Stop storing keys in the nix store for storeKeysOnMachine.

This patch changes storeKeysOnMachine to work roughly the same way when
true as when false. The only difference is that the keys are stored in
/var/keys (which is usually a real disk) as opposed to /run/keys (which
is RAM).

This is an improvement on the previous version of storeKeysOnMachine
where the keys were stored in the Nix store and linked from /run/keys.
The previous version doesn't allow setting unix permissions on the keys,
meaning any process on the server can read all keys.

This solution has the downside that rolling back doesn't roll back the
keys, however this is consistent with how storeKeysOnMachine=false works
so shouldn't be a major concern. Furthermore if someone wants to have
the keys rollback with the system they can use builtins.toFile instead
of the keys mechanism which works the same way that the previous
storeKeysOnMachine=true worked.

Additionally a symlink /etc/nixops-keys has been added that will be set
to the appropriate key location to make it easier to switch between the
options.

doc/manual/nixops.xml

@@ -1943,7 +1943,7 @@ input.  See <command>nixops export</command> for examples.</para>
     <title>Description</title>
     <para>
       This command uploads the keys described in <varname>deployment.keys</varname>
-      to remote machines in the <literal>/run/keys/</literal> directory.
+      to remote machines in the <literal>/etc/nixops-keys/</literal> directory.
     </para>
     <para>
       Keys are <emphasis>not</emphasis> persisted across reboots by default.

doc/manual/overview.xml

@@ -1747,7 +1747,7 @@ deploy the new configuration.
 }
     </screen>
 
-    This will create a file <filename>/run/keys/my-secret</filename>
+    This will create a file <filename>/etc/nixops-keys/my-secret</filename>
     with the specified contents, ownership, and permissions.
   </para>
 
@@ -1786,7 +1786,7 @@ deploy the new configuration.
         after = [ "my-secret-key.service" ];
         wants = [ "my-secret-key.service" ];
         script = ''
-          export MY_SECRET=$(cat /run/keys/my-secret)
+          export MY_SECRET=$(cat /etc/nixops-keys/my-secret)
           run-my-program
         '';
       };

nix/keys.nix

@@ -11,7 +11,7 @@ let
         The text the key should contain. So if the key name is
         <replaceable>password</replaceable> and <literal>foobar</literal>
         is set here, the contents of the file
-        <filename>/run/keys/<replaceable>password</replaceable></filename>
+        <filename>/etc/nixops-keys/<replaceable>password</replaceable></filename>
         will be <literal>foobar</literal>.
       '';
     };
@@ -62,6 +62,7 @@ let
     inherit (keyOptionsType) getSubOptions;
   };
 
+  keyDir = if config.deployment.storeKeysOnMachine then "/var/keys" else "/run/keys";
 in
 
 {
@@ -93,11 +94,11 @@ in
       description = ''
         <para>The set of keys to be deployed to the machine.  Each attribute
         maps a key name to a file that can be accessed as
-        <filename>/run/keys/<replaceable>name</replaceable></filename>.
+        <filename>/etc/nixops-keys/<replaceable>name</replaceable></filename>.
         Thus, <literal>{ password.text = "foobar"; }</literal> causes a
-        file <filename>/run/keys/password</filename> to be created
+        file <filename>/etc/nixops-keys/password</filename> to be created
         with contents <literal>foobar</literal>.  The directory
-        <filename>/run/keys</filename> is only accessible to root and
+        <filename>/etc/nixops-keys</filename> is only accessible to root and
         the <literal>keys</literal> group, so keep in mind to add any
         users that need to have access to a particular key to this group.</para>
 
@@ -115,38 +116,17 @@ in
 
   config = {
 
-    warnings = mkIf config.deployment.storeKeysOnMachine [(
-      "The use of `deployment.storeKeysOnMachine' imposes a security risk " +
-      "because all keys will be put in the Nix store and thus are world-" +
-      "readable. Also, this will have an impact on services like OpenSSH, " +
-      "which require strict permissions to be set on key files, so expect " +
-      "things to break."
-    )];
-
     system.activationScripts.nixops-keys = stringAfter [ "users" "groups" ]
       ''
-        mkdir -p /run/keys -m 0750
-        chown root:keys /run/keys
-
-        ${optionalString config.deployment.storeKeysOnMachine
-            (concatStrings (mapAttrsToList (name: value:
-              let
-                # FIXME: The key file should be marked as private once
-                # https://github.com/NixOS/nix/issues/8 is fixed.
-                keyFile = pkgs.writeText name value.text;
-              in "ln -sfn ${keyFile} /run/keys/${name}\n")
-              config.deployment.keys)
-            + ''
-              # FIXME: delete obsolete keys?
-              touch /run/keys/done
-            '')
-        }
+        mkdir -p ${keyDir} -m 0750
+        chown root:keys ${keyDir}
+        ln -sfn ${keyDir} /etc/nixops-keys
 
         ${concatStringsSep "\n" (flip mapAttrsToList config.deployment.keys (name: value:
           # Make sure each key has correct ownership, since the configured owning
           # user or group may not have existed when first uploaded.
           ''
-            [[ -f "/run/keys/${name}" ]] && chown '${value.user}:${value.group}' "/run/keys/${name}"
+            [[ -f "${keyDir}/${name}" ]] && chown '${value.user}:${value.group}' "${keyDir}/${name}"
           ''
         ))}
       '';
@@ -162,7 +142,7 @@ in
           serviceConfig.RemainAfterExit = true;
           script =
             ''
-              while ! [ -e /run/keys/done ]; do
+              while ! [ -e ${keyDir}/done ]; do
                 sleep 0.1
               done
             '';
@@ -178,19 +158,19 @@ in
           path = [ pkgs.inotifyTools ];
           preStart = ''
             (while read f; do if [ "$f" = "${name}" ]; then break; fi; done \
-              < <(inotifywait -qm --format '%f' -e create /run/keys) ) &
+              < <(inotifywait -qm --format '%f' -e create ${keyDir}) ) &
 
-            if [[ -e "/run/keys/${name}" ]]; then
+            if [[ -e "${keyDir}/${name}" ]]; then
               echo 'flapped down'
               kill %1
               exit 0
             fi
             wait %1
           '';
           script = ''
-            inotifywait -qq -e delete_self "/run/keys/${name}" &
+            inotifywait -qq -e delete_self "${keyDir}/${name}" &
 
-            if [[ ! -e "/run/keys/${name}" ]]; then
+            if [[ ! -e "${keyDir}/${name}" ]]; then
               echo 'flapped up'
               exit 0
             fi

nixops/backends/__init__.py

@@ -199,21 +199,24 @@ def reboot_rescue(self, hard=False):
         self.warn("machine ‘{0}’ doesn't have a rescue"
                   " system.".format(self.name))
 
+    @property
+    def key_dir(self):
+        return "/var/keys" if self.store_keys_on_machine else "/run/keys"
+
     def send_keys(self):
         if self.state == self.RESCUE:
             # Don't send keys when in RESCUE state, because we're most likely
             # bootstrapping plus we probably don't have /run mounted properly
             # so keys will probably end up being written to DISK instead of
             # into memory.
             return
-        if self.store_keys_on_machine: return
-        self.run_command("mkdir -m 0750 -p /run/keys"
-                         " && chown root:keys /run/keys")
+        key_dir = self.key_dir
+        self.run_command("mkdir -pm 0750 {0} && chown root:keys {0}".format(key_dir))
         for k, opts in self.get_keys().items():
             self.log("uploading key ‘{0}’...".format(k))
             tmp = self.depl.tempdir + "/key-" + self.name
             f = open(tmp, "w+"); f.write(opts['text']); f.close()
-            outfile = "/run/keys/" + k
+            outfile = os.path.join(key_dir, k)
             outfile_esc = "'" + outfile.replace("'", r"'\''") + "'"
             self.run_command("rm -f " + outfile_esc)
             self.upload_file(tmp, outfile)
@@ -237,7 +240,7 @@ def send_keys(self):
               )
             )
             os.remove(tmp)
-        self.run_command("touch /run/keys/done")
+        self.run_command("touch {}/done".format(key_dir))
 
     def get_keys(self):
         return self.keys

tests/functional/single_machine_secret_key_ram.nix

@@ -0,0 +1,7 @@
+{
+  machine.deployment = {
+    storeKeysOnMachine = false;
+
+    keys."secret.key" = "12345";
+  };
+}

tests/functional/test_send_keys_sends_keys.py

@@ -5,18 +5,29 @@
 
 parent_dir = path.dirname(__file__)
 
-secret_key_spec = '%s/single_machine_secret_key.nix' % (parent_dir)
+secret_key_ram_spec = '%s/single_machine_secret_key_ram.nix' % (parent_dir)
+secret_key_disk_spec = '%s/single_machine_secret_key_disk.nix' % (parent_dir)
 
 class TestSendKeysSendsKeys(single_machine_test.SingleMachineTest):
     _multiprocess_can_split_ = True
 
     def setup(self):
         super(TestSendKeysSendsKeys,self).setup()
-        self.depl.nix_exprs = self.depl.nix_exprs + [ secret_key_spec ]
 
     def run_check(self):
+        self.depl.nix_exprs = self.depl.nix_exprs + [ secret_key_ram_spec ]
+
         self.depl.deploy()
         self.check_command("test -f /run/keys/secret.key")
         self.check_command("rm -f /run/keys/secret.key")
         self.depl.send_keys()
         self.check_command("test -f /run/keys/secret.key")
+
+    def run_check(self):
+        self.depl.nix_exprs = self.depl.nix_exprs + [ secret_key_disk_spec ]
+
+        self.depl.deploy()
+        self.check_command("test -f /var/keys/secret.key")
+        self.check_command("rm -f /var/keys/secret.key")
+        self.depl.send_keys()
+        self.check_command("test -f /var/keys/secret.key")