Closed Bug 1228410 Opened 5 years ago Closed 5 years ago

Upgrade Firefox 46 to NSS 3.22.1

Categories

(Core :: Security: PSM, defect)

45 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- affected
firefox46 + fixed
firefox47 --- fixed

People

(Reporter: KaiE, Unassigned)

References

Details

Attachments

(2 files, 1 obsolete file)

Upgrade Firefox 45 to NSS 3.22
Keywords: leave-open
Blocks: winclang
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
Nobody has mentioned a reason to have NSS 3.22 in Firefox 46 yet.

It might get postponed to Firefox 47.
It would be very nice to get bug 1233981 merged to mozilla-central as soon as we can...
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;
> }
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)
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.
Attachment #8711478 - Flags: review?(martin.thomson) → review+
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+
Tagged NSS_3_22_BETA2 that includes the fix, will push to mozilla-inbound.
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.
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/3808d0b70b4f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
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.
(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
No longer blocks: winclang
Attachment #8711478 - Flags: review?(rrelyea)
(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)
Summary: Upgrade Firefox 46 to NSS 3.22 → Upgrade Firefox 46 to NSS 3.22.x
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)
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.
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)
yes
Flags: needinfo?(kaie)
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
(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)
Flags: needinfo?(kwilson)
Summary: Upgrade Firefox 46 to NSS 3.22.x → Upgrade Firefox 46 to NSS 3.22.1
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
(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)
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.
All tests looked good on second and third attempt.

Re-landed:
https://hg.mozilla.org/releases/mozilla-aurora/rev/7141f2426f66
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.