nixos/acme: Make account creds check more robust

Fixes #190493

Check if an actual key file exists. This does not
completely cover the work accountHash does to ensure
that a new account is registered when account
related options are changed.

3 files changed, 56 insertions, 11 deletions

diff --git a/nixos/modules/security/acme/default.nix b/nixos/modules/security/acme/default.nix
index 45e4dab087ec6..91ec24ab1f58d 100644
--- a/nixos/modules/security/acme/default.nix
+++ b/nixos/modules/security/acme/default.nix
@@ -377,7 +377,8 @@ let
 
         # 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
+        # We also need an account key. Avoids #190493
+        if cmp -s domainhash.txt certificates/domainhash.txt && [ -e 'certificates/${keyName}.key' -a -e 'certificates/${keyName}.crt' -a -n "$(find accounts -name '${data.email}.key')" ]; 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.
diff --git a/nixos/tests/acme.nix b/nixos/tests/acme.nix
index a31cb12477a0b..d540bc6ec31bb 100644
--- a/nixos/tests/acme.nix
+++ b/nixos/tests/acme.nix
@@ -41,6 +41,16 @@
     inherit documentRoot;
   };
 
+  simpleConfig = {
+    security.acme = {
+      certs."http.example.test" = {
+        listenHTTP = ":80";
+      };
+    };
+
+    networking.firewall.allowedTCPPorts = [ 80 ];
+  };
+
   # Base specialisation config for testing general ACME features
   webserverBasicConfig = {
     services.nginx.enable = true;
@@ -174,15 +184,24 @@ in {
 
       specialisation = {
         # Tests HTTP-01 verification using Lego's built-in web server
-        http01lego.configuration = { ... }: {
-          security.acme = {
-            certs."http.example.test" = {
-              listenHTTP = ":80";
-            };
-          };
+        http01lego.configuration = simpleConfig;
 
-          networking.firewall.allowedTCPPorts = [ 80 ];
-        };
+        renew.configuration = lib.mkMerge [
+          simpleConfig
+          {
+            # Pebble provides 5 year long certs,
+            # needs to be higher than that to test renewal
+            security.acme.certs."http.example.test".validMinDays = 9999;
+          }
+        ];
+
+        # Tests that account creds can be safely changed.
+        accountchange.configuration = lib.mkMerge [
+          simpleConfig
+          {
+            security.acme.certs."http.example.test".email = "admin@example.test";
+          }
+        ];
 
         # First derivation used to test general ACME features
         general.configuration = { ... }: let
@@ -458,12 +477,32 @@ in {
       download_ca_certs(client)
 
       # Perform http-01 w/ lego test first
-      switch_to(webserver, "http01lego")
-
       with subtest("Can request certificate with Lego's built in web server"):
+          switch_to(webserver, "http01lego")
+          webserver.wait_for_unit("acme-finished-http.example.test.target")
+          check_fullchain(webserver, "http.example.test")
+          check_issuer(webserver, "http.example.test", "pebble")
+
+      # Perform renewal test
+      with subtest("Can renew certificates when they expire"):
+          hash = webserver.succeed("sha256sum /var/lib/acme/http.example.test/cert.pem")
+          switch_to(webserver, "renew")
+          webserver.wait_for_unit("acme-finished-http.example.test.target")
+          check_fullchain(webserver, "http.example.test")
+          check_issuer(webserver, "http.example.test", "pebble")
+          hash_after = webserver.succeed("sha256sum /var/lib/acme/http.example.test/cert.pem")
+          assert hash != hash_after
+
+      # Perform account change test
+      with subtest("Handles email change correctly"):
+          hash = webserver.succeed("sha256sum /var/lib/acme/http.example.test/cert.pem")
+          switch_to(webserver, "accountchange")
           webserver.wait_for_unit("acme-finished-http.example.test.target")
           check_fullchain(webserver, "http.example.test")
           check_issuer(webserver, "http.example.test", "pebble")
+          hash_after = webserver.succeed("sha256sum /var/lib/acme/http.example.test/cert.pem")
+          # Has to do a full run to register account, which creates new certs.
+          assert hash != hash_after
 
       # Perform general tests
       switch_to(webserver, "general")
diff --git a/nixos/tests/common/acme/client/default.nix b/nixos/tests/common/acme/client/default.nix
index 503e610d1ac9e..9a37e4f2fd35e 100644
--- a/nixos/tests/common/acme/client/default.nix
+++ b/nixos/tests/common/acme/client/default.nix
@@ -9,6 +9,11 @@ in {
     defaults = {
       server = "https://${caDomain}/dir";
       email = "hostmaster@example.test";
+      # Avoid a random 0-8 minute sleep when testing renewals.
+      # We are not using LE servers in testing so this is not
+      # going to impact their load.
+      # See https://github.com/go-acme/lego/issues/1656
+      extraLegoRenewFlags = ["-no-random-sleep"];
     };
   };