Skip to content

Commit

Permalink
nixos/acme: Make account creds check more robust
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
m1cr0man authored and winterqt committed Oct 6, 2022
1 parent 39796ca commit 657ecbc
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 11 deletions.
3 changes: 2 additions & 1 deletion nixos/modules/security/acme/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
59 changes: 49 additions & 10 deletions nixos/tests/acme.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
5 changes: 5 additions & 0 deletions nixos/tests/common/acme/client/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
};
};

Expand Down

0 comments on commit 657ecbc

Please sign in to comment.