moss

commit 1815f2a4b22223ee58222b0634ecb0efa0405c01
parent 4f0ac189e627c531d9f4b648b158988c67ca6027
Author: Daniel Barlow <dan@telent.net>
Date:   Sat, 14 Aug 2021 20:28:26 +0100

decrypt identity into temporary file, not string

Once secret key material is in managed ruby memory there's no
easy way to guarantee it doesn't end up in swap.  Even if we use mmap
and mlock we still involve Ruby strings to get it into/out of that
mapped region, and they might be swapped out

Diffstat:
M
README.md
 | 
12
++++++------
M
moss.rb
 | 
39
++++++++++++++++++++++++---------------

2 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/README.md b/README.md
@@ -161,12 +161,6 @@ security requirements, look elsewhere.
   loggers, cameras pointing at your keyboard, virtual hosts with
   compromised hypervisors, then moss won't help you
 
-* the first time an encrypted secret is needed (when writing a secet),
-  it is decrypted into memory and retained for the lifetime of the
-  process. If the app is paged out, maybe that data gets written to
-  swap.  This is so that `moss rebuild` prompts only once for the
-  passphrase instead of once per secet
-
 * `pass edit` uses Ruby's Tempfile class to write a temporary
   plaintext file containing the secret. According to the Ruby
   documentation the "filename picking method is both thread-safe and
@@ -180,6 +174,12 @@ security requirements, look elsewhere.
   of interpolating values into strings which may give rise to shell
   shell injection attacks
 
+* the first time an encrypted identity is needed (i.e when reading a
+  secret), we create a temporary file using Tempfile, and unlink it
+  before running age to decrypt the secret into it.  This is so that
+  `moss rebuild` prompts only once for the passphrase instead of once
+  per secret. moss does not read the decrypted file itself.
+
 * reasonable care has been taken to set appropriate file permissions
   on keys and secrets, so you should be good against other non-root
   users on your machine
diff --git a/moss.rb b/moss.rb
@@ -73,6 +73,19 @@ class Moss
     root.join("identity")
   end
 
+  def identity_stream
+    unless @identity
+      if encrypted?(identity_file)
+        @identity = Tempfile.new('id')
+        @identity.unlink
+        Kernel.system(AGE, "-d", identity_file.to_s, { 1 => @identity.fileno })
+      else
+        @identity = File.open(identity_file)
+      end
+    end
+    @identity.tap(&:rewind)
+  end
+
   def git_managed?
     store.join(".git").exist?
   end
@@ -113,23 +126,19 @@ class Moss
     end
   end
 
-  def identity
-    unless @identity
-      @identity = encrypted?(identity_file) ?
-                    capture_output([AGE, "-d", identity_file]) :
-                    identity_file.read
-    end
-    @identity
-  end
-
-
   def read_secret(name)
     pathname = store.join("#{name}.age")
-    File.exist?(pathname) or raise MossError, "Can't open non-existent file #{pathname}"
-    IO.popen([AGE, "-i", "-", "-d", pathname.to_s], "r+") do |f|
-      f.write(identity)
-      f.close_write
-      f.read
+    File.exist?(pathname) or
+      raise MossError, "Can't open non-existent file #{pathname}"
+
+    IO.pipe do |r_io, w_io|
+      pid = Kernel.spawn(AGE, "-i", "-", "-d", pathname.to_s,
+                         { 0 => identity_stream.fileno,
+                           1 => w_io.fileno
+                         })
+      w_io.close
+      Process.waitpid(pid)
+      r_io.read
     end
   end