1368599 - Disable TLS 1.3 by default for FF 54

Status: VERIFIED FIXED

(Core :: Security: PSM, defect)

Description

[Eric Rescorla (:ekr)]

We need to disable TLS 1.3 before we ship FF 54.

Also, I want to disable short headers for this build.

Comment 1

[Eric Rescorla (:ekr)]

Attachment: 0001-Bug-1368599-Disable-TLS-1.3-by-default-for-Release-5.patch

Comment 2

[Eric Rescorla (:ekr)]

Liz, the ni was just FYI

Comment 3

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Adding tracking/affected flags.  if we can land this quickly it may make it into today's build.

Comment 4

[Eric Rescorla (:ekr)]

JC, are you able to review this?

Comment 5

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8873518 [details] [diff] [review]
0001-Bug-1368599-Disable-TLS-1.3-by-default-for-Release-5.patch

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

I'm on it. Slow morning.

Comment 6

[Eric Rescorla (:ekr)]

Comment on attachment 8873518 [details] [diff] [review]
0001-Bug-1368599-Disable-TLS-1.3-by-default-for-Release-5.patch

This is for approval on 54-beta (so it ends up in 54-release)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1310516 enabled TLS 1.3 by default
[User impact if declined]: interoperability concerns, iiuc
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no - this is for 53 only
[Needs manual test from QE? If yes, steps to reproduce]: no, but you could test by going to https://www.cloudflare.com/, opening the page info dialog, going to the security tab, and checking that under "Technical Details" it says TLS 1.2 and not TLS 1.3
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: it just changes the default to a previously-known good value
[String changes made/needed]: none

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8873518 [details] [diff] [review]
0001-Bug-1368599-Disable-TLS-1.3-by-default-for-Release-5.patch

Makes sense, Beta54+

Comment 8

[Eric Rescorla (:ekr)]

I'd actually like to leave this on on Beta as long as I can and didn't read the calendar right (ideally we would just disable this on Beta). What's the last possible date we can land this to be safe for Release 54?

Comment 9

[Eric Rescorla (:ekr)]

Alternately, we can just move to setting the pref in a system add-on targeted at the beta channel only. Would that be better?

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Eric Rescorla (:ekr) from comment #8)
> I'd actually like to leave this on on Beta as long as I can and didn't read
> the calendar right (ideally we would just disable this on Beta). What's the
> last possible date we can land this to be safe for Release 54?

If you want this on Beta54 and not Release54, we could do either of the following:
1) Time this uplift such that it's in the last RC, a bit tough to coordinate but possible.
2) Go-live with this pref on at first for release54 and then push a system add-on update to disable TLS 1.3 post go-live.

The problem with 2) is the lag. With 1) TLS1.3 will be disabled before it hits release 54

Comment 11

[Eric Rescorla (:ekr)]

I agree #2 is bad. WRT #1, could we just do it for the last beta? That would give us some slack, but not be risky?

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Ok. Let's take land this patch in the ~last RC build or latest by Wed 6/7. Gerry, Wesley, Ryan fyi.

Comment 13

[Carsten Book [:Tomcat]]

landed on beta as https://hg.mozilla.org/releases/mozilla-beta/rev/4f0bd6c5b03dafa6025621da30e1bc87008d027b

Comment 14

[Carsten Book [:Tomcat]]

outch ryan told this landed too soon, so backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/a6a79438608114ac87218bcb467b1f8aa5d603f3

Comment 15

[Julien Cristau [:jcristau]]

I don't think we should keep things out of RC1 if we want them in release, that kind of defeats the purpose of a release candidate...

Comment 16

[Julien Cristau [:jcristau]]

Just chatted with Ritu, let's land this for RC1, that way we're all set if nothing else requires a respin this week.  Thanks.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/bfd20b4c396b

Comment 18

[Mihai Boldan, Desktop QA [:mboldan]]

The issue was verified on Firefox 54.0 (20170608175746), under Windows 7x64, Mac OS X 10.12.5 and Ubuntu 16.04x64, following the STR from Comment 6.  
TLS 1.2 is correctly displayed under Technical Details.
I'm marking this issue Verified Fixed.