867432 - Remove nsIX509Cert.verifyForUsage

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

We may not need to fix bug 867430 for this:

On #developers
<gavin> bsmith: we're going to just remove testpilot

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Remove nsIX509Cert.verifyForUsage

Here's the try run: https://tbpl.mozilla.org/?tree=Try&rev=015b6dd0e28b

Comment 3

[:Mook]

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 4

[Camilo Viecco (:cviecco)]

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.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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

Comment 6

[Honza Bambas (:mayhemer)]

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.

Comment 7

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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

Comment 8

[Honza Bambas (:mayhemer)]

Comment on attachment 744359 [details] [diff] [review]
Remove nsIX509Cert.verifyForUsage

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

Sorry for the delay.

sr=honzab

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d8b09a16c50

Thanks for the reviews.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7d8b09a16c50