Closed Bug 1305970 Opened 8 years ago Closed 7 years ago

Upgrade Firefox 51 to NSS 3.28

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 + fixed
firefox52 --- fixed

People

(Reporter: franziskus, Assigned: franziskus)

References

(Blocks 1 open bug)

Details

(Whiteboard: [psm-assigned])

Attachments

(2 files)

tracking NSS 3.28 for Firefox 52
Target Milestone: --- → mozilla52
Should we just reopen bug 1304919 if there's more to do to update Firefox 52 to NSS 3.28 or am I misunderstanding something here?
Flags: needinfo?(franziskuskiefer)
bug 1304919 did more than landing NSS trunk in central. I didn't want to litter that bug. We're trying a new way of landing NSS (about every other day). So this bug is supposed to track those NSS imports all the way to the release.
Flags: needinfo?(franziskuskiefer)
Priority: -- → P1
Whiteboard: [psm-assigned]
Backout by franziskuskiefer@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1475e5ea5dab
Backed out changeset a1b7ab570480 for bustage
Depends on: 1311995
Depends on: 1316307
Depends on: 1317857
Depends on: 1317981
Depends on: 1318633
No longer depends on: 1311995
No longer depends on: 1317981
Blocks: 1317959
Franziskus, I suggest to get approval for uplifting the larger amount of changes on NSS_3_28_BRANCH to aurora, asap.
Attachment #8814751 - Flags: review?(ekr) → review?(ttaubert)
Comment on attachment 8814751 [details]
Bug 1305970 - Update to latest NSS 3.28,

Approval Request Comment
[Feature/regressing bug #]: 1305970
[User impact if declined]: TLS 1.3 will be unusable; fixes for several memory leaks and a crash bug that might affect WebRTC
[Describe test coverage new/current, TreeHerder]: extensive NSS testing, try build in progress
[Risks and why]: Changes to TLS 1.3 are not trivial, mitigated by testing
[String/UUID change made/needed]: n/a
Attachment #8814751 - Flags: approval-mozilla-aurora?
Comment on attachment 8814751 [details]
Bug 1305970 - Update to latest NSS 3.28,

https://reviewboard.mozilla.org/r/95840/#review95902
Attachment #8814751 - Flags: review?(ttaubert) → review+
Comment on attachment 8814751 [details]
Bug 1305970 - Update to latest NSS 3.28,

nss update for tls1.3 goodness, take in aurora52
Attachment #8814751 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Martin Thomson [:mt:] from comment #32)
> Apparently pulsebot is not working its magic for aurora: 
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 071d92d893791d2f0cd39ddcec368f3e03d71e8f

yep only integration + mc tree :) setting also the flag
Depends on: 1320509
No longer depends on: 1317857
Aurora uses NSS 3.28 beta.

Could you please approve the following additional changes, to allow aurora to use a final NSS release versions?

None of them should be any risk for Firefox.

List of changes to NSS 3.28 final

- version information fix
    https://hg.mozilla.org/projects/nss/rev/4e449cb1e8c0

- regression fix, add symbolic definitions for backwards compatibility
    https://hg.mozilla.org/projects/nss/rev/36e8e898af98

- regression fix, affects some programs that open NSS databases multiple times (Firefox doesn't do that)
    https://hg.mozilla.org/projects/nss/rev/b8bbed415405

- not part of the build, support fuzzing tests
    https://hg.mozilla.org/projects/nss/rev/9e9296e620c7
    https://hg.mozilla.org/projects/nss/rev/c84d8cf72ba9

- not affecting firefox, correctness fix, NSS tools might get stuck prompting for a password unnecessarily
    https://hg.mozilla.org/projects/nss/rev/b09fe3c22b24

- not affecting firefox, because firefox doesn't use that code: Distrust WoSign/StartCom roots in NSS' own verify code
    https://hg.mozilla.org/projects/nss/rev/86236577102e

- change NSS version to 3.28 final
Attachment #8818871 - Flags: approval-mozilla-aurora?
Comment on attachment 8818871 [details]
changes to NSS 3.28 final

nss 3.28 final for aurora52
Attachment #8818871 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Are you sure that Aurora is on 3.28?

This suggests that it is on 3.29
https://hg.mozilla.org/releases/mozilla-aurora/file/tip/security/nss/lib/ssl/ssl.def
(In reply to Eric Rescorla (:ekr) from comment #36)
> Are you sure that Aurora is on 3.28?
> 
> This suggests that it is on 3.29
> https://hg.mozilla.org/releases/mozilla-aurora/file/tip/security/nss/lib/ssl/
> ssl.def

That's being fixed in https://hg.mozilla.org/projects/nss/rev/4e449cb1e8c0 as far as I can tell
(In reply to Eric Rescorla (:ekr) from comment #36)
> Are you sure that Aurora is on 3.28?

Yes.

Aurora uses NSS revision 6c26f0cd19ba which is on NSS_3_28_BRANCH, and Julien's comment is correct.

http://kuix.de/mozilla/versions/
https://hg.mozilla.org/projects/nss/rev/6c26f0cd19ba
(In reply to Eric Rescorla (:ekr) from comment #36)
> Are you sure that Aurora is on 3.28?
> 
> This suggests that it is on 3.29
> https://hg.mozilla.org/releases/mozilla-aurora/file/tip/security/nss/lib/ssl/
> ssl.def

Now I see the problem you're referring to:

;+NSS_3.29 {    # NSS 3.29 release
;+    global:
SSL_ExportEarlyKeyingMaterial;
;+    local:
;+*;
;+};


I guess this was added, when Franziskus had "merged over everything" to the 3.28 branch.

If this API is ready to be released, then we must move the definition to the 3.28 section.
Sorry, please ignore the previous comment.

I went through the same confusion again...
I had started an aurora-try build, to verify that our changes are good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=51a24812876975bf04f74e9f64c214e4ab9d27d0

That build had a lot of failures.
I've asked on the Mozilla IRC channel, and I've been told that the only worrying ones are the "bc*" builds on Windows.

To verify if this is really our (NSS) fault, or if that's somehow a property of the base revision I had used for testing, I've submitted another try build for that base revision, without our patch, only for Windows OPT.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=89a978976508f8a2d84c54f15d614d77930561a2
That base revision try build showed the same failures. That means, those failures aren't caused by our NSS change.

(From past experience, I'm not surprised that try builds for older branches (like aurora) show more failures than try builds for mozilla-central.)
I would like to more-or-less simultaneously uplift 3.28 to Beta so that we can take measurements of the performance of TLS 1.3 in the field.

We'll need the new short headers patch
https://hg.mozilla.org/projects/nss/rev/377b0eb048f6e2ac356f60c6c5229ae242c51634

And the patch to enable it which MT r+ed at:
https://nss-review.dev.mozaws.net/D123
Flags: needinfo?(rkothari)
Flags: needinfo?(gchang)
Ritu, Gerry, per our discussion in e-mail
Here is what I believe to be the complete patch set:

e2c2116e0f60  E Enable short headers
d7d947bd6cf2  PM Bug 1321783 - Make updater be networking conservative r=dkeeler r=rstrong r=dragana a=jcristau
50343eee0a40  MT Bug 1304919 - Update WebRTC to latest NSS, r=ekr
a992a437e973  MT Bug 1304919 - Update TLS server tests to expect TLS 1.3 cipher suite, r=ekr
9daa472e39a0  E Bug 1304919 - PSM changes to support TLS 1.3 key exchange, r=mt
15b541961576  E Update NSS to 3.28 branch
Hi Eric,
Per our discussion in email, I will give a GO for this experiment. Please create uplift requests for required patches by Wed (12/20) so that we can make it in 51 beta 10.
Flags: needinfo?(gchang) → needinfo?(ekr)
Ekr's plan to enable this, monitor as a Beta experiment, wait on the outcome to decide whether to do a staged rollout on release seems reasonable. Also the ability to turn off with a system add-on update and pref flip is a good risk mitigation.
Flags: needinfo?(rkothari)
Also, as I need to update the add-on anyway, I think I'll have it sanity check that TLS 1.3 works at all (that's part of the experiment anyway) and disable 1.3 on that machine if it doesn't work. That should give us another safety margin
Flags: needinfo?(ekr)
Any code for firefox beta needs to go into firefox aurora, too, and should be tested on aurora first.

Given comment 48, and based on EKR's statements on IRC, I understand we're allowed to include the latest TLS 1.3 short-headers change into NSS 3.28 and into aurora.

I've tagged a NSS 3.28 release candidate, that includes the above, and will land this into aurora immediately.

If we don't get any problem reports within the next 24 hours, I'll tag the release as final tomorrow.
I'll keep this bug open until tomorrow. If everything will be fine, I'll update aurora again to record the correct RTM tag in the information file TAG-INFO (no code change).
Carsten, FYI, you set status-firefox52 to fixed in comment 33, but we weren't there yet. We usually land multiple intermediate snapshots of NSS, and these kinds of bugs aren't fixed until we land a final NSS RTM tag.
(That's also why we have the leave-open keyword set.)
I haven't seen any problem reports, and have tagged NSS_3_28_RTM,
and have recorded the final tag in aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/2748ea683e9ccf7b3c742539f73a5565f4998fdc
Status: NEW → RESOLVED
Closed: 7 years ago
Keywords: leave-open
Resolution: --- → FIXED
I've received an email from EKR, requesting me to uplift 3.28 RTM to the beta branch, and he claims that he has gotten approval.

Because I don't see other approval flags in this bug, I'll use a=ekr.
Shouldnt the subject be changed to refer to firefox 51 then ?
(In reply to Landry Breuil (:gaston) from comment #57)
> Shouldnt the subject be changed to refer to firefox 51 then ?

Let's wait if the commit sticks, then we can do it.
(In reply to Kai Engert (:kaie) from comment #56)
> https://hg.mozilla.org/releases/mozilla-beta/rev/
> dd227854175f75d00c0e00797a91a82db9ab93ee

backed out:
https://hg.mozilla.org/releases/mozilla-beta/rev/1d8e797ecd63
This will also need patches from bug 8794050, bug 8794051, and bug 8794052. Thanks Kai!
Oops. Sorry about that.
(In reply to Kai Engert (:kaie) from comment #56)
> https://hg.mozilla.org/releases/mozilla-beta/rev/
> dd227854175f75d00c0e00797a91a82db9ab93ee

Please remember to include the bug number in the commit message as well the next time this lands.
I assume you don't need my help with landing.
I see it has landed yesterday into firefox 51 beta:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=3db4e6dd305a9589f11bdb08aed157c94ffb1fe1
Summary: Upgrade Firefox 52 to NSS 3.28 → Upgrade Firefox 51 to NSS 3.28
Target Milestone: mozilla52 → mozilla51
Shouldnt the min version be bumped to 3.28 in https://dxr.mozilla.org/mozilla-beta/source/old-configure#10614 too ?
(In reply to Landry Breuil (:gaston) from comment #66)
> Shouldnt the min version be bumped to 3.28 in
> https://dxr.mozilla.org/mozilla-beta/source/old-configure#10614 too ?

Oh yes, we must do that, thanks for noticing. I guess we can just do that without separate approval?
Kai, does this also need a build peer to sign off?
(In reply to Eric Rescorla (:ekr) from comment #68)
> Kai, does this also need a build peer to sign off?

Do you mean on the code review level? We've always changed the build system to require the NSS mininum version that Firefox is actually using, and in the past, I usually landed that change together with the NSS uplifts.

I was asking how to handle the landing with regards to the approval, because I couldn't see any approval flags in this bug for beta in this bug.
I've gotten approval from Sheriffs on IRC, given that this is standard practice.
Thanks!
You need to log in before you can comment on or make changes to this bug.