acme: share accounts between certificates

[m1cr0man]

Fixes #85152

Motivation for this change

There are strict rate limits on account creation for Let's Encrypt certificates. It is important to reuse credentials when possible.

This is a regression from #84781. Having tested on my own system which had this change committed, this new symlink logic will not affect already updated hosts and not trigger another account refresh.

I am tempted to check if /var/lib/acme/.lego/${cert} is a directory and remove it, thus undoing the effect of #85152 but I'm uneasy about deleting folders automatically :P Let me know what you think.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• [ ]Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[emilazy]

LGTM, thanks!

@GrahamcOfBorg build nixosTests.acme

[emilazy]

I am tempted to check if /var/lib/acme/.lego/${cert} is a directory and remove it, thus undoing the effect of #85152

Guessing you mean /var/lib/acme/.lego/${cert}/accounts? I think it's a bad idea to delete people's account private keys without their consent; they're not terribly valuable, but they do matter for revocations, and CAA records can tie certificate issuance to account IDs, so although the window of opportunity here was small, we should probably support users who already got migrated to one account per certificate.

@GrahamcOfBorg test acme

[emilazy]

(cc @grahamc; not sure whether I was just too impatient or whether ofborg failed to recognize my command until I posted it in a non-review comment?)

[m1cr0man]

@emilazy I fully agree with that. I'll keep the PR as-is then, and support both scenarios. 🙂

[emilazy]

Not sure what's up with either of the tests; the AArch64 test isn't running because I'm not a trusted user, the x86_64 test is failing cryptically. We know the ACME tests are flaky, so let's hope it's just nondeterminism and spin the wheel again...

@GrahamcOfBorg test acme

[lukateras]

@GrahamcOfBorg test acme

[Mic92]

backport:

[detached HEAD ecfd73d] acme: share accounts between certificates
Author: Lucas Savva [email protected]
Date: Mon Apr 13 23:54:44 2020 +0100
1 file changed, 2 insertions(+), 1 deletion(-)