Merge pull request #318599 from pacien/nixos-fcgiwrap-isolation

nixos/fcgiwrap: refactor to fix permissions

nixos/doc/manual/release-notes/rl-2411.section.md

@@ -60,6 +60,20 @@
   it is set, instead of the previous hardcoded default of
   `${networking.hostName}.${security.ipa.domain}`.
 
+- The fcgiwrap module now allows multiple instances running as distinct users.
+  The option `services.fgciwrap` now takes an attribute set of the
+  configuration of each individual instance.
+  This requires migrating any previous configuration keys from
+  `services.fcgiwrap.*` to `services.fcgiwrap.some-instance.*`.
+  The ownership and mode of the UNIX sockets created by this service are now
+  configurable and private by default.
+  Processes also now run as a dynamically allocated user by default instead of
+  root.
+
+- `services.cgit` now runs as the cgit user by default instead of root.
+  This change requires granting access to the repositories to this user or
+  setting the appropriate one through `services.cgit.some-instance.user`.
+
 - `nvimpager` was updated to version 0.13.0, which changes the order of user and
   nvimpager settings: user commands in `-c` and `--cmd` now override the
   respective default settings because they are executed later.

nixos/modules/services/misc/zoneminder.nix

@@ -202,10 +202,10 @@ in {
     ];
 
     services = {
-      fcgiwrap = lib.mkIf useNginx {
-        enable = true;
-        preforkProcesses = cfg.cameras;
-        inherit user group;
+      fcgiwrap.zoneminder = lib.mkIf useNginx {
+        process.prefork = cfg.cameras;
+        process.user = user;
+        process.group = group;
       };
 
       mysql = lib.mkIf cfg.database.createLocally {
@@ -225,9 +225,7 @@ in {
             default = true;
             root = "${pkg}/share/zoneminder/www";
             listen = [ { addr = "0.0.0.0"; inherit (cfg) port; } ];
-            extraConfig = let
-              fcgi = config.services.fcgiwrap;
-            in ''
+            extraConfig = ''
               index index.php;
 
               location / {
@@ -257,7 +255,7 @@ in {
                   fastcgi_param HTTP_PROXY "";
                   fastcgi_intercept_errors on;
 
-                  fastcgi_pass ${fcgi.socketType}:${fcgi.socketAddress};
+                  fastcgi_pass unix:${config.services.fcgiwrap.zoneminder.socket.address};
                 }
 
                 location /cache/ {

nixos/modules/services/networking/cgit.nix

@@ -25,14 +25,14 @@ let
 
   regexLocation = cfg: regexEscape (stripLocation cfg);
 
-  mkFastcgiPass = cfg: ''
+  mkFastcgiPass = name: cfg: ''
     ${if cfg.nginx.location == "/" then ''
       fastcgi_param PATH_INFO $uri;
     '' else ''
       fastcgi_split_path_info ^(${regexLocation cfg})(/.+)$;
       fastcgi_param PATH_INFO $fastcgi_path_info;
     ''
-    }fastcgi_pass unix:${config.services.fcgiwrap.socketAddress};
+    }fastcgi_pass unix:${config.services.fcgiwrap."cgit-${name}".socket.address};
   '';
 
   cgitrcLine = name: value: "${name}=${
@@ -72,25 +72,11 @@ let
     ${cfg.extraConfig}
   '';
 
-  mkCgitReposDir = cfg:
-    if cfg.scanPath != null then
-      cfg.scanPath
-    else
-      pkgs.runCommand "cgit-repos" {
-        preferLocalBuild = true;
-        allowSubstitutes = false;
-      } ''
-        mkdir -p "$out"
-        ${
-          concatStrings (
-            mapAttrsToList
-              (name: value: ''
-                ln -s ${escapeShellArg value.path} "$out"/${escapeShellArg name}
-              '')
-              cfg.repos
-          )
-        }
-      '';
+  fcgiwrapUnitName = name: "fcgiwrap-cgit-${name}";
+  fcgiwrapRuntimeDir = name: "/run/${fcgiwrapUnitName name}";
+  gitProjectRoot = name: cfg: if cfg.scanPath != null
+    then cfg.scanPath
+    else "${fcgiwrapRuntimeDir name}/repos";
 
 in
 {
@@ -154,6 +140,18 @@ in
             type = types.lines;
             default = "";
           };
+
+          user = mkOption {
+            description = "User to run the cgit service as.";
+            type = types.str;
+            default = "cgit";
+          };
+
+          group = mkOption {
+            description = "Group to run the cgit service as.";
+            type = types.str;
+            default = "cgit";
+          };
         };
       }));
     };
@@ -165,18 +163,46 @@ in
       message = "Exactly one of services.cgit.${vhost}.scanPath or services.cgit.${vhost}.repos must be set.";
     }) cfgs;
 
-    services.fcgiwrap.enable = true;
+    users = mkMerge (flip mapAttrsToList cfgs (_: cfg: {
+      users.${cfg.user} = {
+        isSystemUser = true;
+        inherit (cfg) group;
+      };
+      groups.${cfg.group} = { };
+    }));
+
+    services.fcgiwrap = flip mapAttrs' cfgs (name: cfg:
+      nameValuePair "cgit-${name}" {
+        process = { inherit (cfg) user group; };
+        socket = { inherit (config.services.nginx) user group; };
+      }
+    );
+
+    systemd.services = flip mapAttrs' cfgs (name: cfg:
+      nameValuePair (fcgiwrapUnitName name)
+      (mkIf (cfg.repos != { }) {
+        serviceConfig.RuntimeDirectory = fcgiwrapUnitName name;
+        preStart = ''
+          GIT_PROJECT_ROOT=${escapeShellArg (gitProjectRoot name cfg)}
+          mkdir -p "$GIT_PROJECT_ROOT"
+          cd "$GIT_PROJECT_ROOT"
+          ${concatLines (flip mapAttrsToList cfg.repos (name: repo: ''
+            ln -s ${escapeShellArg repo.path} ${escapeShellArg name}
+          ''))}
+        '';
+      }
+    ));
 
     services.nginx.enable = true;
 
-    services.nginx.virtualHosts = mkMerge (mapAttrsToList (_: cfg: {
+    services.nginx.virtualHosts = mkMerge (mapAttrsToList (name: cfg: {
       ${cfg.nginx.virtualHost} = {
         locations = (
           genAttrs'
             [ "cgit.css" "cgit.png" "favicon.ico" "robots.txt" ]
-            (name: nameValuePair "= ${stripLocation cfg}/${name}" {
+            (fileName: nameValuePair "= ${stripLocation cfg}/${fileName}" {
               extraConfig = ''
-                alias ${cfg.package}/cgit/${name};
+                alias ${cfg.package}/cgit/${fileName};
               '';
             })
         ) // {
@@ -184,10 +210,10 @@ in
             fastcgiParams = rec {
               SCRIPT_FILENAME = "${pkgs.git}/libexec/git-core/git-http-backend";
               GIT_HTTP_EXPORT_ALL = "1";
-              GIT_PROJECT_ROOT = mkCgitReposDir cfg;
+              GIT_PROJECT_ROOT = gitProjectRoot name cfg;
               HOME = GIT_PROJECT_ROOT;
             };
-            extraConfig = mkFastcgiPass cfg;
+            extraConfig = mkFastcgiPass name cfg;
           };
           "${stripLocation cfg}/" = {
             fastcgiParams = {
@@ -196,7 +222,7 @@ in
               HTTP_HOST = "$server_name";
               CGIT_CONFIG = mkCgitrc cfg;
             };
-            extraConfig = mkFastcgiPass cfg;
+            extraConfig = mkFastcgiPass name cfg;
           };
         };
       };

nixos/modules/services/networking/smokeping.nix

@@ -337,7 +337,11 @@ in
     };
 
     # use nginx to serve the smokeping web service
-    services.fcgiwrap.enable = mkIf cfg.webService true;
+    services.fcgiwrap.smokeping = mkIf cfg.webService {
+      process.user = cfg.user;
+      process.group = cfg.user;
+      socket = { inherit (config.services.nginx) user group; };
+    };
     services.nginx = mkIf cfg.webService {
       enable = true;
       virtualHosts."smokeping" = {
@@ -349,7 +353,7 @@ in
         locations."/smokeping.fcgi" = {
           extraConfig = ''
             include ${config.services.nginx.package}/conf/fastcgi_params;
-            fastcgi_pass unix:${config.services.fcgiwrap.socketAddress};
+            fastcgi_pass unix:${config.services.fcgiwrap.smokeping.socket.address};
             fastcgi_param SCRIPT_FILENAME ${smokepingHome}/smokeping.fcgi;
             fastcgi_param DOCUMENT_ROOT ${smokepingHome};
           '';

nixos/modules/services/web-servers/fcgiwrap.nix

@@ -3,70 +3,128 @@
 with lib;
 
 let
-  cfg = config.services.fcgiwrap;
+  forEachInstance = f: flip mapAttrs' config.services.fcgiwrap (name: cfg:
+    nameValuePair "fcgiwrap-${name}" (f cfg)
+  );
+
 in {
-
-  options = {
-    services.fcgiwrap = {
-      enable = mkOption {
-        type = types.bool;
-        default = false;
-        description = "Whether to enable fcgiwrap, a server for running CGI applications over FastCGI.";
-      };
-
-      preforkProcesses = mkOption {
-        type = types.int;
+  options.services.fcgiwrap = mkOption {
+    description = "Configuration for fcgiwrap instances.";
+    default = { };
+    type = types.attrsOf (types.submodule ({ config, ... }: { options = {
+      process.prefork = mkOption {
+        type = types.ints.positive;
         default = 1;
         description = "Number of processes to prefork.";
       };
 
-      socketType = mkOption {
+      process.user = mkOption {
+        type = types.nullOr types.str;
+        default = null;
+        description = ''
+          User as which this instance of fcgiwrap will be run.
+          Set to `null` (the default) to use a dynamically allocated user.
+        '';
+      };
+
+      process.group = mkOption {
+        type = types.nullOr types.str;
+        default = null;
+        description = "Group as which this instance of fcgiwrap will be run.";
+      };
+
+      socket.type = mkOption {
         type = types.enum [ "unix" "tcp" "tcp6" ];
         default = "unix";
         description = "Socket type: 'unix', 'tcp' or 'tcp6'.";
       };
 
-      socketAddress = mkOption {
+      socket.address = mkOption {
         type = types.str;
-        default = "/run/fcgiwrap.sock";
+        default = "/run/fcgiwrap-${config._module.args.name}.sock";
         example = "1.2.3.4:5678";
-        description = "Socket address. In case of a UNIX socket, this should be its filesystem path.";
+        description = ''
+          Socket address.
+          In case of a UNIX socket, this should be its filesystem path.
+        '';
       };
 
-      user = mkOption {
+      socket.user = mkOption {
         type = types.nullOr types.str;
         default = null;
-        description = "User permissions for the socket.";
+        description = ''
+          User to be set as owner of the UNIX socket.
+          Defaults to the process running user.
+        '';
       };
 
-      group = mkOption {
+      socket.group = mkOption {
         type = types.nullOr types.str;
         default = null;
-        description = "Group permissions for the socket.";
+        description = ''
+          Group to be set as owner of the UNIX socket.
+          Defaults to the process running group.
+        '';
       };
-    };
+
+      socket.mode = mkOption {
+        type = types.nullOr types.str;
+        default = if config.socket.type == "unix" then "0600" else null;
+        defaultText = literalExpression ''
+          if config.socket.type == "unix" then "0600" else null
+        '';
+        description = ''
+          Mode to be set on the UNIX socket.
+          Defaults to private to the socket's owner.
+        '';
+      };
+    }; }));
   };
 
-  config = mkIf cfg.enable {
-    systemd.services.fcgiwrap = {
+  config = {
+    assertions = concatLists (mapAttrsToList (name: cfg: [
+      {
+        assertion = cfg.socket.user != null -> cfg.socket.type == "unix";
+        message = "Socket owner can only be set for the UNIX socket type.";
+      }
+      {
+        assertion = cfg.socket.group != null -> cfg.socket.type == "unix";
+        message = "Socket owner can only be set for the UNIX socket type.";
+      }
+      {
+        assertion = cfg.socket.mode != null -> cfg.socket.type == "unix";
+        message = "Socket mode can only be set for the UNIX socket type.";
+      }
+    ]) config.services.fcgiwrap);
+
+    systemd.services = forEachInstance (cfg: {
       after = [ "nss-user-lookup.target" ];
-      wantedBy = optional (cfg.socketType != "unix") "multi-user.target";
+      wantedBy = optional (cfg.socket.type != "unix") "multi-user.target";
 
       serviceConfig = {
-        ExecStart = "${pkgs.fcgiwrap}/sbin/fcgiwrap -c ${builtins.toString cfg.preforkProcesses} ${
-          optionalString (cfg.socketType != "unix") "-s ${cfg.socketType}:${cfg.socketAddress}"
-        }";
-      } // (if cfg.user != null && cfg.group != null then {
-        User = cfg.user;
-        Group = cfg.group;
-      } else { } );
-    };
+        ExecStart = ''
+          ${pkgs.fcgiwrap}/sbin/fcgiwrap ${cli.toGNUCommandLineShell {} ({
+            c = cfg.process.prefork;
+          } // (optionalAttrs (cfg.socket.type != "unix") {
+            s = "${cfg.socket.type}:${cfg.socket.address}";
+          }))}
+        '';
+      } // (if cfg.process.user != null then {
+        User = cfg.process.user;
+        Group = cfg.process.group;
+      } else {
+        DynamicUser = true;
+      });
+    });
 
-    systemd.sockets = if (cfg.socketType == "unix") then {
-      fcgiwrap = {
-        wantedBy = [ "sockets.target" ];
-        socketConfig.ListenStream = cfg.socketAddress;
+    systemd.sockets = forEachInstance (cfg: mkIf (cfg.socket.type == "unix") {
+      wantedBy = [ "sockets.target" ];
+      socketConfig = {
+        ListenStream = cfg.socket.address;
+        SocketUser = cfg.socket.user;
+        SocketGroup = cfg.socket.group;
+        SocketMode = cfg.socket.mode;
       };
-    } else { };
+    });
   };
 }

nixos/tests/cgit.nix

@@ -23,7 +23,7 @@ in {
         nginx.location = "/(c)git/";
         repos = {
           some-repo = {
-            path = "/srv/git/some-repo";
+            path = "/tmp/git/some-repo";
             desc = "some-repo description";
           };
         };
@@ -50,12 +50,12 @@ in {
 
     server.fail("curl -fsS http://localhost/robots.txt")
 
-    server.succeed("${pkgs.writeShellScript "setup-cgit-test-repo" ''
+    server.succeed("sudo -u cgit ${pkgs.writeShellScript "setup-cgit-test-repo" ''
       set -e
-      git init --bare -b master /srv/git/some-repo
+      git init --bare -b master /tmp/git/some-repo
       git init -b master reference
       cd reference
-      git remote add origin /srv/git/some-repo
+      git remote add origin /tmp/git/some-repo
       date > date.txt
       git add date.txt
       git -c user.name=test -c user.email=test@localhost commit -m 'add date'

nixos/tests/gitolite-fcgiwrap.nix

@@ -24,7 +24,12 @@ import ./make-test-python.nix (
               {
                 networking.firewall.allowedTCPPorts = [ 80 ];
 
-                services.fcgiwrap.enable = true;
+                services.fcgiwrap.gitolite = {
+                  process.user = "gitolite";
+                  process.group = "gitolite";
+                  socket = { inherit (config.services.nginx) user group; };
+                };
+
                 services.gitolite = {
                   enable = true;
                   adminPubkey = adminPublicKey;
@@ -59,7 +64,7 @@ import ./make-test-python.nix (
                     fastcgi_param SCRIPT_FILENAME ${pkgs.gitolite}/bin/gitolite-shell;
 
                     # use Unix domain socket or inet socket
-                    fastcgi_pass unix:/run/fcgiwrap.sock;
+                    fastcgi_pass unix:${config.services.fcgiwrap.gitolite.socket.address};
                   '';
                 };
 
@@ -82,7 +87,7 @@ import ./make-test-python.nix (
 
           server.wait_for_unit("gitolite-init.service")
           server.wait_for_unit("nginx.service")
-          server.wait_for_file("/run/fcgiwrap.sock")
+          server.wait_for_file("/run/fcgiwrap-gitolite.sock")
 
           client.wait_for_unit("multi-user.target")
           client.succeed(