From b833d4a32d965e6393a63b2c91b46eca2a5030d8 Mon Sep 17 00:00:00 2001
From: Emily <vcs@emily.moe>
Date: Sun, 16 Jul 2023 16:59:43 +0100
Subject: ssh: use symlinks for `authorizedKeys` options
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As explained in the changelog and activation check, the previous
implementation had a nasty security bug that made removing a user’s
authorized keys effectively a no‐op.
---
 modules/system/checks.nix | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

(limited to 'modules/system')

diff --git a/modules/system/checks.nix b/modules/system/checks.nix
index f0f03e8..d527aa8 100644
--- a/modules/system/checks.nix
+++ b/modules/system/checks.nix
@@ -202,6 +202,28 @@ let
         exit 2
     fi
   '';
+
+  # TODO: Remove this a couple years down the line when we can assume
+  # that anyone who cares about security has upgraded.
+  oldSshAuthorizedKeysDirectory = ''
+    if [[ -d /etc/ssh/authorized_keys.d ]]; then
+        printf >&2 '\e[1;31merror: /etc/ssh/authorized_keys.d exists, aborting activation\e[0m\n'
+        printf >&2 'SECURITY NOTICE: The previous implementation of the\n'
+        printf >&2 '`users.users.<name>.openssh.authorizedKeys.*` options would not delete\n'
+        printf >&2 'authorized keys files when the setting for a given user was removed.\n'
+        printf >&2 '\n'
+        printf >&2 "This means that if you previously stopped managing a user's authorized\n"
+        printf >&2 'SSH keys with nix-darwin, or intended to revoke their access by\n'
+        printf >&2 'removing the option, the previous set of keys could still be used to\n'
+        printf >&2 'log in as that user.\n'
+        printf >&2 '\n'
+        printf >&2 'You can check the /etc/ssh/authorized_keys.d directory to see which\n'
+        printf >&2 'keys were permitted; afterwards, please remove the directory and\n'
+        printf >&2 're-run activation. The options continue to be supported and will now\n'
+        printf >&2 'correctly permit only the keys in your current system configuration.\n'
+        exit 2
+    fi
+  '';
 in
 
 {
@@ -245,6 +267,7 @@ in
       (mkIf cfg.verifyNixChannels nixChannels)
       nixInstaller
       (mkIf cfg.verifyNixPath nixPath)
+      oldSshAuthorizedKeysDirectory
     ];
 
     system.activationScripts.checks.text = ''
-- 
cgit v1.2.3


From 36a15e8c6c4686be29ccbf0ae0ac1d6133074615 Mon Sep 17 00:00:00 2001
From: Emily <vcs@emily.moe>
Date: Sun, 16 Jul 2023 17:02:10 +0100
Subject: write-text: remove support for `copy`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is a huge anti‐declarative footgun; `copy` files cannot
distinguish if a previous version is managed by nix-darwin, so they
can’t check the hash, so they’re prone to destroying data, and
copied files are not deleted when they’re removed from the system
configuration, which led to a security bug. Nothing else in‐tree
was using this functionality, so let’s make sure it doesn’t
cause any more bugs.
---
 modules/system/etc.nix | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

(limited to 'modules/system')

diff --git a/modules/system/etc.nix b/modules/system/etc.nix
index 008fb1c..bc60bef 100644
--- a/modules/system/etc.nix
+++ b/modules/system/etc.nix
@@ -10,7 +10,6 @@ let
   };
 
   etc = filter (f: f.enable) (attrValues config.environment.etc);
-  etcCopy = filter (f: f.copy) (attrValues config.environment.etc);
 
 in
 
@@ -34,9 +33,10 @@ in
       ''
         mkdir -p $out/etc
         cd $out/etc
-        ${concatMapStringsSep "\n" (attr: "mkdir -p $(dirname '${attr.target}')") etc}
-        ${concatMapStringsSep "\n" (attr: "ln -s '${attr.source}' '${attr.target}'") etc}
-        ${concatMapStringsSep "\n" (attr: "touch '${attr.target}'.copy") etcCopy}
+        ${concatMapStringsSep "\n" (attr: ''
+          mkdir -p "$(dirname ${escapeShellArg attr.target})"
+          ln -s ${escapeShellArgs [ attr.source attr.target ]}
+        '') etc}
       '';
 
     system.activationScripts.etcChecks.text = ''
@@ -55,10 +55,6 @@ in
         etcStaticFile=/etc/static/$subPath
         etcFile=/etc/$subPath
 
-        if [[ -e $configFile.copy ]]; then
-          continue
-        fi
-
         # We need to check files that exist and aren't already links to
         # $etcStaticFile for known hashes.
         if [[
@@ -109,11 +105,6 @@ in
           mkdir -p "$etcDir"
         fi
 
-        if [[ -e $etcStaticFile.copy ]]; then
-          cp "$etcStaticFile" "$etcFile"
-          continue
-        fi
-
         if [[ -e $etcFile ]]; then
           if [[ $(readlink -- "$etcFile") == "$etcStaticFile" ]]; then
             continue
@@ -130,7 +121,7 @@ in
 
         # Delete stale links into /etc/static.
         if [[
-          $(readlink "$etcFile") == "$etcStaticFile"
+          $(readlink -- "$etcFile") == "$etcStaticFile"
           && ! -e $etcStaticFile
         ]]; then
           rm "$etcFile"
-- 
cgit v1.2.3