Closed Bug 1345368 Opened 7 years ago Closed 7 years ago

Upgrade Firefox 55 to NSS 3.31

Categories

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

Product:
Core
Component:
Security: PSM
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: franziskus, Assigned: franziskus)

References

(Blocks 1 open bug)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file, 1 obsolete file)

ff-certdata.patch
972 bytes, patch
ted
: review+
Details | Diff | Splinter Review 
Tracking NSS 3.31 for Firefox 55.
Keywords: leave-open
Priority: -- → P1
Whiteboard: [psm-assigned]
(In reply to Pulsebot from comment #20)
> Pushed by franziskuskiefer@gmail.com:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/53be95383c09
> land NSS 7228445b43ac, r=me

This change included two new files that look like they shouldn't be there:

  security/manager/ssl/RootHashes.inc.orig
  security/manager/tools/KnownRootHashes.json.orig

fkiefer, were they committed by accident?
Flags: needinfo?(franziskuskiefer)
Hm, that's weird. They indeed shouldn't be there like this. I'll push a follow up and make sure to fix the script that pushed this change.
Flags: needinfo?(franziskuskiefer)
Depends on: 1366187
Just as an FYI it looks like this change increased the # of compiler warnings:

== Change summary for alert #6673 (as of May 17 2017 14:30 UTC) ==

Regressions:

  2%  compiler warnings summary windowsxp debug      230.00 -> 234.00
  2%  compiler warnings summary windows2012-32 debug 230.00 -> 234.00
  2%  compiler warnings summary linux64 debug static-analysis361.67 -> 367.58
  1%  compiler warnings summary linux32 debug        468.00 -> 473.00
  1%  compiler warnings summary linux64-stylo debug  475.00 -> 480.00
  1%  compiler warnings summary linux64 debug        474.00 -> 479.00

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6673
Pushed by franziskuskiefer@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf6ee973f04e
land NSS 0c3800b6eaba UPGRADE_NSS_RELEASE, r=me
Pushed by franziskuskiefer@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea4280e95894
land NSS 29290a4a9bd0 UPGRADE_NSS_RELEASE, r=me
Attached patch ff-certdata.patch (obsolete) — — Splinter Review 
The certdata generation changed in NSS. This will have to land with the next version of NSS.
Attachment #8873467 - Flags: review?(ted)
Comment on attachment 8873467 [details] [diff] [review]
ff-certdata.patch

Review of attachment 8873467 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like certdata.py unconditionally calls 'perl':
https://hg.mozilla.org/projects/nss/file/1943d2f973c7/lib/ckfw/builtins/certdata.py

That's going to break things for anyone who's building with perl that's not in their PATH or with a different binary name. Can we fix that to honor some sort of PERL variable like we currently do?
We don't have access to any of the moz.build variables. But we can use an env variable if that works for you. Something like this https://nss-review.dev.mozaws.net/D346? We might have to set PERL=buildconfig.substs['PERL'] somewhere if that's not the case yet.
Flags: needinfo?(ted)
Set PERL env to buildconfig.substs['PERL'] to make NSS use $PERL instead of `perl`.
Attachment #8873467 - Attachment is obsolete: true
Attachment #8873467 - Flags: review?(ted)
Attachment #8873798 - Flags: review?(ted)
Comment on attachment 8873798 [details] [diff] [review]
ff-certdata.patch

Review of attachment 8873798 [details] [diff] [review]:
-----------------------------------------------------------------

This is OK, but looking at it again it feels like an odd way to go about it. You wrapped the Perl execution in a Python script to make things work better with MSYS/native path differences on Windows, but since we're already in Python here invoking another Python interpreter seems unnecessary. Could we instead just cheat and make the existing script drop the first entry in `inputs`?
Flags: needinfo?(ted)
Comment on attachment 8873798 [details] [diff] [review]
ff-certdata.patch

Review of attachment 8873798 [details] [diff] [review]:
-----------------------------------------------------------------

I'm OK with landing this to unblock things, but it might be nicer to do what I mentioned in my previous comment.
Attachment #8873798 - Flags: review?(ted) → review+
Backed out for failing xpcshell's security/manager/ssl/tests/unit/test_broken_fips.js on Windows:

https://hg.mozilla.org/integration/mozilla-inbound/rev/99e99af157c2a7b38f9919fc3647d26bd36c9c1d
https://hg.mozilla.org/integration/mozilla-inbound/rev/11f9875cfe18ecfbb4b28d9f87d5f5af94258430

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=952cf10f8d8afa91d5b4e86702febfb0f19aa91e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=success&filter-searchStr=windows+xpcshell
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=104828397&repo=mozilla-inbound

03:13:58     INFO -  TEST-START | security/manager/ssl/tests/unit/test_broken_fips.js
03:13:58  WARNING -  TEST-UNEXPECTED-FAIL | security/manager/ssl/tests/unit/test_broken_fips.js | xpcshell return code: 0
03:13:58     INFO -  TEST-INFO took 311ms
03:13:58     INFO -  >>>>>>>
03:13:58     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
03:13:58  WARNING -  TEST-UNEXPECTED-FAIL | security/manager/ssl/tests/unit/test_broken_fips.js | run_test - [run_test : 27] FIPS should not be enabled - false == true
03:13:58     INFO -  c:/slave/test/build/tests/xpcshell/tests/security/manager/ssl/tests/unit/test_broken_fips.js:run_test:27
03:13:58     INFO -  c:\slave\test\build\tests\xpcshell\head.js:_execute_test:544
03:13:58     INFO -  -e:null:1
03:13:58     INFO -  exiting test
03:13:58     INFO -  "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
03:13:58     INFO -  <<<<<<<
Flags: needinfo?(franziskuskiefer)
Depends on: 1337950
David, can we remove that test? The FIPS DB is working again on Windows so that the test fails.
Flags: needinfo?(franziskuskiefer) → needinfo?(dkeeler)
We should just make this line "skip-if = os != 'mac'" here: https://dxr.mozilla.org/mozilla-central/rev/2c6289f56812c30254acfdddabcfec1e149c0336/security/manager/ssl/tests/unit/xpcshell.ini#45 (and update the comment).
Flags: needinfo?(dkeeler)
Pushed by franziskuskiefer@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aafc907d2aae
land NSS NSS_3_31_BETA2 UPGRADE_NSS_RELEASE, r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/d93732fda32c
Disable test_broken_fips on all platforms other than mac, r=keeler
https://hg.mozilla.org/integration/mozilla-inbound/rev/b55ffc5807df
adapt to new NSS certdata.py, r=ted
Pushed by franziskuskiefer@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccf7273933f0
land NSS NSS_3_31_RTM UPGRADE_NSS_RELEASE, r=me
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/ccf7273933f0
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: