Closed Bug 934663 Opened 11 years ago Closed 11 years ago

Change set of cipher suites enabled by default in Gecko to match cipher suite proposal

Categories

(Core :: Security: PSM, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox26 --- unaffected
firefox27 + verified

People

(Reporter: briansmith, Assigned: briansmith)

References

Details

Attachments

(1 file)

See https://briansmith.org/browser-ciphersuites-01.html.

For now, there will be three differences:
1. TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 and TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 are not implemented in NSS, we we can't enable them yet. I will find/file a separate bug for enabling these in PSM when they are enabled in NSS.

2. I kept TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA and TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
enabled because we should make sure we have the ephemeral key exchange version of every RSA key exchange cipher suite enabled if possible. (Note that there aren't any TLS_DHE_RSA_WITH_RC4_* cipher suites.) I will update the proposal to add these two cipher suites.

3. The ordering of cipher suites doesn't match the ordering in the proposal, because the ordering is controlled by NSS. I will file a separate bug about fixing the ordering.
To verify this fix, try one or more of the following:

1. Visit https://www.mikestoolbox.net/ (Yes, it really does require a certificate error exception) and compare the reported cipher suites to the set recommended at https://briansmith.org/browser-ciphersuites-01.html, taking into consideration the expected differences in comment 0.

2. Make a connection to Google and verify that the TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 cipher suite is being used.

3. Visit any HTTPS site with Wireshark active and verify that the ClientHello we send contains all the above cipher suites.
Assignee: nobody → brian
Status: NEW → ASSIGNED
Attachment #826989 - Flags: review?(cviecco)
One more thing: I removed the entries for the cipher suites preferences from netwerk/base/public/security-prefs.js. The effect that this has is to hide these prefs from the about:config dialog box. This is a good idea because we don't really want people tweaking these prefs, IMO. Also, it makes it easier for us to do the right thing when modifying the list since we won't have two lists to keep in sync.
So the patch has the following suites enabled:
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
TLS_DHE_RSA_WITH_AES_128_CBC_SHA
TLS_DHE_RSA_WITH_AES_256_CBC_SHA
SSL_DHE_RSA_WITH_3DES_EDE_CBC_SHA
TLS_DHE_DSS_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_RC4_128_SHA
TLS_ECDHE_ECDSA_WITH_RC4_128_SHA
TLS_RSA_WITH_AES_128_CBC_SHA
TLS_RSA_WITH_AES_256_CBC_SHA
SSL_RSA_WITH_3DES_EDE_CBC_SHA
SSL_RSA_WITH_RC4_128_SHA
SSL_RSA_WITH_RC4_128_MD5

(I think you want to get them in that order, but I understand that it requires changes in other places?)

What I'm missing is:
TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (I suggest it as first TLS_DHE_*)
TLS_RSA_WITH_AES_128_GCM_SHA256 (I suggest it was first TKS_RSA_*)

Those (and aes256 version) are supported by apache 2.2 while you need 2.4 for the ECDHE versions.

PS: Shouldn't those that are disabled by default actually get a "false"?
(In reply to Kurt Roeckx from comment #3)
> (I think you want to get them in that order, but I understand that it
> requires changes in other places?)

Right. I will file the NSS bug for that soon.

> What I'm missing is:
> TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (I suggest it as first TLS_DHE_*)
> TLS_RSA_WITH_AES_128_GCM_SHA256 (I suggest it was first TKS_RSA_*)

Let's discuss whether we should add these on the mailing list thread. It's out of scope for this bug. I will post a message to the mailing list thread soon explaining again why I don't want to add these (yet).

> PS: Shouldn't those that are disabled by default actually get a "false"?

Static variables are zero-initialized so it isn't necessary. I avoided adding the explicit 'false' to minimize the changes in the diff, to make it easier to review. I can add a follow-up patch to make the 'false' explicit if desired.
Comment on attachment 826989 [details] [diff] [review]
Change set of enabled cipher suites

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

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +881,1 @@
>   {"security.ssl3.dhe_rsa_camellia_256_sha", TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA}, // 256-bit Camellia encryption with RSA, DHE, and a SHA1 MAC

From here on the last parameter is not explicit. I would add a note saying somethink like: since this is a static const the missing parameters are set to zero.
Attachment #826989 - Flags: review?(cviecco) → review+
I disagree about preferring 128 bit variants over 256 bit variants, and also about removing cipher preferences from about:config, user choice is important.
(In reply to BoerenkoolMetWorst from comment #6)
> I disagree about preferring 128 bit variants over 256 bit variants, and also
> about removing cipher preferences from about:config, user choice is
> important.

This patch does not change the preference from 256 to 128 bits. For more information please see the mozilla-dev-crypto maillist, where this was discussed in depth.
Also, you can enable the disabled prefs by adding a new preference value (yes this disables easy modification). User choice, while user-unfriendly, remains there.
Are bug 926773 and bug 663320 duplicates of this one?
(In reply to Coyote from comment #8)
> Are bug 926773 and bug 663320 duplicates of this one?

bug 926773 is superseeded by this one.
bug 663320 is not nor a dup. (AES_256_CGM)

(In reply to Camilo Viecco (:cviecco) from comment #7)
> (In reply to BoerenkoolMetWorst from comment #6)
> > I disagree about preferring 128 bit variants over 256 bit variants, and also
> > about removing cipher preferences from about:config, user choice is
> > important.
> 
> This patch does not change the preference from 256 to 128 bits. For more
> information please see the mozilla-dev-crypto maillist, where this was
> discussed in depth.
> Also, you can enable the disabled prefs by adding a new preference value
> (yes this disables easy modification). User choice, while user-unfriendly,
> remains there.

Thank you for the clarification.
I landed a slightly different version of the patch:
https://hg.mozilla.org/integration/mozilla-inbound/rev/23e213d57704

The difference is that I did not disable the Camellia cipher suites. During IETF 88, I learned some things that might change our decision about whether to disable them or not, so I'm taking the conservative approach and leaving them enabled. We can disable them later if we decided not to amend the "spec" for this work.

This conservative approach will also make it more reasonable to uplift this to mozilla-aurora.
I will ask for uplift to mozilla-aurora next week.
Flags: needinfo?(brian)
Target Milestone: --- → mozilla28
Depends on: 839310
Flags: needinfo?(brian)
Flags: needinfo?(brian)
No longer depends on: 936818
(In reply to Brian Smith from comment #4)
> (In reply to Kurt Roeckx from comment #3)
> > What I'm missing is:
> > TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (I suggest it as first TLS_DHE_*)
> > TLS_RSA_WITH_AES_128_GCM_SHA256 (I suggest it was first TKS_RSA_*)
> 
> Let's discuss whether we should add these on the mailing list thread. It's
> out of scope for this bug. I will post a message to the mailing list thread
> soon explaining again why I don't want to add these (yet).

I've asked about those on the list before and nobody replied to that.  I'm also still waiting for that message to the list.
https://hg.mozilla.org/mozilla-central/rev/23e213d57704
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 937789
With today's aurora update I ended up with those ciphers enabled by default:
ECDHE-ECDSA-AES128-SHA
ECDHE-RSA-AES128-SHA
ECDHE-ECDSA-AES256-SHA
ECDHE-RSA-AES256-SHA
ECDHE-ECDSA-3DES-EDE-SHA
ECDHE-RSA-3DES-EDE-SHA
ECDHE-ECDSA-RC4128-SHA
ECDHE-RSA-RC4128-SHA
DHE-RSA-AES128-SHA
DHE-DSS-AES128-SHA
DHE-RSA-CAMELLIA128-SHA
DHE-DSS-CAMELLIA128-SHA
DHE-RSA-AES256-SHA
DHE-DSS-AES256-SHA
DHE-RSA-CAMELLIA256-SHA
DHE-DSS-CAMELLIA256-SHA
DHE-RSA-3DES-EDE-SHA
DHE-DSS-3DES-EDE-SHA
ECDH-ECDSA-AES128-SHA
ECDH-RSA-AES128-SHA
ECDH-ECDSA-AES256-SHA
ECDH-RSA-AES256-SHA
ECDH-ECDSA-3DES-EDE-SHA
ECDH-RSA-3DES-EDE-SHA
ECDH-ECDSA-RC4128-SHA
ECDH-RSA-RC4128-SHA
RSA-AES128-SHA
RSA-CAMELLIA128-SHA
RSA-AES256-SHA
RSA-CAMELLIA256-SHA
RSA-SEED-SHA
RSA-FIPS-3DES-EDE-SHA
RSA-3DES-EDE-SHA
RSA-RC4128-SHA
RSA-RC4128-MD5

This isn't what I expected.  I for instance didn't expect any ECDH-* ciphers to be enabled.

The following 2 seem to be disabled by default in the prefs, but when enabled end up at the top of the list.
ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
ECDHE_RSA_WITH_AES_128_GCM_SHA256
(In reply to Kurt Roeckx from comment #17)
> With today's aurora update I ended up with those ciphers enabled by default:
(snip)
> This isn't what I expected.  I for instance didn't expect any ECDH-* ciphers
> to be enabled.

In 27 aurora and earlier, cipher suites has not changed at all.
Only 28 nightly has been affected by this bug.

Ciphers enabled by default in 28 nightly (no ECDH-* cuites is enabled)
ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
ECDHE_RSA_WITH_AES_128_GCM_SHA256
ECDHE_ECDSA_WITH_AES_128_CBC_SHA
ECDHE_RSA_WITH_AES_128_CBC_SHA
ECDHE_ECDSA_WITH_AES_256_CBC_SHA
ECDHE_RSA_WITH_AES_256_CBC_SHA
ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
ECDHE_ECDSA_WITH_RC4_128_SHA
ECDHE_RSA_WITH_RC4_128_SHA
DHE_RSA_WITH_AES_128_CBC_SHA
DHE_DSS_WITH_AES_128_CBC_SHA
DHE_RSA_WITH_CAMELLIA_128_CBC_SHA
DHE_RSA_WITH_AES_256_CBC_SHA
DHE_DSS_WITH_AES_256_CBC_SHA
DHE_RSA_WITH_CAMELLIA_256_CBC_SHA
DHE_RSA_WITH_3DES_EDE_CBC_SHA
RSA_WITH_AES_128_CBC_SHA
RSA_WITH_CAMELLIA_128_CBC_SHA
RSA_WITH_AES_256_CBC_SHA
RSA_WITH_CAMELLIA_256_CBC_SHA
RSA_WITH_3DES_EDE_CBC_SHA
RSA_WITH_RC4_128_SHA
RSA_WITH_RC4_128_MD5

> The following 2 seem to be disabled by default in the prefs, but when
> enabled end up at the top of the list.
> ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
> ECDHE_RSA_WITH_AES_128_GCM_SHA256

These are already enabled in 28 nightly.
(In reply to Kosuke Kaizuka from comment #18)
> (In reply to Kurt Roeckx from comment #17)
> > With today's aurora update I ended up with those ciphers enabled by default:
> In 27 aurora and earlier, cipher suites has not changed at all.
> Only 28 nightly has been affected by this bug.

I'm telling you that in aurora this morning the list didn't change yet, but this afternoon (CET) it changed to the list I showed.
This might be the(In reply to Kurt Roeckx from comment #19)
> I'm telling you that in aurora this morning the list didn't change yet, but
> this afternoon (CET) it changed to the list I showed.

That might be the result of an update to NSS and not firefox itself.
Comment on attachment 826989 [details] [diff] [review]
Change set of enabled cipher suites

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 926116

User impact if declined: AES-GCM doesn't work, which is the main motivation for using/testing TLS 1.2.

Testing completed (on m-c, etc.): This has been on mozilla-central since revision 23e213d57704 was checked in on 2013-11-11 in comment 14. Additionally, Google Chrome shipped a release with the AES-GCM code.

Risk to taking this patch (and alternatives if risky): This is very low risk. We disabled a few never-used cipher suites with this patch. There is some small risk that doing so might have some compatibility impact, but we won't fully know for sure until we get user feedback from testing. Note that the telemetry in bug 707275 will help with this, especially if bug 707275 gets uplifted to mozilla-beta.

String or IDL/UUID changes made by this patch: none.
Attachment #826989 - Flags: approval-mozilla-aurora?
(In reply to Kurt Roeckx from comment #20)
> This might be the(In reply to Kurt Roeckx from comment #19)
> > I'm telling you that in aurora this morning the list didn't change yet, but
> > this afternoon (CET) it changed to the list I showed.
> 
> That might be the result of an update to NSS and not firefox itself.

The order is controlled by NSS and was changed in NSS bug 936828. The updated NSS (NSS 3.15.4 beta 3) landed in mozilla-central yesterday when I landed the latest patch in bug 898431.

I just asked for this patch to be uplifted to mozilla-aurora above, so that AES-GCM would be enabled in Aurora if the uplift request is approved.
(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #22)
> The order is controlled by NSS and was changed in NSS bug 936828. The
> updated NSS (NSS 3.15.4 beta 3) landed in mozilla-central yesterday when I
> landed the latest patch in bug 898431.

s/mozilla-central/mozilla-aurora/. It landed in mozilla-central on 2013-11-11, as I mentioned in comment 21.
Maybe not the right place here, but there's something weird to me with this site:
https://sparen.nibcdirect.nl/

Nightly negotiates: SSL_RSA_WITH_RC4_128_SHA

However, this site should be able to negotiate TLS 1.2 according to ssllabs; these ciphers are available:

TLS_RSA_WITH_RC4_128_SHA	
TLS_RSA_WITH_AES_256_CBC_SHA	
TLS_RSA_WITH_AES_128_CBC_SHA

For instance, Chrome seems to do TLS 1.2 according to the SSLlabs simulation report.
(In reply to Coyote from comment #25)
> Maybe not the right place here, but there's something weird to me with this
> site:
> https://sparen.nibcdirect.nl/
> 
> Nightly negotiates: SSL_RSA_WITH_RC4_128_SHA

Thanks for the feedback. This is also what Chrome Release and Chrome Canary negotiate. Note that Firefox doesn't show the TLS version in the UI like Chrome does, but that doesn't mean that Firefox isn't using TLS 1.2.
Ah ok, I thought that Firefox did show if SSL was negotiated or TLS 1.x and that the part not shown what TLS 1.x version.

So this feature should be on a wishlist ;-)

Thanks for the explanation.
Keywords: qawanted
QA Contact: mwobensmith
So as far as I know this isn't in aurora yet.  Will this get uplifted?
Attachment #826989 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Kurt Roeckx from comment #28)
> So as far as I know this isn't in aurora yet.  Will this get uplifted?

yep, I meant to approve it last time when setting the flags/ QA contact, it slipped. I've a+ed it, should land today/tomorrow.
I will land this today along with the NSS bugfix that it requires.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (X11; Linux i686; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0

Reproduced with Firefox 26 beta 1 (Build ID: 20131028225529): TLS_ECDHE_RSA_WITH_RC4_128_SHA cipher suite is used when making a connection to Google.

Verified as fixed on Firefox 27 beta 4 (build ID: 20140106141415) and latest Aurora 28.0a2 (Build ID: 20140107004003) with STR from comment 1:
1. The following ciphers are enabled by default (including the differences from comment 0):
ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
ECDHE_RSA_WITH_AES_128_GCM_SHA256
ECDHE_ECDSA_WITH_AES_128_CBC_SHA
ECDHE_RSA_WITH_AES_128_CBC_SHA
ECDHE_ECDSA_WITH_AES_256_CBC_SHA
ECDHE_RSA_WITH_AES_256_CBC_SHA
ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
ECDHE_ECDSA_WITH_RC4_128_SHA
ECDHE_RSA_WITH_RC4_128_SHA
DHE_RSA_WITH_AES_128_CBC_SHA
DHE_DSS_WITH_AES_128_CBC_SHA
DHE_RSA_WITH_CAMELLIA_128_CBC_SHA
DHE_RSA_WITH_AES_256_CBC_SHA
DHE_DSS_WITH_AES_256_CBC_SHA
DHE_RSA_WITH_CAMELLIA_256_CBC_SHA
DHE_RSA_WITH_3DES_EDE_CBC_SHA
RSA_WITH_AES_128_CBC_SHA
RSA_WITH_CAMELLIA_128_CBC_SHA
RSA_WITH_AES_256_CBC_SHA
RSA_WITH_CAMELLIA_256_CBC_SHA
RSA_WITH_3DES_EDE_CBC_SHA
RSA_WITH_RC4_128_SHA
RSA_WITH_RC4_128_MD5

2. TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 cipher suite is used when making a connection to Google.
3. When navigating to https://www.mozilla.org with Wireshark active, the Client Hello contains all the above cipher suites.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Why no TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 and TLS_RSA_WITH_AES_128_GCM_SHA256?
Wikipedia for example only has support for the RSA version:
https://www.ssllabs.com/ssltest/analyze.html?d=wikipedia.org
Hi,
I see only:
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b)
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)

Where are others? For example:
TLS_RSA_WITH_AES_256_CBC_SHA256 (0x3d)

Many web-sites have only TLS_RSA_WITH_AES_256_CBC_SHA256 as kind of strong(even without PFS) and weak RC4 and 3DES. If I have not TLS_RSA_WITH_AES_256_CBC_SHA256 server will choose RC4 or 3DES!

Why Mozilla doesn't add others "SHA256" cipher suits? What is the problem to add them, is it so hard? :)
(In reply to BoerenkoolMetWorst from comment #33)
> Why no TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 and
> TLS_RSA_WITH_AES_128_GCM_SHA256?
> Wikipedia for example only has support for the RSA version:
> https://www.ssllabs.com/ssltest/analyze.html?d=wikipedia.org

> Why Mozilla doesn't add others "SHA256" cipher suits? What is the problem to
> add them, is it so hard? :)

I will replyto the thread that Rasj started on the dev-tech-crypto mailing list:
https://groups.google.com/d/msg/mozilla.dev.tech.crypto/xNcq_2g30BU/kOyr-wkfZb0J

You can subscribe to dev-tech-crypto here: https://lists.mozilla.org/listinfo/dev-tech-crypto

Please don't continue the discussion of the cipher suite decisions in this bug.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: