1036765 - Disable cipher suites that are not in the "Browser Cipher Suite" proposal that are still enabled

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: remove-obsolete-cipher-suites.patch

Let's disable the cipher suites that are still enabled that were not in the cipher suite proposal.

Telemetry data:

The first column represents the percentage of full handshakes that the Firefox 30 release negotiated using that cipher suite. The middle column represents the code for the cipher suite in the SSL_CIPHER_SUITE_* telemetry probes.

In total, 0.0316% (three hundredths of one percent) of handshakes used one of these cipher suites. Thus, the impact of this change is very likely to be minimal, both for security and for compatibility.

---------+----+---------------------------------------
0.0151%  | 22 | TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA
0.0087%  | 24 | TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA
0.0002%  | 62 | TLS_RSA_WITH_CAMELLIA_128_CBC_SHA
0.0005%  | 64 | TLS_RSA_WITH_CAMELLIA_256_CBC_SHA
---------+----+--------------------------------------
0.0009%  |  7 | TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
0.0061%  | 25 | TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
0.0000%  | 28 | TLS_DHE_DSS_WITH_AES_256_CBC_SHA
---------+----+--------------------------------------
0.0316%



You can verify this change using these websites:

  https://www.ssllabs.com/ssltest/viewMyClient.html
  https://cc.dcsec.uni-hannover.de/
  https://mikestoolbox.net/ (requires cert error override)

This is a continuation of the work from bug 934663 and friends.

If/when this is r+'d I will file another bug about removing the prefs in the release after this ships.

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8453510 [details] [diff] [review]
remove-obsolete-cipher-suites.patch

I think this is the wrong patch - it's removing cipher suites that were already removed.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: disable-obsolete-cipher-suites.patch

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8453876 [details] [diff] [review]
disable-obsolete-cipher-suites.patch

Review of attachment 8453876 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, but why not remove TLS_DHE_DSS_WITH_AES_128_CBC_SHA along with TLS_DHE_DSS_WITH_AES_256_CBC_SHA?

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +838,3 @@
>  
>   { "security.ssl3.dhe_dss_aes_128_sha",
>     TLS_DHE_DSS_WITH_AES_128_CBC_SHA, true }, // deprecated (DSS)

what about this one?

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #3)
> ::: security/manager/ssl/src/nsNSSComponent.cpp
> @@ +838,3 @@
> >  
> >   { "security.ssl3.dhe_dss_aes_128_sha",
> >     TLS_DHE_DSS_WITH_AES_128_CBC_SHA, true }, // deprecated (DSS)
> 
> what about this one?

I believe the telemetry data shows we won't break too many things if we remove that one. However, the removal of that is outside the scope of this bug, because TLS_DHE_DSS_WITH_AES_128_CBC_SHA is included in the proposal. It is the last DSS-based cipher suite, and it was chosen primarily because Google has it enabled and I didn't want to take the compatibility risk of disabling it. Feel free to bring it up on the list if you think we should remove all DSA/DSS support.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Thanks for the quick review!

I landed this on Mozilla-Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6684d05944b6

I recommend that we re-run the SSL compatibility test suite near the middle of the Mozilla Aurora 33 timeframe to assess the compatibility impact of this change plus the changes to mozilla::pkix for Firefox 33.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6684d05944b6