diff --git a/nixos/modules/security/acme.nix b/nixos/modules/security/acme.nix index be4762da8d13..9242a12fd35f 100644 --- a/nixos/modules/security/acme.nix +++ b/nixos/modules/security/acme.nix @@ -156,6 +156,7 @@ let ${toString data.ocspMustStaple} ${data.keyType} ''; certDir = mkHash hashData; + # TODO remove domainHash usage entirely. Waiting on go-acme/lego#1532 domainHash = mkHash "${concatStringsSep " " extraDomains} ${data.domain}"; accountHash = (mkAccountHash acmeServer data); accountDir = accountDirRoot + accountHash; @@ -372,22 +373,19 @@ let echo '${domainHash}' > domainhash.txt - # Check if we can renew - if [ -e 'certificates/${keyName}.key' -a -e 'certificates/${keyName}.crt' -a -n "$(ls -1 accounts)" ]; then + # Check if we can renew. + # We can only renew if the list of domains has not changed. + if cmp -s domainhash.txt certificates/domainhash.txt && [ -e 'certificates/${keyName}.key' -a -e 'certificates/${keyName}.crt' -a -n "$(ls -1 accounts)" ]; then - # When domains are updated, there's no need to do a full - # Lego run, but it's likely renew won't work if days is too low. - if [ -e certificates/domainhash.txt ] && cmp -s domainhash.txt certificates/domainhash.txt; then + # Even if a cert is not expired, it may be revoked by the CA. + # Try to renew, and silently fail if the cert is not expired. + # Avoids #85794 and resolves #129838 + if ! lego ${renewOpts} --days ${toString cfg.validMinDays}; then if is_expiration_skippable out/full.pem; then - echo 1>&2 "nixos-acme: skipping renewal because expiration isn't within the coming ${toString cfg.validMinDays} days" + echo 1>&2 "nixos-acme: Ignoring failed renewal because expiration isn't within the coming ${toString cfg.validMinDays} days" else - echo 1>&2 "nixos-acme: renewing now, because certificate expires within the configured ${toString cfg.validMinDays} days" - lego ${renewOpts} --days ${toString cfg.validMinDays} + exit 3 fi - else - echo 1>&2 "certificate domain(s) have changed; will renew now" - # Any number > 90 works, but this one is over 9000 ;-) - lego ${renewOpts} --days 9001 fi # Otherwise do a full run @@ -406,8 +404,7 @@ let chown 'acme:${data.group}' certificates/* # Copy all certs to the "real" certs directory - CERT='certificates/${keyName}.crt' - if [ -e "$CERT" ] && ! cmp -s "$CERT" out/fullchain.pem; then + if ! cmp -s 'certificates/${keyName}.crt' out/fullchain.pem; then touch out/renewed echo Installing new certificate cp -vp 'certificates/${keyName}.crt' out/fullchain.pem diff --git a/nixos/tests/acme.nix b/nixos/tests/acme.nix index 72b7bb8a396a..80b85502d4e8 100644 --- a/nixos/tests/acme.nix +++ b/nixos/tests/acme.nix @@ -113,11 +113,19 @@ in import ./make-test-python.nix ({ lib, ... }: { # Now adding an alias to ensure that the certs are updated specialisation.nginx-aliases.configuration = { pkgs, ... }: { - services.nginx.virtualHosts."a.example.test" = { + services.nginx.virtualHosts."a.example.test" = (vhostBase pkgs) // { serverAliases = [ "b.example.test" ]; }; }; + # Must be run after nginx-aliases + specialisation.remove-extra-domain.configuration = { pkgs, ... } : { + # This also validates that useACMEHost doesn't unexpectedly add the domain. + services.nginx.virtualHosts."b.example.test" = (vhostBase pkgs) // { + useACMEHost = "a.example.test"; + }; + }; + # Test OCSP Stapling specialisation.ocsp-stapling.configuration = { pkgs, ... }: { security.acme.certs."a.example.test" = { @@ -408,6 +416,18 @@ in import ./make-test-python.nix ({ lib, ... }: { check_connection(client, "a.example.test") check_connection(client, "b.example.test") + with subtest("Can remove extra domains from a cert"): + switch_to(webserver, "remove-extra-domain") + webserver.wait_for_unit("acme-finished-a.example.test.target") + webserver.wait_for_unit("nginx.service") + check_connection(client, "a.example.test") + rc, _ = client.execute( + "openssl s_client -CAfile /tmp/ca.crt -connect b.example.test:443" + " /dev/null | openssl x509 -noout -text" + " | grep DNS: | grep b.example.test" + ) + assert rc > 0, "Removed extraDomainName was not removed from the cert" + with subtest("Can request certificates for vhost + aliases (apache-httpd)"): try: switch_to(webserver, "httpd-aliases")