Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/acme: change default keyType to ec256 #83121

Merged
merged 1 commit into from May 3, 2020

Conversation

@emilazy
Copy link
Member

emilazy commented Mar 22, 2020

Motivation for this change

Currently, the NixOS ACME module defaults 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 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").

While the NIST curves have many flaws (see 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 (see "Other standard primes"), isn't usable for Web PKI (it's not supported by BoringSSL by default and hence doesn't work in Chromium/Chrome, and Let's Encrypt don't support it either).

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 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 due to Go's lack of accelerated assembly implementations for it, and the difference there seems even more extreme: this golang-nuts post 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...

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.)

cc @aanderse @arianvp @m1cr0man

@GrahamcOfBorg test acme

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.
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]: mozilla/server-side-tls#190 (comment)
[caddy]: https://github.com/caddyserver/caddy/blob/2cab475ba516fa725d012f53ca417c3e039607de/modules/caddytls/values.go#L113-L124
[crypto/elliptic]: https://github.com/golang/go/tree/2910c5b4a01a573ebc97744890a07c1a3122c67a/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.)
@emilazy
Copy link
Member Author

emilazy commented Mar 22, 2020

@GrahamcOfBorg test acme

@Mic92
Copy link
Contributor

Mic92 commented Mar 22, 2020

In my setup I actually introduced an option to use rsa keys side by side with ec256. The reason is that the Android (android 9?) and iOS (latest) stack (not the browsers but libraries) do not support elliptic curve certificates yet. Nginx has a way of supporting both certificates and use it based on what clients supprt. This rises the question if elliptic curves are ready for general use as a default or if we should wait longer.

@Mic92
Copy link
Contributor

Mic92 commented Mar 22, 2020

Here are the patches:
Mic92@a16d6a2
Mic92@5c45c46

They are quite ugly, which is why I never up streamed them.

@grahamc
Copy link
Member

grahamc commented Mar 22, 2020

I am surprised we express a default here, instead of allowing lego to pick a default on its own. I see lego also picks ec384:

https://github.com/go-acme/lego/blob/59ea57daf66d8201bd9690e227fa397732eab7f6/cmd/flags.go#L43-L47

I wonder if we should take this issue upstream to lego?

@Mic92
Copy link
Contributor

Mic92 commented Mar 22, 2020

cc @ldez

@ldez
Copy link

ldez commented Mar 22, 2020

Hi folks,

the arguments of @emilazy seem right, but I cannot change the default value like that in lego, because it's a breaking change.

So feel free to open an issue on lego as a reminder for the next major version of lego.

Thanks.

@emilazy
Copy link
Member Author

emilazy commented Mar 22, 2020

@Mic92

This rises the question if elliptic curves are ready for general use as a default or if we should wait longer.

I personally think it's fine; Mozilla's TLS config guidelines which we use for services.nginx.recommendedTlsSettings recommend P-256 for certificates:

  • ECDSA certificates are recommended over RSA certificates, as they allow the use of ECDHE with Windows 7 clients using Internet Explorer 11, as well as allow connections from IE11 on Windows Server 2008 R2

I'm surprised to hear iOS libraries don't support ECDSA when the browser does, but I think the general performance difference on mobile platforms vs. RSA probably outweighs the fuss of tweaking settings for developers. I'm not sure what problem you're having with Android; the Android docs suggest that the API should support ECDSA natively, and the SSL Labs report for my site says that handshakes work fine with ec256 certificates and recommendedTlsSettings back to Android 4.4.2 / iOS 9.

@grahamc We also specify preferred key algorithms and ciphers for OpenSSH; in general I think it's reasonable to be opinionated by default about cryptography configuration policy as many package/system defaults are woefully outdated and insecure, though for ACME deferring to client logic would probably be fine too.

@ldez Opened go-acme/lego#1091, thanks! I'm not sure this should really qualify as a breaking change since any client that supports P-384 should support P-256 and it'll only affect certificate generation.

@Mic92
Copy link
Contributor

Mic92 commented Mar 23, 2020

@emilazy It definitely did not work a few weeks ago when I was trying to get home-assistant on Android 9 and iOS with my web server and the ec384 certificate. And I debugged this down to the native library level. I don't know about ec256 though however I assumed it also does not work. Have you tested this?

@yegortimoshenko
Copy link
Member

yegortimoshenko commented Mar 23, 2020

Hm, iOS apps definitely support TLS ECDSA: https://www.ssllabs.com/ssltest/viewClient.html?name=Apple%20ATS&version=9&platform=iOS%209&key=112 (ATS docs)

Same with Android, going as far back as Android 5: https://www.ssllabs.com/ssltest/viewClient.html?name=Android&version=5.0.0&key=88

@Mic92 I can help look into why it's happening (but would be a better fit for a new issue).

@emilazy
Copy link
Member Author

emilazy commented Mar 24, 2020

For what it's worth, at least one non-Let's Encrypt ACME provider, Buypass Go SSL, supports P-256 but not P-384 (at least according to that page and their Certification Practice Statement).

@Mic92
Copy link
Contributor

Mic92 commented Mar 24, 2020

Hm, iOS apps definitely supports TLS ECDSA: ssllabs.com/ssltest/viewClient.html?name=Apple%20ATS&version=9&platform=iOS%209&key=112 (ATS docs)

Same with Android, going as far back as Android 5: ssllabs.com/ssltest/viewClient.html?name=Android&version=5.0.0&key=88

@Mic92 I can help look into why it's happening (but would be a better fit for a new issue).

I think this is just the web browser. In example can you confirm you can connect to a web server using a non-browser application. For me okhttp on Android broke with ec384 ceritificates. Okhttp uses Android's builtin TLS stack for connection. My girlfriends IPhone also failed to connect after upgrading the certificate. The webview of the application itself worked but not the internal http client that connected to the API. I don't think this should be moved to a new issue because the whole point of this PR is finding a default that works reasonable well with current platforms.

@yegortimoshenko
Copy link
Member

yegortimoshenko commented Mar 24, 2020

@Mic92 I've tested this scenario on two Android devices (versions 8.1.0 and 10) against an ECDSA P-256 certificate (https://ecc256.badssl.com) using Httper, which uses Okhttp3: https://github.com/MuShare/Httper-Android/blob/99a59acb60d82cb80455d78b0c271b8efd3a34b7/app/src/main/java/org/mushare/httper/utils/RestClient.java#L9

It works:
signal-attachment-2020-03-24-171304_001
signal-attachment-2020-03-24-171304_002

iOS ATS results I've linked to above are specifically for iOS HTTPS client, not WKWebView, see: https://developer.apple.com/documentation/bundleresources/information_property_list/nsapptransportsecurity

@emilazy
Copy link
Member Author

emilazy commented May 3, 2020

cc @NixOS/acme

It'd be nice to get this merged; lego seems to just be waiting on a major version bump to switch, and in the meantime the major performance disadvantages I mentioned in my original comment apply to everyone using the default NixOS ACME TLS setup. Being able to serve dual ECDSA/RSA setups for legacy clients is both possible to set up now that the patch splitting certificates into their own directories has been merged, and an orthogonal issue to this default setting which is also used for e.g. ACME account keys.

I don't believe this is a breaking change for us since it won't affect existing account and TLS keys, merely the default of newly-generated ones.

@emilazy emilazy requested a review from NixOS/acme May 3, 2020
Copy link
Contributor

andrew-d left a comment

I agree with the proposed change. As someone that works professionally in the infosec field, I would personally use P-256 and think it is the correct default.

Copy link
Member

yegortimoshenko left a comment

$ openssl speed ecdsap256 ecdsap384
Doing 256 bits sign ecdsa's for 10s: 432630 256 bits ECDSA signs in 9.94s 
Doing 256 bits verify ecdsa's for 10s: 140272 256 bits ECDSA verify in 9.97s
Doing 384 bits sign ecdsa's for 10s: 9724 384 bits ECDSA signs in 9.99s 
Doing 384 bits verify ecdsa's for 10s: 13028 384 bits ECDSA verify in 10.00s
OpenSSL 1.1.1d  10 Sep 2019
built on: Tue Sep 10 13:13:07 2019 UTC
options:bn(64,64) rc4(16x,int) des(int) aes(partial) idea(int) blowfish(ptr) 
compiler: gcc -fPIC -pthread -m64 -Wa,--noexecstack -Wall -O3 -DOPENSSL_USE_NODELETE -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DKECCAK1600_ASM -DRC4_ASM -DMD5_ASM -DVPAES_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DX25519_ASM -DPOLY1305_ASM -DNDEBUG
                              sign    verify    sign/s verify/s
 256 bits ecdsa (nistp256)   0.0000s   0.0001s  43524.1  14069.4
 384 bits ecdsa (nistp384)   0.0010s   0.0008s    973.4   1302.8

This change only affects newly created private keys, as @emilazy noted previously. LGTM.

@yegortimoshenko yegortimoshenko merged commit 235f4c4 into NixOS:master May 3, 2020
15 checks passed
15 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
tests.acme on aarch64-linux Success
Details
tests.acme on x86_64-linux Success
Details
@emilazy emilazy deleted the emilazy:acme-use-ec256 branch May 8, 2020
@emilazy
Copy link
Member Author

emilazy commented May 18, 2020

@NixOS/acme Should this be backported to 20.03? Switching from P-384 to P-256 is extremely unlikely to break anything, and most people are going to regenerate their account and certificate keys on upgrade to 20.03 due to the switch to lego. That feels like a good opportunity to pick the right key type up-front, and otherwise it may be an indefinitely long time before keys are recycled and they benefit from this change.

Asking now because the key type is mentioned in the release notes in #87911.

@emilazy emilazy mentioned this pull request May 18, 2020
0 of 10 tasks complete
dtzWill added a commit to dtzWill/nixpkgs that referenced this pull request May 21, 2020
nixos/acme: change default keyType to ec256

(cherry picked from commit 235f4c4)
@emilazy emilazy mentioned this pull request May 24, 2020
1 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.