From 3c2e82337dd25cb7e07bf2eba0e622b97690470b Mon Sep 17 00:00:00 2001
From: ThinkChaos <ThinkChaos@users.noreply.github.com>
Date: Sat, 24 Aug 2024 19:18:07 -0400
Subject: [PATCH] nixos/web-servers: assert ACME cert access via service user
 and groups

Allows giving access using SupplementaryGroups.
---
 .../acme/mk-cert-ownership-assertion.nix      | 23 ++++++++++++++++---
 .../web-servers/apache-httpd/default.nix      |  6 ++---
 .../services/web-servers/caddy/default.nix    |  4 ++--
 .../services/web-servers/nginx/default.nix    |  6 ++---
 4 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/nixos/modules/security/acme/mk-cert-ownership-assertion.nix b/nixos/modules/security/acme/mk-cert-ownership-assertion.nix
index b80d89aeb9fc..53a3fbaadd2e 100644
--- a/nixos/modules/security/acme/mk-cert-ownership-assertion.nix
+++ b/nixos/modules/security/acme/mk-cert-ownership-assertion.nix
@@ -1,4 +1,21 @@
-{ cert, group, groups, user }: {
-  assertion = cert.group == group || builtins.any (u: u == user) groups.${cert.group}.members;
-  message = "Group for certificate ${cert.domain} must be ${group}, or user ${user} must be a member of group ${cert.group}";
+lib:
+
+{ cert, groups, services }:
+let
+  catSep = builtins.concatStringsSep;
+
+  svcGroups = svc:
+    (lib.optional (svc.serviceConfig ? Group) svc.serviceConfig.Group)
+    ++ (svc.serviceConfig.SupplementaryGroups or [ ]);
+in
+{
+  assertion = builtins.all (svc:
+    svc.serviceConfig.User or "root" == "root"
+    || builtins.elem svc.serviceConfig.User groups.${cert.group}.members
+    || builtins.elem cert.group (svcGroups svc)
+  ) services;
+
+  message = "Certificate ${cert.domain} (group=${cert.group}) must be readable by service(s) ${
+    catSep ", " (map (svc: "${svc.name} (user=${svc.serviceConfig.User} groups=${catSep " " (svcGroups svc)})") services)
+  }";
 }
diff --git a/nixos/modules/services/web-servers/apache-httpd/default.nix b/nixos/modules/services/web-servers/apache-httpd/default.nix
index e64fbff00fd5..1ac86c1a5c1d 100644
--- a/nixos/modules/services/web-servers/apache-httpd/default.nix
+++ b/nixos/modules/services/web-servers/apache-httpd/default.nix
@@ -373,7 +373,7 @@ let
       echo "$options" >> $out
     '';
 
-  mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix;
+  mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix lib;
 in
 
 
@@ -643,9 +643,9 @@ in
         '';
       }
     ] ++ map (name: mkCertOwnershipAssertion {
-      inherit (cfg) group user;
       cert = config.security.acme.certs.${name};
       groups = config.users.groups;
+      services = [ config.systemd.services.httpd ] ++ lib.optional (vhostCertNames != []) config.systemd.services.httpd-config-reload;
     }) vhostCertNames;
 
     warnings =
@@ -795,7 +795,7 @@ in
     systemd.services.httpd-config-reload = let
       sslServices = map (certName: "acme-${certName}.service") vhostCertNames;
       sslTargets = map (certName: "acme-finished-${certName}.target") vhostCertNames;
-    in mkIf (sslServices != []) {
+    in mkIf (vhostCertNames != []) {
       wantedBy = sslServices ++ [ "multi-user.target" ];
       # Before the finished targets, after the renew services.
       # This service might be needed for HTTP-01 challenges, but we only want to confirm
diff --git a/nixos/modules/services/web-servers/caddy/default.nix b/nixos/modules/services/web-servers/caddy/default.nix
index a221e578f769..8f8a4da35cc5 100644
--- a/nixos/modules/services/web-servers/caddy/default.nix
+++ b/nixos/modules/services/web-servers/caddy/default.nix
@@ -55,7 +55,7 @@ let
 
   configPath = "/etc/${etcConfigFile}";
 
-  mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix;
+  mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix lib;
 in
 {
   imports = [
@@ -331,9 +331,9 @@ in
         message = "To specify an adapter other than 'caddyfile' please provide your own configuration via `services.caddy.configFile`";
       }
     ] ++ map (name: mkCertOwnershipAssertion {
-      inherit (cfg) group user;
       cert = config.security.acme.certs.${name};
       groups = config.users.groups;
+      services = [ config.systemd.services.caddy ];
     }) vhostCertNames;
 
     services.caddy.globalConfig = ''
diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix
index 848671c16f6f..922df1ea03ab 100644
--- a/nixos/modules/services/web-servers/nginx/default.nix
+++ b/nixos/modules/services/web-servers/nginx/default.nix
@@ -473,7 +473,7 @@ let
     '') authDef)
   );
 
-  mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix;
+  mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix lib;
 
   oldHTTP2 = (versionOlder cfg.package.version "1.25.1" && !(cfg.package.pname == "angie" || cfg.package.pname == "angieQuic"));
 in
@@ -1211,9 +1211,9 @@ in
         '';
       }
     ] ++ map (name: mkCertOwnershipAssertion {
-      inherit (cfg) group user;
       cert = config.security.acme.certs.${name};
       groups = config.users.groups;
+      services = [ config.systemd.services.nginx ] ++ lib.optional (cfg.enableReload || vhostCertNames != []) config.systemd.services.nginx-config-reload;
     }) vhostCertNames;
 
     services.nginx.additionalModules = optional cfg.recommendedBrotliSettings pkgs.nginxModules.brotli
@@ -1322,7 +1322,7 @@ in
     systemd.services.nginx-config-reload = let
       sslServices = map (certName: "acme-${certName}.service") vhostCertNames;
       sslTargets = map (certName: "acme-finished-${certName}.target") vhostCertNames;
-    in mkIf (cfg.enableReload || sslServices != []) {
+    in mkIf (cfg.enableReload || vhostCertNames != []) {
       wants = optionals cfg.enableReload [ "nginx.service" ];
       wantedBy = sslServices ++ [ "multi-user.target" ];
       # Before the finished targets, after the renew services.