From b76c36dcdc4502bb5837abeeb505f0e60bef5c90 Mon Sep 17 00:00:00 2001 From: Leona Maroni Date: Mon, 22 Sep 2025 15:18:12 +0200 Subject: [PATCH 1/2] nixos/tests/acme: introduce new test nginx without reload --- nixos/tests/acme/default.nix | 54 ++++++++++++++++++----------- pkgs/servers/http/nginx/generic.nix | 1 + 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/nixos/tests/acme/default.nix b/nixos/tests/acme/default.nix index 2faca3a0b842..6ec14191c2ed 100644 --- a/nixos/tests/acme/default.nix +++ b/nixos/tests/acme/default.nix @@ -1,6 +1,26 @@ -{ runTest }: +{ lib, runTest }: let domain = "example.test"; + nginxBaseModule = { + services.nginx = { + enable = true; + logError = "stderr info"; + # This tests a number of things at once: + # - Self-signed certs are in place before the webserver startup + # - Nginx is started before acme renewal is attempted + # - useACMEHost behaves as expected + # - acmeFallbackHost behaves as expected + virtualHosts.default = { + default = true; + addSSL = true; + useACMEHost = "proxied.example.test"; + acmeFallbackHost = "localhost:8080"; + }; + }; + specialisation.nullroot.configuration = { + services.nginx.virtualHosts."nullroot.${domain}".acmeFallbackHost = "localhost:8081"; + }; + }; in { http01-builtin = runTest ./http01-builtin.nix; @@ -11,26 +31,18 @@ in inherit domain; serverName = "nginx"; group = "nginx"; - baseModule = { - services.nginx = { - enable = true; - enableReload = true; - logError = "stderr info"; - # This tests a number of things at once: - # - Self-signed certs are in place before the webserver startup - # - Nginx is started before acme renewal is attempted - # - useACMEHost behaves as expected - # - acmeFallbackHost behaves as expected - virtualHosts.default = { - default = true; - addSSL = true; - useACMEHost = "proxied.example.test"; - acmeFallbackHost = "localhost:8080"; - }; - }; - specialisation.nullroot.configuration = { - services.nginx.virtualHosts."nullroot.${domain}".acmeFallbackHost = "localhost:8081"; - }; + baseModule = lib.recursiveUpdate nginxBaseModule { + services.nginx.enableReload = true; + }; + } + ); + nginx-without-reload = runTest ( + import ./webserver.nix { + inherit domain; + serverName = "nginx"; + group = "nginx"; + baseModule = lib.recursiveUpdate nginxBaseModule { + services.nginx.enableReload = false; }; } ); diff --git a/pkgs/servers/http/nginx/generic.nix b/pkgs/servers/http/nginx/generic.nix index ac58212aed82..9e3eaa8548cd 100644 --- a/pkgs/servers/http/nginx/generic.nix +++ b/pkgs/servers/http/nginx/generic.nix @@ -298,6 +298,7 @@ stdenv.mkDerivation { ; variants = lib.recurseIntoAttrs nixosTests.nginx-variants; acme-integration = nixosTests.acme.nginx; + acme-integration-without-reload = nixosTests.acme.nginx-without-reload; } // passthru.tests; }; From b3a76d495ec78b1a7b1a098eaac862d19712bdd5 Mon Sep 17 00:00:00 2001 From: Leona Maroni Date: Mon, 22 Sep 2025 15:18:12 +0200 Subject: [PATCH 2/2] nixos/nginx: allow adding new ACME certificates without nginx restart Currently, nginx gets restarted when adding a new ACME certificate, even when `services.nginx.enableReload = true` because of changes in the Wants/After/Before sections of `nginx.service`. This change moves these dependencies to `nginx-config-reload.service` and the respective ACME systemd units. --- .../services/web-servers/nginx/default.nix | 336 ++++++++++-------- nixos/tests/acme/webserver.nix | 15 +- nixos/tests/all-tests.nix | 5 +- 3 files changed, 205 insertions(+), 151 deletions(-) diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix index a29ad656cad9..87007b00ecca 100644 --- a/nixos/modules/services/web-servers/nginx/default.nix +++ b/nixos/modules/services/web-servers/nginx/default.nix @@ -1491,164 +1491,202 @@ in }; }; - systemd.services.nginx = { - description = "Nginx Web Server"; - wantedBy = [ "multi-user.target" ]; - wants = concatLists (map (certName: [ "acme-${certName}.service" ]) vhostCertNames); - after = [ - "network.target" - ] - # Ensure nginx runs with baseline certificates in place. - ++ map (certName: "acme-${certName}.service") vhostCertNames; - # Ensure nginx runs (with current config) before the actual ACME jobs run - before = map (certName: "acme-order-renew-${certName}.service") vhostCertNames; - stopIfChanged = false; - preStart = '' - ${cfg.preStart} - ${execCommand} -t - ''; + systemd.services = { + nginx = { + description = "Nginx Web Server"; + wantedBy = [ "multi-user.target" ]; + wants = lib.optionals (!cfg.enableReload) ( + concatLists (map (certName: [ "acme-${certName}.service" ]) vhostCertNames) + ); + after = [ + "network.target" + ] + # Ensure nginx runs with baseline certificates in place. + ++ lib.optionals (!cfg.enableReload) (map (certName: "acme-${certName}.service") vhostCertNames); + # Ensure nginx runs (with current config) before the actual ACME jobs run + before = lib.optionals (!cfg.enableReload) ( + map (certName: "acme-order-renew-${certName}.service") vhostCertNames + ); + stopIfChanged = false; + preStart = '' + ${cfg.preStart} + ${execCommand} -t + ''; - startLimitIntervalSec = 60; - serviceConfig = { - ExecStart = execCommand; - ExecReload = [ - "${execCommand} -t" - "${pkgs.coreutils}/bin/kill -HUP $MAINPID" - ]; - Restart = "always"; - RestartSec = "10s"; - # User and group - User = cfg.user; - Group = cfg.group; - # Runtime directory and mode - RuntimeDirectory = "nginx"; - RuntimeDirectoryMode = "0750"; - # Cache directory and mode - CacheDirectory = "nginx"; - CacheDirectoryMode = "0750"; - # Logs directory and mode - LogsDirectory = "nginx"; - LogsDirectoryMode = "0750"; - # Proc filesystem - ProcSubset = "pid"; - ProtectProc = "invisible"; - # New file permissions - UMask = "0027"; # 0640 / 0750 - # Capabilities - AmbientCapabilities = [ - "CAP_NET_BIND_SERVICE" - "CAP_SYS_RESOURCE" - ] - ++ optionals cfg.enableQuicBPF [ - "CAP_SYS_ADMIN" - "CAP_NET_ADMIN" - ]; - CapabilityBoundingSet = [ - "CAP_NET_BIND_SERVICE" - "CAP_SYS_RESOURCE" - ] - ++ optionals cfg.enableQuicBPF [ - "CAP_SYS_ADMIN" - "CAP_NET_ADMIN" - ]; - # Security - NoNewPrivileges = true; - # Sandboxing (sorted by occurrence in https://www.freedesktop.org/software/systemd/man/systemd.exec.html) - ProtectSystem = "strict"; - ProtectHome = mkDefault true; - PrivateTmp = true; - PrivateDevices = true; - ProtectHostname = true; - ProtectClock = true; - ProtectKernelTunables = true; - ProtectKernelModules = true; - ProtectKernelLogs = true; - ProtectControlGroups = true; - RestrictAddressFamilies = [ - "AF_UNIX" - "AF_INET" - "AF_INET6" - ]; - RestrictNamespaces = true; - LockPersonality = true; - MemoryDenyWriteExecute = - !( - (builtins.any (mod: (mod.allowMemoryWriteExecute or false)) cfg.package.modules) - || (cfg.package == pkgs.openresty) - ); - RestrictRealtime = true; - RestrictSUIDSGID = true; - RemoveIPC = true; - PrivateMounts = true; - # System Call Filtering - SystemCallArchitectures = "native"; - SystemCallFilter = [ - "~@cpu-emulation @debug @keyring @mount @obsolete @privileged @setuid" - ] - ++ optional cfg.enableQuicBPF [ "bpf" ]; + startLimitIntervalSec = 60; + serviceConfig = { + ExecStart = execCommand; + ExecReload = [ + "${execCommand} -t" + "${pkgs.coreutils}/bin/kill -HUP $MAINPID" + ]; + Restart = "always"; + RestartSec = "10s"; + # User and group + User = cfg.user; + Group = cfg.group; + # Runtime directory and mode + RuntimeDirectory = "nginx"; + RuntimeDirectoryMode = "0750"; + # Cache directory and mode + CacheDirectory = "nginx"; + CacheDirectoryMode = "0750"; + # Logs directory and mode + LogsDirectory = "nginx"; + LogsDirectoryMode = "0750"; + # Proc filesystem + ProcSubset = "pid"; + ProtectProc = "invisible"; + # New file permissions + UMask = "0027"; # 0640 / 0750 + # Capabilities + AmbientCapabilities = [ + "CAP_NET_BIND_SERVICE" + "CAP_SYS_RESOURCE" + ] + ++ optionals cfg.enableQuicBPF [ + "CAP_SYS_ADMIN" + "CAP_NET_ADMIN" + ]; + CapabilityBoundingSet = [ + "CAP_NET_BIND_SERVICE" + "CAP_SYS_RESOURCE" + ] + ++ optionals cfg.enableQuicBPF [ + "CAP_SYS_ADMIN" + "CAP_NET_ADMIN" + ]; + # Security + NoNewPrivileges = true; + # Sandboxing (sorted by occurrence in https://www.freedesktop.org/software/systemd/man/systemd.exec.html) + ProtectSystem = "strict"; + ProtectHome = mkDefault true; + PrivateTmp = true; + PrivateDevices = true; + ProtectHostname = true; + ProtectClock = true; + ProtectKernelTunables = true; + ProtectKernelModules = true; + ProtectKernelLogs = true; + ProtectControlGroups = true; + RestrictAddressFamilies = [ + "AF_UNIX" + "AF_INET" + "AF_INET6" + ]; + RestrictNamespaces = true; + LockPersonality = true; + MemoryDenyWriteExecute = + !( + (builtins.any (mod: (mod.allowMemoryWriteExecute or false)) cfg.package.modules) + || (cfg.package == pkgs.openresty) + ); + RestrictRealtime = true; + RestrictSUIDSGID = true; + RemoveIPC = true; + PrivateMounts = true; + # System Call Filtering + SystemCallArchitectures = "native"; + SystemCallFilter = [ + "~@cpu-emulation @debug @keyring @mount @obsolete @privileged @setuid" + ] + ++ optional cfg.enableQuicBPF [ "bpf" ]; + }; }; - }; + + # This service waits for all certificates to be available + # before reloading nginx configuration. + # sslTargets are added to wantedBy + before + # which allows the acme-order-renew-$cert.service to signify the successful updating + # of certs end-to-end. + nginx-config-reload = + let + sslServices = map (certName: "acme-${certName}.service") vhostCertNames; + sslOrderRenewServices = map (certName: "acme-order-renew-${certName}.service") vhostCertNames; + in + mkIf (cfg.enableReload || vhostCertNames != [ ]) { + wants = optionals cfg.enableReload [ "nginx.service" ]; + # Reload config directly after the self-signed certificates have been requested + # This is required for HTTP-01 ACME challenges, as the vHost with `.well-known/acme-challenge` + # must already exist. Another reload with the actual certificate is triggered + # with `security.acme.certs.<...>.reloadServices` + wantedBy = [ "multi-user.target" ] ++ optionals cfg.enableReload sslServices; + after = optionals cfg.enableReload sslServices; + before = optionals cfg.enableReload sslOrderRenewServices; + restartTriggers = optionals cfg.enableReload [ configFile ]; + # Block reloading if not all certs exist yet. + # Happens when config changes add new vhosts/certs. + unitConfig = { + ConditionPathExists = optionals (vhostCertNames != [ ]) ( + map (certName: certs.${certName}.directory + "/fullchain.pem") vhostCertNames + ); + # Disable rate limiting for this, because it may be triggered quickly a bunch of times + # if a lot of certificates are renewed in quick succession. The reload itself is cheap, + # so even doing a lot of them in a short burst is fine. + # FIXME: there's probably a better way to do this. + StartLimitIntervalSec = 0; + }; + serviceConfig = { + Type = "oneshot"; + TimeoutSec = 60; + ExecCondition = "/run/current-system/systemd/bin/systemctl -q is-active nginx.service"; + ExecStart = "/run/current-system/systemd/bin/systemctl reload nginx.service"; + }; + }; + } + # When reload is enabled, add the systemd dependency to the acme unit to prevent restarts + # of the nginx.service unit. + # This needs to be here, because of how switch-to-configuration works in the case that nginx.service + # is not started at the moment where new certificates are requested. + # Configuring this relationship in nginx.service would lead to s-t-c restarting nginx.service + # when a new certificate is added, as it will restart a unit when their direct unit properties, + # including After and Wants, change. + // lib.optionalAttrs cfg.enableReload ( + lib.listToAttrs ( + map ( + name: + lib.nameValuePair "acme-${name}" { + before = [ "nginx.service" ]; + wantedBy = [ "nginx.service" ]; + } + ) vhostCertNames + ) + ); environment.etc."nginx/nginx.conf" = mkIf cfg.enableReload { source = configFile; }; - # This service waits for all certificates to be available - # before reloading nginx configuration. - # sslTargets are added to wantedBy + before - # which allows the acme-order-renew-$cert.service to signify the successful updating - # of certs end-to-end. - systemd.services.nginx-config-reload = - let - sslOrderRenewServices = map (certName: "acme-order-renew-${certName}.service") vhostCertNames; - in - mkIf (cfg.enableReload || vhostCertNames != [ ]) { - wants = optionals cfg.enableReload [ "nginx.service" ]; - wantedBy = sslOrderRenewServices ++ [ "multi-user.target" ]; - # XXX Before the finished targets, after the renew services. - # This service might be needed for HTTP-01 challenges, but we only want to confirm - # certs are updated _after_ config has been reloaded. - after = sslOrderRenewServices; - restartTriggers = optionals cfg.enableReload [ configFile ]; - # Block reloading if not all certs exist yet. - # Happens when config changes add new vhosts/certs. - unitConfig = { - ConditionPathExists = optionals (vhostCertNames != [ ]) ( - map (certName: certs.${certName}.directory + "/fullchain.pem") vhostCertNames - ); - # Disable rate limiting for this, because it may be triggered quickly a bunch of times - # if a lot of certificates are renewed in quick succession. The reload itself is cheap, - # so even doing a lot of them in a short burst is fine. - # FIXME: there's probably a better way to do this. - StartLimitIntervalSec = 0; - }; - serviceConfig = { - Type = "oneshot"; - TimeoutSec = 60; - ExecCondition = "/run/current-system/systemd/bin/systemctl -q is-active nginx.service"; - ExecStart = "/run/current-system/systemd/bin/systemctl reload nginx.service"; - }; - }; - security.acme.certs = let - acmePairs = map ( - vhostConfig: - let - hasRoot = vhostConfig.acmeRoot != null; - in - nameValuePair vhostConfig.serverName { - group = mkDefault cfg.group; - # if acmeRoot is null inherit config.security.acme - # Since config.security.acme.certs..webroot's own default value - # should take precedence set priority higher than mkOptionDefault - webroot = mkOverride (if hasRoot then 1000 else 2000) vhostConfig.acmeRoot; - # Also nudge dnsProvider to null in case it is inherited - dnsProvider = mkOverride (if hasRoot then 1000 else 2000) null; - extraDomainNames = vhostConfig.serverAliases; - # Filter for enableACME-only vhosts. Don't want to create dud certs - } - ) (filter (vhostConfig: vhostConfig.useACMEHost == null) acmeEnabledVhosts); + # Here are two cases: + # - when no `useACMEHost` is set, the `serverName` acme certificate is the primary name and we need to configure it + # - when `useACMEHost` is set, this is also the primary name and we only need to configure the reloadServices property + acmePairs = + map ( + vhostConfig: + let + hasRoot = vhostConfig.acmeRoot != null; + in + nameValuePair vhostConfig.serverName { + reloadServices = [ "nginx.service" ]; + group = mkDefault cfg.group; + # if acmeRoot is null inherit config.security.acme + # Since config.security.acme.certs..webroot's own default value + # should take precedence set priority higher than mkOptionDefault + webroot = mkOverride (if hasRoot then 1000 else 2000) vhostConfig.acmeRoot; + # Also nudge dnsProvider to null in case it is inherited + dnsProvider = mkOverride (if hasRoot then 1000 else 2000) null; + extraDomainNames = vhostConfig.serverAliases; + # Filter for enableACME-only vhosts. Don't want to create dud certs + } + ) (filter (vhostConfig: vhostConfig.useACMEHost == null) acmeEnabledVhosts) + ++ map ( + vhostConfig: + nameValuePair vhostConfig.useACMEHost { + reloadServices = [ "nginx.service" ]; + } + ) (filter (vhostConfig: vhostConfig.useACMEHost != null) acmeEnabledVhosts); in listToAttrs acmePairs; diff --git a/nixos/tests/acme/webserver.nix b/nixos/tests/acme/webserver.nix index 5dcd8857e8d2..17fe915245d1 100644 --- a/nixos/tests/acme/webserver.nix +++ b/nixos/tests/acme/webserver.nix @@ -226,5 +226,18 @@ # Ensure the timer works, due to our shenanigans with # RemainAfterExit=true webserver.wait_until_succeeds(f"journalctl --cursor-file=/tmp/cursor | grep 'Starting Order (and renew) ACME certificate for zeroconf3.{domain}...'") - ''; + '' + + + lib.optionalString + (config.nodes.webserver.services.nginx.enable && config.nodes.webserver.services.nginx.enableReload) + '' + with subtest("Ensure that adding a second vhost does not restart nginx"): + switch_to(webserver, "zeroconf") + webserver.wait_for_unit("renew-triggered.target") + webserver.succeed("journalctl --cursor-file=/tmp/cursor") + switch_to(webserver, "zeroconf3") + webserver.wait_for_unit("renew-triggered.target") + output = webserver.succeed("journalctl --cursor-file=/tmp/cursor") + t.assertNotIn("Stopping Nginx Web Server...", output) + ''; } diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index 466fff654e07..ff1571e0d41b 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -183,7 +183,10 @@ in # keep-sorted start case=no numeric=no block=yes _3proxy = runTest ./3proxy.nix; aaaaxy = runTest ./aaaaxy.nix; - acme = import ./acme/default.nix { inherit runTest; }; + acme = import ./acme/default.nix { + inherit runTest; + inherit (pkgs) lib; + }; acme-dns = runTest ./acme-dns.nix; activation = pkgs.callPackage ../modules/system/activation/test.nix { }; activation-etc-overlay-immutable = runTest ./activation/etc-overlay-immutable.nix;