Bug 950129: Make OCSP fetching policy for OCSP response signing certificates consistent, r=wtc, r=rrelyea
authorBrian Smith <brian@briansmith.org>
Sun, 12 Jan 2014 23:13:10 -0800
changeset 163109 2e2c276950b91b1eec31db74cff8f7d2b7826363
parent 163108 80a27198344a0cedc444aa80071380d5726b5a39
child 163110 0e14b3468b944b59b9275d39469bcbc37843d890
push id25981
push userbrian@briansmith.org
push dateMon, 13 Jan 2014 21:47:33 +0000
treeherdermozilla-central@1b892043a386 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswtc, rrelyea
bugs950129
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 950129: Make OCSP fetching policy for OCSP response signing certificates consistent, r=wtc, r=rrelyea
security/nss/coreconf/coreconf.dep
security/nss/lib/certdb/certi.h
security/nss/lib/certhigh/certvfy.c
security/nss/lib/certhigh/ocsp.c
security/patches/README
security/patches/bug-950129.patch
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -5,9 +5,8 @@
 
 /*
  * A dummy header file that is a dependency for all the object files.
  * Used to force a full recompilation of NSS in Mozilla's Tinderbox
  * depend builds.  See comments in rules.mk.
  */
 
 #error "Do not include this header file."
-
--- a/security/nss/lib/certdb/certi.h
+++ b/security/nss/lib/certdb/certi.h
@@ -256,16 +256,38 @@ void ReleaseDPCache(CRLDPCache* dpcache,
 
 /*
  * map Stan errors into NSS errors
  * This function examines the stan error stack and automatically sets
  * PORT_SetError(); to the appropriate SEC_ERROR value.
  */
 void CERT_MapStanError();
 
+/* Like CERT_VerifyCert, except with an additional argument, flags. The
+ * flags are defined immediately below.
+ *
+ * OCSP checking is always skipped when certUsage is certUsageStatusResponder.
+ */
+SECStatus
+cert_VerifyCertWithFlags(CERTCertDBHandle *handle, CERTCertificate *cert,
+                         PRBool checkSig, SECCertUsage certUsage, PRTime t,
+                         PRUint32 flags, void *wincx, CERTVerifyLog *log);
+
+/* Use the default settings.
+ * cert_VerifyCertWithFlags(..., CERT_VERIFYCERT_USE_DEFAULTS) is equivalent
+ * to CERT_VerifyCert(...);
+ */
+#define CERT_VERIFYCERT_USE_DEFAULTS 0
+
+/* Skip all the OCSP checks during certificate verification, regardless of
+ * the global OCSP settings. By default, certificate |cert| will have its
+ * revocation status checked via OCSP according to the global OCSP settings.
+ */
+#define CERT_VERIFYCERT_SKIP_OCSP 1
+
 /* Interface function for libpkix cert validation engine:
  * cert_verify wrapper. */
 SECStatus
 cert_VerifyCertChainPkix(CERTCertificate *cert,
                          PRBool checkSig,
                          SECCertUsage     requiredUsage,
                          PRTime           time,
                          void            *wincx,
--- a/security/nss/lib/certhigh/certvfy.c
+++ b/security/nss/lib/certhigh/certvfy.c
@@ -1195,17 +1195,17 @@ CERT_VerifyCertificate(CERTCertDBHandle 
 
         if (rv != SECSuccess) {
             /* EXIT_IF_NOT_LOGGING(log); XXX ???? */
             INVALID_USAGE();
         }
 
         /*
          * Check OCSP revocation status, but only if the cert we are checking
-         * is not a status reponder itself.  We only do this in the case
+         * is not a status responder itself. We only do this in the case
          * where we checked the cert chain (above); explicit trust "wins"
          * (avoids status checking, just as it avoids CRL checking) by
          * bypassing this code.
          */
 
         if (PR_FALSE == checkedOCSP) {
             checkedOCSP = PR_TRUE; /* only check OCSP once */
             statusConfig = CERT_GetStatusConfig(handle);
@@ -1230,20 +1230,29 @@ loser:
     return(valid);
 }
 
 SECStatus
 CERT_VerifyCert(CERTCertDBHandle *handle, CERTCertificate *cert,
 		PRBool checkSig, SECCertUsage certUsage, PRTime t,
 		void *wincx, CERTVerifyLog *log)
 {
+    return cert_VerifyCertWithFlags(handle, cert, checkSig, certUsage, t,
+                                    CERT_VERIFYCERT_USE_DEFAULTS, wincx, log);
+}
+
+SECStatus
+cert_VerifyCertWithFlags(CERTCertDBHandle *handle, CERTCertificate *cert,
+                         PRBool checkSig, SECCertUsage certUsage, PRTime t,
+                         PRUint32 flags, void *wincx, CERTVerifyLog *log)
+{
     SECStatus rv;
     unsigned int requiredKeyUsage;
     unsigned int requiredCertType;
-    unsigned int flags;
+    unsigned int failedFlags;
     unsigned int certType;
     PRBool       trusted;
     PRBool       allowOverride;
     SECCertTimeValidity validity;
     CERTStatusConfig *statusConfig;
    
 #ifdef notdef 
     /* check if this cert is in the Evil list */
@@ -1302,41 +1311,43 @@ CERT_VerifyCert(CERTCertDBHandle *handle
 	PORT_SetError(SEC_ERROR_INADEQUATE_KEY_USAGE);
 	LOG_ERROR_OR_EXIT(log,cert,0,requiredKeyUsage);
     }
     if ( !( certType & requiredCertType ) ) {
 	PORT_SetError(SEC_ERROR_INADEQUATE_CERT_TYPE);
 	LOG_ERROR_OR_EXIT(log,cert,0,requiredCertType);
     }
 
-    rv = cert_CheckLeafTrust(cert,certUsage, &flags, &trusted);
+    rv = cert_CheckLeafTrust(cert, certUsage, &failedFlags, &trusted);
     if (rv  == SECFailure) {
 	PORT_SetError(SEC_ERROR_UNTRUSTED_CERT);
-	LOG_ERROR_OR_EXIT(log,cert,0,flags);
+	LOG_ERROR_OR_EXIT(log, cert, 0, failedFlags);
     } else if (trusted) {
 	goto done;
     }
 
 
     rv = CERT_VerifyCertChain(handle, cert, checkSig, certUsage,
 			      t, wincx, log);
     if (rv != SECSuccess) {
 	EXIT_IF_NOT_LOGGING(log);
     }
 
     /*
-     * Check revocation status, but only if the cert we are checking
-     * is not a status reponder itself.  We only do this in the case
-     * where we checked the cert chain (above); explicit trust "wins"
-     * (avoids status checking, just as it avoids CRL checking, which
-     * is all done inside VerifyCertChain) by bypassing this code.
+     * Check revocation status, but only if the cert we are checking is not a
+     * status responder itself and the caller did not ask us to skip the check.
+     * We only do this in the case where we checked the cert chain (above);
+     * explicit trust "wins" (avoids status checking, just as it avoids CRL
+     * checking, which is all done inside VerifyCertChain) by bypassing this
+     * code.
      */
-    statusConfig = CERT_GetStatusConfig(handle);
-    if (certUsage != certUsageStatusResponder && statusConfig != NULL) {
-	if (statusConfig->statusChecker != NULL) {
+    if (!(flags & CERT_VERIFYCERT_SKIP_OCSP) &&
+	certUsage != certUsageStatusResponder) {
+	statusConfig = CERT_GetStatusConfig(handle);
+	if (statusConfig && statusConfig->statusChecker) {
 	    rv = (* statusConfig->statusChecker)(handle, cert,
 							 t, wincx);
 	    if (rv != SECSuccess) {
 		LOG_ERROR_OR_EXIT(log,cert,0,0);
 	    }
 	}
     }
 
--- a/security/nss/lib/certhigh/ocsp.c
+++ b/security/nss/lib/certhigh/ocsp.c
@@ -13,16 +13,17 @@
 #include "prnetdb.h"
 
 #include "seccomon.h"
 #include "secitem.h"
 #include "secoidt.h"
 #include "secasn1.h"
 #include "secder.h"
 #include "cert.h"
+#include "certi.h"
 #include "xconst.h"
 #include "secerr.h"
 #include "secoid.h"
 #include "hasht.h"
 #include "sechash.h"
 #include "secasn1.h"
 #include "plbase64.h"
 #include "keyhi.h"
@@ -4179,18 +4180,19 @@ CERT_VerifyOCSPResponseSignature(CERTOCS
         rv = SECSuccess;
     } else {
         SECCertUsage certUsage;
         if (CERT_IsCACert(signerCert, NULL)) {
             certUsage = certUsageAnyCA;
         } else {
             certUsage = certUsageStatusResponder;
         }
-        rv = CERT_VerifyCert(handle, signerCert, PR_TRUE,
-                             certUsage, producedAt, pwArg, NULL);
+        rv = cert_VerifyCertWithFlags(handle, signerCert, PR_TRUE, certUsage,
+                                      producedAt, CERT_VERIFYCERT_SKIP_OCSP,
+                                      pwArg, NULL);
         if (rv != SECSuccess) {
             PORT_SetError(SEC_ERROR_OCSP_INVALID_SIGNING_CERT);
             goto finish;
         }
     }
 
     rv = ocsp_VerifyResponseSignature(signerCert, signature,
                                       tbsResponseDataDER,
--- a/security/patches/README
+++ b/security/patches/README
@@ -1,2 +1,5 @@
 This directory contains patches that were added locally
 on top of the NSS release.
+
+bug-950129.patch  Make OCSP fetching policy for OCSP response signing
+                  certificates consistent.
new file mode 100644
--- /dev/null
+++ b/security/patches/bug-950129.patch
@@ -0,0 +1,195 @@
+# HG changeset patch
+# Parent 352d188c67d9fefd82524c4439a5d04679687945
+# User Brian Smith <brian@briansmith.org>
+diff --git a/security/nss/lib/certdb/certi.h b/security/nss/lib/certdb/certi.h
+--- a/security/nss/lib/certdb/certi.h
++++ b/security/nss/lib/certdb/certi.h
+@@ -256,16 +256,38 @@ void ReleaseDPCache(CRLDPCache* dpcache,
+ 
+ /*
+  * map Stan errors into NSS errors
+  * This function examines the stan error stack and automatically sets
+  * PORT_SetError(); to the appropriate SEC_ERROR value.
+  */
+ void CERT_MapStanError();
+ 
++/* Like CERT_VerifyCert, except with an additional argument, flags. The
++ * flags are defined immediately below.
++ *
++ * OCSP checking is always skipped when certUsage is certUsageStatusResponder.
++ */
++SECStatus
++cert_VerifyCertWithFlags(CERTCertDBHandle *handle, CERTCertificate *cert,
++                         PRBool checkSig, SECCertUsage certUsage, PRTime t,
++                         PRUint32 flags, void *wincx, CERTVerifyLog *log);
++
++/* Use the default settings.
++ * cert_VerifyCertWithFlags(..., CERT_VERIFYCERT_USE_DEFAULTS) is equivalent
++ * to CERT_VerifyCert(...);
++ */
++#define CERT_VERIFYCERT_USE_DEFAULTS 0
++
++/* Skip all the OCSP checks during certificate verification, regardless of
++ * the global OCSP settings. By default, certificate |cert| will have its
++ * revocation status checked via OCSP according to the global OCSP settings.
++ */
++#define CERT_VERIFYCERT_SKIP_OCSP 1
++
+ /* Interface function for libpkix cert validation engine:
+  * cert_verify wrapper. */
+ SECStatus
+ cert_VerifyCertChainPkix(CERTCertificate *cert,
+                          PRBool checkSig,
+                          SECCertUsage     requiredUsage,
+                          PRTime           time,
+                          void            *wincx,
+diff --git a/security/nss/lib/certhigh/certvfy.c b/security/nss/lib/certhigh/certvfy.c
+--- a/security/nss/lib/certhigh/certvfy.c
++++ b/security/nss/lib/certhigh/certvfy.c
+@@ -1195,17 +1195,17 @@ CERT_VerifyCertificate(CERTCertDBHandle 
+ 
+         if (rv != SECSuccess) {
+             /* EXIT_IF_NOT_LOGGING(log); XXX ???? */
+             INVALID_USAGE();
+         }
+ 
+         /*
+          * Check OCSP revocation status, but only if the cert we are checking
+-         * is not a status reponder itself.  We only do this in the case
++         * is not a status responder itself. We only do this in the case
+          * where we checked the cert chain (above); explicit trust "wins"
+          * (avoids status checking, just as it avoids CRL checking) by
+          * bypassing this code.
+          */
+ 
+         if (PR_FALSE == checkedOCSP) {
+             checkedOCSP = PR_TRUE; /* only check OCSP once */
+             statusConfig = CERT_GetStatusConfig(handle);
+@@ -1230,20 +1230,29 @@ loser:
+     return(valid);
+ }
+ 
+ SECStatus
+ CERT_VerifyCert(CERTCertDBHandle *handle, CERTCertificate *cert,
+ 		PRBool checkSig, SECCertUsage certUsage, PRTime t,
+ 		void *wincx, CERTVerifyLog *log)
+ {
++    return cert_VerifyCertWithFlags(handle, cert, checkSig, certUsage, t,
++                                    CERT_VERIFYCERT_USE_DEFAULTS, wincx, log);
++}
++
++SECStatus
++cert_VerifyCertWithFlags(CERTCertDBHandle *handle, CERTCertificate *cert,
++                         PRBool checkSig, SECCertUsage certUsage, PRTime t,
++                         PRUint32 flags, void *wincx, CERTVerifyLog *log)
++{
+     SECStatus rv;
+     unsigned int requiredKeyUsage;
+     unsigned int requiredCertType;
+-    unsigned int flags;
++    unsigned int failedFlags;
+     unsigned int certType;
+     PRBool       trusted;
+     PRBool       allowOverride;
+     SECCertTimeValidity validity;
+     CERTStatusConfig *statusConfig;
+    
+ #ifdef notdef 
+     /* check if this cert is in the Evil list */
+@@ -1302,41 +1311,43 @@ CERT_VerifyCert(CERTCertDBHandle *handle
+ 	PORT_SetError(SEC_ERROR_INADEQUATE_KEY_USAGE);
+ 	LOG_ERROR_OR_EXIT(log,cert,0,requiredKeyUsage);
+     }
+     if ( !( certType & requiredCertType ) ) {
+ 	PORT_SetError(SEC_ERROR_INADEQUATE_CERT_TYPE);
+ 	LOG_ERROR_OR_EXIT(log,cert,0,requiredCertType);
+     }
+ 
+-    rv = cert_CheckLeafTrust(cert,certUsage, &flags, &trusted);
++    rv = cert_CheckLeafTrust(cert, certUsage, &failedFlags, &trusted);
+     if (rv  == SECFailure) {
+ 	PORT_SetError(SEC_ERROR_UNTRUSTED_CERT);
+-	LOG_ERROR_OR_EXIT(log,cert,0,flags);
++	LOG_ERROR_OR_EXIT(log, cert, 0, failedFlags);
+     } else if (trusted) {
+ 	goto done;
+     }
+ 
+ 
+     rv = CERT_VerifyCertChain(handle, cert, checkSig, certUsage,
+ 			      t, wincx, log);
+     if (rv != SECSuccess) {
+ 	EXIT_IF_NOT_LOGGING(log);
+     }
+ 
+     /*
+-     * Check revocation status, but only if the cert we are checking
+-     * is not a status reponder itself.  We only do this in the case
+-     * where we checked the cert chain (above); explicit trust "wins"
+-     * (avoids status checking, just as it avoids CRL checking, which
+-     * is all done inside VerifyCertChain) by bypassing this code.
++     * Check revocation status, but only if the cert we are checking is not a
++     * status responder itself and the caller did not ask us to skip the check.
++     * We only do this in the case where we checked the cert chain (above);
++     * explicit trust "wins" (avoids status checking, just as it avoids CRL
++     * checking, which is all done inside VerifyCertChain) by bypassing this
++     * code.
+      */
+-    statusConfig = CERT_GetStatusConfig(handle);
+-    if (certUsage != certUsageStatusResponder && statusConfig != NULL) {
+-	if (statusConfig->statusChecker != NULL) {
++    if (!(flags & CERT_VERIFYCERT_SKIP_OCSP) &&
++	certUsage != certUsageStatusResponder) {
++	statusConfig = CERT_GetStatusConfig(handle);
++	if (statusConfig && statusConfig->statusChecker) {
+ 	    rv = (* statusConfig->statusChecker)(handle, cert,
+ 							 t, wincx);
+ 	    if (rv != SECSuccess) {
+ 		LOG_ERROR_OR_EXIT(log,cert,0,0);
+ 	    }
+ 	}
+     }
+ 
+diff --git a/security/nss/lib/certhigh/ocsp.c b/security/nss/lib/certhigh/ocsp.c
+--- a/security/nss/lib/certhigh/ocsp.c
++++ b/security/nss/lib/certhigh/ocsp.c
+@@ -13,16 +13,17 @@
+ #include "prnetdb.h"
+ 
+ #include "seccomon.h"
+ #include "secitem.h"
+ #include "secoidt.h"
+ #include "secasn1.h"
+ #include "secder.h"
+ #include "cert.h"
++#include "certi.h"
+ #include "xconst.h"
+ #include "secerr.h"
+ #include "secoid.h"
+ #include "hasht.h"
+ #include "sechash.h"
+ #include "secasn1.h"
+ #include "plbase64.h"
+ #include "keyhi.h"
+@@ -4179,18 +4180,19 @@ CERT_VerifyOCSPResponseSignature(CERTOCS
+         rv = SECSuccess;
+     } else {
+         SECCertUsage certUsage;
+         if (CERT_IsCACert(signerCert, NULL)) {
+             certUsage = certUsageAnyCA;
+         } else {
+             certUsage = certUsageStatusResponder;
+         }
+-        rv = CERT_VerifyCert(handle, signerCert, PR_TRUE,
+-                             certUsage, producedAt, pwArg, NULL);
++        rv = cert_VerifyCertWithFlags(handle, signerCert, PR_TRUE, certUsage,
++                                      producedAt, CERT_VERIFYCERT_SKIP_OCSP,
++                                      pwArg, NULL);
+         if (rv != SECSuccess) {
+             PORT_SetError(SEC_ERROR_OCSP_INVALID_SIGNING_CERT);
+             goto finish;
+         }
+     }
+ 
+     rv = ocsp_VerifyResponseSignature(signerCert, signature,
+                                       tbsResponseDataDER,