commit 6690cd6c246a1377c2d72c6bfcceaa4a65219c54
parent 71c66d37b481c905ff3e3a4a96ff69d8603ca88b
Author: Daniel Barlow <dan@telent.net>
Date: Wed, 11 Aug 2021 10:04:09 +0100
remove variable interpolation in strings
easy source of shell injection
Diffstat:
3 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/features/moss.feature b/features/moss.feature
@@ -101,3 +101,12 @@ Feature: command-line password management
Then the store file "home/ebay.age" is readable only by me
And the store file "home/" is readable only by me
And my identity file is readable only by me
+
+ Scenario: I can create files with funny characters
+ Given I am using a temporary password store
+ When I store a secret for "home/with space" with content "horsebattery"
+ Then "home/with space.age" plaintext is "horsebattery"
+
+ When I store a secret for "silly/two\nlines" with content "horsebattery"
+ Then "silly/two\nlines.age" plaintext is "horsebattery"
+
diff --git a/features/step_definitions/steps.rb b/features/step_definitions/steps.rb
@@ -4,7 +4,7 @@ require 'tmpdir'
require 'fileutils'
def shell(s)
- output = %x{bash -c #{s.inspect}}
+ output = IO.popen(["bash", "-c", s]) do |f| f.read end
$?.exitstatus.zero? or raise "#{$?} : #{@i_see}"
output
end
@@ -29,8 +29,6 @@ def recipient_for_identity(identity_file)
raise "can't get pubkey for identity #{identity_file}"
end
-
-
Given("I have the identity {string}") do |keyfile|
@identity_path = "fixtures/keys/#{keyfile}"
end
@@ -56,7 +54,7 @@ end
When("I store a secret for {string} with content {string}") do |name, content|
@stored_secret_name = name
- shell "echo -n #{content} | #{MOSS} insert #{name}"
+ shell "echo -n #{content} | #{MOSS} insert #{name.inspect}"
end
Then("I see a {int} character string") do |len|
@@ -71,7 +69,7 @@ Then("{string} plaintext is {string}") do |name, expected|
path_name = store_path(name)
contents = File.read(path_name)
expect(contents).to match /AGE ENCRYPTED FILE/
- plaintext = shell "age -i #{IDENTITY_FILE} -d #{path_name}"
+ plaintext = shell "age -i #{IDENTITY_FILE} -d #{path_name.to_s.inspect}"
expect(plaintext).to eq expected
end
@@ -79,7 +77,7 @@ Then("I can decrypt it with key {string} to {string}") do |keyfile, expected|
path_name = store_path(@stored_secret_name + '.age')
contents = File.read(path_name)
expect(contents).to match /AGE ENCRYPTED FILE/
- plaintext = shell "age -i fixtures/keys/#{keyfile} -d #{path_name}"
+ plaintext = shell "age -i fixtures/keys/#{keyfile} -d #{path_name.to_s.inspect}"
expect(plaintext).to eq expected
end
@@ -88,14 +86,14 @@ Then("I cannot decrypt it with key {string}") do |keyfile|
contents = File.read(path_name)
expect(contents).to match /AGE ENCRYPTED FILE/
# we expect age to fail, redirect stderr to avoid messing up test output
- shell "! age -i fixtures/keys/#{keyfile} -d #{path_name} 2>&1"
+ shell "! age -i fixtures/keys/#{keyfile} -d #{path_name.to_s.inspect} 2>&1"
end
Then("{string} plaintext matches {word}") do |name, re|
path_name = store_path(name)
contents = File.read(path_name)
expect(contents).to match /AGE ENCRYPTED FILE/
- shell "age -i #{IDENTITY_FILE} -d #{path_name}"
+ shell "age -i #{IDENTITY_FILE} -d #{path_name.to_s.inspect}"
expect(@i_see).to match Regexp.new(re)
end
diff --git a/moss.rb b/moss.rb
@@ -28,23 +28,31 @@ class Moss
@root = Pathname.new(root)
end
- # This is the method that "moss init" calls, which could be better
- # named. "initialize" would be a good name if it weren't special to
- # Ruby
-
def encrypted?(filename)
magic = "age-encryption.org/v1"
File.read(filename, magic.length) == magic
end
+ # this works like backticks, but can be passed an array instead of a
+ # string, thus avoiding shell quoting pitfalls
+ private def capture_output(command)
+ if command.respond_to?(:first) then command = command.map(&:to_s) end
+ IO.popen(command) {|f|
+ f.read
+ }
+ end
+
def pubkey_for_identity(filename)
pubkey =
if encrypted?(filename)
- IO.popen("set -o pipefail; #{AGE} -d #{filename} | #{AGE_KEYGEN} -y -") {|f|
+ plaintext = capture_output([AGE, "-d", filename])
+ IO.popen([AGE_KEYGEN, "-y", "-"], "r+") do |f|
+ f.write(plaintext)
+ f.close_write
f.read
- }
+ end
else
- IO.popen("#{AGE_KEYGEN} -y #{filename}") { |f| f.read }
+ capture_output([AGE_KEYGEN, "-y", filename.to_s])
end
if $?.exitstatus.zero?
pubkey
@@ -53,6 +61,9 @@ class Moss
end
end
+ # This is the method that "moss init" calls, which could be better
+ # named. "initialize" would be a good name if it weren't special to
+ # Ruby
def create(keyfile)
keyfile.exist? or raise "Cannot read identity at #{keyfile}"
recipient = pubkey_for_identity(keyfile)
@@ -71,6 +82,8 @@ class Moss
store.join(".git").exist?
end
+ # ascend the directory hierarchy looking for the filename,
+ # stopping at the store directory. this method is badly named
private def find_in_subtree(subtree, filename)
pathname = subtree.join(filename)
case
@@ -92,18 +105,25 @@ class Moss
def write_secret(name, content)
pathname = store.join("#{name}.age")
pathname.dirname.mkpath
- IO.popen("#{AGE} -a --recipients-file #{recipients_for_secret(name)} -o #{pathname.to_s}", "w") do |f|
+ IO.popen([AGE,
+ "-a", "--recipients-file", recipients_for_secret(name).to_s,
+ "-o", pathname.to_s], "w") do |f|
f.write(content)
end
if git_managed?
- Kernel.system("cd #{store.to_s} && git add #{pathname.relative_path_from(store).to_s.inspect} && git commit -m'new secret'")
+ Kernel.system(GIT, "add", pathname.relative_path_from(store).to_s,
+ {chdir: MOSS.store})
+ Kernel.system(GIT, "commit", "-m", "new secret",
+ {chdir: MOSS.store})
end
end
def read_secret(name)
pathname = store.join("#{name}.age")
File.exist?(pathname) or raise "Can't open #{pathname}: $!"
- `#{AGE} -i #{identity_file} -d #{pathname}`
+ capture_output([AGE,
+ "-i", identity_file,
+ "-d", pathname])
end
def secrets
@@ -119,6 +139,11 @@ class Moss
git: git_managed?
}
end
+
+ def git_operation(parameters)
+ raise "not a git repo" unless git_managed?
+ Kernel.system(GIT, *parameters, {chdir: store})
+ end
end
@@ -186,7 +211,7 @@ when 'init'
keyfile = Pathname.new(parameters.first)
MOSS.create(keyfile)
when 'git'
- Kernel.system(GIT, *parameters, {chdir: MOSS.store})
+ MOSS.git_operation(parameters)
else
raise "command #{action} unrecognized"
end
