From b99d3a4f8be2b53046b2b2c4a22c72236c0d4f2d Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Wed, 21 Aug 2024 17:46:30 -0400 Subject: [PATCH 1/6] nixos/apache: not "before" ACME certs using DNS validation Relax dependency with certs that are validated via DNS challenge since we know the HTTP server is not required for that validation. This allows marking the server's service as depending on the cert. --- .../web-servers/apache-httpd/default.nix | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/nixos/modules/services/web-servers/apache-httpd/default.nix b/nixos/modules/services/web-servers/apache-httpd/default.nix index 46cb09959579..e64fbff00fd5 100644 --- a/nixos/modules/services/web-servers/apache-httpd/default.nix +++ b/nixos/modules/services/web-servers/apache-httpd/default.nix @@ -33,7 +33,9 @@ let certName = if hostOpts.useACMEHost != null then hostOpts.useACMEHost else hostOpts.hostName; }) (filter (hostOpts: hostOpts.enableACME || hostOpts.useACMEHost != null) vhosts); - dependentCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts); + vhostCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts); + dependentCertNames = filter (cert: certs.${cert}.dnsProvider == null) vhostCertNames; # those that might depend on the HTTP server + independentCertNames = filter (cert: certs.${cert}.dnsProvider != null) vhostCertNames; # those that don't depend on the HTTP server mkListenInfo = hostOpts: if hostOpts.listen != [] then @@ -644,7 +646,7 @@ in inherit (cfg) group user; cert = config.security.acme.certs.${name}; groups = config.users.groups; - }) dependentCertNames; + }) vhostCertNames; warnings = mapAttrsToList (name: hostOpts: '' @@ -747,8 +749,10 @@ in systemd.services.httpd = { description = "Apache HTTPD"; wantedBy = [ "multi-user.target" ]; - wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) dependentCertNames); - after = [ "network.target" ] ++ map (certName: "acme-selfsigned-${certName}.service") dependentCertNames; + wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) vhostCertNames); + after = [ "network.target" ] + ++ map (certName: "acme-selfsigned-${certName}.service") vhostCertNames + ++ map (certName: "acme-${certName}.service") independentCertNames; # avoid loading self-signed key w/ real cert, or vice-versa before = map (certName: "acme-${certName}.service") dependentCertNames; restartTriggers = [ cfg.configFile ]; @@ -789,8 +793,8 @@ in # which allows the acme-finished-$cert.target to signify the successful updating # of certs end-to-end. systemd.services.httpd-config-reload = let - sslServices = map (certName: "acme-${certName}.service") dependentCertNames; - sslTargets = map (certName: "acme-finished-${certName}.target") dependentCertNames; + sslServices = map (certName: "acme-${certName}.service") vhostCertNames; + sslTargets = map (certName: "acme-finished-${certName}.target") vhostCertNames; in mkIf (sslServices != []) { wantedBy = sslServices ++ [ "multi-user.target" ]; # Before the finished targets, after the renew services. @@ -801,7 +805,7 @@ in restartTriggers = [ cfg.configFile ]; # Block reloading if not all certs exist yet. # Happens when config changes add new vhosts/certs. - unitConfig.ConditionPathExists = map (certName: certs.${certName}.directory + "/fullchain.pem") dependentCertNames; + unitConfig.ConditionPathExists = map (certName: certs.${certName}.directory + "/fullchain.pem") vhostCertNames; serviceConfig = { Type = "oneshot"; TimeoutSec = 60; From 26d6294deb867f9690a60321290b9c07c7e8649b Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Wed, 21 Aug 2024 17:46:39 -0400 Subject: [PATCH 2/6] nixos/caddy: not "before" ACME certs using DNS validation Relax dependency with certs that are validated via DNS challenge since we know the HTTP server is not required for that validation. This allows marking the server's service as depending on the cert. --- .../services/web-servers/caddy/default.nix | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/nixos/modules/services/web-servers/caddy/default.nix b/nixos/modules/services/web-servers/caddy/default.nix index 496705beff7b..a221e578f769 100644 --- a/nixos/modules/services/web-servers/caddy/default.nix +++ b/nixos/modules/services/web-servers/caddy/default.nix @@ -5,8 +5,12 @@ with lib; let cfg = config.services.caddy; + certs = config.security.acme.certs; virtualHosts = attrValues cfg.virtualHosts; - acmeVHosts = filter (hostOpts: hostOpts.useACMEHost != null) virtualHosts; + acmeEnabledVhosts = filter (hostOpts: hostOpts.useACMEHost != null) virtualHosts; + vhostCertNames = unique (map (hostOpts: hostOpts.useACMEHost) acmeEnabledVhosts); + dependentCertNames = filter (cert: certs.${cert}.dnsProvider == null) vhostCertNames; # those that might depend on the HTTP server + independentCertNames = filter (cert: certs.${cert}.dnsProvider != null) vhostCertNames; # those that don't depend on the HTTP server mkVHostConf = hostOpts: let @@ -51,8 +55,6 @@ let configPath = "/etc/${etcConfigFile}"; - acmeHosts = unique (catAttrs "useACMEHost" acmeVHosts); - mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix; in { @@ -332,7 +334,7 @@ in inherit (cfg) group user; cert = config.security.acme.certs.${name}; groups = config.users.groups; - }) acmeHosts; + }) vhostCertNames; services.caddy.globalConfig = '' ${optionalString (cfg.email != null) "email ${cfg.email}"} @@ -348,9 +350,10 @@ in systemd.packages = [ cfg.package ]; systemd.services.caddy = { - wants = map (hostOpts: "acme-finished-${hostOpts.useACMEHost}.target") acmeVHosts; - after = map (hostOpts: "acme-selfsigned-${hostOpts.useACMEHost}.service") acmeVHosts; - before = map (hostOpts: "acme-${hostOpts.useACMEHost}.service") acmeVHosts; + wants = map (certName: "acme-finished-${certName}.target") vhostCertNames; + after = map (certName: "acme-selfsigned-${certName}.service") vhostCertNames + ++ map (certName: "acme-${certName}.service") independentCertNames; # avoid loading self-signed key w/ real cert, or vice-versa + before = map (certName: "acme-${certName}.service") dependentCertNames; wantedBy = [ "multi-user.target" ]; startLimitIntervalSec = 14400; @@ -397,10 +400,10 @@ in security.acme.certs = let - certCfg = map (useACMEHost: nameValuePair useACMEHost { + certCfg = map (certName: nameValuePair certName { group = mkDefault cfg.group; reloadServices = [ "caddy.service" ]; - }) acmeHosts; + }) vhostCertNames; in listToAttrs certCfg; From 03122b43c8380bc8671640e5bbd1aa9fec938b5e Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Wed, 21 Aug 2024 17:46:49 -0400 Subject: [PATCH 3/6] nixos/nginx: not "before" ACME certs using DNS validation Relax dependency with certs that are validated via DNS challenge since we know the HTTP server is not required for that validation. This allows marking the server's service as depending on the cert. --- .../services/web-servers/nginx/default.nix | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix index 1e43554b7818..848671c16f6f 100644 --- a/nixos/modules/services/web-servers/nginx/default.nix +++ b/nixos/modules/services/web-servers/nginx/default.nix @@ -7,7 +7,9 @@ let inherit (config.security.acme) certs; vhostsConfigs = mapAttrsToList (vhostName: vhostConfig: vhostConfig) virtualHosts; acmeEnabledVhosts = filter (vhostConfig: vhostConfig.enableACME || vhostConfig.useACMEHost != null) vhostsConfigs; - dependentCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts); + vhostCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts); + dependentCertNames = filter (cert: certs.${cert}.dnsProvider == null) vhostCertNames; # those that might depend on the HTTP server + independentCertNames = filter (cert: certs.${cert}.dnsProvider != null) vhostCertNames; # those that don't depend on the HTTP server virtualHosts = mapAttrs (vhostName: vhostConfig: let serverName = if vhostConfig.serverName != null @@ -1212,7 +1214,7 @@ in inherit (cfg) group user; cert = config.security.acme.certs.${name}; groups = config.users.groups; - }) dependentCertNames; + }) vhostCertNames; services.nginx.additionalModules = optional cfg.recommendedBrotliSettings pkgs.nginxModules.brotli ++ lib.optional cfg.recommendedZstdSettings pkgs.nginxModules.zstd; @@ -1236,8 +1238,10 @@ in systemd.services.nginx = { description = "Nginx Web Server"; wantedBy = [ "multi-user.target" ]; - wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) dependentCertNames); - after = [ "network.target" ] ++ map (certName: "acme-selfsigned-${certName}.service") dependentCertNames; + wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) vhostCertNames); + after = [ "network.target" ] + ++ map (certName: "acme-selfsigned-${certName}.service") vhostCertNames + ++ map (certName: "acme-${certName}.service") independentCertNames; # avoid loading self-signed key w/ real cert, or vice-versa # Nginx needs to be started in order to be able to request certificates # (it's hosting the acme challenge after all) # This fixes https://github.com/NixOS/nixpkgs/issues/81842 @@ -1316,8 +1320,8 @@ in # which allows the acme-finished-$cert.target to signify the successful updating # of certs end-to-end. systemd.services.nginx-config-reload = let - sslServices = map (certName: "acme-${certName}.service") dependentCertNames; - sslTargets = map (certName: "acme-finished-${certName}.target") dependentCertNames; + sslServices = map (certName: "acme-${certName}.service") vhostCertNames; + sslTargets = map (certName: "acme-finished-${certName}.target") vhostCertNames; in mkIf (cfg.enableReload || sslServices != []) { wants = optionals cfg.enableReload [ "nginx.service" ]; wantedBy = sslServices ++ [ "multi-user.target" ]; @@ -1329,7 +1333,7 @@ in restartTriggers = optionals cfg.enableReload [ configFile ]; # Block reloading if not all certs exist yet. # Happens when config changes add new vhosts/certs. - unitConfig.ConditionPathExists = optionals (sslServices != []) (map (certName: certs.${certName}.directory + "/fullchain.pem") dependentCertNames); + unitConfig.ConditionPathExists = optionals (sslServices != []) (map (certName: certs.${certName}.directory + "/fullchain.pem") vhostCertNames); serviceConfig = { Type = "oneshot"; TimeoutSec = 60; From 3c2e82337dd25cb7e07bf2eba0e622b97690470b Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Sat, 24 Aug 2024 19:18:07 -0400 Subject: [PATCH 4/6] 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. From 1bd7f1374dd10c59b7d0fcee91925d101ccd7e6d Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Thu, 7 Nov 2024 20:17:46 -0500 Subject: [PATCH 5/6] nixos/acme: use non deprecated CLI flag for `dnsPropagationCheck` --- nixos/modules/security/acme/default.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/modules/security/acme/default.nix b/nixos/modules/security/acme/default.nix index a9cb396f13fd..4af92d8779ef 100644 --- a/nixos/modules/security/acme/default.nix +++ b/nixos/modules/security/acme/default.nix @@ -217,7 +217,7 @@ let protocolOpts = if useDns then ( [ "--dns" data.dnsProvider ] - ++ lib.optionals (!data.dnsPropagationCheck) [ "--dns.disable-cp" ] + ++ lib.optionals (!data.dnsPropagationCheck) [ "--dns.propagation-disable-ans" ] ++ lib.optionals (data.dnsResolver != null) [ "--dns.resolvers" data.dnsResolver ] ) else if data.s3Bucket != null then [ "--http" "--http.s3-bucket" data.s3Bucket ] else if data.listenHTTP != null then [ "--http" "--http.port" data.listenHTTP ] From b432e86cafe1bc1ba2a1e471cd4eeaae22694fcf Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Thu, 7 Nov 2024 20:18:08 -0500 Subject: [PATCH 6/6] nixos/acme: remove unused binding in tests --- nixos/tests/common/acme/server/default.nix | 5 ----- 1 file changed, 5 deletions(-) diff --git a/nixos/tests/common/acme/server/default.nix b/nixos/tests/common/acme/server/default.nix index 457495cdb2c0..893c6924027f 100644 --- a/nixos/tests/common/acme/server/default.nix +++ b/nixos/tests/common/acme/server/default.nix @@ -54,11 +54,6 @@ let testCerts = import ./snakeoil-certs.nix; domain = testCerts.domain; - resolver = let - message = "You need to define a resolver for the acme test module."; - firstNS = lib.head config.networking.nameservers; - in if config.networking.nameservers == [] then throw message else firstNS; - pebbleConf.pebble = { listenAddress = "0.0.0.0:443"; managementListenAddress = "0.0.0.0:15000";