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

Add `./mach build --with-rustls` #24764

Closed
wants to merge 1 commit into from
Closed

Conversation

@Darkspirit
Copy link
Contributor

Darkspirit commented Nov 17, 2019

I would like to merge this as an optional feature so everyone can already use it for testing and further development. There were earlier attempts with #17938 and #15329. Legacy TLS versions and DHE have already been disabled for OpenSSL.
Main motivation is doing this for Redox, but other platforms will benefit from it as build option as well, so this doesn't need to be an all-or-nothing decision about whether Servo potentially wanted to keep defaulting to a ton of not-safe-Rust TLS and certificate validation code for specific build targets or not(?).

With ./mach build --with-rustls we get

  • Certificate Transparency SCT verification
  • HTTP/2 enabled by default
  • a lot tested, tidy and safe Rust code

Rustls as a dependency is a blocker for integration of

Therefore it would be good to develop and test behind a build flag until you have gotten enough confidence to use it by default at the end.

Next steps should be

https://github.com/ctz/rustls is the well received Rust TLS library used by time.cloudflare.com, Baidu, and others. It depends on https://github.com/briansmith/webpki which does certificate validation and is written by the author of the same functionality in Firefox. Both depend on https://github.com/briansmith/ring which originally based on BoringSSL, Google's improved fork of OpenSSL, but was further tidied up and got lots of C and C++ replaced with Rust. These libraries were developed for Servo, based on meetings far in the past. (See wiki.) Ring is also used by Google Fuchsia.

https://blog.cloudflare.com/announcing-cfnts/

We decided to use Rustls over OpenSSL or BoringSSL because of the crate's consistent error codes and default support for authentication that is difficult to disable accidentally.

https://www.zdnet.com/article/a-rust-based-tls-library-outperformed-openssl-in-almost-every-category/
https://twitter.com/BRIAN_____/status/1146187144030896129

Rustls vs OpenSSL: https://jbp.io/2019/07/01/rustls-vs-openssl-performance.html:

  • 15% quicker to send data
  • 5% quicker to receive data
  • 20-40% quicker to full handshake (client)
  • 10% quicker to full handshake (server)
  • 30-70% quicker to resume (client)
  • 10-20% quicker to resume (server)
  • Uses <1/2 as much RAM

https://twitter.com/repi/status/1146337760921489408

love it, we nuked our OpenSSL usage at EmbarkStudios
and just use Rustls statically linked in and never want to go back

With Baidu's MesaLink there also exists a wrapper providing an OpenSSL API based on Rustls:
https://www.reddit.com/r/rust/comments/b9gd10/mesalink_10_openssl_c_apis_done_with_rust_10m/

Baidu has been using MesaLink in production with >10M monthly active users as of 12/2018.


  • ./mach build -d does not report any errors
  • ./mach build -d --with-rustls does not report any errors
  • ./mach test-tidy does not report any errors
@highfive
Copy link

highfive commented Nov 17, 2019

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/command_base.py
  • @KiChjang: components/net/Cargo.toml, components/net/resource_thread.rs, components/net/http_loader.rs, components/net/connector.rs, components/net/tests/main.rs and 1 more
@jdm
Copy link
Member

jdm commented Nov 19, 2019

Thank you for this PR! I think it's really promising that the changes required to use a different TLS engine under the hood are relatively self-contained, and it bodes well for the future. That being said, I see some drawbacks to merging this PR right now - the core team is actively focused on several high priority projects right now, and deciding on the future of the SSL stack in Servo takes time and energy that are in short supply. I think we conceptually like the idea of moving away from OpenSSL to RusTLS, but we're not comfortable making that switch without due diligence.

While choosing to merge this PR does not require making a commitment to switch to RusTLS, it does require maintaining this code across all of our existing supported platforms and dealing with any problems that arise when we update our Rust compiler toolchain. I would prefer not to do that until we're ready to consider integrating RusTLS into Servo, given the lack of time and resources we face in our existing work.

I'm going to go ahead and close this PR, but I want to emphasize that it's really interesting! The network stack of Servo is generally some of the least worked-on code in the project, and when we next focus on improving it perhaps we can revisit this topic.

@jdm jdm closed this Nov 19, 2019
@Darkspirit Darkspirit mentioned this pull request Nov 30, 2019
2 of 2 tasks complete
bors-servo added a commit that referenced this pull request Dec 9, 2019
Add ALPN and signature algorithms to OpenSSL config

* Updated http crate from 0.1.17 [to 0.1.20](https://github.com/hyperium/http/blob/master/CHANGELOG.md#0120-november-26-2019).
* Updated hyper from 0.12.33 [to 0.12.35](hyperium/hyper@v0.12.33...v0.12.35).
* Updated hyper-openssl from 0.7.0 [to 0.7.1](https://github.com/sfackler/hyper-openssl/blob/master/CHANGELOG.md#v071---2019-03-01).
* Updated openssl crate from 0.10.11 [to 0.10.26](https://github.com/sfackler/rust-openssl/blob/master/openssl/CHANGELOG.md#v01026---2019-11-22).
  * Set ALPN to h2+http/1.1 for https (Enabled HTTP2) and http/1.1 for websockets.
  * Restricted signature algorithms to the same list across platforms: EdDSA at first, then ECDSA certificates, then TLS 1.3's RSA(-PSS), then classic RSA (PKCS 1.5).
Thereby we disabled the [following](https://www.ssllabs.com/ssltest/viewClient.html?name=OpenSSL&version=1.1.1c&key=165) non-web-standard signature algorithms: SHA512/ECDSA, SHA224/ECDSA, SHA1/ECDSA, SHA224/RSA, SHA224/DSA, SHA1/DSA, SHA256/DSA, SHA384/DSA, SHA512/DSA. [SHA1/RSA](https://tools.ietf.org/html/draft-ietf-tls-md5-sha1-deprecate-00) is almost dead and now only used by a few old and broken F5 load balancers: Like TLS 1.0 deprecation, we can again deprecate this some months earlier than other browsers.
  * Added Chacha20 to TLS 1.2 ciphersuite list and preferred it like Firefox and [Rustls](https://github.com/ctz/rustls/blob/4b13a322c05c9310173513782c5d12c5afedfaf5/rustls/src/suites.rs#L377). Removed legacy and privacy-hostile plain RSA (AES256-SHA, AES128-SHA).
Compared to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) we keep ECDHE-RSA-AES256-SHA and ECDHE-RSA-AES128-SHA for now, but won't reintroduce deprecated DHE.
  * Switched server-side to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) in components/net/tests/main.rs. (The new `modern` would have been TLSv1.3-only, therefore only worked with OpenSSL 1.1.1 which is unfortunately not yet used on every target.)
  * Renamed `ssl_connector_builder(certs)` to the more neutral and long-term better fitting `create_tls_config(certs, alpn)` as it was done in #24764.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
bors-servo added a commit that referenced this pull request Dec 9, 2019
Add ALPN and signature algorithms to OpenSSL config

* Updated http crate from 0.1.17 [to 0.1.20](https://github.com/hyperium/http/blob/master/CHANGELOG.md#0120-november-26-2019).
* Updated hyper from 0.12.33 [to 0.12.35](hyperium/hyper@v0.12.33...v0.12.35).
* Updated hyper-openssl from 0.7.0 [to 0.7.1](https://github.com/sfackler/hyper-openssl/blob/master/CHANGELOG.md#v071---2019-03-01).
* Updated openssl crate from 0.10.11 [to 0.10.26](https://github.com/sfackler/rust-openssl/blob/master/openssl/CHANGELOG.md#v01026---2019-11-22).
  * Set ALPN to h2+http/1.1 for https (Enabled HTTP2) and http/1.1 for websockets.
  * Restricted signature algorithms to the same list across platforms: EdDSA at first, then ECDSA certificates, then TLS 1.3's RSA(-PSS), then classic RSA (PKCS 1.5).
Thereby we disabled the [following](https://www.ssllabs.com/ssltest/viewClient.html?name=OpenSSL&version=1.1.1c&key=165) non-web-standard signature algorithms: SHA512/ECDSA, SHA224/ECDSA, SHA1/ECDSA, SHA224/RSA, SHA224/DSA, SHA1/DSA, SHA256/DSA, SHA384/DSA, SHA512/DSA. [SHA1/RSA](https://tools.ietf.org/html/draft-ietf-tls-md5-sha1-deprecate-00) is almost dead and now only used by a few old and broken F5 load balancers: Like TLS 1.0 deprecation, we can again deprecate this some months earlier than other browsers.
  * Added Chacha20 to TLS 1.2 ciphersuite list and preferred it like Firefox and [Rustls](https://github.com/ctz/rustls/blob/4b13a322c05c9310173513782c5d12c5afedfaf5/rustls/src/suites.rs#L377). Removed legacy and privacy-hostile plain RSA (AES256-SHA, AES128-SHA).
Compared to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) we keep ECDHE-RSA-AES256-SHA and ECDHE-RSA-AES128-SHA for now, but won't reintroduce deprecated DHE.
  * Switched server-side to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) in components/net/tests/main.rs. (The new `modern` would have been TLSv1.3-only, therefore only worked with OpenSSL 1.1.1 which is unfortunately not yet used on every target.)
  * Renamed `ssl_connector_builder(certs)` to the more neutral and long-term better fitting `create_tls_config(certs, alpn)` as it was done in #24764.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
bors-servo added a commit that referenced this pull request Dec 9, 2019
Add ALPN and signature algorithms to OpenSSL config

* Updated http crate from 0.1.17 [to 0.1.20](https://github.com/hyperium/http/blob/master/CHANGELOG.md#0120-november-26-2019).
* Updated hyper from 0.12.33 [to 0.12.35](hyperium/hyper@v0.12.33...v0.12.35).
* Updated hyper-openssl from 0.7.0 [to 0.7.1](https://github.com/sfackler/hyper-openssl/blob/master/CHANGELOG.md#v071---2019-03-01).
* Updated openssl crate from 0.10.11 [to 0.10.26](https://github.com/sfackler/rust-openssl/blob/master/openssl/CHANGELOG.md#v01026---2019-11-22).
  * Set ALPN to h2+http/1.1 for https (Enabled HTTP2) and http/1.1 for websockets.
  * Restricted signature algorithms to the same list across platforms: EdDSA at first, then ECDSA certificates, then TLS 1.3's RSA(-PSS), then classic RSA (PKCS 1.5).
Thereby we disabled the [following](https://www.ssllabs.com/ssltest/viewClient.html?name=OpenSSL&version=1.1.1c&key=165) non-web-standard signature algorithms: SHA512/ECDSA, SHA224/ECDSA, SHA1/ECDSA, SHA224/RSA, SHA224/DSA, SHA1/DSA, SHA256/DSA, SHA384/DSA, SHA512/DSA. [SHA1/RSA](https://tools.ietf.org/html/draft-ietf-tls-md5-sha1-deprecate-00) is almost dead and now only used by a few old and broken F5 load balancers: Like TLS 1.0 deprecation, we can again deprecate this some months earlier than other browsers.
  * Added Chacha20 to TLS 1.2 ciphersuite list and preferred it like Firefox and [Rustls](https://github.com/ctz/rustls/blob/4b13a322c05c9310173513782c5d12c5afedfaf5/rustls/src/suites.rs#L377). Removed legacy and privacy-hostile plain RSA (AES256-SHA, AES128-SHA).
Compared to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) we keep ECDHE-RSA-AES256-SHA and ECDHE-RSA-AES128-SHA for now, but won't reintroduce deprecated DHE.
  * Switched server-side to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) in components/net/tests/main.rs. (The new `modern` would have been TLSv1.3-only, therefore only worked with OpenSSL 1.1.1 which is unfortunately not yet used on every target.)
  * Renamed `ssl_connector_builder(certs)` to the more neutral and long-term better fitting `create_tls_config(certs, alpn)` as it was done in #24764.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
bors-servo added a commit that referenced this pull request Dec 10, 2019
Add ALPN and signature algorithms to OpenSSL config

* Updated http crate from 0.1.17 [to 0.1.20](https://github.com/hyperium/http/blob/master/CHANGELOG.md#0120-november-26-2019).
* Updated hyper from 0.12.33 [to 0.12.35](hyperium/hyper@v0.12.33...v0.12.35).
* Updated hyper-openssl from 0.7.0 [to 0.7.1](https://github.com/sfackler/hyper-openssl/blob/master/CHANGELOG.md#v071---2019-03-01).
* Updated openssl crate from 0.10.11 [to 0.10.26](https://github.com/sfackler/rust-openssl/blob/master/openssl/CHANGELOG.md#v01026---2019-11-22).
  * Set ALPN to h2+http/1.1 for https (Enabled HTTP2) and http/1.1 for websockets.
  * Restricted signature algorithms to the same list across platforms: EdDSA at first, then ECDSA certificates, then TLS 1.3's RSA(-PSS), then classic RSA (PKCS 1.5).
Thereby we disabled the [following](https://www.ssllabs.com/ssltest/viewClient.html?name=OpenSSL&version=1.1.1c&key=165) non-web-standard signature algorithms: SHA512/ECDSA, SHA224/ECDSA, SHA1/ECDSA, SHA224/RSA, SHA224/DSA, SHA1/DSA, SHA256/DSA, SHA384/DSA, SHA512/DSA. [SHA1/RSA](https://tools.ietf.org/html/draft-ietf-tls-md5-sha1-deprecate-00) is almost dead and now only used by a few old and broken F5 load balancers: Like TLS 1.0 deprecation, we can again deprecate this some months earlier than other browsers.
  * Added Chacha20 to TLS 1.2 ciphersuite list and preferred it like Firefox and [Rustls](https://github.com/ctz/rustls/blob/4b13a322c05c9310173513782c5d12c5afedfaf5/rustls/src/suites.rs#L377). Removed legacy and privacy-hostile plain RSA (AES256-SHA, AES128-SHA).
Compared to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) we keep ECDHE-RSA-AES256-SHA and ECDHE-RSA-AES128-SHA for now, but won't reintroduce deprecated DHE.
  * Switched server-side to [Mozilla intermediate v5](https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28recommended.29) in components/net/tests/main.rs. (The new `modern` would have been TLSv1.3-only, therefore only worked with OpenSSL 1.1.1 which is unfortunately not yet used on every target.)
  * Renamed `ssl_connector_builder(certs)` to the more neutral and long-term better fitting `create_tls_config(certs, alpn)` as it was done in #24764.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
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

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