Closed Bug 1453741 Opened 2 years ago Closed 2 years ago

remove synchronous certificate verification APIs from nsIX509CertDB

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: keeler, Assigned: keeler)

References

(Blocks 1 open bug)

Details

(Whiteboard: [psm-assigned])

Attachments

(2 files)

nsIX509CertDB exposes some synchronous certificate verification APIs. Removing them will contribute to the goal of prohibiting OCSP fetching on the main thread, which will enable some implementation simplifications that hopefully will address some stability issues we've been seeing (e.g. potentially bug 1435966). It's also possible that doing this will help with the intermittent failures we've been seeing in many PSM xpcshell tests.
findCertByEmailAddress is still used by comm-central, so I filed bug 1453778.
Comment on attachment 8967512 [details]
bug 1453741 - (2/2) remove nsIX509CertDB.findCertByEmailAddress

https://reviewboard.mozilla.org/r/236190/#review242018
Attachment #8967512 - Flags: review?(jjones) → review+
Comment on attachment 8967511 [details]
bug 1453741 - (1/2) remove nsIX509CertDB.verifyCert{AtTime,Now}

https://reviewboard.mozilla.org/r/236188/#review242020

I'm not nearly as accomplished a linter, but this looks good to me. I scanned the bulk of the test edits for obvious errors, and looked closer at the removals / `head_psm` work, which all look fine.
Attachment #8967511 - Flags: review?(jjones) → review+
Comment on attachment 8967511 [details]
bug 1453741 - (1/2) remove nsIX509CertDB.verifyCert{AtTime,Now}

https://reviewboard.mozilla.org/r/236188/#review242126

Couldn't find anything wrong with this either :)
Attachment #8967511 - Flags: review?(franziskuskiefer) → review+
Comment on attachment 8967512 [details]
bug 1453741 - (2/2) remove nsIX509CertDB.findCertByEmailAddress

https://reviewboard.mozilla.org/r/236190/#review242124

lgtm
Attachment #8967512 - Flags: review?(franziskuskiefer) → review+
Thanks for the reviews!
try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3cdc4704b18ee0ed998509b5e8d9572004c9d44
I think bug 1453778 is going to land soonish, so I'll wait a bit to land this.
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c76fdea69868
(1/2) remove nsIX509CertDB.verifyCert{AtTime,Now} r=fkiefer,jcj
https://hg.mozilla.org/integration/autoland/rev/00a9881a5ceb
(2/2) remove nsIX509CertDB.findCertByEmailAddress r=fkiefer,jcj
https://hg.mozilla.org/mozilla-central/rev/c76fdea69868
https://hg.mozilla.org/mozilla-central/rev/00a9881a5ceb
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.