Closed Bug 1435644 Opened 3 years ago Closed 3 years ago

Cert xpcshell tests are permafailing due to expiration, e.g. security/manager/ssl/tests/unit/test_cert_chains.js - closed trees


(Core :: Security, defect)

Not set



Tracking Status
firefox-esr52 --- verified
firefox58 --- verified
firefox59 --- verified
firefox60 --- verified


(Reporter: emilio, Assigned: keeler)



(Whiteboard: [stockwell fixed:other])


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

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

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
Regenerate the security/manager/ssl test certificates and keys. r=franziskus a=Aryx on a CLOSED TREE
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
There are still failures:

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


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.

Pushed by
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]

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]

Review of attachment 8948467 [details] [diff] [review]:

This looks right. Thanks for taking care of this!
Attachment #8948467 - Flags: review?(dkeeler) → review+
Pushed by
Fix and regenerate test_cert_eku/ certs to get test_cert_eku.js passing again. r=keeler on a CLOSED TREE
Duplicate of this bug: 1435645
Comment on attachment 8948347 [details]
Bug 1435644: Also regenerate the signed apps.

Thanks so much for finishing this up!
Attachment #8948347 - Flags: review?(dkeeler)
Closed: 3 years ago3 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:

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 (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
> ( 
> 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
> ( 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.
We are indeed fixed across all affected branches now. Thanks for the patches!
Assignee: nobody → dkeeler
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.