Closed Bug 1435644 Opened 6 years ago Closed 6 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)

defect
Not set
blocker

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
firefox-esr52 --- verified
firefox58 --- verified
firefox59 --- verified
firefox60 --- verified

People

(Reporter: emilio, Assigned: keeler)

References

Details

(Whiteboard: [stockwell fixed:other])

Attachments

(2 files, 1 obsolete file)

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.
David, looks like you're familiar with this test, could you take a look? Thanks!
Flags: needinfo?(dkeeler)
Depends on: 1435645
Flags: needinfo?(cykesiopka.bmo)
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
Looks like these certificates (and therefore start/end dates) were generated at build time until bug 1256495.
Blocks: 1256495
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
I guess I'll give this a shot so I can land my stuff hopefully today...
Assignee: nobody → emilio
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+
Missed a few which didn't call GeneratedTestKey, thanks for making me double-check ;).

Fixed.
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+
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
https://hg.mozilla.org/mozilla-central/rev/95fd9deac911
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
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 → ---
(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
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.
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
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
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
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 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)
Hopefully this is correct now.
Attachment #8948450 - Attachment is obsolete: true
Attachment #8948467 - Flags: review?(dkeeler)
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+
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
Comment on attachment 8948347 [details]
Bug 1435644: Also regenerate the signed apps.

Thanks so much for finishing this up!
Attachment #8948347 - Flags: review?(dkeeler)
https://hg.mozilla.org/mozilla-central/rev/d47979847362
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Flags: needinfo?(dkeeler)
David, looks like this'll need rebased patches for mozilla-release (Fx58). Can you please attach those?
Flags: needinfo?(franziskuskiefer) → needinfo?(dkeeler)
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?
Sure (also, just fyi - my irc client isn't able to connect to irc.m.o at the moment, but I'm on slack).
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)
This looks like the certs in dom/security/test/contentverifier weren't updated.
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 :(
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?
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)
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
We are indeed fixed across all affected branches now. Thanks for the patches!
Assignee: nobody → dkeeler
Status: RESOLVED → VERIFIED
Whiteboard: [stockwell fixed:other\
Whiteboard: [stockwell fixed:other\ → [stockwell fixed:other]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: