728321 - Workaround bug 551429 (Stack overflow during revocation checking of OCSP response signing cert) in Gecko

Status: RESOLVED INVALID

(Core :: Security: PSM, defect)

Description

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

Attachment: Prevent stack overflow and ensure addon compatibility when NSS_ENABLE_PKIX_VERIFY=1 by forcing CERT_SetUsePKIXForValidation(false)

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

[Note that this patch is the same patch I attached to bug 551429.]

The best fix for bug 551429 will probably not be ready for Firefox 13. My goal is to have libpkix enabled by default in Firefox 13. In order to do so, we should work around bug 551429 until the more complicated fix in libpkix itself should be made. (I know how to fix the bug in libpkix; I just don't have time to do it this release cycle.)

Note that NSS_Initialize (or something it calls) calls CERT_SetUsePKIXForValidation(PR_TRUE) when NSS_ENABLE_PKIX_VERIFY=1 in the environment. This patch causes Gecko-based applications to effectively ignore the
NSS_ENABLE_PKIX_VERIFY=1 environment variable.

As noted in the other bug, we must force CERT_SetUsePKIXForValidation(PR_FALSE) for addon compatibility as well.

Comment 1

[Kyle Hamilton]

I'm probably wrong, but it looks as though this would cause NSS to initialize libpkix for its use (certificate verification), but then the certificate verifier is told to never use it.  i.e. enable libpkix in name only.  If it is enabled in name only, its initialization spends resources that are never recouped in functionality.

We need libpkix because we need cross-certificates, policy mapping, and other things its certificate verifier implements.  We can't say "oh it's enabled" and not allow the blocked bugs (such as US Federal Bridge CA inclusion, bug 478418) to be resolved.

If it's not ready for prime time in Firefox 13, it's best to hold off on it until it is ready.  It's definitely not in the user's interest to ship security code which doesn't integrate well with its environment.  I think this was :kaie's point on bug 5541 comment 17:  "We need a real fix, not CERT_SetUsePKIXForValidation(false)."

Comment 2

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

(In reply to Kyle Hamilton from comment #1)
> I'm probably wrong, but it looks as though this would cause NSS to
> initialize libpkix for its use (certificate verification), but then the
> certificate verifier is told to never use it.

CERT_SetUsePKIXForValidation tells CERT_VerifyCert[ificate] to implement itself in terms of CERT_PKIXVerifyCert. But, the translation is not 100% correct.

All the callers within Gecko already have explicit calls to CERT_PKIXVerifyCert that is used when the libpkix pref is enabled and call CERT_VerifyCert[ificate] when the libpkix pref is disabled.

Comment 3

[Kai Engert (:KaiE:)]

Comment on attachment 598301 [details] [diff] [review]
Prevent stack overflow and ensure addon compatibility when NSS_ENABLE_PKIX_VERIFY=1 by forcing CERT_SetUsePKIXForValidation(false)

I don't like this change, it should be either completely PKIX or no PKIX, depending on the pref.

But feel free to find another reviewer who agrees with you.

Comment 4

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

We simply don't support NSS_ENABLE_PKIX_VERIFY=1. Also, the switch to insanity::pkix will make this a non-issue.