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.
This commit is contained in:
Niklas Hambüchen 2021-04-30 18:13:31 +02:00
parent c8dff328e5
commit 0dc08b4138

View file

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