653572 - PSM should use CERT_SetUsePKIXForValidation() to switch all calls to libpkix (calls from with NSS, or from extensions)

Status: RESOLVED INVALID

(Core :: Security: PSM, enhancement)

Description

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

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

There is a question as to whether we should call CERT_SetUsePKIXForValidation(PR_TRUE) so that extensions start using libpkix instead of the old certificate validation semantics. My opinion is we shouldn't do so, because (a) we cannot make these calls use the same verification options as we are using for other verifications (which is really what extensions would want, I would guess), and (b) some extensions may be somehow relying on the old validation behavior. I recommend WONTFIX.

Comment 1

[Kai Engert (:KaiE:)]

(In reply to comment #0)
> 
> There is a question as to whether we should call
> CERT_SetUsePKIXForValidation(PR_TRUE)

More precisely, my proposal is:
- query the preference that either enables or disables the use of libpkix
  in PSM

- use either TRUE or FALSE for this function call during PSM init,
  depending on the preference.


When using PR_FALSE, it's a noop and keeps the old behaviour.


When the preference is set to TRUE, we really want to use libpkix for all uses, including potential indirect calls triggered from NSS itself.



> so that extensions start using libpkix
> instead of the old certificate validation semantics. 

Not limited to extensions, this affects calls from inside NSS to verifications, too. (I don't know whether such code paths exist that affect PSM. I haven't read all of NSS. My argue is, let's just make sure we're switching completely.)

Regarding extensions, I think you refer to Firefox add-ons that:
- do not use the XPCOM interfaces that PSM provides
- directly link to NSS and directly call C interfaces


Difficult question and decision.

In my opinion, the driving factor for answering this question should be, what's our strategy?

Is it
(a) continue to keep the old verifications APIs forever, with the old semantic

or is it
(b) encourage everyone to switch to libPKIX.


I suspect it's (b).

If it's really (b), then having libPKIX use globally enabled, this would be a good opportunity for the developers of such extensions to notice that things are changing. If they don't like the new behaviour, they are free to update their code to use their own defaults.

Comment 2

[Kai Engert (:KaiE:)]

Attachment: Patch v1 - Do not check in until bug 551429 is fixed (by way of a libPKIX improvement)

Let's get a third opinion

Comment 3

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

I think this won't work, for the same reason we aren't using CERT_SetUsePKIXForValidation for the core validaiton in PSM: there isn't a way to set the validation options (CRL options, OCSP options, missing cert fetching, etc.). That's why I said that this is a third set of semantics. Also, as we saw with the need for us to use SkipOcsp/SkipOcspOff() previously, the old API is really problematic when concurrently validating certificate chains.

I would rather have extensions continue to work the same way they always have, and document that the old API is deprecated in Mozilla. I've already started auditing the code in NSS to find places where calls to the old API are done and I will file bugs about any calls to those functions. (I know one case is the handling of SSL step up in SSL, which should be disabled in PSM anyway.)

Comment 4

[Robert Relyea]

Comment on attachment 529514 [details] [diff] [review]
Patch v1 - Do not check in until bug 551429 is fixed (by way of a libPKIX improvement)

r+ rrelyea

Comment 5

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

This will cause stack overflows due to bug 635384, that do not occur without it.

Comment 6

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

From Sean Leonard on the dev-tech-crypto thread "libpkix maintenance plan (was Re: What exactly are the benefits of libpkix over the old certificate path validation library?)":

> As stated, we would like to move away from using
> CERT_VerifyCertificate, provided that CERT_PKIXVerifyCert is
> strictly better. CERT_PKIXVerifyCert has significant gaps in
> functionality that need to be addressed first.

So, calling CERT_SetUsePKIXForValidation(PR_TRUE) may actually have negative effects w.r.t. addon compatibility.

Comment 7

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

The patch in bug 551429 does the exact opposite of this. Because of the addon compatibility issues, I am proposing we WONTFIX this. Re-open if you disagree.

Comment 8

[Kai Engert (:KaiE:)]

I disagree. Reopening.

We must improve libPKIX to work correctly.

Comment 9

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

(In reply to Kai Engert (:kaie) from comment #8)
> I disagree. Reopening.
> 
> We must improve libPKIX to work correctly.

There is no way libpkix will be enabled by default in mozilla13 if you insist that all the bugs in libpkix that would block this bug be fixed. There is a lot of work that needs to be done to make libpkix backward-compatible enough to make it the default for all addon use cases too. Again, see Sean's email on dev-tech-crypto.

Comment 10

[Kyle Hamilton]

Please acknowledge that libpkix won't be the default in moz13 because the functionality which it provides is insufficient for effective transition.  Until it's fixed, this bug needs to depend on bug 651246 and bug 728321.

Otherwise, you're pushing an increased library initialization footprint with no benefit, also called "dead code".

Comment 11

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

There's no need for calling CERT_SetUsePKIXForValidation in Gecko given that we're ready to switch to insanity::pkix in Firefox. Additionally, I am planning to make it so that Gecko builds NSS without building libpkix to save footprint, so CERT_SetUsePKIXForValidation won't even be available to call.