Commit graph

88 commits

Author SHA1 Message Date
Lucas Savva
34b5c5c1a4
nixos/acme: More features and fixes
- Allow for key reuse when domains are the only thing that
  were changed.
- Fixed systemd service failure when preliminarySelfsigned
  was set to false
2020-09-06 01:28:19 +01:00
Lucas Savva
67a5d660cb
nixos/acme: Run postRun script as root 2020-09-04 19:34:10 +01:00
Lucas Savva
1b6cfd9796
nixos/acme: Fix race condition, dont be smart with keys
Attempting to reuse keys on a basis different to the cert (AKA,
storing the key in a directory with a hashed name different to
the cert it is associated with) was ineffective since when
"lego run" is used it will ALWAYS generate a new key. This causes
issues when you revert changes since your "reused" key will not
be the one associated with the old cert. As such, I tore out the
whole keyDir implementation.

As for the race condition, checking the mtime of the cert file
was not sufficient to detect changes. In testing, selfsigned
and full certs could be generated/installed within 1 second of
each other. cmp is now used instead.

Also, I removed the nginx/httpd reload waiters in favour of
simple retry logic for the curl-based tests
2020-09-04 01:09:43 +01:00
Lucas Savva
982c5a1f0e
nixos/acme: Restructure module
- Use an acme user and group, allow group override only
- Use hashes to determine when certs actually need to regenerate
- Avoid running lego more than necessary
- Harden permissions
- Support "systemctl clean" for cert regeneration
- Support reuse of keys between some configuration changes
- Permissions fix services solves for previously root owned certs
- Add a note about multiple account creation and emails
- Migrate extraDomains to a list
- Deprecate user option
- Use minica for self-signed certs
- Rewrite all tests

I thought of a few more cases where things may go wrong,
and added tests to cover them. In particular, the web server
reload services were depending on the target - which stays alive,
meaning that the renewal timer wouldn't be triggering a reload
and old certs would stay on the web servers.

I encountered some problems ensuring that the reload took place
without accidently triggering it as part of the test. The sync
commands I added ended up being essential and I'm not sure why,
it seems like either node.succeed ends too early or there's an
oddity of the vm's filesystem I'm not aware of.

- Fix duplicate systemd rules on reload services

Since useACMEHost is not unique to every vhost, if one cert
was reused many times it would create duplicate entries in
${server}-config-reload.service for wants, before and
ConditionPathExists
2020-09-02 19:22:43 +01:00
Lassulus
8a141825a3
Merge pull request #89779 from jktr/acme-extra-flags
nixos/acme: extra lego flags
2020-08-22 14:29:39 +02:00
Mario Rodas
af5765b0dc
Merge pull request #85681 from Beskhue/improve-descriptions
nixos/acme: improve some descriptions
2020-07-23 00:03:05 -05:00
datafoo
cc37d7edd7 nixos/acme: execute a single lego command
Stop trying to execute `lego renew` if that is not necessary.

Fix #86184.
2020-06-19 14:56:17 +02:00
J. Konrad Tegtmeier-Rottach
1719353619 nixos/acme: add extraLegoRunFlags option 2020-06-08 02:18:31 +02:00
J. Konrad Tegtmeier-Rottach
a0189a4c49 nixos/acme: add extraLegoFlags option 2020-06-08 02:17:55 +02:00
Florian Klink
7457c78989
Merge pull request #86347 from m1cr0man/dnsdocs
nixos/acme: update documentation
2020-05-04 14:11:28 +02:00
Yegor Timoshenko
235f4c4a91
Merge pull request #83121 from emilazy/acme-use-ec256
nixos/acme: change default keyType to ec256
2020-05-03 12:41:23 +03:00
Lucas Savva
037ef70d5c
nixos/acme: fix incorrect example 2020-05-02 00:07:50 +01:00
Lucas Savva
47da7aafdf
nixos/acme: update documentation 2020-04-29 20:31:17 +01:00
Thomas Churchman
8a061ebdef nixos/acme: improve some descriptions 2020-04-21 15:50:31 +02:00
Emily
ef7e6eeaf4 nixos/acme: set maintainers to acme team 2020-04-20 01:39:31 +01:00
worldofpeace
996ae856b6
Merge pull request #85365 from immae/fix_acme_postrun
nixos/acme: Fix postRun in acme certificate being ran at every run
2020-04-18 13:16:16 -04:00
Ismaël Bouya
8e88b8dce2
nixos/acme: Fix postRun in acme certificate being ran at every run 2020-04-17 22:16:50 +02:00
Arian van Putten
5c1c642939 Revert "nixos/acme: Fix allowKeysForGroup not applying immediately"
This reverts commit 5532065d06.

As far as I can tell setting RemainAfterExit=true here completely breaks
certificate renewal, which is really bad!

the sytemd timer will activate the service unit every OnCalendar=,
however with RemainAfterExit=true the service is already active! So the
timer doesn't rerun the service!

The commit also broke the actual tests, (As it broke activation too)
but this was fixed later in https://github.com/NixOS/nixpkgs/pull/76052
I wrongly assumed that PR fixed renewal too, which it didn't!

testing renewals is hard, as we need to sleep in tests.
2020-04-16 10:37:04 +02:00
Lucas Savva
827d5e6b44
acme: share accounts between certificates
There are strict rate limits on account creation for Let's Encrypt
certificates. It is important to reuse credentails when possible.
2020-04-14 00:15:16 +01:00
Jörg Thalheim
d7ff6ab94a
acme: create certificates in subdirectory
This allows to have multiple certificates with the same common name.
Lego uses in its internal directory the common name to name the certificate.

fixes #84409
2020-04-09 08:26:07 +01:00
Maximilian Bosch
1a5289f803
nixos/acme: don't depend on multi-user.target inside a container
On boot, a container doesn't have an uplink and would run into a timeout
while waiting for cert renewal[1].

[1] https://github.com/NixOS/nixpkgs/pull/81371#issuecomment-605526099
2020-03-29 19:59:52 +02:00
Emily
62e34d1c87 nixos/acme: change default keyType to ec256
Previously, the NixOS ACME module defaulted to using P-384 for
TLS certificates. I believe that this is a mistake, and that we
should use P-256 instead, despite it being theoretically
cryptographically weaker.

The security margin of a 256-bit elliptic curve cipher is substantial;
beyond a certain level, more bits in the key serve more to slow things
down than add meaningful protection. It's much more likely that ECDSA
will be broken entirely, or some fatal flaw will be found in the NIST
curves that makes them all insecure, than that the security margin
will be reduced enough to put P-256 at risk but not P-384. It's also
inconsistent to target a curve with a 192-bit security margin when our
recommended nginx TLS configuration allows 128-bit AES. [This Stack
Exchange answer][pornin] by cryptographer Thomas Pornin conveys the
general attitude among experts:

> Use P-256 to minimize trouble. If you feel that your manhood is
> threatened by using a 256-bit curve where a 384-bit curve is
> available, then use P-384: it will increases your computational and
> network costs (a factor of about 3 for CPU, a few extra dozen bytes
> on the network) but this is likely to be negligible in practice (in a
> SSL-powered Web server, the heavy cost is in "Web", not "SSL").

[pornin]: https://security.stackexchange.com/a/78624

While the NIST curves have many flaws (see [SafeCurves][safecurves]),
P-256 and P-384 are no different in this respect; SafeCurves gives
them the same rating. The only NIST curve Bernstein [thinks better of,
P-521][bernstein] (see "Other standard primes"), isn't usable for Web
PKI (it's [not supported by BoringSSL by default][boringssl] and hence
[doesn't work in Chromium/Chrome][chromium], and Let's Encrypt [don't
support it either][letsencrypt]).

[safecurves]: https://safecurves.cr.yp.to/
[bernstein]: https://blog.cr.yp.to/20140323-ecdsa.html
[boringssl]: https://boringssl.googlesource.com/boringssl/+/e9fc3e547e557492316932b62881c3386973ceb2
[chromium]: https://bugs.chromium.org/p/chromium/issues/detail?id=478225
[letsencrypt]: https://letsencrypt.org/docs/integration-guide/#supported-key-algorithms

So there's no real benefit to using P-384; what's the cost? In the
Stack Exchange answer I linked, Pornin estimates a factor of 3×
CPU usage, which wouldn't be so bad; unfortunately, this is wildly
optimistic in practice, as P-256 is much more common and therefore
much better optimized. [This GitHub comment][openssl] measures the
performance differential for raw Diffie-Hellman operations with OpenSSL
1.1.1 at a whopping 14× (even P-521 fares better!); [Caddy disables
P-384 by default][caddy] due to Go's [lack of accelerated assembly
implementations][crypto/elliptic] for it, and the difference there seems
even more extreme: [this golang-nuts post][golang-nuts] measures the key
generation performance differential at 275×. It's unlikely to be the
bottleneck for anyone, but I still feel kind of bad for anyone having
lego generate hundreds of certificates and sign challenges with them
with performance like that...

[openssl]: https://github.com/mozilla/server-side-tls/issues/190#issuecomment-421831599
[caddy]: 2cab475ba5/modules/caddytls/values.go (L113-L124)
[crypto/elliptic]: 2910c5b4a0/src/crypto/elliptic
[golang-nuts]: https://groups.google.com/forum/#!topic/golang-nuts/nlnJkBMMyzk

In conclusion, there's no real reason to use P-384 in general: if you
don't care about Web PKI compatibility and want to use a nicer curve,
then Ed25519 or P-521 are better options; if you're a NIST-fearing
paranoiac, you should use good old RSA; but if you're a normal person
running a web server, then you're best served by just using P-256. Right
now, NixOS makes an arbitrary decision between two equally-mediocre
curves that just so happens to slow down ECDH key agreement for every
TLS connection by over an order of magnitude; this commit fixes that.

Unfortunately, it seems like existing P-384 certificates won't get
migrated automatically on renewal without manual intervention, but
that's a more general problem with the existing ACME module (see #81634;
I know @yegortimoshenko is working on this). To migrate your
certificates manually, run:

    $ sudo find /var/lib/acme/.lego/certificates -type f -delete
    $ sudo find /var/lib/acme -name '*.pem' -delete
    $ sudo systemctl restart 'acme-*.service' nginx.service

(No warranty. If it breaks, you get to keep both pieces. But it worked
for me.)
2020-03-22 05:27:20 +00:00
Silvan Mosberger
4f69262c19
Merge pull request #81369 from mweinelt/pr/acme-chmod
nixos/acme: apply chmod and ownership unconditionally
2020-03-07 03:24:46 +01:00
Yegor Timoshenko
c32da2ed9c nixos/acme: force symlink from fullchain.pem to cert.pem
Co-authored-by: emily <vcs@emily.moe>
2020-03-04 12:52:12 +03:00
Yegor Timoshenko
c16f2218da
Merge pull request #80900 from emilazy/acme-must-staple
nixos/acme: Must-Staple and extra flags
2020-03-03 03:57:40 +03:00
Yegor Timoshenko
31aefc74c5
Merge pull request #80856 from emilazy/adjust-acme
nixos/acme: adjust renewal timer options
2020-03-03 03:49:33 +03:00
Martin Weinelt
3575555fa8
nixos/acme: apply chmod and ownership unconditionally
Also separate directory and file permissions so the certificate files
don't end up with the executable bit.

Fixes #81335
2020-02-29 20:17:14 +01:00
Emily
ffb7b984b2 nixos/acme: add extraLegoRenewFlags option 2020-02-29 16:44:04 +00:00
Emily
b522aeda5a nixos/acme: add ocspMustStaple option 2020-02-29 16:44:04 +00:00
Emily
7b14bbd734 nixos/acme: adjust renewal timer options
The current weekly setting causes every NixOS server to try to renew
its certificate at midnight on the dot on Monday. This contributes to
the general problem of periodic load spikes for Let's Encrypt; NixOS
is probably not a major contributor to that problem, but we can lead by
example by picking good defaults here.

The values here were chosen after consulting with @yuriks, an SRE at
Let's Encrypt:

* Randomize the time certificates are renewed within a 24 hour period.

* Check for renewal every 24 hours, to ensure the certificate is always
  renewed before an expiry notice is sent out.

* Increase the AccuracySec (thus lowering the accuracy(!)), so that
  systemd can coalesce the renewal with other timers being run.

  (You might be worried that this would defeat the purpose of the time
  skewing, but systemd is documented as avoiding this by picking a
  random time.)
2020-02-29 14:03:36 +00:00
Martin Weinelt
5ff9441471
nixos/acme: renew after rebuild and on boot
Fixes #81069
2020-02-29 14:40:34 +01:00
Emily
8ecbd97f82 nixos/acme: move the crt to fullchain.pem
lego already bundles the chain with the certificate,[1] so the current
code, designed for simp_le, was resulting in duplicate certificate
chains, manifesting as "Chain issues: Incorrect order, Extra certs" on
the Qualys SSL Server Test.

cert.pem stays around as a symlink for backwards compatibility.

[1] 5cdc0002e9/acme/api/certificate.go (L40-L44)
2020-02-23 04:10:34 +00:00
Lucas Savva
636eb23157
nixos/acme: Fix b.example.com test 2020-02-09 11:34:17 +00:00
Lucas Savva
ac983cff48
nixos/acme: add dns-01 test, fix cert locating bug 2020-02-09 02:09:34 +00:00
Lucas Savva
2181313c54
nixos/acme: simplify email resolve logic 2020-02-03 21:37:22 +00:00
Lucas Savva
769fbf9254 nixos/acme: fix some descriptions, default acceptTerms to false 2020-01-19 18:24:04 +00:00
Lucas Savva
61665e3363 nixos/acme: ignore tmpfiles rules for null webroots 2020-01-15 09:17:11 +00:00
Lucas Savva
9467f2ba2c nixos/acme: Add logic to select right email address 2020-01-12 21:52:28 +00:00
Lucas Savva
1e3607d331 nixos/acme: replace simp-le with lego client
Lego allows users to use the DNS-01 challenge to validate their
certificates. It is mostly backwards compatible, with a few
caveats.

 - extraDomains can no longer have different webroots to the
   main webroot for the cert.
 - An email address is now mandatory for account creation

The following other changes were required:
 - Deprecate security.acme.certs.<name>.plugins, as this was
   specific to simp-le
 - Rename security.acme.validMin to validMinDays, to avoid
   confusion and errors. Lego requires the TTL to be specified in
   days
 - Add options to cover DNS challenge (dnsProvider,
   credentialsFile, dnsPropagationCheck)
 - A shared state directory is now used (/var/lib/acme/.lego)
   to avoid account creation rate limits and share credentials
   between certs
2020-01-12 21:28:53 +00:00
Ben Price
83972b80b4 nixos/acme: implement postRun using ExecStartPost
In 5532065d06, acme was changed to be
RemainAfterExit=true, but `postRun` commands are implemented as
`ExecStopPost`. Systemd now considers the service to be still running
after simp_le is finished, so won't run these commands (e.g. to reload
certificates in a webserver). Change `postRun` to use `ExecStartPost` to
ensure the commands are run in a timely manner.
2019-12-19 17:39:59 +00:00
Silvan Mosberger
4ee3e8b21d
nixos/treewide: Move rename.nix imports to their respective modules
A centralized list for these renames is not good because:
- It breaks disabledModules for modules that have a rename defined
- Adding/removing renames for a module means having to find them in the
central file
- Merge conflicts due to multiple people editing the central file
2019-12-10 02:51:19 +01:00
Silvan Mosberger
c482b65abe
nixos/acme: Fix allowKeysForGroup not applying immediately (#72056)
nixos/acme: Fix allowKeysForGroup not applying immediately
2019-11-13 23:51:34 +01:00
Félix Baylac-Jacqué
5671fa2396 nixos/modules/security/acme.nix: add server option
Add a new option permitting to point certbot to an ACME Directory
Resource URI other than Let's Encrypt production/staging one.

In the meantime, we are deprecating the now useless Let's Encrypt
production flag.
2019-10-30 11:08:12 +01:00
Franz Pletz
5d22f7afe1
nixos/acme: fix staging endpoint url
fixes #72067
2019-10-28 10:12:39 +01:00
Silvan Mosberger
5532065d06
nixos/acme: Fix allowKeysForGroup not applying immediately
Previously setting `allowKeysForGroup = true; group = "foo"` would not
apply the group permission change of the certificates until the service
gets restarted. This commit fixes this by making systemd restart the
service every time it changes.

Note that applying this commit to a system with an already running acme
systemd service doesn't fix this immediately and you still need to wait
for the next refresh (or call `systemctl restart acme-<domain>`). Once
everybody's service has restarted once this should be a problem of the
past.
2019-10-27 00:49:32 +02:00
Félix Baylac-Jacqué
0c0af28cd5 nixos/tests/letsencrypt: use Pebble instead of Boulder
Let's encrypt bumped ACME to V2. We need to update our nixos test to
be compatible with this new protocol version.

We decided to drop the Boulder ACME server in favor of the more
integration test friendly Pebble.

- overriding cacert not necessary
- this avoids rebuilding lots of packages needlessly
- nixos/tests/acme: use pebble's ca for client tests
- pebble always generates its own ca which has to be fetched

TODO: write proper commit msg :)
2019-10-23 21:17:17 +02:00
Félix Baylac-Jacqué
38e84151e0 certbot: 0.31.0 -> 0.39.0
Updating:

- nixos module to use the new `account_reg.json` file.
- use nixpkgs pebble for integration tests.

Co-authored-by: Florian Klink <flokli@flokli.de>

Replace certbot-embedded pebble
2019-10-23 13:12:11 +02:00
Frederik Rietdijk
ad1d58c622 Merge staging-next into staging 2019-08-31 10:04:20 +02:00
Arian van Putten
604b7c139f Fix letsencrypt (#60219)
* nixos/acme: Fix ordering of cert requests

When subsequent certificates would be added, they would
not wake up nginx correctly due to target units only being triggered
once. We now added more fine-grained systemd dependencies to make sure
nginx always is aware of new certificates and doesn't restart too early
resulting in a crash.

Furthermore, the acme module has been refactored. Mostly to get
rid of the deprecated PermissionStartOnly systemd options which were
deprecated. Below is a summary of changes made.

* Use SERVICE_RESULT to determine status
This was added in systemd v232. we don't have to keep track
of the EXITCODE ourselves anymore.

* Add regression test for requesting mutliple domains

* Deprecate 'directory' option
We now use systemd's StateDirectory option to manage
create and permissions of the acme state directory.

* The webroot is created using a systemd.tmpfiles.rules rule
instead of the preStart script.

* Depend on certs directly

By getting rid of the target units, we make sure ordering
is correct in the case that you add new certs after already
having deployed some.

Reason it broke before:  acme-certificates.target would
be in active state, and if you then add a new cert, it
would still be active and hence nginx would restart
without even requesting a new cert. Not good!  We
make the dependencies more fine-grained now. this should fix that

* Remove activationDelay option

It complicated the code a lot, and is rather arbitrary. What if
your activation script takes more than activationDelay seconds?

Instead, one should use systemd dependencies to make sure some
action happens before setting the certificate live.

e.g. If you want to wait until your cert is published in DNS DANE /
TLSA, you could create a unit that blocks until it appears in DNS:

```
RequiredBy=acme-${cert}.service
After=acme-${cert}.service
ExecStart=publish-wait-for-dns-script
```
2019-08-29 16:32:59 +02:00
volth
35d68ef143 treewide: remove redundant quotes 2019-08-26 21:40:19 +00:00