* Move the setuid wrappers activation scriptlet to

modules/security/setuid-wrappers.nix.
* Removed the "path" activation scriptlet.  The partial ordering was
  underspecified (there was nothing ensuring that it came near the end
  of the activation script), and it wasn't needed in any case.

svn path=/nixos/branches/modular-nixos/; revision=15726

etc/default.nix

@@ -39,7 +39,7 @@ let
   nixEnvVars = config.nix.envVars;
   modulesTree = config.system.modulesTree;
   nssModulesPath = config.system.nssModules.path;
-  wrapperDir = config.system.wrapperDir;
+  wrapperDir = config.security.wrapperDir;
   systemPath = config.system.path;
   binsh = config.system.build.binsh;
 

helpers/setuid/default.nix

@@ -1,10 +0,0 @@
-{stdenv, wrapperDir}:
-
-stdenv.mkDerivation {
-  name = "setuid-wrapper";
-  buildCommand = ''
-    ensureDir $out/bin
-    gcc -Wall -O2 -DWRAPPER_DIR=\"${wrapperDir}\" ${./setuid-wrapper.c} -o $out/bin/setuid-wrapper
-    strip -s $out/bin/setuid-wrapper
-  '';
-}

modules/security/setuid-wrappers.nix

@@ -1,47 +1,127 @@
 {pkgs, config, ...}:
 
+with pkgs.lib;
+
 let
-  inherit (pkgs.lib) mergeOneOption mkOption mkIf;
+
+  options = {
+    security = {
+      setuidPrograms = mkOption {
+        default = [
+          "passwd" "su" "crontab" "ping" "ping6"
+          "fusermount" "wodim" "cdrdao" "growisofs"
+        ];
+        description = ''
+          Only the programs from system path listed her will be made setuid root
+          (through a wrapper program).  It's better to set
+          <option>security.extraSetuidPrograms</option>.
+        '';
+      };
+
+      # !!! obsolete
+      extraSetuidPrograms = mkOption {
+        default = [];
+        example = ["fusermount"];
+        description = ''
+          This option lists additional programs that must be made setuid
+          root.
+        '';
+      };
+
+      setuidOwners = mkOption {
+        default = [];
+        example = [{
+          program = "sendmail";
+          owner = "nobody";
+          group = "postdrop";
+          setuid = false;
+          setgid = true;
+        }];
+        description = ''
+          This option allows the ownership and permissions on the setuid
+          wrappers for specific programs to be overriden from the
+          default (setuid root, but not setgid root).
+        '';
+      };
+
+      wrapperDir = mkOption {
+        default = "/var/setuid-wrappers";
+        description = ''
+          This option defines the path to the setuid wrappers.  It
+          should generally not be overriden.
+        '';
+      };
+    };
+  };
+
+  setuidWrapper = pkgs.stdenv.mkDerivation {
+    name = "setuid-wrapper";
+    buildCommand = ''
+      ensureDir $out/bin
+      gcc -Wall -O2 -DWRAPPER_DIR=\"${config.security.wrapperDir}\" ${./setuid-wrapper.c} -o $out/bin/setuid-wrapper
+      strip -s $out/bin/setuid-wrapper
+    '';
+  };
+
 in
 
 {
-  security = {
+  require = [options];
-    setuidPrograms = mkOption {
-      default = [
-        "passwd" "su" "crontab" "ping" "ping6"
-        "fusermount" "wodim" "cdrdao" "growisofs"
-      ];
-      description = "
-        Only the programs from system path listed her will be made setuid root
-        (through a wrapper program).  It's better to set
-        <option>security.extraSetuidPrograms</option>.
-      ";
-    };
 
-    extraSetuidPrograms = mkOption {
+  system = {
-      default = [];
+    activationScripts = {
-      example = ["fusermount"];
+      setuid =
-      description = "
+        let
-        This option lists additional programs that must be made setuid
+          setuidPrograms = builtins.toString (
-        root.
+            config.security.setuidPrograms ++
-      ";
+            config.security.extraSetuidPrograms ++
-    };
+            map (x: x.program) config.security.setuidOwners
+          );
 
-    setuidOwners = mkOption {
+          adjustSetuidOwner = concatStrings (map
-      default = [];
+            (_entry: let entry = {
-      example = [{
+              owner = "nobody";
-        program = "sendmail";
+              group = "nogroup";
-        owner = "nodody";
+              setuid = false;
-        group = "postdrop";
+              setgid = false;
-        setuid = false;
+            } //_entry; in
-        setgid = true;
+            ''
-      }];
+              chown ${entry.owner}.${entry.group} $wrapperDir/${entry.program}
-      description = ''
+              chmod u${if entry.setuid then "+" else "-"}s $wrapperDir/${entry.program} 
-        List of non-trivial setuid programs from system path, like Postfix sendmail. Default 
+              chmod g${if entry.setgid then "+" else "-"}s $wrapperDir/${entry.program}
-        should probably be nobody:nogroup:false:false - if you are bothering
+            '')
-        doing anything with a setuid program, "root.root u+s g-s" is not what
+            config.security.setuidOwners);
-        you are aiming at..
+
-      '';
+        in fullDepEntry ''
+          # Look in the system path and in the default profile for
+          # programs to be wrapped.  However, having setuid programs
+          # in a profile is problematic, since the NixOS activation
+          # script won't be rerun automatically when you install a
+          # wrappable program in the profile with nix-env.
+          SETUID_PATH=/nix/var/nix/profiles/default/sbin:/nix/var/nix/profiles/default/bin:${config.system.path}/bin:${config.system.path}/sbin
+
+          wrapperDir=${config.security.wrapperDir}
+          if test -d $wrapperDir; then rm -f $wrapperDir/*; fi # */
+          mkdir -p $wrapperDir
+          
+          for i in ${setuidPrograms}; do
+              program=$(PATH=$SETUID_PATH type -tP $i)
+              if test -z "$program"; then
+                  # XXX: It would be preferable to detect this problem before
+                  # `activate-configuration' is invoked.
+                  #echo "WARNING: No executable named \`$i' was found" >&2
+                  #echo "WARNING: but \`$i' was specified as a setuid program." >&2
+                  true
+              else
+                  cp ${setuidWrapper}/bin/setuid-wrapper $wrapperDir/$i
+                  echo -n "$program" > $wrapperDir/$i.real
+                  chown root.root $wrapperDir/$i
+                  chmod 4755 $wrapperDir/$i
+              fi
+          done
+
+          ${adjustSetuidOwner}
+        '' [ "defaultPath" "users" ];
     };
   };
 }

system/system-options.nix

@@ -26,22 +26,14 @@ let
       shell = mkOption {
         default = "/var/run/current-system/sw/bin/bash";
         description = ''
-          You should not redefine this option unless you want to change the
+          This option defines the path to the Bash shell.  It should
-          bash version for security issues.
+          generally not be overriden.
         '';
         merge = list:
           assert list != [] && builtins.tail list == [];
           builtins.head list;
       };
 
-      wrapperDir = mkOption {
-        default = "/var/setuid-wrappers";
-        description = ''
-          You should not redefine this option unless you want to change the
-          path for security issues.
-        '';
-      };
-
       overridePath = mkOption {
         default = [];
         description = ''
@@ -75,7 +67,6 @@ in
 let
   inherit (pkgs.stringsWithDeps) noDepEntry fullDepEntry packEntry;
   inherit (pkgs.lib) mapRecordFlatten;
-
 in
 
 {
@@ -103,14 +94,14 @@ in
         let path = [
           pkgs.coreutils pkgs.gnugrep pkgs.findutils
           pkgs.glibc # needed for getent
-          pkgs.glibcLocales # needed for getent
           pkgs.pwdutils
+          pkgs.nettools # needed for hostname
         ]; in noDepEntry ''
-        export PATH=/empty
+          export PATH=/empty
-        for i in ${toString path}; do
+          for i in ${toString path}; do
-          PATH=$PATH:$i/bin:$i/sbin;
+            PATH=$PATH:$i/bin:$i/sbin;
-        done
+          done
-      '';
+        '';
 
       stdio = fullDepEntry ''
         # Needed by some programs.
@@ -220,64 +211,6 @@ in
         "users" # nixbld group
       ];
 
-      path = fullDepEntry ''
-        PATH=${config.system.path}/bin:${config.system.path}/sbin:$PATH
-      '' [ "defaultPath" ];
-
-      setuid =
-        let
-          setuidPrograms = builtins.toString (
-            config.security.setuidPrograms ++
-            config.security.extraSetuidPrograms ++
-            map (x: x.program) config.security.setuidOwners
-          );
-
-          adjustSetuidOwner = pkgs.lib.concatStrings (map
-            (_entry: let entry = {
-              owner = "nobody";
-              group = "nogroup";
-              setuid = false;
-              setgid = false;
-            } //_entry; in
-            ''
-              chown ${entry.owner}.${entry.group} $wrapperDir/${entry.program}
-              chmod u${if entry.setuid then "+" else "-"}s $wrapperDir/${entry.program} 
-              chmod g${if entry.setgid then "+" else "-"}s $wrapperDir/${entry.program}
-            '')
-            config.security.setuidOwners);
-
-        in fullDepEntry ''
-        # Make a few setuid programs work.
-        save_PATH="$PATH"
-
-        # Add the default profile to the search path for setuid executables.
-        PATH="/nix/var/nix/profiles/default/sbin:$PATH"
-        PATH="/nix/var/nix/profiles/default/bin:$PATH"
-
-        wrapperDir=${config.system.wrapperDir}
-        if test -d $wrapperDir; then rm -f $wrapperDir/*; fi # */
-        mkdir -p $wrapperDir
-        for i in ${setuidPrograms}; do
-            program=$(type -tp $i)
-            if test -z "$program"; then
-        	# XXX: It would be preferable to detect this problem before
-        	# `activate-configuration' is invoked.
-        	#echo "WARNING: No executable named \`$i' was found" >&2
-        	#echo "WARNING: but \`$i' was specified as a setuid program." >&2
-                true
-            else
-                cp "$(type -tp setuid-wrapper)" $wrapperDir/$i
-                echo -n "$program" > $wrapperDir/$i.real
-                chown root.root $wrapperDir/$i
-                chmod 4755 $wrapperDir/$i
-            fi
-        done
-
-        ${adjustSetuidOwner}
-
-        PATH="$save_PATH"
-      '' [ "path" "users" ];
-
       hostname = fullDepEntry ''
         # Set the host name.  Don't clear it if it's not configured in the
         # NixOS configuration, since it may have been set by dhclient in the
@@ -290,7 +223,7 @@ in
               hostname ""
             fi
         ''}
-      '' [ "path" ];
+      '' [ "defaultPath" ];
 
       # The activation has to be done at the end. This is forced at the apply
       # function of activationScripts option
@@ -302,7 +235,7 @@ in
         ln -sfn "$(readlink -f "$systemConfig")" /var/run/current-system
 
         # Prevent the current configuration from being garbage-collected.
-       ln -sfn /var/run/current-system /nix/var/nix/gcroots/current-system
+        ln -sfn /var/run/current-system /nix/var/nix/gcroots/current-system
       '';
     };
   };

system/system.nix

@@ -34,8 +34,6 @@ rec {
 
   kernel = kernelPackages.kernel;
 
-  modulesTree = config.system.modulesTree;
-
 
   # The initial ramdisk.
   initialRamdiskStuff = import ../modules/system/boot/stage-1.nix {
@@ -61,10 +59,6 @@ rec {
   modprobe = config.system.sbin.modprobe;
 
 
-  # Environment variables for running Nix.
-  nixEnvVars = config.nix.envVars;
-
-
   # The static parts of /etc.
   etc = config.system.build.etc;
 
@@ -73,16 +67,6 @@ rec {
   fontDir = config.system.build.x11Fonts;
 
   
-  # The wrapper setuid programs (since we can't have setuid programs
-  # in the Nix store).
-  wrapperDir = config.system.wrapperDir;
-  
-  setuidWrapper = import ../helpers/setuid {
-    inherit (pkgs) stdenv;
-    inherit wrapperDir;
-  };
-
-
   # A patched `mount' command that looks in a directory in the Nix
   # store instead of in /sbin for mount helpers (like mount.ntfs-3g or
   # mount.cifs).
@@ -104,7 +88,6 @@ rec {
         nixosTools.nixosCheckout
         nixosTools.nixosHardwareScan
         nixosTools.nixosGenSeccureKeys
-        setuidWrapper
       ];
       path =
         pkgs.lib.optionals (!config.environment.cleanStart) [