Remove nsIX509Cert.verifyForUsage

RESOLVED FIXED in mozilla24

Status

()

Core
Security: PSM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: briansmith, Assigned: briansmith)

Tracking

(Blocks: 1 bug, 4 keywords)

unspecified
mozilla24
addon-compat, APIchange, main-thread-io, perf
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Snappy])

Attachments

(1 attachment)

+++ This bug was initially created as a clone of Bug #775698 +++

Certificate validation does disk I/O and/or network I/O so it should never be done on the main thread. Unfortunately, nsIX509Cert.verifyForUsage is impossible to implement with its current synchronous signature without doing network I/O or disk I/O on the main thread.

Also, the existence of this function complicates the insanity::pkix integration into Firefox.

The only user in the tree is Test Pilot. I looked at the code in Test Pilot and it seems unnecessary. I filed bug 867430 about that.
Summary: [Tracking] Remove all nsIX509Cert.verifyForUsage → Remove nsIX509Cert.verifyForUsage
We may not need to fix bug 867430 for this:

On #developers
<gavin> bsmith: we're going to just remove testpilot
See Also: → bug 867437
Depends on: 867445
No longer depends on: 867430
Assignee: nobody → bsmith
Created attachment 744359 [details] [diff] [review]
Remove nsIX509Cert.verifyForUsage

Here's the try run: https://tbpl.mozilla.org/?tree=Try&rev=015b6dd0e28b
Attachment #744359 - Flags: superreview?(honzab.moz)
Attachment #744359 - Flags: review?(cviecco)
Keywords: APIchange

Comment 3

4 years ago
Drive-by:

Note that you might want to update the documentation to stop using this method:

https://developer.mozilla.org/en-US/docs/How_to_check_the_security_state_of_an_XMLHTTPRequest_over_SSL

It may also be useful to document (asynchronous) methods for addons to migrate to.
Comment on attachment 744359 [details] [diff] [review]
Remove nsIX509Cert.verifyForUsage

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

r+ with comment addressed.

::: security/manager/ssl/public/nsIX509Cert.idl
@@ +155,5 @@
>    
>    /**
>     *  Constants that describe the certified usages of a certificate.
> +   *
> +   *  Deprecated and unused

File bug to remove these in the future or remove them now.
Attachment #744359 - Flags: review?(cviecco) → review+
(In reply to :Mook from comment #3)
> Drive-by:
> 
> Note that you might want to update the documentation to stop using this
> method:
> 
> https://developer.mozilla.org/en-US/docs/
> How_to_check_the_security_state_of_an_XMLHTTPRequest_over_SSL

I updated the wiki. Please review:

https://developer.mozilla.org/en-US/docs/How_to_check_the_security_state_of_an_XMLHTTPRequest_over_SSL

> It may also be useful to document (asynchronous) methods for addons to
> migrate to.

There isn't a general-purpose async counterpart, currently.
What is the plan for:

http://mxr.mozilla.org/comm-central/search?find=%2Fmailnews%2F&string=VerifyForUsage

We should provide comm-central e.g. a static function duplicating the original method.  Or do you have a different idea?

This patch will effectively break comm-central builds.
(In reply to Honza Bambas (:mayhemer) from comment #6)
> What is the plan for:
> 
> http://mxr.mozilla.org/comm-central/
> search?find=%2Fmailnews%2F&string=VerifyForUsage
> 
> We should provide comm-central e.g. a static function duplicating the
> original method.  Or do you have a different idea?

See bug 867437. The calls to verifyForUsage in comm-central can/should just be removed.

> This patch will effectively break comm-central builds.

Yes, lots of the patches related to this will break comm-central until comm-central can be updated to undo the breakage.

In general, all the S/MIME-related functionality should be moved from mozilla-central to comm-central so that we don't have to maintain it as part of PSM.
Blocks: 867437
Comment on attachment 744359 [details] [diff] [review]
Remove nsIX509Cert.verifyForUsage

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

Sorry for the delay.

sr=honzab
Attachment #744359 - Flags: superreview?(honzab.moz) → superreview+
No longer blocks: 867437
Depends on: 867437
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d8b09a16c50

Thanks for the reviews.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla24
https://hg.mozilla.org/mozilla-central/rev/7d8b09a16c50
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.