-
Notifications
You must be signed in to change notification settings - Fork 887
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
Force strong TLS cipher suite in rustup-init.sh for curl and wget if support is detected #2287
Conversation
If we get rid of the new variable RUSTUP_STRONG_TLS_CS and make it a private string, then I can more easily add support for GnuTLS, in addition to OpenSSL/LibreSSL/BoringSSL. For wget (untested example):
The strong cipher suites match what is used by Firefox 68 ESR with all weak cipher suites disabled using |
What I'd suggest is that the default for the variable should be the empty string. If the user sets it non-empty then we use that, otherwise we attempt to detect the correct set by looking for gnutls vs. openssl. |
I need to redo the GnuTLS cipher suite string (aka priority string). |
Unlike OpenSSL, GnuTLS doesn't seem as forgiving of unknown values in the cipher suites list. So I didn't bother with removing/adding SIGN or GROUP categories. This GnuTLS priority string isn't exactly in the same sequence as OpenSSL's cipher suite but it's close enough:
|
@kinnison the MR was updated to incorporate your suggestion. Please let me know if it works for you. I tested on Fedora 30 and OpenSUSE Leap 15.1 clients with an nginx server logging which cipher suite was used for download. Also, I updated GnuTLS priority string to enable TLS 1.3 (if supported) without explicitly specifying it. This may improve potential compatibility issues on older systems using wget+gnutls. The resulting cipher suites and TLS versions remain the same. The ordering is slightly different from OpenSSL but not bad enough to devote more time.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks excellent thank you for your efforts.
I've been through the checks and they look good. My only concern is that macos builders seem to be skipping strong cipher suites.
If you expected this then that's fine, otherwise it may be worth popping some debugging into it and looking at the macos builder behaviour. Sadly I do not have a mac so cannot help you determine if this is expected on our builder or not.
Once you let me know your intent with the macos side of things, if it's to wait and see, I'll merge.
Great catch on the macOS! I'll investigate on macOS 10.14 and let you know what I find within 2 days (hopefully sooner). |
@kinnison good news, strong cipher suites are detected and used on macOS 10.14 with /usr/bin/curl (Sep 20, 2019 timestamp and LibreSSL). Thanks again for your excellent suggestions and feedback. Please squash+merge if you'd like. Proposed squashed commit msg:
|
Could you please do the squash etc, so that I'm merging your commit? I'm not in a position to do that right now because I'm at work, but I can push buttons on github :D |
@kinnison yes, I'll be happy to use git to squash tonight if needed. I was limiting my GitHub activities to a browser, but my concerns about git vs browser privacy were mostly addressed (a 3rd-party's language about GitHub activity data collection caused confusion). BTW, GitHub added a repository setting to provide "Squash & Merge" button you can use from a web browser by clicking on the down arrow portion of the normal "Merge" button. |
Force curl and wget to use strong TLS cipher suites if supported by local tools. If RUSTUP_TLS_CIPHERSUITES variable is set by user then use it. Closes rust-lang#2284. curl and wget TLS backends supported for strong TLS cipher suites: GnuTLS, OpenSSL, LibreSSL and BoringSSL. Other backends (NSS, WolfSSL, etc.) fall back to prior behavior but can also handle user-specified cipher suites in any syntax required (syntax is not checked by script). curl and wget (if support is detected) will use the same strong TLS 1.2-1.3 cipher suite as Firefox 68 ESR with all weak cipher suites disabled using about:config. Sequence of all 9 cipher suites for OpenSSL is identical to Firefox (slightly different for GnuTLS). DHE is excluded from TLS 1.2 because servers often use bad DH params (see RFC 7919). GnuTLS priority string produces: TLS_AES_128_GCM_SHA256 0x13, 0x01 TLS1.3 TLS_CHACHA20_POLY1305_SHA256 0x13, 0x03 TLS1.3 TLS_AES_256_GCM_SHA384 0x13, 0x02 TLS1.3 TLS_ECDHE_ECDSA_AES_128_GCM_SHA256 0xc0, 0x2b TLS1.2 TLS_ECDHE_ECDSA_CHACHA20_POLY1305 0xcc, 0xa9 TLS1.2 TLS_ECDHE_ECDSA_AES_256_GCM_SHA384 0xc0, 0x2c TLS1.2 TLS_ECDHE_RSA_AES_128_GCM_SHA256 0xc0, 0x2f TLS1.2 TLS_ECDHE_RSA_CHACHA20_POLY1305 0xcc, 0xa8 TLS1.2 TLS_ECDHE_RSA_AES_256_GCM_SHA384 0xc0, 0x30 TLS1.2 GnuTLS priority string could be hardened more but it isn't forgiving of unknown/unsupported values, so the bare minimum was specified.
@kinnison MR is squashed and ready to merge, no other changes to the script. 🎉 b2sum of new rustup-init.sh: 726ff184dc8358847d6bc145a489fd2aea029cda6876e1ea87bf840f4d73d0665974c3891b74cd4ee13c6814e8705e7604ce02381a0685338dac3b5e0d61fbcf |
Force strong TLS cipher suite in rustup-init.sh for curl and wget if support is detected
Force curl and wget to use strong TLS cipher suites if supported by local tools. If RUSTUP_TLS_CIPHERSUITES variable is set by user then use it. Closes #2284.
curl and wget TLS backends supported for strong TLS cipher suites: GnuTLS, OpenSSL, LibreSSL and BoringSSL. Other backends (NSS, WolfSSL, etc.) fall back to prior behavior but can also handle user-specified cipher suites in any syntax required (syntax is not checked by script).
curl and wget (if support is detected) will use the same strong TLS 1.2-1.3 cipher suite as Firefox 68 ESR with all weak cipher suites disabled using
about:config
. Sequence of all 9 cipher suites is identical for OpenSSL (to Firefox) and slightly different for GnuTLS.(click to expand) 📷 SSL Client Test for Firefox with Strong TLS 1.2-1.3 Cipher Suites
Status
Detection of local tools other than OpenSSL or GnuTLS is out of scope due to time required for all possible combinations (NSS, Schannel, WolfSSL, etc.).
Special thanks to @kinnison for his excellent feedback and suggestions.
Details
Tests used nginx server configured for TLS 1.2.
If local tools support it, make curl and wget require TLS 1.3 + TLS 1.2 with strong cipher suites specified in RUSTUP_STRONG_TLS_CS.
RUSTUP_STRONG_TLS_CS can be overridden by the user if they need to specify syntax other than OpenSSL/LibreSSL/BoringSSL such as NSS, Schannel, WolfSSL, etc.
(click to expand) More details
Strong cipher suite list defaults to:
TLS_AES_128_GCM_SHA256:
TLS_CHACHA20_POLY1305_SHA256:
TLS_AES_256_GCM_SHA384:
ECDHE-ECDSA-AES128-GCM-SHA256:
ECDHE-RSA-AES128-GCM-SHA256:
ECDHE-ECDSA-CHACHA20-POLY1305:
ECDHE-RSA-CHACHA20-POLY1305:
ECDHE-ECDSA-AES256-GCM-SHA384:
ECDHE-RSA-AES256-GCM-SHA384
The first 3 cipher suites are TLS 1.3. The remaining are TLS 1.2
cipher suites limited to ECDHE key exchange + AEAD cipher.
DHE is disabled for TLS 1.2 due to frequent misconfiguration on
servers such as not specifying 2048-bit or stronger DH group with
safe primes (see RFC 7919).
The OpenSSL syntax was chosen for cipher suites because:
RUSTUP_TLS_CIPHERSUITES can be specified by user with any syntax.