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

Cleanup of default ciphersuite list #22243

Merged
merged 1 commit into from Dec 22, 2018

Conversation

Projects
None yet
6 participants
@Darkspirit
Copy link
Contributor

Darkspirit commented Nov 21, 2018

  • don't offer DHE ciphersuites like Chrome (Firefox is in progress of deprecating DHE as well)
  • don't offer AES-CBC-SHA2 like Firefox and Chrome
  • don't offer AES-GCM for plain RSA like Firefox
  • don't offer ECDSA with AES-CBC like Chrome
  • don't offer weak DES-CBC3-SHA
  • prefer AES256 over AES128 like Mozilla Modern, Safari and Edge

(The last line of cipher suites would be removed in the future when Servo deprecates TLS 1.0/1.1 and switches to Rustls.)

You can compare Firefox and Chrome with https://www.ssllabs.com/ssltest/viewMyClient.html.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors

This change is Reviewable

Cleanup of default ciphersuite list
* don't offer DHE ciphersuites like Chrome
* don't offer AES-CBC-SHA2 like Firefox and Chrome
* don't offer AES-GCM for plain RSA like Firefox
* don't offer ECDSA with AES-CBC like Chrome
* don't offer weak DES-CBC3-SHA
* prefer AES256 over AES128 like Mozilla Modern, Safari and Edge
@highfive

This comment has been minimized.

Copy link

highfive commented Nov 21, 2018

Heads up! This PR modifies the following files:

@highfive

This comment has been minimized.

Copy link

highfive commented Nov 21, 2018

warning Warning warning

  • These commits modify net code, but no tests are modified. Please consider adding a test!
@jdm

This comment has been minimized.

Copy link
Member

jdm commented Nov 21, 2018

@highfive highfive assigned avadacatavra and unassigned Manishearth Nov 21, 2018

@Darkspirit

This comment has been minimized.

Copy link
Contributor

Darkspirit commented Nov 27, 2018

Update: Added Firefox telemetry to the image.
Click to enlarge:

cipherlist_comparison2

Beta 64 Telemetry: TLS versions, Ciphersuites (Legend)

  • 0.99% are using TLS 1.0,
  • but 5.13% (Nightly: 3.44%) are still using plain RSA. 🤮

@avadacatavra, are you fine with this change or would you like to go beyond and remove plain RSA?

@avadacatavra

This comment has been minimized.

Copy link
Contributor

avadacatavra commented Dec 20, 2018

Sorry for the delay--for now, let's leave in plain RSA. 5% is a bit too high IMO, particularly since we're still working on compat in other components.

@avadacatavra

This comment has been minimized.

Copy link
Contributor

avadacatavra commented Dec 20, 2018

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 20, 2018

📌 Commit 027154e has been approved by avadacatavra

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 20, 2018

⌛️ Testing commit 027154e with merge 3e96bf4...

bors-servo added a commit that referenced this pull request Dec 20, 2018

Auto merge of #22243 - Darkspirit:cipherlist_cleanup, r=avadacatavra
Cleanup of default ciphersuite list

* don't offer DHE ciphersuites like Chrome (Firefox is in [progress](https://www.fxsitecompat.com/en-CA/docs/2018/dhe-cipher-suites-are-no-longer-supported-in-webrtc/) of deprecating DHE as well)
* don't offer AES-CBC-SHA2 like Firefox and Chrome
* don't offer AES-GCM for plain RSA like Firefox
* don't offer ECDSA with AES-CBC like Chrome
* don't offer weak DES-CBC3-SHA
* prefer AES256 over AES128 like [Mozilla Modern](https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility), Safari and Edge

(The last line of cipher suites would be removed in the future when Servo deprecates TLS 1.0/1.1 and switches to Rustls.)

You can compare Firefox and Chrome with https://www.ssllabs.com/ssltest/viewMyClient.html.

---
<!-- 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

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22243)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 20, 2018

💔 Test failed - status-taskcluster

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Dec 22, 2018

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 22, 2018

⌛️ Testing commit 027154e with merge 8af6d99...

bors-servo added a commit that referenced this pull request Dec 22, 2018

Auto merge of #22243 - Darkspirit:cipherlist_cleanup, r=avadacatavra
Cleanup of default ciphersuite list

* don't offer DHE ciphersuites like Chrome (Firefox is in [progress](https://www.fxsitecompat.com/en-CA/docs/2018/dhe-cipher-suites-are-no-longer-supported-in-webrtc/) of deprecating DHE as well)
* don't offer AES-CBC-SHA2 like Firefox and Chrome
* don't offer AES-GCM for plain RSA like Firefox
* don't offer ECDSA with AES-CBC like Chrome
* don't offer weak DES-CBC3-SHA
* prefer AES256 over AES128 like [Mozilla Modern](https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility), Safari and Edge

(The last line of cipher suites would be removed in the future when Servo deprecates TLS 1.0/1.1 and switches to Rustls.)

You can compare Firefox and Chrome with https://www.ssllabs.com/ssltest/viewMyClient.html.

---
<!-- 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

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22243)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 22, 2018

💔 Test failed - linux-rel-wpt

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Dec 22, 2018

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 22, 2018

⌛️ Testing commit 027154e with merge 0fcc9b4...

bors-servo added a commit that referenced this pull request Dec 22, 2018

Auto merge of #22243 - Darkspirit:cipherlist_cleanup, r=avadacatavra
Cleanup of default ciphersuite list

* don't offer DHE ciphersuites like Chrome (Firefox is in [progress](https://www.fxsitecompat.com/en-CA/docs/2018/dhe-cipher-suites-are-no-longer-supported-in-webrtc/) of deprecating DHE as well)
* don't offer AES-CBC-SHA2 like Firefox and Chrome
* don't offer AES-GCM for plain RSA like Firefox
* don't offer ECDSA with AES-CBC like Chrome
* don't offer weak DES-CBC3-SHA
* prefer AES256 over AES128 like [Mozilla Modern](https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility), Safari and Edge

(The last line of cipher suites would be removed in the future when Servo deprecates TLS 1.0/1.1 and switches to Rustls.)

You can compare Firefox and Chrome with https://www.ssllabs.com/ssltest/viewMyClient.html.

---
<!-- 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

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22243)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 22, 2018

@bors-servo bors-servo merged commit 027154e into servo:master Dec 22, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@Darkspirit Darkspirit deleted the Darkspirit:cipherlist_cleanup branch Dec 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment