Closed
Bug 1228410
Opened 9 years ago
Closed 9 years ago
Upgrade Firefox 46 to NSS 3.22.1
Categories
(Core :: Security: PSM, defect)
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: KaiE, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
1.19 KB,
patch
|
mt
:
review+
KaiE
:
checkin+
|
Details | Diff | Splinter Review |
41 bytes,
text/plain
|
KaiE
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details |
Upgrade Firefox 45 to NSS 3.22
Reporter | ||
Updated•9 years ago
|
Keywords: leave-open
Reporter | ||
Comment 1•9 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•9 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•9 years ago
|
||
It would be very nice to get bug 1233981 merged to mozilla-central as soon as we can...
Reporter | ||
Comment 4•9 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
Comment 5•9 years ago
|
||
NSS_GetAlgorithmPolicy is on the stack. I think that this is Bug 1009429.
Comment 6•9 years ago
|
||
(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•9 years ago
|
||
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)
Comment 8•9 years ago
|
||
Patch from Kai, with my patch applied:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2a3ef377470
Comment 9•9 years ago
|
||
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)
Comment 10•9 years ago
|
||
Comment 11•9 years ago
|
||
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•9 years ago
|
Attachment #8711478 -
Flags: review?(martin.thomson) → review+
Reporter | ||
Comment 12•9 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•9 years ago
|
||
Tagged NSS_3_22_BETA2 that includes the fix, will push to mozilla-inbound.
Comment 14•9 years ago
|
||
Reporter | ||
Comment 15•9 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.
Comment 16•9 years ago
|
||
bugherder |
Comment 17•9 years ago
|
||
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•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 19•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment 20•9 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•9 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•9 years ago
|
Attachment #8711478 -
Flags: review?(rrelyea)
Reporter | ||
Comment 22•9 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•9 years ago
|
Summary: Upgrade Firefox 46 to NSS 3.22 → Upgrade Firefox 46 to NSS 3.22.x
Reporter | ||
Comment 23•9 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)
Comment 24•9 years ago
|
||
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•9 years ago
|
||
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?
Comment 26•9 years ago
|
||
Marking 45 as affected and tracking it as we intend to uplift this work.
status-firefox46:
--- → affected
tracking-firefox46:
--- → +
Comment 27•9 years ago
|
||
Oops. I meant, marking 46 as affected.
Comment 28•9 years ago
|
||
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+
Comment 29•9 years ago
|
||
kai, i guess you want to do the uplift yourself right ? :)
Flags: needinfo?(kaie)
Reporter | ||
Comment 31•9 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•9 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)
Flags: needinfo?(kwilson)
Reporter | ||
Updated•9 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
Flags: needinfo?(kaie)
Comment 34•9 years ago
|
||
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•9 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•9 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•9 years ago
|
||
All tests looked good on second and third attempt.
Re-landed:
https://hg.mozilla.org/releases/mozilla-aurora/rev/7141f2426f66
Reporter | ||
Comment 38•9 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.
Updated•10 months ago
|
Blocks: nss-uplift
You need to log in
before you can comment on or make changes to this bug.
Description
•