Closed
Bug 1435644
Opened 7 years ago
Closed 7 years ago
Cert xpcshell tests are permafailing due to expiration, e.g. security/manager/ssl/tests/unit/test_cert_chains.js - closed trees
Categories
(Core :: Security, defect)
Core
Security
Tracking
()
VERIFIED
FIXED
mozilla60
People
(Reporter: emilio, Assigned: keeler)
References
Details
(Whiteboard: [stockwell fixed:other])
Attachments
(2 files, 1 obsolete file)
(deleted),
text/x-review-board-request
|
Details | |
(deleted),
patch
|
keeler
:
review+
|
Details | Diff | Splinter Review |
Looks like it is permafailing about a push of mine containing a trivial layout cleanup, and it has a very suspicious message:
FAIL | security/manager/ssl/tests/mochitest/browser/browser_certViewer.js | should have successful verification message - "Could not verify this certificate because it has expired." == "This certificate has been verified for the following uses:"
I suspect the certificate just expired and that it needs to be renewed.
Reporter | ||
Comment 1•7 years ago
|
||
David, looks like you're familiar with this test, could you take a look? Thanks!
Flags: needinfo?(dkeeler)
Updated•7 years ago
|
status-firefox58:
--- → affected
status-firefox59:
--- → affected
status-firefox60:
--- → affected
status-firefox-esr52:
--- → affected
Flags: needinfo?(cykesiopka.bmo)
Comment 2•7 years ago
|
||
Maybe we could stay open with just the browser-chrome failure, but xpcshell has something over 30 tests failing, and the chance of a sheriff noticing either that there was a newly-added 35th failure after them, or that someone landed a security patch and caused a new failure in the middle of that spew of failure is approximately zero.
Closed mozilla-inbound, mozilla-central, autoland, mozilla-beta, mozilla-release, and mozilla-esr52 for this, so when you have a fix please coordinate landing it directly on mozilla-central with a sheriff so they can merge it around and we can reopen trunk trees as quickly as possible.
Severity: normal → blocker
Comment 3•7 years ago
|
||
Looks like these certificates (and therefore start/end dates) were generated at build time until bug 1256495.
Blocks: 1256495
Updated•7 years ago
|
Summary: security/manager/ssl/tests/mochitest/browser/browser_certViewer.js is permafailing, at least on windows → Cert xpcshell tests are permafailing due to expiration, e.g. security/manager/ssl/tests/unit/test_cert_chains.js - closed trees
Reporter | ||
Comment 4•7 years ago
|
||
I guess I'll give this a shot so I can land my stuff hopefully today...
Assignee: nobody → emilio
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8948347 [details]
Bug 1435644: Also regenerate the signed apps.
https://reviewboard.mozilla.org/r/217826/#review223562
I didn't check all certs but lgtm. Land it if you checked that these are all certs that need renewing.
Attachment #8948347 -
Flags: review+
Comment hidden (mozreview-request) |
Reporter | ||
Comment 8•7 years ago
|
||
Missed a few which didn't call GeneratedTestKey, thanks for making me double-check ;).
Fixed.
Reporter | ||
Comment 9•7 years ago
|
||
Comment on attachment 8948347 [details]
Bug 1435644: Also regenerate the signed apps.
I think carrying over Franziskus' review is ok for this.
Attachment #8948347 -
Flags: review?(dkeeler) → review+
Comment 10•7 years ago
|
||
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/mozilla-central/rev/95fd9deac911
Regenerate the security/manager/ssl test certificates and keys. r=franziskus a=Aryx on a CLOSED TREE
Comment 11•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 12•7 years ago
|
||
There are still failures:
https://treeherder.mozilla.org/logviewer.html#?job_id=160393168&repo=mozilla-central
Looks like some certs got updated which should not have been, e.g. security/manager/ssl/tests/unit/test_baseline_requirements_subject_common_name.js passed before
Status: RESOLVED → REOPENED
Flags: needinfo?(franziskuskiefer)
Resolution: FIXED → ---
Target Milestone: mozilla60 → ---
Reporter | ||
Comment 13•7 years ago
|
||
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #12)
> There are still failures:
>
> https://treeherder.mozilla.org/logviewer.html#?job_id=160393168&repo=mozilla-
> central
>
> Looks like some certs got updated which should not have been, e.g.
> security/manager/ssl/tests/unit/
> test_baseline_requirements_subject_common_name.js passed before
/me does not know what he's supposed to do with those.
Assignee: emilio → nobody
Reporter | ||
Comment 14•7 years ago
|
||
Looks like some of them are just testing certs in a specific date, so some of the certs should be reverted I guess. Aryx and I are going through those and trying to figure them out.
Comment hidden (mozreview-request) |
Comment 16•7 years ago
|
||
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/48def8fbcc6f
Also regenerate the signed apps. r=me a=Aryx on a CLOSED TREE
Comment 17•7 years ago
|
||
Regarding the one remaining failure [1] in test_cert_eku.js:
The issuer of the failing test cert, "int-nsSGC-recent.pem" [2] is only valid through 20170824, so regenerating the end entity [3] with no specified validity is by default creating an end entity issued after the intermediate has expired, which will cause a failure.
Since the intermediate's validity is:
validity:20160824-20170824
we should add probably the same line to the end entity, to ensure it's both valid on 25 August 2016 as well as issued during the validity of the intermediate.
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6c09abccbdad23f885c3355f9a68e3057738a63&selectedJob=160409302
[2] https://searchfox.org/mozilla-central/source/security/manager/ssl/tests/unit/test_cert_eku/int-nsSGC-recent.pem.certspec
[3] https://searchfox.org/mozilla-central/source/security/manager/ssl/tests/unit/test_cert_eku/ee-int-nsSGC-recent.pem.certspec
Comment 18•7 years ago
|
||
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/mozilla-central/rev/3df7961bad2c
Revert update of certs which have to remain outdated or are checked at a fixed point in time, update metadata hardcoded in tests. r=Try a=Try on a CLOSED TREE
Comment 19•7 years ago
|
||
Apologies for not catching this earlier, and thanks to all for getting this sorted out so far.
This patch fixes the test_cert_eku.js failures for me locally.
Flags: needinfo?(cykesiopka.bmo)
Attachment #8948450 -
Flags: review?(dkeeler)
Comment 20•7 years ago
|
||
Comment on attachment 8948450 [details] [diff] [review]
bug1435644_fix-test_cert_eku.js-failures_v1.patch
Actually, no, this isn't quite right.
Attachment #8948450 -
Flags: review?(dkeeler)
Comment 21•7 years ago
|
||
Hopefully this is correct now.
Attachment #8948450 -
Attachment is obsolete: true
Attachment #8948467 -
Flags: review?(dkeeler)
Assignee | ||
Comment 22•7 years ago
|
||
Comment on attachment 8948467 [details] [diff] [review]
bug1435644_fix-test_cert_eku.js-failures_v2.patch
Review of attachment 8948467 [details] [diff] [review]:
-----------------------------------------------------------------
This looks right. Thanks for taking care of this!
Attachment #8948467 -
Flags: review?(dkeeler) → review+
Comment 23•7 years ago
|
||
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/d47979847362
Fix and regenerate test_cert_eku/ certs to get test_cert_eku.js passing again. r=keeler on a CLOSED TREE
Reporter | ||
Comment 25•7 years ago
|
||
Comment on attachment 8948347 [details]
Bug 1435644: Also regenerate the signed apps.
Thanks so much for finishing this up!
Attachment #8948347 -
Flags: review?(dkeeler)
Comment 26•7 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 27•7 years ago
|
||
bugherder uplift |
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(dkeeler)
Comment 28•7 years ago
|
||
David, looks like this'll need rebased patches for mozilla-release (Fx58). Can you please attach those?
Flags: needinfo?(franziskuskiefer) → needinfo?(dkeeler)
Comment 29•7 years ago
|
||
bugherder uplift |
Comment 30•7 years ago
|
||
Getting this grafted to m-r was pretty straightforward when done on top of bug 1413336. However, that bug is not grafting cleanly to ESR52 and I think it's going to be more risky trying to get it to do so vs. coming up with an ESR52-specific version of these patches. David, can you please take a look?
Assignee | ||
Comment 31•7 years ago
|
||
Sure (also, just fyi - my irc client isn't able to connect to irc.m.o at the moment, but I'm on slack).
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 33•7 years ago
|
||
https://hg.mozilla.org/releases/mozilla-esr52/rev/9822da488aa651b49f030d0366681c8d0b7da73d
https://hg.mozilla.org/releases/mozilla-esr52/rev/138e37bb3fd27d1f71a28942cb354951ae6d19be
Flags: needinfo?(dkeeler)
Comment 34•7 years ago
|
||
ESR52 still has some failures that need addressing:
https://treeherder.mozilla.org/logviewer.html#?job_id=160520264&repo=mozilla-esr52
Can you please take a look, Franziskus?
Flags: needinfo?(franziskuskiefer)
Comment 35•7 years ago
|
||
This looks like the certs in dom/security/test/contentverifier weren't updated.
Comment 36•7 years ago
|
||
I don't see any cert-related changes to those tests since bug 1336654, and that got backported to ESR52 at the time as well :(
Assignee | ||
Comment 37•7 years ago
|
||
It appears https://hg.mozilla.org/mozilla-central/rev/e393e6c239cd#l20.12 (bug 1340181) is preventing that entire test directory from running, which is why it isn't failing on mozilla-central (or anything after 54).
Looks like this was deliberate, so that's comforting, at least:
(In reply to Ursula Sarracini (:ursula) from bug 1340181 comment #5)
> Updated patch addresses review comments above
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1340181#c2).
>
> One thing to notice here is that there is a bunch of logic for content
> signing that was put in place for remote about:newtab
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1226928). I think it's out of
> the scope of this ticket to remove all that stuff, since we're going to be
> filing a ticket to properly remove all the remote about:newtab code that
> currently lives in central, and removing the content signing stuff would be
> part of that work. I did remove the parts that were hitting any checks
> against content signing code, and I also disabled the content signing tests
> found in dom/security/test/contentverifier/.
Since these certificates don't have certspec files and since I believe the end-entity has to have a special key, I don't know how to regenerate them. Franziskus?
Comment 38•7 years ago
|
||
Ah, right, the code was mostly removed so the tests aren't really necessary anymore.
According to file_contentserver.sjs
// This cert chain is copied from
// security/manager/ssl/tests/unit/test_content_signing/
// using the certificates
// * content_signing_remote_newtab_ee.pem
// * content_signing_int.pem
// * content_signing_root.pem
So replacing goodChain.pem with those three certs should fix the tests
Flags: needinfo?(franziskuskiefer)
Assignee | ||
Comment 39•7 years ago
|
||
Ok - thanks. I had to update the root hash preference value in the tests as well, since the root changed.
https://hg.mozilla.org/releases/mozilla-esr52/rev/e091863355ff51122e8077f7d8f15553321eb24e
Assignee | ||
Updated•7 years ago
|
Comment 40•7 years ago
|
||
We are indeed fixed across all affected branches now. Thanks for the patches!
Assignee: nobody → dkeeler
Status: RESOLVED → VERIFIED
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Whiteboard: [stockwell fixed:other\
Updated•7 years ago
|
Whiteboard: [stockwell fixed:other\ → [stockwell fixed:other]
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•