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] 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
         '';
       };