1228410 - Upgrade Firefox 46 to NSS 3.22.1

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Kai Engert (:KaiE:)]

Upgrade Firefox 45 to NSS 3.22

Comment 1

[Kai Engert (:KaiE:)]

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.

Comment 2

[Kai Engert (:KaiE:)]

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

It might get postponed to Firefox 47.

Comment 3

[(no longer active)]

It would be very nice to get bug 1233981 merged to mozilla-central as soon as we can...

Comment 4

[Kai Engert (:KaiE:)]

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

Comment 5

[Martin Thomson [:mt:]]

NSS_GetAlgorithmPolicy is on the stack.  I think that this is Bug 1009429.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

(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;
> }

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 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.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Patch from Kai, with my patch applied:

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

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 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.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 12

[Kai Engert (:KaiE:)]

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

Comment 13

[Kai Engert (:KaiE:)]

Tagged NSS_3_22_BETA2 that includes the fix, will push to mozilla-inbound.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e763694e778

Comment 15

[Kai Engert (:KaiE:)]

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.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7e763694e778

Comment 17

[Martin Thomson [:mt:]]

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.

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3808d0b70b4f

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3808d0b70b4f

Comment 20

[(no longer active)]

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.

Comment 21

[Kai Engert (:KaiE:)]

(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

Comment 22

[Kai Engert (:KaiE:)]

(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.

Comment 23

[Kai Engert (:KaiE:)]

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.

Comment 24

[Martin Thomson [:mt:]]

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.

Comment 25

[Kai Engert (:KaiE:)]

Attachment: 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.

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Marking 45 as affected and tracking it as we intend to uplift this work.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Oops. I meant, marking 46 as affected.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8718863 [details]
script to uplift Aurora 46 to NSS 3.22

OK to uplift 3.22.1 to aurora.

Comment 29

[Carsten Book [:Tomcat]]

kai, i guess you want to do the uplift yourself right ? :)

Comment 30

[Kai Engert (:KaiE:)]

yes

Comment 31

[Kai Engert (: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

Comment 32

[Kai Engert (:KaiE:)]

(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)

Comment 33

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 34

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 35

[Kai Engert (:KaiE:)]

(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.)

Comment 36

[Kai Engert (: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.

Comment 37

[Kai Engert (:KaiE:)]

All tests looked good on second and third attempt.

Re-landed:
https://hg.mozilla.org/releases/mozilla-aurora/rev/7141f2426f66

Comment 38

[Kai Engert (:KaiE:)]

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.