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

switched to rustls for certificate verification #15329

Closed
wants to merge 1 commit into from

Conversation

avadacatavra
Copy link
Contributor

@avadacatavra avadacatavra commented Feb 1, 2017

Changed the connection code to use rustls and webpki for verification.

There is possibly a performance hit, but this is the first step to reducing dependence on openssl. I've done some investigation on performance (planning on more).

Performance evaluation: I connected to 13 https sites using rustls::verify_server_cert, rustls::parallel_verify_server_cert, and the openssl code currently used in servo.

Average time to establish a connection using:

  • verify_server_cert: 110.6259529
  • parallel_verify_server_cert: 87.99443197
  • openssl: 77.41738468 ms

This is blocked on rustls/rustls#47 and blocks #15010


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix OpenSSL1.1.0 #14203 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because I need to figure out a good way to test this

This change is Reviewable

@highfive
Copy link

highfive commented Feb 1, 2017

Heads up! This PR modifies the following files:

  • @asajeffrey: components/webdriver_server/Cargo.toml
  • @jgraham: components/webdriver_server/Cargo.toml
  • @fitzgen: components/script/dom/document.rs, components/script/Cargo.toml, components/devtools_traits/Cargo.toml, components/devtools_traits/Cargo.toml, components/devtools/Cargo.toml, components/script/webdriver_handlers.rs, components/script_traits/Cargo.toml, components/script_traits/Cargo.toml, components/script/dom/websocket.rs
  • @KiChjang: components/net/Cargo.toml, components/script/dom/document.rs, components/script/Cargo.toml, components/net/cookie.rs, components/net/lib.rs, components/net/http_loader.rs, components/net/subresource_integrity.rs, components/net_traits/lib.rs, components/net_traits/lib.rs, components/script/webdriver_handlers.rs, components/script_traits/Cargo.toml, components/script_traits/Cargo.toml, components/net_traits/Cargo.toml, components/net_traits/Cargo.toml, components/script/dom/websocket.rs, components/net/resource_thread.rs, components/net/connector.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 1, 2017
@wafflespeanut wafflespeanut added the S-needs-rebase There are merge conflict errors. label Feb 6, 2017
@wafflespeanut
Copy link
Contributor

r? @jdm

@highfive highfive assigned jdm and unassigned wafflespeanut Feb 6, 2017
@wafflespeanut wafflespeanut added the S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. label Feb 6, 2017
@avadacatavra avadacatavra mentioned this pull request Feb 8, 2017
@jdm jdm removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 13, 2017
@nox
Copy link
Contributor

nox commented Sep 24, 2017

What's blocking this?

@jdm
Copy link
Member

jdm commented Nov 15, 2017

Closing due to inactivity.

@jdm jdm closed this Nov 15, 2017
@Darkspirit Darkspirit mentioned this pull request Nov 17, 2019
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-rebase There are merge conflict errors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenSSL1.1.0
5 participants