From 904f68fb0fc01cf4072c1215416eb4e2b9fc4e56 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Wed, 9 Jun 2021 01:41:26 +0200
Subject: [PATCH 01/10] nixos/security/wrappers: make well-typed

The security.wrappers option is morally a set of submodules but it's
actually (un)typed as a generic attribute set. This is bad for several
reasons:

1. Some of the "submodule" option are not document;
2. the default values are not documented and are chosen based on
   somewhat bizarre rules (issue #23217);
3. It's not possible to override an existing wrapper due to the
   dumb types.attrs.merge strategy;
4. It's easy to make mistakes that will go unnoticed, which is
   really bad given the sensitivity of this module (issue #47839).

This makes the option a proper set of submodule and add strict types and
descriptions to every sub-option. Considering it's not yet clear if the
way the default values are picked is intended, this reproduces the current
behavior, but it's now documented explicitly.
---
 nixos/modules/security/wrappers/default.nix | 176 +++++++++++++-------
 1 file changed, 119 insertions(+), 57 deletions(-)

diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix
index 1e65f4515155..74dfd86b86af 100644
--- a/nixos/modules/security/wrappers/default.nix
+++ b/nixos/modules/security/wrappers/default.nix
@@ -5,23 +5,108 @@ let
 
   parentWrapperDir = dirOf wrapperDir;
 
-  programs =
-    (lib.mapAttrsToList
-      (n: v: (if v ? program then v else v // {program=n;}))
-      wrappers);
-
   securityWrapper = pkgs.callPackage ./wrapper.nix {
     inherit parentWrapperDir;
   };
 
+  fileModeType =
+    let
+      # taken from the chmod(1) man page
+      symbolic = "[ugoa]*([-+=]([rwxXst]*|[ugo]))+|[-+=][0-7]+";
+      numeric = "[-+=]?[0-7]{0,4}";
+      mode = "((${symbolic})(,${symbolic})*)|(${numeric})";
+    in
+     lib.types.strMatching mode
+     // { description = "file mode string"; };
+
+  wrapperType = lib.types.submodule ({ name, config, ... }: {
+    options.source = lib.mkOption
+      { type = lib.types.path;
+        description = "The absolute path to the program to be wrapped.";
+      };
+    options.program = lib.mkOption
+      { type = with lib.types; nullOr str;
+        default = name;
+        description = ''
+          The name of the wrapper program. Defaults to the attribute name.
+        '';
+      };
+    options.owner = lib.mkOption
+      { type = lib.types.str;
+        default = with config;
+          if (capabilities != "") || !(setuid || setgid || permissions != null)
+          then "root"
+          else "nobody";
+        description = ''
+          The owner of the wrapper program. Defaults to <literal>root</literal>
+          if any capability is set and setuid/setgid/permissions are not, otherwise to
+          <literal>nobody</litera>.
+        '';
+      };
+    options.group = lib.mkOption
+      { type = lib.types.str;
+        default = with config;
+          if (capabilities != "") || !(setuid || setgid || permissions != null)
+          then "root"
+          else "nogroup";
+        description = ''
+          The group of the wrapper program. Defaults to <literal>root</literal>
+          if any capability is set and setuid/setgid/permissions are not,
+          otherwise to <literal>nogroup</litera>.
+        '';
+      };
+    options.permissions = lib.mkOption
+      { type = lib.types.nullOr fileModeType;
+        default = null;
+        example = "u+rx,g+x,o+x";
+        apply = x: if x == null then "u+rx,g+x,o+x" else x;
+        description = ''
+          The permissions of the wrapper program. The format is that of a
+          symbolic or numeric file mode understood by <command>chmod</command>.
+        '';
+      };
+    options.capabilities = lib.mkOption
+      { type = lib.types.commas;
+        default = "";
+        description = ''
+          A comma-separated list of capabilities to be given to the wrapper
+          program. For capabilities supported by the system check the
+          <citerefentry>
+            <refentrytitle>capabilities</refentrytitle>
+            <manvolnum>7</manvolnum>
+          </citerefentry>
+          manual page.
+
+          <note><para>
+            <literal>cap_setpcap</literal>, which is required for the wrapper
+            program to be able to raise caps into the Ambient set is NOT raised
+            to the Ambient set so that the real program cannot modify its own
+            capabilities!! This may be too restrictive for cases in which the
+            real program needs cap_setpcap but it at least leans on the side
+            security paranoid vs. too relaxed.
+          </para></note>
+        '';
+      };
+    options.setuid = lib.mkOption
+      { type = lib.types.bool;
+        default = false;
+        description = "Whether to add the setuid bit the wrapper program.";
+      };
+    options.setgid = lib.mkOption
+      { type = lib.types.bool;
+        default = false;
+        description = "Whether to add the setgid bit the wrapper program.";
+      };
+  });
+
   ###### Activation script for the setcap wrappers
   mkSetcapProgram =
     { program
     , capabilities
     , source
-    , owner  ? "nobody"
-    , group  ? "nogroup"
-    , permissions ? "u+rx,g+x,o+x"
+    , owner
+    , group
+    , permissions
     , ...
     }:
     assert (lib.versionAtLeast (lib.getVersion config.boot.kernelPackages.kernel) "4.3");
@@ -46,11 +131,11 @@ let
   mkSetuidProgram =
     { program
     , source
-    , owner  ? "nobody"
-    , group  ? "nogroup"
-    , setuid ? false
-    , setgid ? false
-    , permissions ? "u+rx,g+x,o+x"
+    , owner
+    , group
+    , setuid
+    , setgid
+    , permissions
     , ...
     }:
     ''
@@ -66,24 +151,11 @@ let
 
   mkWrappedPrograms =
     builtins.map
-      (s: if (s ? capabilities)
-          then mkSetcapProgram
-                 ({ owner = "root";
-                    group = "root";
-                  } // s)
-          else if
-             (s ? setuid && s.setuid) ||
-             (s ? setgid && s.setgid) ||
-             (s ? permissions)
-          then mkSetuidProgram s
-          else mkSetuidProgram
-                 ({ owner  = "root";
-                    group  = "root";
-                    setuid = true;
-                    setgid = false;
-                    permissions = "u+rx,g+x,o+x";
-                  } // s)
-      ) programs;
+      (opts:
+        if opts.capabilities != ""
+          then mkSetcapProgram opts
+          else mkSetuidProgram opts
+      ) (lib.attrValues wrappers);
 in
 {
   imports = [
@@ -95,7 +167,7 @@ in
 
   options = {
     security.wrappers = lib.mkOption {
-      type = lib.types.attrs;
+      type = lib.types.attrsOf wrapperType;
       default = {};
       example = lib.literalExample
         ''
@@ -109,31 +181,11 @@ in
           }
         '';
       description = ''
-        This option allows the ownership and permissions on the setuid
-        wrappers for specific programs to be overridden from the
-        default (setuid root, but not setgid root).
-
-        <note>
-          <para>The sub-attribute <literal>source</literal> is mandatory,
-          it must be the absolute path to the program to be wrapped.
-          </para>
-
-          <para>The sub-attribute <literal>program</literal> is optional and
-          can give the wrapper program a new name. The default name is the same
-          as the attribute name itself.</para>
-
-          <para>Additionally, this option can set capabilities on a
-          wrapper program that propagates those capabilities down to the
-          wrapped, real program.</para>
-
-          <para>NOTE: cap_setpcap, which is required for the wrapper
-          program to be able to raise caps into the Ambient set is NOT
-          raised to the Ambient set so that the real program cannot
-          modify its own capabilities!! This may be too restrictive for
-          cases in which the real program needs cap_setpcap but it at
-          least leans on the side security paranoid vs. too
-          relaxed.</para>
-        </note>
+        This option effectively allows adding setuid/setgid bits, capabilities,
+        changing file ownership and permissions of a program without directly
+        modifying it. This works by creating a wrapper program under the
+        <option>security.wrapperDir</option> directory, which is then added to
+        the shell <literal>PATH</literal>.
       '';
     };
 
@@ -151,6 +203,16 @@ in
   ###### implementation
   config = {
 
+    assertions = lib.mapAttrsToList
+      (name: opts:
+        { assertion = opts.setuid || opts.setgid -> opts.capabilities == "";
+          message = ''
+            The security.wrappers.${name} wrapper is not valid:
+                setuid/setgid and capabilities are mutually exclusive.
+          '';
+        }
+      ) wrappers;
+
     security.wrappers = {
       # These are mount related wrappers that require the +s permission.
       fusermount.source = "${pkgs.fuse}/bin/fusermount";

From 22004f7e8febc6ae6553c44ecd8bf9da9ddc5260 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Wed, 9 Jun 2021 19:59:39 +0200
Subject: [PATCH 02/10] nixos/security/wrappers: use fixed defaults

To keep backward compatibility and have a typing would require making
all options null by default, adding a defaultText containing the actual
value, write the default value logic based on `!= null` and replacing
the nulls laters. This pretty much defeats the point of having used
a submodule type.
---
 nixos/modules/security/wrappers/default.nix | 35 ++++++---------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix
index 74dfd86b86af..8b1f5da2ba2d 100644
--- a/nixos/modules/security/wrappers/default.nix
+++ b/nixos/modules/security/wrappers/default.nix
@@ -33,33 +33,18 @@ let
       };
     options.owner = lib.mkOption
       { type = lib.types.str;
-        default = with config;
-          if (capabilities != "") || !(setuid || setgid || permissions != null)
-          then "root"
-          else "nobody";
-        description = ''
-          The owner of the wrapper program. Defaults to <literal>root</literal>
-          if any capability is set and setuid/setgid/permissions are not, otherwise to
-          <literal>nobody</litera>.
-        '';
+        default = "root";
+        description = "The owner of the wrapper program.";
       };
     options.group = lib.mkOption
       { type = lib.types.str;
-        default = with config;
-          if (capabilities != "") || !(setuid || setgid || permissions != null)
-          then "root"
-          else "nogroup";
-        description = ''
-          The group of the wrapper program. Defaults to <literal>root</literal>
-          if any capability is set and setuid/setgid/permissions are not,
-          otherwise to <literal>nogroup</litera>.
-        '';
+        default = "root";
+        description = "The group of the wrapper program.";
       };
     options.permissions = lib.mkOption
-      { type = lib.types.nullOr fileModeType;
-        default = null;
-        example = "u+rx,g+x,o+x";
-        apply = x: if x == null then "u+rx,g+x,o+x" else x;
+      { type = fileModeType;
+        default  = "u+rx,g+x,o+x";
+        example = "a+rx";
         description = ''
           The permissions of the wrapper program. The format is that of a
           symbolic or numeric file mode understood by <command>chmod</command>.
@@ -89,7 +74,7 @@ let
       };
     options.setuid = lib.mkOption
       { type = lib.types.bool;
-        default = false;
+        default = true;
         description = "Whether to add the setuid bit the wrapper program.";
       };
     options.setgid = lib.mkOption
@@ -153,8 +138,8 @@ let
     builtins.map
       (opts:
         if opts.capabilities != ""
-          then mkSetcapProgram opts
-          else mkSetuidProgram opts
+        then mkSetcapProgram opts
+        else mkSetuidProgram opts
       ) (lib.attrValues wrappers);
 in
 {

From 7d8b303e3fd76ccf58cfe26348e889def3663546 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Thu, 10 Jun 2021 14:57:52 +0200
Subject: [PATCH 03/10] nixos/security/wrappers: check that sources exist

Add a shell script that checks if the paths of all wrapped programs
actually exist to catch mistakes. This only checks for Nix store paths,
which are always expected to exist at build time.
---
 nixos/modules/security/wrappers/default.nix | 30 ++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix
index 8b1f5da2ba2d..2ce26854be44 100644
--- a/nixos/modules/security/wrappers/default.nix
+++ b/nixos/modules/security/wrappers/default.nix
@@ -226,7 +226,7 @@ in
       ]}"
     '';
 
-    ###### setcap activation script
+    ###### wrappers activation script
     system.activationScripts.wrappers =
       lib.stringAfter [ "specialfs" "users" ]
         ''
@@ -257,5 +257,33 @@ in
             ln --symbolic $wrapperDir ${wrapperDir}
           fi
         '';
+
+    ###### wrappers consistency checks
+    system.extraDependencies = lib.singleton (pkgs.runCommandLocal
+      "ensure-all-wrappers-paths-exist" { }
+      ''
+        # make sure we produce output
+        mkdir -p $out
+
+        echo -n "Checking that Nix store paths of all wrapped programs exist... "
+
+        declare -A wrappers
+        ${lib.concatStringsSep "\n" (lib.mapAttrsToList (n: v:
+          "wrappers['${n}']='${v.source}'") wrappers)}
+
+        for name in "''${!wrappers[@]}"; do
+          path="''${wrappers[$name]}"
+          if [[ "$path" =~ /nix/store ]] && [ ! -e "$path" ]; then
+            test -t 1 && echo -ne '\033[1;31m'
+            echo "FAIL"
+            echo "The path $path does not exist!"
+            echo 'Please, check the value of `security.wrappers."'$name'".source`.'
+            test -t 1 && echo -ne '\033[0m'
+            exit 1
+          fi
+        done
+
+        echo "OK"
+      '');
   };
 }

From 936e8eaf411248e34ceef219fb94acfbb66060a0 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Sun, 12 Sep 2021 16:14:40 +0200
Subject: [PATCH 04/10] nixos/security/wrappers: fix shell quoting

---
 nixos/modules/security/wrappers/default.nix | 36 ++++++++++-----------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix
index 2ce26854be44..2f2c170e4607 100644
--- a/nixos/modules/security/wrappers/default.nix
+++ b/nixos/modules/security/wrappers/default.nix
@@ -96,20 +96,20 @@ let
     }:
     assert (lib.versionAtLeast (lib.getVersion config.boot.kernelPackages.kernel) "4.3");
     ''
-      cp ${securityWrapper}/bin/security-wrapper $wrapperDir/${program}
-      echo -n "${source}" > $wrapperDir/${program}.real
+      cp ${securityWrapper}/bin/security-wrapper "$wrapperDir/${program}"
+      echo -n "${source}" > "$wrapperDir/${program}.real"
 
       # Prevent races
-      chmod 0000 $wrapperDir/${program}
-      chown ${owner}.${group} $wrapperDir/${program}
+      chmod 0000 "$wrapperDir/${program}"
+      chown ${owner}.${group} "$wrapperDir/${program}"
 
       # Set desired capabilities on the file plus cap_setpcap so
       # the wrapper program can elevate the capabilities set on
       # its file into the Ambient set.
-      ${pkgs.libcap.out}/bin/setcap "cap_setpcap,${capabilities}" $wrapperDir/${program}
+      ${pkgs.libcap.out}/bin/setcap "cap_setpcap,${capabilities}" "$wrapperDir/${program}"
 
       # Set the executable bit
-      chmod ${permissions} $wrapperDir/${program}
+      chmod ${permissions} "$wrapperDir/${program}"
     '';
 
   ###### Activation script for the setuid wrappers
@@ -124,14 +124,14 @@ let
     , ...
     }:
     ''
-      cp ${securityWrapper}/bin/security-wrapper $wrapperDir/${program}
-      echo -n "${source}" > $wrapperDir/${program}.real
+      cp ${securityWrapper}/bin/security-wrapper "$wrapperDir/${program}"
+      echo -n "${source}" > "$wrapperDir/${program}.real"
 
       # Prevent races
-      chmod 0000 $wrapperDir/${program}
-      chown ${owner}.${group} $wrapperDir/${program}
+      chmod 0000 "$wrapperDir/${program}"
+      chown ${owner}.${group} "$wrapperDir/${program}"
 
-      chmod "u${if setuid then "+" else "-"}s,g${if setgid then "+" else "-"}s,${permissions}" $wrapperDir/${program}
+      chmod "u${if setuid then "+" else "-"}s,g${if setgid then "+" else "-"}s,${permissions}" "$wrapperDir/${program}"
     '';
 
   mkWrappedPrograms =
@@ -238,7 +238,7 @@ in
 
           # We want to place the tmpdirs for the wrappers to the parent dir.
           wrapperDir=$(mktemp --directory --tmpdir="${parentWrapperDir}" wrappers.XXXXXXXXXX)
-          chmod a+rx $wrapperDir
+          chmod a+rx "$wrapperDir"
 
           ${lib.concatStringsSep "\n" mkWrappedPrograms}
 
@@ -246,15 +246,15 @@ in
             # Atomically replace the symlink
             # See https://axialcorps.com/2013/07/03/atomically-replacing-files-and-directories/
             old=$(readlink -f ${wrapperDir})
-            if [ -e ${wrapperDir}-tmp ]; then
-              rm --force --recursive ${wrapperDir}-tmp
+            if [ -e "${wrapperDir}-tmp" ]; then
+              rm --force --recursive "${wrapperDir}-tmp"
             fi
-            ln --symbolic --force --no-dereference $wrapperDir ${wrapperDir}-tmp
-            mv --no-target-directory ${wrapperDir}-tmp ${wrapperDir}
-            rm --force --recursive $old
+            ln --symbolic --force --no-dereference "$wrapperDir" "${wrapperDir}-tmp"
+            mv --no-target-directory "${wrapperDir}-tmp" "${wrapperDir}"
+            rm --force --recursive "$old"
           else
             # For initial setup
-            ln --symbolic $wrapperDir ${wrapperDir}
+            ln --symbolic "$wrapperDir" "${wrapperDir}"
           fi
         '';
 

From 27dcb04cde501f731a79ea5d5cf25a21de5a91ad Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Sun, 12 Sep 2021 19:29:22 +0200
Subject: [PATCH 05/10] nixos/security/wrappers: remove WRAPPER_PATH

This appears to be a leftover from 628e6a83.
---
 nixos/modules/security/wrappers/default.nix | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix
index 2f2c170e4607..8c9d0b487bbc 100644
--- a/nixos/modules/security/wrappers/default.nix
+++ b/nixos/modules/security/wrappers/default.nix
@@ -230,10 +230,6 @@ in
     system.activationScripts.wrappers =
       lib.stringAfter [ "specialfs" "users" ]
         ''
-          # Look in the system path and in the default profile for
-          # programs to be wrapped.
-          WRAPPER_PATH=${config.system.path}/bin:${config.system.path}/sbin
-
           chmod 755 "${parentWrapperDir}"
 
           # We want to place the tmpdirs for the wrappers to the parent dir.

From 41a498578e612cf34e2aa60eb0d8fc6a5b0d4d79 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Thu, 10 Jun 2021 01:46:26 +0200
Subject: [PATCH 06/10] nixos/mail: reuse security.wrappers type

---
 nixos/modules/services/mail/mail.nix | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/nixos/modules/services/mail/mail.nix b/nixos/modules/services/mail/mail.nix
index fed313e4738e..fcc7ff6db91b 100644
--- a/nixos/modules/services/mail/mail.nix
+++ b/nixos/modules/services/mail/mail.nix
@@ -1,4 +1,4 @@
-{ config, lib, ... }:
+{ config, options, lib, ... }:
 
 with lib;
 
@@ -11,6 +11,7 @@ with lib;
     services.mail = {
 
       sendmailSetuidWrapper = mkOption {
+        type = types.nullOr options.security.wrappers.type.nestedTypes.elemType;
         default = null;
         internal = true;
         description = ''

From 8f76a6eefcfa0c9904e0749f04b27090527ce09f Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Thu, 10 Jun 2021 01:47:55 +0200
Subject: [PATCH 07/10] nixos: add implict security.wrappers options

This is to keep the same permissions/setuid/setgid as before the change
in security.wrappers defaults.
---
 nixos/modules/programs/ccache.nix                   | 2 ++
 nixos/modules/programs/msmtp.nix                    | 2 ++
 nixos/modules/programs/ssmtp.nix                    | 2 ++
 nixos/modules/security/pam.nix                      | 1 +
 nixos/modules/services/mail/opensmtpd.nix           | 5 ++++-
 nixos/modules/services/mail/postfix.nix             | 4 ++++
 nixos/modules/services/networking/x2goserver.nix    | 2 ++
 nixos/modules/services/scheduling/fcron.nix         | 2 ++
 nixos/modules/services/x11/desktop-managers/cde.nix | 5 +++--
 9 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/nixos/modules/programs/ccache.nix b/nixos/modules/programs/ccache.nix
index d672e1da017a..35a4373f6128 100644
--- a/nixos/modules/programs/ccache.nix
+++ b/nixos/modules/programs/ccache.nix
@@ -28,7 +28,9 @@ in {
 
       # "nix-ccache --show-stats" and "nix-ccache --clear"
       security.wrappers.nix-ccache = {
+        owner = "nobody";
         group = "nixbld";
+        setuid = false;
         setgid = true;
         source = pkgs.writeScript "nix-ccache.pl" ''
           #!${pkgs.perl}/bin/perl
diff --git a/nixos/modules/programs/msmtp.nix b/nixos/modules/programs/msmtp.nix
index 217060e6b3b3..9c067bdc9695 100644
--- a/nixos/modules/programs/msmtp.nix
+++ b/nixos/modules/programs/msmtp.nix
@@ -78,6 +78,8 @@ in {
       source = "${pkgs.msmtp}/bin/sendmail";
       setuid = false;
       setgid = false;
+      owner = "root";
+      group = "root";
     };
 
     environment.etc."msmtprc".text = let
diff --git a/nixos/modules/programs/ssmtp.nix b/nixos/modules/programs/ssmtp.nix
index 8b500f0383f4..e28a14538ecd 100644
--- a/nixos/modules/programs/ssmtp.nix
+++ b/nixos/modules/programs/ssmtp.nix
@@ -181,6 +181,8 @@ in
       source = "${pkgs.ssmtp}/bin/sendmail";
       setuid = false;
       setgid = false;
+      owner = "root";
+      group = "root";
     };
 
   };
diff --git a/nixos/modules/security/pam.nix b/nixos/modules/security/pam.nix
index 163d75d7caf2..0bc774af3a66 100644
--- a/nixos/modules/security/pam.nix
+++ b/nixos/modules/security/pam.nix
@@ -871,6 +871,7 @@ in
       unix_chkpwd = {
         source = "${pkgs.pam}/sbin/unix_chkpwd.orig";
         owner = "root";
+        group = "nogroup";
         setuid = true;
       };
     };
diff --git a/nixos/modules/services/mail/opensmtpd.nix b/nixos/modules/services/mail/opensmtpd.nix
index c838d3b949db..dc209e8add4e 100644
--- a/nixos/modules/services/mail/opensmtpd.nix
+++ b/nixos/modules/services/mail/opensmtpd.nix
@@ -103,12 +103,15 @@ in {
     };
 
     security.wrappers.smtpctl = {
+      owner = "nobody";
       group = "smtpq";
+      setuid = false;
       setgid = true;
       source = "${cfg.package}/bin/smtpctl";
     };
 
-    services.mail.sendmailSetuidWrapper = mkIf cfg.setSendmail security.wrappers.smtpctl;
+    services.mail.sendmailSetuidWrapper = mkIf cfg.setSendmail
+      security.wrappers.smtpctl // { program = "sendmail"; };
 
     systemd.tmpfiles.rules = [
       "d /var/spool/smtpd 711 root - - -"
diff --git a/nixos/modules/services/mail/postfix.nix b/nixos/modules/services/mail/postfix.nix
index 9b0a5bba2feb..2b8edb9c51f8 100644
--- a/nixos/modules/services/mail/postfix.nix
+++ b/nixos/modules/services/mail/postfix.nix
@@ -673,6 +673,7 @@ in
       services.mail.sendmailSetuidWrapper = mkIf config.services.postfix.setSendmail {
         program = "sendmail";
         source = "${pkgs.postfix}/bin/sendmail";
+        owner = "nobody";
         group = setgidGroup;
         setuid = false;
         setgid = true;
@@ -681,6 +682,7 @@ in
       security.wrappers.mailq = {
         program = "mailq";
         source = "${pkgs.postfix}/bin/mailq";
+        owner = "nobody";
         group = setgidGroup;
         setuid = false;
         setgid = true;
@@ -689,6 +691,7 @@ in
       security.wrappers.postqueue = {
         program = "postqueue";
         source = "${pkgs.postfix}/bin/postqueue";
+        owner = "nobody";
         group = setgidGroup;
         setuid = false;
         setgid = true;
@@ -697,6 +700,7 @@ in
       security.wrappers.postdrop = {
         program = "postdrop";
         source = "${pkgs.postfix}/bin/postdrop";
+        owner = "nobody";
         group = setgidGroup;
         setuid = false;
         setgid = true;
diff --git a/nixos/modules/services/networking/x2goserver.nix b/nixos/modules/services/networking/x2goserver.nix
index 48020fc1ceca..554e51f9d4ff 100644
--- a/nixos/modules/services/networking/x2goserver.nix
+++ b/nixos/modules/services/networking/x2goserver.nix
@@ -88,12 +88,14 @@ in {
       source = "${pkgs.x2goserver}/lib/x2go/libx2go-server-db-sqlite3-wrapper.pl";
       owner = "x2go";
       group = "x2go";
+      setuid = false;
       setgid = true;
     };
     security.wrappers.x2goprintWrapper = {
       source = "${pkgs.x2goserver}/bin/x2goprint";
       owner = "x2go";
       group = "x2go";
+      setuid = false;
       setgid = true;
     };
 
diff --git a/nixos/modules/services/scheduling/fcron.nix b/nixos/modules/services/scheduling/fcron.nix
index 42bed21bf25b..4f5d99ddf38f 100644
--- a/nixos/modules/services/scheduling/fcron.nix
+++ b/nixos/modules/services/scheduling/fcron.nix
@@ -136,9 +136,11 @@ in
         owner = "fcron";
         group = "fcron";
         setgid = true;
+        setuid = false;
       };
       fcronsighup = {
         source = "${pkgs.fcron}/bin/fcronsighup";
+        owner = "root";
         group = "fcron";
       };
     };
diff --git a/nixos/modules/services/x11/desktop-managers/cde.nix b/nixos/modules/services/x11/desktop-managers/cde.nix
index 3f1575a0ca63..24ca82fca796 100644
--- a/nixos/modules/services/x11/desktop-managers/cde.nix
+++ b/nixos/modules/services/x11/desktop-managers/cde.nix
@@ -49,9 +49,10 @@ in {
     users.groups.mail = {};
     security.wrappers = {
       dtmail = {
-        source = "${pkgs.cdesktopenv}/bin/dtmail";
-        group = "mail";
         setgid = true;
+        owner = "nobody";
+        group = "mail";
+        source = "${pkgs.cdesktopenv}/bin/dtmail";
       };
     };
 

From fedd7cd6901646cb7e2a94a148d300f7b632d7e0 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Sun, 12 Sep 2021 18:53:48 +0200
Subject: [PATCH 08/10] nixos: explicitely set security.wrappers ownership

This is slightly more verbose and inconvenient, but it forces you
to think about what the wrapper ownership and permissions will be.
---
 nixos/modules/programs/bandwhich.nix          |  4 +-
 nixos/modules/programs/captive-browser.nix    |  4 ++
 nixos/modules/programs/firejail.nix           |  7 ++-
 nixos/modules/programs/gamemode.nix           |  2 +
 nixos/modules/programs/iftop.nix              |  4 +-
 nixos/modules/programs/iotop.nix              |  4 +-
 nixos/modules/programs/kbdlight.nix           |  7 ++-
 nixos/modules/programs/liboping.nix           |  4 +-
 nixos/modules/programs/mtr.nix                |  4 +-
 nixos/modules/programs/noisetorch.nix         |  4 +-
 nixos/modules/programs/shadow.nix             | 21 ++++---
 nixos/modules/programs/singularity.nix        |  7 ++-
 nixos/modules/programs/slock.nix              |  7 ++-
 nixos/modules/programs/traceroute.nix         |  4 +-
 nixos/modules/programs/udevil.nix             |  7 ++-
 nixos/modules/programs/wavemon.nix            |  4 +-
 nixos/modules/programs/wshowkeys.nix          |  7 ++-
 .../security/chromium-suid-sandbox.nix        |  7 ++-
 nixos/modules/security/doas.nix               |  9 ++-
 nixos/modules/security/duosec.nix             |  7 ++-
 nixos/modules/security/pam_usb.nix            | 14 ++++-
 nixos/modules/security/polkit.nix             | 14 ++++-
 nixos/modules/security/wrappers/default.nix   | 57 +++++++++++++------
 .../services/desktops/gnome/gnome-keyring.nix |  4 +-
 nixos/modules/services/mail/exim.nix          |  7 ++-
 nixos/modules/services/misc/mame.nix          |  4 +-
 nixos/modules/services/misc/weechat.nix       |  7 ++-
 nixos/modules/services/monitoring/incron.nix  |  7 ++-
 .../services/monitoring/zabbix-proxy.nix      |  7 ++-
 .../modules/services/networking/smokeping.nix | 14 ++++-
 nixos/modules/services/scheduling/cron.nix    |  7 ++-
 nixos/modules/services/scheduling/fcron.nix   |  1 +
 .../modules/services/video/replay-sorcery.nix |  4 +-
 .../x11/desktop-managers/enlightenment.nix    | 21 ++++++-
 .../services/x11/desktop-managers/plasma5.nix | 24 ++++++--
 nixos/modules/tasks/filesystems/ecryptfs.nix  | 14 ++++-
 nixos/modules/tasks/network-interfaces.nix    |  9 ++-
 nixos/modules/virtualisation/libvirtd.nix     |  3 +
 .../virtualisation/spice-usb-redirection.nix  |  6 +-
 39 files changed, 276 insertions(+), 72 deletions(-)

diff --git a/nixos/modules/programs/bandwhich.nix b/nixos/modules/programs/bandwhich.nix
index 1cffb5fa2765..610d602ad2cc 100644
--- a/nixos/modules/programs/bandwhich.nix
+++ b/nixos/modules/programs/bandwhich.nix
@@ -22,8 +22,10 @@ in {
   config = mkIf cfg.enable {
     environment.systemPackages = with pkgs; [ bandwhich ];
     security.wrappers.bandwhich = {
-      source = "${pkgs.bandwhich}/bin/bandwhich";
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_raw,cap_net_admin+ep";
+      source = "${pkgs.bandwhich}/bin/bandwhich";
     };
   };
 }
diff --git a/nixos/modules/programs/captive-browser.nix b/nixos/modules/programs/captive-browser.nix
index d7684d08c6c7..4e8abdeecf0b 100644
--- a/nixos/modules/programs/captive-browser.nix
+++ b/nixos/modules/programs/captive-browser.nix
@@ -105,11 +105,15 @@ in
       );
 
     security.wrappers.udhcpc = {
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_raw+p";
       source = "${pkgs.busybox}/bin/udhcpc";
     };
 
     security.wrappers.captive-browser = {
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_raw+p";
       source = pkgs.writeShellScript "captive-browser" ''
         export PREV_CONFIG_HOME="$XDG_CONFIG_HOME"
diff --git a/nixos/modules/programs/firejail.nix b/nixos/modules/programs/firejail.nix
index ad4ef1a39459..9384b01b3674 100644
--- a/nixos/modules/programs/firejail.nix
+++ b/nixos/modules/programs/firejail.nix
@@ -81,7 +81,12 @@ in {
   };
 
   config = mkIf cfg.enable {
-    security.wrappers.firejail.source = "${lib.getBin pkgs.firejail}/bin/firejail";
+    security.wrappers.firejail =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${lib.getBin pkgs.firejail}/bin/firejail";
+      };
 
     environment.systemPackages = [ pkgs.firejail ] ++ [ wrappedBins ];
   };
diff --git a/nixos/modules/programs/gamemode.nix b/nixos/modules/programs/gamemode.nix
index 03949bf98df6..102788f5b019 100644
--- a/nixos/modules/programs/gamemode.nix
+++ b/nixos/modules/programs/gamemode.nix
@@ -56,6 +56,8 @@ in
       polkit.enable = true;
       wrappers = mkIf cfg.enableRenice {
         gamemoded = {
+          owner = "root";
+          group = "root";
           source = "${pkgs.gamemode}/bin/gamemoded";
           capabilities = "cap_sys_nice+ep";
         };
diff --git a/nixos/modules/programs/iftop.nix b/nixos/modules/programs/iftop.nix
index a98a9a8187d4..c74714a9a6d6 100644
--- a/nixos/modules/programs/iftop.nix
+++ b/nixos/modules/programs/iftop.nix
@@ -11,8 +11,10 @@ in {
   config = mkIf cfg.enable {
     environment.systemPackages = [ pkgs.iftop ];
     security.wrappers.iftop = {
-      source = "${pkgs.iftop}/bin/iftop";
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_raw+p";
+      source = "${pkgs.iftop}/bin/iftop";
     };
   };
 }
diff --git a/nixos/modules/programs/iotop.nix b/nixos/modules/programs/iotop.nix
index 5512dbc62f72..b7c1c69f9ddd 100644
--- a/nixos/modules/programs/iotop.nix
+++ b/nixos/modules/programs/iotop.nix
@@ -10,8 +10,10 @@ in {
   };
   config = mkIf cfg.enable {
     security.wrappers.iotop = {
-      source = "${pkgs.iotop}/bin/iotop";
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_admin+p";
+      source = "${pkgs.iotop}/bin/iotop";
     };
   };
 }
diff --git a/nixos/modules/programs/kbdlight.nix b/nixos/modules/programs/kbdlight.nix
index 58e45872fac8..8a2a0057cf2d 100644
--- a/nixos/modules/programs/kbdlight.nix
+++ b/nixos/modules/programs/kbdlight.nix
@@ -11,6 +11,11 @@ in
 
   config = mkIf cfg.enable {
     environment.systemPackages = [ pkgs.kbdlight ];
-    security.wrappers.kbdlight.source = "${pkgs.kbdlight.out}/bin/kbdlight";
+    security.wrappers.kbdlight =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${pkgs.kbdlight.out}/bin/kbdlight";
+      };
   };
 }
diff --git a/nixos/modules/programs/liboping.nix b/nixos/modules/programs/liboping.nix
index 4e4c235ccde4..4433f9767d6e 100644
--- a/nixos/modules/programs/liboping.nix
+++ b/nixos/modules/programs/liboping.nix
@@ -13,8 +13,10 @@ in {
     security.wrappers = mkMerge (map (
       exec: {
         "${exec}" = {
-          source = "${pkgs.liboping}/bin/${exec}";
+          owner = "root";
+          group = "root";
           capabilities = "cap_net_raw+p";
+          source = "${pkgs.liboping}/bin/${exec}";
         };
       }
     ) [ "oping" "noping" ]);
diff --git a/nixos/modules/programs/mtr.nix b/nixos/modules/programs/mtr.nix
index 75b710c1584f..63516c58440e 100644
--- a/nixos/modules/programs/mtr.nix
+++ b/nixos/modules/programs/mtr.nix
@@ -31,8 +31,10 @@ in {
     environment.systemPackages = with pkgs; [ cfg.package ];
 
     security.wrappers.mtr-packet = {
-      source = "${cfg.package}/bin/mtr-packet";
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_raw+p";
+      source = "${cfg.package}/bin/mtr-packet";
     };
   };
 }
diff --git a/nixos/modules/programs/noisetorch.nix b/nixos/modules/programs/noisetorch.nix
index 5f3b0c8f5d1e..bca68b0064c0 100644
--- a/nixos/modules/programs/noisetorch.nix
+++ b/nixos/modules/programs/noisetorch.nix
@@ -18,8 +18,10 @@ in {
 
   config = mkIf cfg.enable {
     security.wrappers.noisetorch = {
-      source = "${cfg.package}/bin/noisetorch";
+      owner = "root";
+      group = "root";
       capabilities = "cap_sys_resource=+ep";
+      source = "${cfg.package}/bin/noisetorch";
     };
   };
 }
diff --git a/nixos/modules/programs/shadow.nix b/nixos/modules/programs/shadow.nix
index 386ded9d98b6..e021f184179d 100644
--- a/nixos/modules/programs/shadow.nix
+++ b/nixos/modules/programs/shadow.nix
@@ -43,6 +43,13 @@ let
 
     '';
 
+  mkSetuidRoot = source:
+    { setuid = true;
+      owner = "root";
+      group = "root";
+      inherit source;
+    };
+
 in
 
 {
@@ -109,14 +116,14 @@ in
       };
 
     security.wrappers = {
-      su.source        = "${pkgs.shadow.su}/bin/su";
-      sg.source        = "${pkgs.shadow.out}/bin/sg";
-      newgrp.source    = "${pkgs.shadow.out}/bin/newgrp";
-      newuidmap.source = "${pkgs.shadow.out}/bin/newuidmap";
-      newgidmap.source = "${pkgs.shadow.out}/bin/newgidmap";
+      su        = mkSetuidRoot "${pkgs.shadow.su}/bin/su";
+      sg        = mkSetuidRoot "${pkgs.shadow.out}/bin/sg";
+      newgrp    = mkSetuidRoot "${pkgs.shadow.out}/bin/newgrp";
+      newuidmap = mkSetuidRoot "${pkgs.shadow.out}/bin/newuidmap";
+      newgidmap = mkSetuidRoot "${pkgs.shadow.out}/bin/newgidmap";
     } // lib.optionalAttrs config.users.mutableUsers {
-      chsh.source      = "${pkgs.shadow.out}/bin/chsh";
-      passwd.source    = "${pkgs.shadow.out}/bin/passwd";
+      chsh   = mkSetuidRoot "${pkgs.shadow.out}/bin/chsh";
+      passwd = mkSetuidRoot "${pkgs.shadow.out}/bin/passwd";
     };
   };
 }
diff --git a/nixos/modules/programs/singularity.nix b/nixos/modules/programs/singularity.nix
index 6ac64a81fc24..db935abe4bb4 100644
--- a/nixos/modules/programs/singularity.nix
+++ b/nixos/modules/programs/singularity.nix
@@ -16,7 +16,12 @@ in {
 
   config = mkIf cfg.enable {
       environment.systemPackages = [ singularity ];
-      security.wrappers.singularity-suid.source = "${singularity}/libexec/singularity/bin/starter-suid.orig";
+      security.wrappers.singularity-suid =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${singularity}/libexec/singularity/bin/starter-suid.orig";
+      };
       systemd.tmpfiles.rules = [
         "d /var/singularity/mnt/session 0770 root root -"
         "d /var/singularity/mnt/final 0770 root root -"
diff --git a/nixos/modules/programs/slock.nix b/nixos/modules/programs/slock.nix
index 0e1281e62cd7..ce80fcc5d4a8 100644
--- a/nixos/modules/programs/slock.nix
+++ b/nixos/modules/programs/slock.nix
@@ -21,6 +21,11 @@ in
 
   config = mkIf cfg.enable {
     environment.systemPackages = [ pkgs.slock ];
-    security.wrappers.slock.source = "${pkgs.slock.out}/bin/slock";
+    security.wrappers.slock =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${pkgs.slock.out}/bin/slock";
+      };
   };
 }
diff --git a/nixos/modules/programs/traceroute.nix b/nixos/modules/programs/traceroute.nix
index 4eb0be3f0e0b..6e04057ac503 100644
--- a/nixos/modules/programs/traceroute.nix
+++ b/nixos/modules/programs/traceroute.nix
@@ -19,8 +19,10 @@ in {
 
   config = mkIf cfg.enable {
     security.wrappers.traceroute = {
-      source = "${pkgs.traceroute}/bin/traceroute";
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_raw+p";
+      source = "${pkgs.traceroute}/bin/traceroute";
     };
   };
 }
diff --git a/nixos/modules/programs/udevil.nix b/nixos/modules/programs/udevil.nix
index ba5670f9dfe9..0dc08c435df4 100644
--- a/nixos/modules/programs/udevil.nix
+++ b/nixos/modules/programs/udevil.nix
@@ -9,6 +9,11 @@ in {
   options.programs.udevil.enable = mkEnableOption "udevil";
 
   config = mkIf cfg.enable {
-    security.wrappers.udevil.source = "${lib.getBin pkgs.udevil}/bin/udevil";
+    security.wrappers.udevil =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${lib.getBin pkgs.udevil}/bin/udevil";
+      };
   };
 }
diff --git a/nixos/modules/programs/wavemon.nix b/nixos/modules/programs/wavemon.nix
index ac665fe4a023..e5ccacba75d4 100644
--- a/nixos/modules/programs/wavemon.nix
+++ b/nixos/modules/programs/wavemon.nix
@@ -21,8 +21,10 @@ in {
   config = mkIf cfg.enable {
     environment.systemPackages = with pkgs; [ wavemon ];
     security.wrappers.wavemon = {
-      source = "${pkgs.wavemon}/bin/wavemon";
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_admin+ep";
+      source = "${pkgs.wavemon}/bin/wavemon";
     };
   };
 }
diff --git a/nixos/modules/programs/wshowkeys.nix b/nixos/modules/programs/wshowkeys.nix
index 09b008af1d5d..f7b71d2bb0c8 100644
--- a/nixos/modules/programs/wshowkeys.nix
+++ b/nixos/modules/programs/wshowkeys.nix
@@ -17,6 +17,11 @@ in {
   };
 
   config = mkIf cfg.enable {
-    security.wrappers.wshowkeys.source = "${pkgs.wshowkeys}/bin/wshowkeys";
+    security.wrappers.wshowkeys =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${pkgs.wshowkeys}/bin/wshowkeys";
+      };
   };
 }
diff --git a/nixos/modules/security/chromium-suid-sandbox.nix b/nixos/modules/security/chromium-suid-sandbox.nix
index b83dbc4202a8..bb99c053f718 100644
--- a/nixos/modules/security/chromium-suid-sandbox.nix
+++ b/nixos/modules/security/chromium-suid-sandbox.nix
@@ -28,6 +28,11 @@ in
 
   config = mkIf cfg.enable {
     environment.systemPackages = [ sandbox ];
-    security.wrappers.${sandbox.passthru.sandboxExecutableName}.source = "${sandbox}/bin/${sandbox.passthru.sandboxExecutableName}";
+    security.wrappers.${sandbox.passthru.sandboxExecutableName} =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${sandbox}/bin/${sandbox.passthru.sandboxExecutableName}";
+      };
   };
 }
diff --git a/nixos/modules/security/doas.nix b/nixos/modules/security/doas.nix
index 27f6870aaf37..35f618b03e8e 100644
--- a/nixos/modules/security/doas.nix
+++ b/nixos/modules/security/doas.nix
@@ -241,9 +241,12 @@ in
       }
     ];
 
-    security.wrappers = {
-      doas.source = "${doas}/bin/doas";
-    };
+    security.wrappers.doas =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${doas}/bin/doas";
+      };
 
     environment.systemPackages = [
       doas
diff --git a/nixos/modules/security/duosec.nix b/nixos/modules/security/duosec.nix
index c47be80b9dc3..bbe246fe229e 100644
--- a/nixos/modules/security/duosec.nix
+++ b/nixos/modules/security/duosec.nix
@@ -186,7 +186,12 @@ in
   config = mkIf (cfg.ssh.enable || cfg.pam.enable) {
     environment.systemPackages = [ pkgs.duo-unix ];
 
-    security.wrappers.login_duo.source = "${pkgs.duo-unix.out}/bin/login_duo";
+    security.wrappers.login_duo =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${pkgs.duo-unix.out}/bin/login_duo";
+      };
 
     system.activationScripts = {
       login_duo = mkIf cfg.ssh.enable ''
diff --git a/nixos/modules/security/pam_usb.nix b/nixos/modules/security/pam_usb.nix
index c695ba075ca9..51d81e823f86 100644
--- a/nixos/modules/security/pam_usb.nix
+++ b/nixos/modules/security/pam_usb.nix
@@ -32,8 +32,18 @@ in
 
     # Make sure pmount and pumount are setuid wrapped.
     security.wrappers = {
-      pmount.source = "${pkgs.pmount.out}/bin/pmount";
-      pumount.source = "${pkgs.pmount.out}/bin/pumount";
+      pmount =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.pmount.out}/bin/pmount";
+        };
+      pumount =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.pmount.out}/bin/pumount";
+        };
     };
 
     environment.systemPackages = [ pkgs.pmount ];
diff --git a/nixos/modules/security/polkit.nix b/nixos/modules/security/polkit.nix
index f556cca23cdc..d9c58152f1fa 100644
--- a/nixos/modules/security/polkit.nix
+++ b/nixos/modules/security/polkit.nix
@@ -83,8 +83,18 @@ in
     security.pam.services.polkit-1 = {};
 
     security.wrappers = {
-      pkexec.source = "${pkgs.polkit.bin}/bin/pkexec";
-      polkit-agent-helper-1.source = "${pkgs.polkit.out}/lib/polkit-1/polkit-agent-helper-1";
+      pkexec =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.polkit.bin}/bin/pkexec";
+        };
+      polkit-agent-helper-1 =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.polkit.out}/lib/polkit-1/polkit-agent-helper-1";
+        };
     };
 
     systemd.tmpfiles.rules = [
diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix
index 8c9d0b487bbc..2697ab0bde8f 100644
--- a/nixos/modules/security/wrappers/default.nix
+++ b/nixos/modules/security/wrappers/default.nix
@@ -33,12 +33,10 @@ let
       };
     options.owner = lib.mkOption
       { type = lib.types.str;
-        default = "root";
         description = "The owner of the wrapper program.";
       };
     options.group = lib.mkOption
       { type = lib.types.str;
-        default = "root";
         description = "The group of the wrapper program.";
       };
     options.permissions = lib.mkOption
@@ -74,7 +72,7 @@ let
       };
     options.setuid = lib.mkOption
       { type = lib.types.bool;
-        default = true;
+        default = false;
         description = "Whether to add the setuid bit the wrapper program.";
       };
     options.setgid = lib.mkOption
@@ -156,13 +154,30 @@ in
       default = {};
       example = lib.literalExample
         ''
-          { sendmail.source = "/nix/store/.../bin/sendmail";
-            ping = {
-              source  = "${pkgs.iputils.out}/bin/ping";
-              owner   = "nobody";
-              group   = "nogroup";
-              capabilities = "cap_net_raw+ep";
-            };
+          {
+            # a setuid root program
+            doas =
+              { setuid = true;
+                owner = "root";
+                group = "root";
+                source = "''${pkgs.doas}/bin/doas";
+              };
+
+            # a setgid program
+            locate =
+              { setgid = true;
+                owner = "root";
+                group = "mlocate";
+                source = "''${pkgs.locate}/bin/locate";
+              };
+
+            # a program with the CAP_NET_RAW capability
+            ping =
+              { owner = "root";
+                group = "root";
+                capabilities = "cap_net_raw+ep";
+                source = "''${pkgs.iputils.out}/bin/ping";
+              };
           }
         '';
       description = ''
@@ -198,13 +213,21 @@ in
         }
       ) wrappers;
 
-    security.wrappers = {
-      # These are mount related wrappers that require the +s permission.
-      fusermount.source = "${pkgs.fuse}/bin/fusermount";
-      fusermount3.source = "${pkgs.fuse3}/bin/fusermount3";
-      mount.source = "${lib.getBin pkgs.util-linux}/bin/mount";
-      umount.source = "${lib.getBin pkgs.util-linux}/bin/umount";
-    };
+    security.wrappers =
+      let
+        mkSetuidRoot = source:
+          { setuid = true;
+            owner = "root";
+            group = "root";
+            inherit source;
+          };
+      in
+      { # These are mount related wrappers that require the +s permission.
+        fusermount  = mkSetuidRoot "${pkgs.fuse}/bin/fusermount";
+        fusermount3 = mkSetuidRoot "${pkgs.fuse3}/bin/fusermount3";
+        mount  = mkSetuidRoot "${lib.getBin pkgs.util-linux}/bin/mount";
+        umount = mkSetuidRoot "${lib.getBin pkgs.util-linux}/bin/umount";
+      };
 
     boot.specialFileSystems.${parentWrapperDir} = {
       fsType = "tmpfs";
diff --git a/nixos/modules/services/desktops/gnome/gnome-keyring.nix b/nixos/modules/services/desktops/gnome/gnome-keyring.nix
index cda44bab8bfa..d821da164beb 100644
--- a/nixos/modules/services/desktops/gnome/gnome-keyring.nix
+++ b/nixos/modules/services/desktops/gnome/gnome-keyring.nix
@@ -52,8 +52,10 @@ with lib;
     security.pam.services.login.enableGnomeKeyring = true;
 
     security.wrappers.gnome-keyring-daemon = {
-      source = "${pkgs.gnome.gnome-keyring}/bin/gnome-keyring-daemon";
+      owner = "root";
+      group = "root";
       capabilities = "cap_ipc_lock=ep";
+      source = "${pkgs.gnome.gnome-keyring}/bin/gnome-keyring-daemon";
     };
 
   };
diff --git a/nixos/modules/services/mail/exim.nix b/nixos/modules/services/mail/exim.nix
index 8927d84b478c..25b533578c94 100644
--- a/nixos/modules/services/mail/exim.nix
+++ b/nixos/modules/services/mail/exim.nix
@@ -104,7 +104,12 @@ in
       gid = config.ids.gids.exim;
     };
 
-    security.wrappers.exim.source = "${cfg.package}/bin/exim";
+    security.wrappers.exim =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${cfg.package}/bin/exim";
+      };
 
     systemd.services.exim = {
       description = "Exim Mail Daemon";
diff --git a/nixos/modules/services/misc/mame.nix b/nixos/modules/services/misc/mame.nix
index 4b9a04be7c29..dd6c5ef9aa00 100644
--- a/nixos/modules/services/misc/mame.nix
+++ b/nixos/modules/services/misc/mame.nix
@@ -45,8 +45,10 @@ in
     environment.systemPackages = [ pkgs.mame ];
 
     security.wrappers."${mame}" = {
-      source = "${pkgs.mame}/bin/${mame}";
+      owner = "root";
+      group = "root";
       capabilities = "cap_net_admin,cap_net_raw+eip";
+      source = "${pkgs.mame}/bin/${mame}";
     };
 
     systemd.services.mame = {
diff --git a/nixos/modules/services/misc/weechat.nix b/nixos/modules/services/misc/weechat.nix
index b71250f62e0f..9ac2b0ea490c 100644
--- a/nixos/modules/services/misc/weechat.nix
+++ b/nixos/modules/services/misc/weechat.nix
@@ -52,7 +52,12 @@ in
       wants = [ "network.target" ];
     };
 
-    security.wrappers.screen.source = "${pkgs.screen}/bin/screen";
+    security.wrappers.screen =
+      { setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${pkgs.screen}/bin/screen";
+      };
   };
 
   meta.doc = ./weechat.xml;
diff --git a/nixos/modules/services/monitoring/incron.nix b/nixos/modules/services/monitoring/incron.nix
index dc97af58562e..255e1d9e30ba 100644
--- a/nixos/modules/services/monitoring/incron.nix
+++ b/nixos/modules/services/monitoring/incron.nix
@@ -71,7 +71,12 @@ in
 
     environment.systemPackages = [ pkgs.incron ];
 
-    security.wrappers.incrontab.source = "${pkgs.incron}/bin/incrontab";
+    security.wrappers.incrontab =
+    { setuid = true;
+      owner = "root";
+      group = "root";
+      source = "${pkgs.incron}/bin/incrontab";
+    };
 
     # incron won't read symlinks
     environment.etc."incron.d/system" = {
diff --git a/nixos/modules/services/monitoring/zabbix-proxy.nix b/nixos/modules/services/monitoring/zabbix-proxy.nix
index 2c8b8b92cb38..8c7a2970e9b3 100644
--- a/nixos/modules/services/monitoring/zabbix-proxy.nix
+++ b/nixos/modules/services/monitoring/zabbix-proxy.nix
@@ -262,7 +262,12 @@ in
     };
 
     security.wrappers = {
-      fping.source = "${pkgs.fping}/bin/fping";
+      fping =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.fping}/bin/fping";
+        };
     };
 
     systemd.services.zabbix-proxy = {
diff --git a/nixos/modules/services/networking/smokeping.nix b/nixos/modules/services/networking/smokeping.nix
index 4470c18fd533..0a6477487369 100644
--- a/nixos/modules/services/networking/smokeping.nix
+++ b/nixos/modules/services/networking/smokeping.nix
@@ -278,8 +278,18 @@ in
       }
     ];
     security.wrappers = {
-      fping.source = "${pkgs.fping}/bin/fping";
-      fping6.source = "${pkgs.fping}/bin/fping6";
+      fping =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.fping}/bin/fping";
+        };
+      fping6 =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.fping}/bin/fping6";
+        };
     };
     environment.systemPackages = [ pkgs.fping ];
     users.users.${cfg.user} = {
diff --git a/nixos/modules/services/scheduling/cron.nix b/nixos/modules/services/scheduling/cron.nix
index 3bc31832946b..c28956b3bfeb 100644
--- a/nixos/modules/services/scheduling/cron.nix
+++ b/nixos/modules/services/scheduling/cron.nix
@@ -93,7 +93,12 @@ in
 
     { services.cron.enable = mkDefault (allFiles != []); }
     (mkIf (config.services.cron.enable) {
-      security.wrappers.crontab.source = "${cronNixosPkg}/bin/crontab";
+      security.wrappers.crontab =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${cronNixosPkg}/bin/crontab";
+        };
       environment.systemPackages = [ cronNixosPkg ];
       environment.etc.crontab =
         { source = pkgs.runCommand "crontabs" { inherit allFiles; preferLocalBuild = true; }
diff --git a/nixos/modules/services/scheduling/fcron.nix b/nixos/modules/services/scheduling/fcron.nix
index 4f5d99ddf38f..acaa995f7395 100644
--- a/nixos/modules/services/scheduling/fcron.nix
+++ b/nixos/modules/services/scheduling/fcron.nix
@@ -142,6 +142,7 @@ in
         source = "${pkgs.fcron}/bin/fcronsighup";
         owner = "root";
         group = "fcron";
+        setuid = true;
       };
     };
     systemd.services.fcron = {
diff --git a/nixos/modules/services/video/replay-sorcery.nix b/nixos/modules/services/video/replay-sorcery.nix
index d78e782c7968..7ce5be8a5a1c 100644
--- a/nixos/modules/services/video/replay-sorcery.nix
+++ b/nixos/modules/services/video/replay-sorcery.nix
@@ -44,8 +44,10 @@ in
 
     security.wrappers = mkIf cfg.enableSysAdminCapability {
       replay-sorcery = {
-        source = "${pkgs.replay-sorcery}/bin/replay-sorcery";
+        owner = "root";
+        group = "root";
         capabilities = "cap_sys_admin+ep";
+        source = "${pkgs.replay-sorcery}/bin/replay-sorcery";
       };
     };
 
diff --git a/nixos/modules/services/x11/desktop-managers/enlightenment.nix b/nixos/modules/services/x11/desktop-managers/enlightenment.nix
index 3a7ab64510b5..e3d876e82fdd 100644
--- a/nixos/modules/services/x11/desktop-managers/enlightenment.nix
+++ b/nixos/modules/services/x11/desktop-managers/enlightenment.nix
@@ -65,9 +65,24 @@ in
 
     # Wrappers for programs installed by enlightenment that should be setuid
     security.wrappers = {
-      enlightenment_ckpasswd.source = "${pkgs.enlightenment.enlightenment}/lib/enlightenment/utils/enlightenment_ckpasswd";
-      enlightenment_sys.source = "${pkgs.enlightenment.enlightenment}/lib/enlightenment/utils/enlightenment_sys";
-      enlightenment_system.source = "${pkgs.enlightenment.enlightenment}/lib/enlightenment/utils/enlightenment_system";
+      enlightenment_ckpasswd =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.enlightenment.enlightenment}/lib/enlightenment/utils/enlightenment_ckpasswd";
+        };
+      enlightenment_sys =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.enlightenment.enlightenment}/lib/enlightenment/utils/enlightenment_sys";
+        };
+      enlightenment_system =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.enlightenment.enlightenment}/lib/enlightenment/utils/enlightenment_system";
+        };
     };
 
     environment.etc."X11/xkb".source = xcfg.xkbDir;
diff --git a/nixos/modules/services/x11/desktop-managers/plasma5.nix b/nixos/modules/services/x11/desktop-managers/plasma5.nix
index aac905fea437..d8dc2675f068 100644
--- a/nixos/modules/services/x11/desktop-managers/plasma5.nix
+++ b/nixos/modules/services/x11/desktop-managers/plasma5.nix
@@ -197,12 +197,24 @@ in
       };
 
       security.wrappers = {
-        kcheckpass.source = "${lib.getBin libsForQt5.kscreenlocker}/libexec/kcheckpass";
-        start_kdeinit.source = "${lib.getBin libsForQt5.kinit}/libexec/kf5/start_kdeinit";
-        kwin_wayland = {
-          source = "${lib.getBin plasma5.kwin}/bin/kwin_wayland";
-          capabilities = "cap_sys_nice+ep";
-        };
+        kcheckpass =
+          { setuid = true;
+            owner = "root";
+            group = "root";
+            source = "${lib.getBin libsForQt5.kscreenlocker}/libexec/kcheckpass";
+          };
+        start_kdeinit =
+          { setuid = true;
+            owner = "root";
+            group = "root";
+            source = "${lib.getBin libsForQt5.kinit}/libexec/kf5/start_kdeinit";
+          };
+        kwin_wayland =
+          { owner = "root";
+            group = "root";
+            capabilities = "cap_sys_nice+ep";
+            source = "${lib.getBin plasma5.kwin}/bin/kwin_wayland";
+          };
       };
 
       # DDC support
diff --git a/nixos/modules/tasks/filesystems/ecryptfs.nix b/nixos/modules/tasks/filesystems/ecryptfs.nix
index 12a407cabbfb..8138e6591610 100644
--- a/nixos/modules/tasks/filesystems/ecryptfs.nix
+++ b/nixos/modules/tasks/filesystems/ecryptfs.nix
@@ -7,8 +7,18 @@ with lib;
   config = mkIf (any (fs: fs == "ecryptfs") config.boot.supportedFilesystems) {
     system.fsPackages = [ pkgs.ecryptfs ];
     security.wrappers = {
-      "mount.ecryptfs_private".source = "${pkgs.ecryptfs.out}/bin/mount.ecryptfs_private";
-      "umount.ecryptfs_private".source = "${pkgs.ecryptfs.out}/bin/umount.ecryptfs_private";
+      "mount.ecryptfs_private" =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.ecryptfs.out}/bin/mount.ecryptfs_private";
+        };
+      "umount.ecryptfs_private" =
+        { setuid = true;
+          owner = "root";
+          group = "root";
+          source = "${pkgs.ecryptfs.out}/bin/umount.ecryptfs_private";
+        };
     };
   };
 }
diff --git a/nixos/modules/tasks/network-interfaces.nix b/nixos/modules/tasks/network-interfaces.nix
index 8f9c66b01572..d934e3cf0224 100644
--- a/nixos/modules/tasks/network-interfaces.nix
+++ b/nixos/modules/tasks/network-interfaces.nix
@@ -1133,11 +1133,16 @@ in
     # kernel because we need the ambient capability
     security.wrappers = if (versionAtLeast (getVersion config.boot.kernelPackages.kernel) "4.3") then {
       ping = {
-        source  = "${pkgs.iputils.out}/bin/ping";
+        owner = "root";
+        group = "root";
         capabilities = "cap_net_raw+p";
+        source = "${pkgs.iputils.out}/bin/ping";
       };
     } else {
-      ping.source = "${pkgs.iputils.out}/bin/ping";
+      setuid = true;
+      owner = "root";
+      group = "root";
+      source = "${pkgs.iputils.out}/bin/ping";
     };
     security.apparmor.policies."bin.ping".profile = lib.mkIf config.security.apparmor.policies."bin.ping".enable (lib.mkAfter ''
       /run/wrappers/bin/ping {
diff --git a/nixos/modules/virtualisation/libvirtd.nix b/nixos/modules/virtualisation/libvirtd.nix
index f45f1802d91c..3c291397a998 100644
--- a/nixos/modules/virtualisation/libvirtd.nix
+++ b/nixos/modules/virtualisation/libvirtd.nix
@@ -183,6 +183,9 @@ in {
     };
 
     security.wrappers.qemu-bridge-helper = {
+      setuid = true;
+      owner = "root";
+      group = "root";
       source = "/run/${dirName}/nix-helpers/qemu-bridge-helper";
     };
 
diff --git a/nixos/modules/virtualisation/spice-usb-redirection.nix b/nixos/modules/virtualisation/spice-usb-redirection.nix
index 4168cebe79b1..255327f2622c 100644
--- a/nixos/modules/virtualisation/spice-usb-redirection.nix
+++ b/nixos/modules/virtualisation/spice-usb-redirection.nix
@@ -14,9 +14,11 @@
 
   config = lib.mkIf config.virtualisation.spiceUSBRedirection.enable {
     environment.systemPackages = [ pkgs.spice-gtk ]; # For polkit actions
-    security.wrappers.spice-client-glib-usb-acl-helper ={
-      source = "${pkgs.spice-gtk}/bin/spice-client-glib-usb-acl-helper";
+    security.wrappers.spice-client-glib-usb-acl-helper = {
+      owner = "root";
+      group = "root";
       capabilities = "cap_fowner+ep";
+      source = "${pkgs.spice-gtk}/bin/spice-client-glib-usb-acl-helper";
     };
   };
 

From 65e83b0e23038abf3acdc9d5f6a550c05be10acd Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Sun, 12 Sep 2021 19:04:55 +0200
Subject: [PATCH 09/10] nixos: fix nobody/nogroup in security.wrappers

---
 nixos/modules/security/pam.nix | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/nixos/modules/security/pam.nix b/nixos/modules/security/pam.nix
index 0bc774af3a66..8b1f653d4e2c 100644
--- a/nixos/modules/security/pam.nix
+++ b/nixos/modules/security/pam.nix
@@ -869,10 +869,10 @@ in
 
     security.wrappers = {
       unix_chkpwd = {
-        source = "${pkgs.pam}/sbin/unix_chkpwd.orig";
-        owner = "root";
-        group = "nogroup";
         setuid = true;
+        owner = "root";
+        group = "root";
+        source = "${pkgs.pam}/sbin/unix_chkpwd.orig";
       };
     };
 

From 27b0c53d237b6c0411dc5798376b0ba6fbad0df0 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Sun, 12 Sep 2021 20:53:44 +0200
Subject: [PATCH 10/10] doc/release-notes: mention security.wrappers changes

---
 .../manual/from_md/release-notes/rl-2111.section.xml   | 10 ++++++++++
 nixos/doc/manual/release-notes/rl-2111.section.md      |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml b/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml
index c723a6dd2add..d6bb5d06ede5 100644
--- a/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml
+++ b/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml
@@ -244,6 +244,16 @@
   <section xml:id="sec-release-21.11-incompatibilities">
     <title>Backward Incompatibilities</title>
     <itemizedlist>
+      <listitem>
+        <para>
+          The <literal>security.wrappers</literal> option now requires
+          to always specify an owner, group and whether the
+          setuid/setgid bit should be set. This is motivated by the fact
+          that before NixOS 21.11, specifying either setuid or setgid
+          but not owner/group resulted in wrappers owned by
+          nobody/nogroup, which is unsafe.
+        </para>
+      </listitem>
       <listitem>
         <para>
           The <literal>paperless</literal> module and package have been
diff --git a/nixos/doc/manual/release-notes/rl-2111.section.md b/nixos/doc/manual/release-notes/rl-2111.section.md
index 440069988d0c..a96d8ada4d9e 100644
--- a/nixos/doc/manual/release-notes/rl-2111.section.md
+++ b/nixos/doc/manual/release-notes/rl-2111.section.md
@@ -75,6 +75,8 @@ subsonic-compatible api. Available as [navidrome](#opt-services.navidrome.enable
 
 ## Backward Incompatibilities {#sec-release-21.11-incompatibilities}
 
+- The `security.wrappers` option now requires to always specify an owner, group and whether the setuid/setgid bit should be set.
+  This is motivated by the fact that before NixOS 21.11, specifying either setuid or setgid but not owner/group resulted in wrappers owned by nobody/nogroup, which is unsafe.
 
 - The `paperless` module and package have been removed. All users should migrate to the
   successor `paperless-ng` instead. The Paperless project [has been