Upgrade Firefox 46 to NSS 3.22.1

RESOLVED FIXED in Firefox 46

Status

()

Core
Security: PSM
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kaie, Unassigned)

Tracking

45 Branch
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 affected, firefox46+ fixed, firefox47 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Upgrade Firefox 45 to NSS 3.22
(Reporter)

Updated

2 years ago
Keywords: leave-open

Updated

2 years ago
Blocks: 752004
(Reporter)

Comment 1

2 years ago
Firefox 45 primary development is finished, and we haven't landed any beta into it.

We should probably keep Firefox 45 at NSS 3.21, and morph this bug,
to land NSS 3.22 into Firefox 46.
Summary: Upgrade Firefox 45 to NSS 3.22 → Upgrade Firefox 46 to NSS 3.22
(Reporter)

Comment 2

2 years ago
Nobody has mentioned a reason to have NSS 3.22 in Firefox 46 yet.

It might get postponed to Firefox 47.

Comment 3

2 years ago
It would be very nice to get bug 1233981 merged to mozilla-central as soon as we can...
Blocks: 1216109
Blocks: 1191936
(Reporter)

Comment 4

2 years ago
Snapshot NSS_3_22_BETA1 doesn't work with mozilla-central, I see crashes:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f60fceb6f67

I've started a second try run based, to have more test data:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ce7ead31d41
NSS_GetAlgorithmPolicy is on the stack.  I think that this is Bug 1009429.
(In reply to Martin Thomson [:mt:] from comment #5)
> NSS_GetAlgorithmPolicy is on the stack.  I think that this is Bug 1009429.

Indeed. params2ecName() passes an OID to SECOID_FindOID() that looks like this:

06082a8648ce3d030107 = OID 1.2.840.10045.3.1.7 (ANSI X9.62 named elliptic curve: prime256v1)

|oidData->offset| is thus SEC_OID_ANSIX962_EC_PRIME256V1 (208) and that's a lot bigger than the number of entries of ecName2OIDTag[].

> if ((oidData = SECOID_FindOID(&oid)) == NULL) return ec_noName;
> if ((NSS_GetAlgorithmPolicy(ecName2OIDTag[oidData->offset], &policyFlags) == SECSuccess) &&
>     !(policyFlags & NSS_USE_ALG_IN_SSL_KX)) {
>   return ec_noName;
> }
Created attachment 8711473 [details] [diff] [review]
0001-Bug-1228410-Fix-crashes-when-trying-to-uplift-NSS-3..patch

We have to pass |oidData->offset| instead of |ecName2OIDTag[oidData->offset]| to NSS_GetAlgorithmPolicy() here. I cleaned this up some more so that we use SECKEY_GetECCOid() to parse the SECItem.

Our ssl_gtests, the coverage tests, and the crashing xpcshell tests pass with this patch applied.
Attachment #8711473 - Flags: review?(rrelyea)
Attachment #8711473 - Flags: review?(martin.thomson)
Created attachment 8711478 [details] [diff] [review]
0001-Bug-1228410-Fix-crashes-when-trying-to-uplift-NSS-3..patch

Darn, try was failing because I forgot to remove a now unused variable. Also, local builds started failing because the linker complains about SECKEY_GetECCOid_FindOID(), hmm. Back to the minimal version of this patch.
Attachment #8711473 - Attachment is obsolete: true
Attachment #8711473 - Flags: review?(rrelyea)
Attachment #8711473 - Flags: review?(martin.thomson)
Attachment #8711478 - Flags: review?(rrelyea)
Attachment #8711478 - Flags: review?(martin.thomson)
We should probably add a test for this in a follow-up. It's bad that none of our SSL tests seems to catch this.

Updated

2 years ago
Attachment #8711478 - Flags: review?(martin.thomson) → review+
(Reporter)

Comment 12

2 years ago
Comment on attachment 8711478 [details] [diff] [review]
0001-Bug-1228410-Fix-crashes-when-trying-to-uplift-NSS-3..patch

Thank you for the quick fix!
https://hg.mozilla.org/projects/nss/rev/72122a7dc17e
Attachment #8711478 - Flags: checkin+
(Reporter)

Comment 13

2 years ago
Tagged NSS_3_22_BETA2 that includes the fix, will push to mozilla-inbound.
(Reporter)

Comment 15

2 years ago
The beta uplift to mozilla-inbound happened after today's branch merge.

I've asked Martin to advice, if and when we shall requesting approval for uplifting to aurora 46.
There is nothing in NSS 3.22 that Firefox really needs.  Let's leave it to soak on m-c for a while and then take the release into Aurora when it proves to be stable.  We're early enough into the Aurora release that it won't hurt to have Dev Edition testing of the release.
(Reporter)

Updated

2 years ago
Keywords: leave-open

Comment 19

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3808d0b70b4f
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47

Comment 20

2 years ago
Hmm, this seems to not include the fix to bug 1233981.  Is there another bug tracking merging that to mozilla-central?  I would like to stop maintaining these patches out of the tree if at all possible.
(Reporter)

Comment 21

2 years ago
(In reply to :Ehsan Akhgari from comment #20)
> Hmm, this seems to not include the fix to bug 1233981.  Is there another bug
> tracking merging that to mozilla-central?  I would like to stop maintaining
> these patches out of the tree if at all possible.

m-c should already have your patch, because it has been upgraded to 3.22

Updated

2 years ago
No longer blocks: 752004
Attachment #8711478 - Flags: review?(rrelyea)
(Reporter)

Comment 22

2 years ago
(In reply to Martin Thomson [:mt:] from comment #17)
> There is nothing in NSS 3.22 that Firefox really needs.  Let's leave it to
> soak on m-c for a while and then take the release into Aurora when it proves
> to be stable.  We're early enough into the Aurora release that it won't hurt
> to have Dev Edition testing of the release.

Martin, are you OK uplifting it to aurora now?

Actually, Kathleen has asked for an update of the CA certificates list in Firefox 46. I currently consider to release that as an NSS 3.22.x release. That release with the CA update is expected around Feb 26.

Because Feb 26 is rather late in the aurora cycle, it would be reasonable to start testing the NSS 3.22.x branch earlier with the aurora branch.

Tomorrow, I expect to create a NSS 3.22.1 for another purpose, it will include one patch on top of 3.22, which has already been tested on m-c.

So, I'm requesting uplift of NSS 3.22.1 to aurora 46, as soon as possible.

That meeans, the CA update release available Feb 26 will probably be NSS 3.22.2.
I'll ask for uplift approval on that date.

I suggest that we reuse this bug for uplifting the NSS 3.22.x versions into Firefox 46 aurora.
Flags: needinfo?(martin.thomson)
(Reporter)

Updated

2 years ago
Summary: Upgrade Firefox 46 to NSS 3.22 → Upgrade Firefox 46 to NSS 3.22.x
(Reporter)

Comment 23

2 years ago
Kathleen, I'm setting a needinfo flag, to make it easier for you to keep this bug on your radar.

Tomorrow, I'll ask for approval of NSS 3.22.1

On Feb 26, I'll ask for approval of NSS 3.22.2

It would be nice to get your help with asking for approvals.
Flags: needinfo?(kwilson)
Yes Kai, I think that we should get the latest uplifted.  3.22 has proven to be stable.  And reducing the number of branches we have is good, especially if 46 needs the same roots.  Of course, the release manager will have the final say, but please do ask.
Flags: needinfo?(martin.thomson)
(Reporter)

Comment 25

2 years ago
Created attachment 8718863 [details]
script to uplift Aurora 46 to NSS 3.22

has rs=mt

We have agreement that we need to uplift NSS 3.22.x to aurora.
Attachment #8718863 - Flags: review+
Attachment #8718863 - Flags: approval-mozilla-aurora?
Marking 45 as affected and tracking it as we intend to uplift this work.
status-firefox46: --- → affected
tracking-firefox46: --- → +
Oops. I meant, marking 46 as affected.
Comment on attachment 8718863 [details]
script to uplift Aurora 46 to NSS 3.22

OK to uplift 3.22.1 to aurora.
Attachment #8718863 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
kai, i guess you want to do the uplift yourself right ? :)
Flags: needinfo?(kaie)
(Reporter)

Comment 30

2 years ago
yes
Flags: needinfo?(kaie)
(Reporter)

Comment 31

2 years ago
Beware, the following link is big, and it causes my firefox 44.0 to be stuck for long times.

https://hg.mozilla.org/releases/mozilla-aurora/rev/da0d1b0806f1
(Reporter)

Comment 32

2 years ago
(ok, let's declare this one as fixed, and let's use a separate bug for uplifting the future 3.22.2 related for root ca certificates)
status-firefox46: affected → fixed
Flags: needinfo?(kwilson)
(Reporter)

Updated

2 years ago
Summary: Upgrade Firefox 46 to NSS 3.22.x → Upgrade Firefox 46 to NSS 3.22.1
Multiple jobs are crashing like https://treeherder.mozilla.org/logviewer.html#?job_id=1958951&repo=mozilla-aurora

Backed out from aurora in https://hg.mozilla.org/releases/mozilla-aurora/rev/e0f6a424331b
status-firefox46: fixed → affected
Flags: needinfo?(kaie)
It's a SIGSEGV in SSL_PeerCertificateChain(), but only on Linux 32-bit systems. Other platforms do not seem to be affected.

https://treeherder.mozilla.org/#/jobs?repo=mozilla-aurora&revision=da0d1b0806f1&selectedJob=1958964
(Reporter)

Comment 35

2 years ago
(In reply to Tim Taubert [:ttaubert] from comment #34)
> It's a SIGSEGV in SSL_PeerCertificateChain(), but only on Linux 32-bit
> systems. Other platforms do not seem to be affected.
> 
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> aurora&revision=da0d1b0806f1&selectedJob=1958964

That's confusing, because I had submitted a try build of almost the same snapshot,
which didn't had that crash:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b73089a97f22

(Only differences are in version numbers and configure.in requirements.)
Flags: needinfo?(kaie)
(Reporter)

Comment 36

2 years ago
There's evidence that the build failures were related to an infrastructure failure.

A re-trigger of the builds succeeded.

I've done a second re-trigger. If this third build also completes without crashes, I'll re-land into aurora.
(Reporter)

Comment 37

2 years ago
All tests looked good on second and third attempt.

Re-landed:
https://hg.mozilla.org/releases/mozilla-aurora/rev/7141f2426f66
status-firefox46: affected → fixed
(Reporter)

Comment 38

2 years ago
Just in case the failure happens again:

Before backing out, please check if the failures are ONLY on one machine, and have someone briefly check if there is any infrastructure failure with that machine. Thank you.
You need to log in before you can comment on or make changes to this bug.