setcap-wrapper: Addressing more PR feedback, unifying drvs, and cleaning up a bit

4 files changed, 48 insertions, 111 deletions

diff --git a/nixos/modules/security/wrappers/default.nix b/nixos/modules/security/wrappers/default.nix
index d12209b375b8..69b62d7b2ff2 100644
--- a/nixos/modules/security/wrappers/default.nix
+++ b/nixos/modules/security/wrappers/default.nix
@@ -3,17 +3,27 @@ let
 
   inherit (config.security) wrapperDir;
 
-  isNotNull = v: if v != null || v != "" then true else false;
-
-  cfg = config.security.wrappers;
-
-  setcapWrappers = import ./setcap-wrapper-drv.nix {
-    inherit config lib pkgs;
-  };
-
-  setuidWrappers = import ./setuid-wrapper-drv.nix {
-    inherit config lib pkgs;
-  };
+  wrappers  = config.security.wrappers;
+  mkWrapper = { program, source ? null, ...}: ''
+    if ! source=${if source != null then source else "$(readlink -f $(PATH=$WRAPPER_PATH type -tP ${program}))"}; then
+        # If we can't find the program, fall back to the
+        # system profile.
+        source=/nix/var/nix/profiles/default/bin/${program}
+    fi
+
+    gcc -Wall -O2 -DSOURCE_PROG=\"$source\" -DWRAPPER_DIR=\"${config.security.wrapperDir}\" \
+        -lcap-ng -lcap ${./permissions-wrapper.c} -o $out/bin/${program}.wrapper -L ${pkgs.libcap.lib}/lib -L ${pkgs.libcap_ng}/lib \
+        -I ${pkgs.libcap.dev}/include -I ${pkgs.libcap_ng}/include -I ${pkgs.linuxHeaders}/include
+  '';
+
+  wrappedPrograms = pkgs.stdenv.mkDerivation {
+    name         = "permissions-wrapper";
+    unpackPhase  = "true";
+    installPhase = ''
+      mkdir -p $out/bin
+      ${lib.concatMapStrings mkWrapper wrappers}
+    '';
+  }
 
   ###### Activation script for the setcap wrappers
   mkSetcapProgram =
@@ -23,8 +33,10 @@ let
     , owner  ? "nobody"
     , group  ? "nogroup"
     ...
-    }: ''
-      cp ${setcapWrappers}/bin/${program}.wrapper $wrapperDir/${program}
+    }: 
+    assert (lib.versionAtLeast (lib.getVersion config.boot.kernelPackages.kernel) "4.3");
+    ''
+      cp ${wrappedPrograms}/bin/${program}.wrapper $wrapperDir/${program}
 
       # Prevent races
       chmod 0000 $wrapperDir/${program}
@@ -33,9 +45,6 @@ let
       # 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.
-      #
-      # Only set the capabilities though if we're being told to
-      # do so.
       ${pkgs.libcap.out}/bin/setcap "cap_setpcap,${capabilities}" $wrapperDir/${program}
 
       # Set the executable bit
@@ -53,7 +62,7 @@ let
     , permissions ? "u+rx,g+x,o+x"
     ...
     }: ''
-      cp ${setuidWrappers}/bin/${program}.wrapper $wrapperDir/${program}
+      cp ${wrappedPrograms}/bin/${program}.wrapper $wrapperDir/${program}
 
       # Prevent races
       chmod 0000 $wrapperDir/${program}
@@ -147,10 +156,10 @@ in
 
   ###### implementation
   config = {
-    # Make sure our setcap-wrapper dir exports to the PATH env
-    # variable when initializing the shell
+    # Make sure our wrapperDir exports to the PATH env variable when
+    # initializing the shell
     environment.extraInit = ''
-      # The permissions wrappers override other bin directories.
+      # Wrappers override other bin directories.
       export PATH="${wrapperDir}:$PATH"
     '';
 
@@ -162,16 +171,17 @@ in
             config.security.setuidPrograms)
             ++ lib.mapAttrsToList
                  (n: v: (if v ? "program" then v else v // {program=n;}))
-                 cfg.wrappers;
+                 wrappers;
 
-        wrapperPrograms =
+        mkWrappedPrograms =
           builtins.map
-            (s: if (s ? "setuid"  && s.setuid  == true) ||
+            (s: if (s ? "capabilities")
+                then mkSetcapProgram s
+                else if 
+                   (s ? "setuid"  && s.setuid  == true) ||
                    (s ? "setguid" && s.setguid == true) ||
                    (s ? "permissions")
                 then mkSetuidProgram s
-                else if (s ? "capabilities")
-                then mkSetcapProgram s
                 else ""
             ) programs;
 
@@ -185,7 +195,7 @@ in
           wrapperDir=$(mktemp --directory --tmpdir=${wrapperDir} wrappers.XXXXXXXXXX)
           chmod a+rx $wrapperDir
 
-          ${lib.concatStringsSep "\n" (builtins.filter isNotNull cfg.wrappers)}
+          ${lib.concatStringsSep "\n" mkWrappedPrograms}
         '';
   };
 }
diff --git a/nixos/modules/security/wrappers/permissions-wrapper.c b/nixos/modules/security/wrappers/permissions-wrapper.c
index cb9d8d6b37b2..608bd3a378c7 100644
--- a/nixos/modules/security/wrappers/permissions-wrapper.c
+++ b/nixos/modules/security/wrappers/permissions-wrapper.c
@@ -26,16 +26,6 @@ extern char **environ;
 static char * sourceProg = SOURCE_PROG;
 static char * wrapperDir = WRAPPER_DIR;
 
-// Make sure we have the WRAPPER_TYPE macro specified at compile
-// time...
-#ifdef WRAPPER_SETCAP
-static char * wrapperType = "setcap";
-#elif defined WRAPPER_SETUID
-static char * wrapperType = "setuid";
-#else
-#error "Program must be compiled with either the WRAPPER_SETCAP or WRAPPER_SETUID macro"
-#endif
-
 // Update the capabilities of the running process to include the given
 // capability in the Ambient set.
 static void set_ambient_cap(cap_value_t cap)
@@ -66,7 +56,7 @@ static int make_caps_ambient(const char *selfPath)
 
     if(!caps)
     {
-        fprintf(stderr, "could not retreive the capability set for this file\n");
+        fprintf(stderr, "no caps set or could not retrieve the caps for this file, not doing anything...\n");
         return 1;
     }
 
@@ -171,6 +161,16 @@ int main(int argc, char * * argv)
 
     assert(selfPathSize > 0);
 
+    // Assert we have room for the zero byte, this ensures the path
+    // isn't being truncated because it's too big for the buffer.
+    //
+    // A better way to handle this might be to use something like the
+    // whereami library (https://github.com/gpakosz/whereami) or a
+    // loop that resizes the buffer and re-reads the link if the
+    // contents are being truncated.
+    assert(selfPathSize < sizeof(selfPath));
+
+    // Set the zero byte since readlink doesn't do that for us.
     selfPath[selfPathSize] = '\0';
 
     // Make sure that we are being executed from the right location,
@@ -207,8 +207,7 @@ int main(int argc, char * * argv)
     // Read the capabilities set on the file and raise them in to the
     // Ambient set so the program we're wrapping receives the
     // capabilities too!
-    if (strcmp(wrapperType, "setcap") == 0)
-        assert(!make_caps_ambient(selfPath));
+    make_caps_ambient(selfPath);
 
     execve(sourceProg, argv, environ);
     
diff --git a/nixos/modules/security/wrappers/setcap-wrapper-drv.nix b/nixos/modules/security/wrappers/setcap-wrapper-drv.nix
deleted file mode 100644
index 03dca5c9f42b..000000000000
--- a/nixos/modules/security/wrappers/setcap-wrapper-drv.nix
+++ /dev/null
@@ -1,37 +0,0 @@
-{ config, lib, pkgs, ... }:
-
-let  
-     cfg = config.security.wrappers;
-
-     # Produce a shell-code splice intended to be stitched into one of
-     # the build or install phases within the derivation.
-     mkSetcapWrapper = { program, source ? null, ...}: ''
-       if ! source=${if source != null then source else "$(readlink -f $(PATH=$PERMISSIONS_WRAPPER_PATH type -tP ${program}))"}; then
-         # If we can't find the program, fall back to the
-         # system profile.
-         source=/nix/var/nix/profiles/default/bin/${program}
-       fi
-
-       gcc -Wall -O2 -DWRAPPER_SETCAP=1 -DSOURCE_PROG=\"$source\" -DWRAPPER_DIR=\"${config.security.run-wrapperDir}\" \
-           -lcap-ng -lcap ${./permissions-wrapper.c} -o $out/bin/${program}.wrapper -L ${pkgs.libcap.lib}/lib -L ${pkgs.libcap_ng}/lib \
-           -I ${pkgs.libcap.dev}/include -I ${pkgs.libcap_ng}/include -I ${pkgs.linuxHeaders}/include
-     '';
-in
-
-# This is only useful for Linux platforms and a kernel version of
-# 4.3 or greater
-assert pkgs.stdenv.isLinux;
-assert lib.versionAtLeast (lib.getVersion config.boot.kernelPackages.kernel) "4.3";
-
-pkgs.stdenv.mkDerivation {
-  name         = "setcap-wrapper";
-  unpackPhase  = "true";
-  buildInputs  = [ pkgs.linuxHeaders ];
-  installPhase = ''
-    mkdir -p $out/bin
-
-    # Concat together all of our shell splices to compile
-    # binary wrapper programs for all configured setcap programs.
-    ${lib.concatMapStrings mkSetcapWrapper cfg.setcap}
-  '';
-}
diff --git a/nixos/modules/security/wrappers/setuid-wrapper-drv.nix b/nixos/modules/security/wrappers/setuid-wrapper-drv.nix
deleted file mode 100644
index e08ae799bf40..000000000000
--- a/nixos/modules/security/wrappers/setuid-wrapper-drv.nix
+++ /dev/null
@@ -1,35 +0,0 @@
-{ config, lib, pkgs, ... }:
-
-let  
-     cfg = config.security.wrappers;
-
-     # Produce a shell-code splice intended to be stitched into one of
-     # the build or install phases within the derivation.
-     mkSetuidWrapper = { program, source ? null, ...}: ''
-       if ! source=${if source != null then source else "$(readlink -f $(PATH=$WRAPPER_PATH type -tP ${program}))"}; then
-           # If we can't find the program, fall back to the
-           # system profile.
-           source=/nix/var/nix/profiles/default/bin/${program}
-       fi
-
-       gcc -Wall -O2 -DWRAPPER_SETUID=1 -DSOURCE_PROG=\"$source\" -DWRAPPER_DIR=\"${config.security.run-wrapperDir}\" \
-           -lcap-ng -lcap ${./permissions-wrapper.c} -o $out/bin/${program}.wrapper -L ${pkgs.libcap.lib}/lib -L ${pkgs.libcap_ng}/lib \
-           -I ${pkgs.libcap.dev}/include -I ${pkgs.libcap_ng}/include -I ${pkgs.linuxHeaders}/include
-     '';
-in
-
-# This is only useful for Linux platforms and a kernel version of
-# 4.3 or greater
-assert pkgs.stdenv.isLinux;
-
-pkgs.stdenv.mkDerivation {
-  name         = "setuid-wrapper";
-  unpackPhase  = "true";
-  installPhase = ''
-    mkdir -p $out/bin
-
-    # Concat together all of our shell splices to compile
-    # binary wrapper programs for all configured setcap programs.
-    ${lib.concatMapStrings mkSetuidWrapper cfg.setuid}
-  '';
-}