1487517 - Cannot establish connections with cipher TLS_DHE_RSA_WITH_AES_256_CBC_SHA

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

https://forums.databricks.com works in 61.0.2 and not 63; connections fail with SSL_ERROR_NO_CYPHER_OVERLAP.

Connections in 61.0.2 use the TLS_DHE_RSA_WITH_AES_256_CBC_SHA cipher suite.

:jcj ran a mozregression and pointed to Bug 1479501 as a likely cause. Additionally, in 63.0b1 (dev edition), resetting the value of the "security.tls.version.fallback-limit" preference to 3 (from the new value of 4) fixes the problem.

Comment 1

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

In our TLS 1.3 handshake, we don't offer TLS_DHE_RSA_WITH_AES_256_CBC_SHA, presumably because AES-CBC ciphersuites have been removed from TLS 1.3, according to a random presentation I found. However, we do we do offer TLS_RSA_WITH_AES_256_CBC_SHA (presumably for servers that don't implement TLS 1.3, so they have a non-TLS 1.3 ciphersuite to choose?). Maybe we're supposed to include the other ciphersuites we have enabled for TLS <1.3?

Comment 2

[Martin Thomson [:mt:]]

I think perhaps we need to consider reverting Bug 1279479 for the moment.  It has probably helped our DHE numbers, but my plan (hope) now is to remove the DHE suites when we get rid of TLS 1.0 (which seems like it might be feasible in the next few years).

Comment 3

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

Heh - got mid-aired basically saying that.

Comment 4

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

Attachment: bug 1487517 - un-do ciphersuite hiding from bug 1279479 and bug 1316300 r?mt

In bug 1279479 we hid DHE ciphersuites from TLS 1.3 handshakes, assuming we
would fall back to TLS 1.2 if the peer needed such a ciphersuite. However, as of
bug 1479501, we don't fall back by default, so this just means we can't
negotiate these ciphersuites. This patch un-hides these ciphersuites from the
TLS 1.3 handshake.

Comment 5

[Martin Thomson [:mt:]]

Comment on attachment 9005503 [details]
bug 1487517 - un-do ciphersuite hiding from bug 1279479 and bug 1316300 r?mt

Martin Thomson [:mt:] has approved the revision.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07600cea6657
un-do ciphersuite hiding from bug 1279479 and bug 1316300 r=mt

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/07600cea6657

Comment 8

[Mike Taylor [:miketaylr]]

Ryan, Martin, do we think this is important (and safe) enough to nominate for uplift to 62 (...this late in the cycle)?

Comment 9

[ht16cq+b33nskirkppis]

I would like to point out to everyone that Chrome has removed all DHE ciphersuits years ago with no issues. Firefox is the last browser still keeping DHE support around for no reason.

For as long as https://dh1024.badssl.com/ shows no error on Firefox, then Firefox users will remain vulnerable.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

We've already built the RC build for Fx62 and don't have another on the horizon. Unless someone feels strongly that this should drive an RC respin, it'll need to wait for a dot release or Fx63.

Comment 11

[Darkspirit]

(In reply to trumopudre from comment #9)
> For as long as https://dh1024.badssl.com/ shows no error on Firefox, then Firefox users will remain vulnerable.

That's bug 1367617 and should be done rather soon. (bug 1227519 has been repurposed for WebRTC.)

(Tim Smith 👨‍🔬 [:tdsmith] from comment #0)
> https://forums.databricks.com

https://www.ssllabs.com/ssltest/analyze.html?d=forums.databricks.com
Chrome can only connect because it supports TLS_RSA_WITH_AES_256_GCM_SHA384.
(I think it's a shame that they even shipped plain RSA with AES-GCM.) It would be nice if they could deprecate it and break that website's messy TLS config.

Comment 12

[Masatoshi Kimura [:emk]]

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #11)
> (In reply to trumopudre from comment #9)
> > For as long as https://dh1024.badssl.com/ shows no error on Firefox, then Firefox users will remain vulnerable.
> 
> That's bug 1367617 and should be done rather soon. (bug 1227519 has been
> repurposed for WebRTC.)

Note that bug 1367617 will break forums.databricks.com again. If we can bump the DH key size to 2048 bits, we can just disable DHE.

Comment 13

[Martin Thomson [:mt:]]

Wait for the dot release as ryanvm says.  It's a safe, simple change, but don't spend real money on this.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

We're likely to be spinning a 62.0.1 release in the near future. Please create a rebased patch and nominate for release uplift if you want this to be considered for it.

Comment 15

[Martin Thomson [:mt:]]

Attachment: bug 1487517 - un-do ciphersuite hiding from bug 1279479 and bug 1316300 r=mt

In bug 1279479 and bug 1316300 we hid some ciphersuites from TLS 1.3
handshakes, assuming we would fall back to TLS 1.2 if the peer needed them.
However, as of bug 1479501, we don't fall back by default, so this just means
we can't negotiate these ciphersuites. This patch un-hides these ciphersuites
from the TLS 1.3 handshake.

Comment 16

[Martin Thomson [:mt:]]

Comment on attachment 9007093 [details]
bug 1487517 - un-do ciphersuite hiding from bug 1279479 and bug 1316300 r=mt

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1487517
[User impact if declined]: Can't connect to sites that only support DHE ciphersuites.  Though there are few of these it tends to drive people to use other browsers.  For instance, some of these sites work in chrome because chrome supports some ciphersuites that we don't.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It deletes inconsequential code.
[String changes made/needed]: None

Comment 17

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

Thanks for rebasing the patch, :mt

Comment 18

[ht16cq+b33nskirkppis]

forums.databricks.com has fixed its cipher suit configuration and now works in Chrome & Firefox.

No patch is therefore necessary - please do not commit it to the point release.

DHE can finally die the death it deserves.

Comment 19

[Martin Thomson [:mt:]]

Even if that were the only site, deleting dead code is always desirable.  If we are going to disable those ciphersuites, we have much better ways of doing so.

Comment 20

[ht16cq+b33nskirkppis]

It was the only site (and I have no idea why it got preferential treatment) as evidenced by https://bugzilla.mozilla.org/show_bug.cgi?id=1227519

Comment 21

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Marking this as qe-verify -, per comment 16.

Comment 22

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9007093 [details]
bug 1487517 - un-do ciphersuite hiding from bug 1279479 and bug 1316300 r=mt

fix compat issues with some servers' tls config; approved for 62.0.2

Comment 23

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-release/rev/14803cf456f3

Comment 24

[ht16cq+b33nskirkppis]

Please note that I strongly disagree with reverting this change and thus making Firefox less secure for all users.