Closed
Bug 1368599
Opened 7 years ago
Closed 7 years ago
Disable TLS 1.3 by default for FF 54
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
VERIFIED
FIXED
mozilla54
People
(Reporter: ekr, Assigned: ekr)
References
Details
Attachments
(1 file)
2.44 KB,
patch
|
keeler
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
We need to disable TLS 1.3 before we ship FF 54.
Also, I want to disable short headers for this build.
Assignee | ||
Comment 1•7 years ago
|
||
Flags: needinfo?(lhenry)
Attachment #8873518 -
Flags: review?(dkeeler)
Assignee | ||
Comment 2•7 years ago
|
||
Liz, the ni was just FYI
Comment 3•7 years ago
|
||
Adding tracking/affected flags. if we can land this quickly it may make it into today's build.
Comment 5•7 years ago
|
||
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.
Attachment #8873518 -
Flags: review?(dkeeler) → review+
Updated•7 years ago
|
Flags: needinfo?(jjones)
Assignee | ||
Comment 6•7 years ago
|
||
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
Attachment #8873518 -
Flags: approval-mozilla-beta?
Comment on attachment 8873518 [details] [diff] [review]
0001-Bug-1368599-Disable-TLS-1.3-by-default-for-Release-5.patch
Makes sense, Beta54+
Attachment #8873518 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 8•7 years ago
|
||
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?
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(rkothari)
Assignee | ||
Comment 9•7 years ago
|
||
Alternately, we can just move to setting the pref in a system add-on targeted at the beta channel only. Would that be better?
Flags: needinfo?(laura)
(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
Flags: needinfo?(rkothari)
Assignee | ||
Comment 11•7 years ago
|
||
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?
Ok. Let's take land this patch in the ~last RC build or latest by Wed 6/7. Gerry, Wesley, Ryan fyi.
Flags: needinfo?(wkocher)
Flags: needinfo?(ryanvm)
Flags: needinfo?(gchang)
Comment 13•7 years ago
|
||
landed on beta as https://hg.mozilla.org/releases/mozilla-beta/rev/4f0bd6c5b03dafa6025621da30e1bc87008d027b
Flags: needinfo?(wkocher)
Flags: needinfo?(ryanvm)
Updated•7 years ago
|
Comment 14•7 years ago
|
||
outch ryan told this landed too soon, so backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/a6a79438608114ac87218bcb467b1f8aa5d603f3
Comment 15•7 years ago
|
||
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•7 years ago
|
||
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.
Flags: needinfo?(ryanvm)
Comment 17•7 years ago
|
||
uplift |
Assignee: nobody → ekr
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(ryanvm)
Flags: needinfo?(laura)
Flags: needinfo?(gchang)
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 18•7 years ago
|
||
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.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•