From 0dc08b41385ef5275dcb76fe479c7730c58035d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niklas=20Hamb=C3=BCchen?= <mail@nh2.me>
Date: Fri, 30 Apr 2021 18:13:31 +0200
Subject: [PATCH 1/2] wireguard module: generatePrivateKeyFile: Fix chmod
 security race. Fixes #121288

Until now, the `touch + chmod 600 + write` approach made it possible for
an unprivileged local user read the private key file, by opening
the file after the touch, before the read permissions are restricted.

This was only the case if `generatePrivateKeyFile = true` and the parent
directory of `privateKeyFile` already existed and was readable.

This commit fixes it by using `umask`, which ensures kernel-side that
the `touch` creates the file with the correct permissions atomically.

This commit also:

* Removes `mkdir --mode 0644 -p "${dirOf values.privateKeyFile}"`
  because setting permissions `drw-r--r--` ("nobody can enter that dir")
  is awkward. `drwx------` would perhaps make sense, like for `.ssh`.
  However, setting the permissions on the private key file is enough,
  and likely better, because `privateKeyFile` is about that file
  specifically and no docs suggest that there's something special
  about its parent dir.
* Removes the `chmod 0400 "${values.privateKeyFile}"`
  because there isn't really a point in removing write access from
  the owner of the private key.
---
 nixos/modules/services/networking/wireguard.nix | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/nixos/modules/services/networking/wireguard.nix b/nixos/modules/services/networking/wireguard.nix
index 34c86934535..043bce16e54 100644
--- a/nixos/modules/services/networking/wireguard.nix
+++ b/nixos/modules/services/networking/wireguard.nix
@@ -246,12 +246,15 @@ let
         };
 
         script = ''
-          mkdir --mode 0644 -p "${dirOf values.privateKeyFile}"
+          set -e
+
+          # If the parent dir does not already exist, create it.
+          # Otherwise, does nothing, keeping existing permisions intact.
+          mkdir -p --mode 0755 "${dirOf values.privateKeyFile}"
+
           if [ ! -f "${values.privateKeyFile}" ]; then
-            touch "${values.privateKeyFile}"
-            chmod 0600 "${values.privateKeyFile}"
-            wg genkey > "${values.privateKeyFile}"
-            chmod 0400 "${values.privateKeyFile}"
+            # Write private key file with atomically-correct permissions.
+            (set -e; umask 077; wg genkey > "${values.privateKeyFile}")
           fi
         '';
       };

From a874a8a98b5cd197acf9b2a40b71107db3718f6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niklas=20Hamb=C3=BCchen?= <mail@nh2.me>
Date: Fri, 30 Apr 2021 19:28:04 +0200
Subject: [PATCH 2/2] release notes: Mention wireguard `generatePrivateKeyFile`
 permission changes

---
 nixos/doc/manual/release-notes/rl-2105.xml | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/nixos/doc/manual/release-notes/rl-2105.xml b/nixos/doc/manual/release-notes/rl-2105.xml
index 6e4a9e7114b..4b7d71147b4 100644
--- a/nixos/doc/manual/release-notes/rl-2105.xml
+++ b/nixos/doc/manual/release-notes/rl-2105.xml
@@ -333,6 +333,17 @@
       <literal>vim</literal> switched to Python 3, dropping all Python 2 support.
     </para>
    </listitem>
+   <listitem>
+    <para>
+     <link linkend="opt-networking.wireguard.interfaces">networking.wireguard.interfaces.&lt;name&gt;.generatePrivateKeyFile</link>,
+     which is off by default, had a <literal>chmod</literal> race condition
+     fixed. As an aside, the parent directory's permissions were widened,
+     and the key files were made owner-writable.
+     This only affects newly created keys.
+     However, if the exact permissions are important for your setup, read
+     <link xlink:href="https://github.com/NixOS/nixpkgs/pull/121294">#121294</link>.
+    </para>
+   </listitem>
    <listitem>
      <para>
       <link linkend="opt-boot.zfs.forceImportAll">boot.zfs.forceImportAll</link>