Bug 1130757 - Move OneCRL check to NSSCertDBTrustDomain::GetCertTrust. r=dkeeler, a=sledru
authorMark Goodwin <mgoodwin@mozilla.com>
Thu, 26 Feb 2015 04:38:00 +0100
changeset 258115 03f55ec25d9a3378ea637e14a506a25ba0e4ee25
parent 258114 578c3d9bd490d88764c4d4435450c730ca30f5a5
child 258116 85246855810995cbe53961804b7ba30f6bde286e
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdkeeler, sledru
bugs1130757
milestone38.0a2
Bug 1130757 - Move OneCRL check to NSSCertDBTrustDomain::GetCertTrust. r=dkeeler, a=sledru
security/certverifier/NSSCertDBTrustDomain.cpp
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -204,16 +204,38 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
   SECItem candidateCertDERSECItem = UnsafeMapInputToSECItem(candidateCertDER);
   ScopedCERTCertificate candidateCert(
     CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &candidateCertDERSECItem,
                             nullptr, false, true));
   if (!candidateCert) {
     return MapPRErrorCodeToResult(PR_GetError());
   }
 
+  // Check the certificate against the OneCRL cert blocklist
+  if (!mCertBlocklist) {
+    return Result::FATAL_ERROR_LIBRARY_FAILURE;
+  }
+
+  bool isCertRevoked;
+  nsresult nsrv = mCertBlocklist->IsCertRevoked(
+                    candidateCert->derIssuer.data,
+                    candidateCert->derIssuer.len,
+                    candidateCert->serialNumber.data,
+                    candidateCert->serialNumber.len,
+                    &isCertRevoked);
+  if (NS_FAILED(nsrv)) {
+    return Result::FATAL_ERROR_LIBRARY_FAILURE;
+  }
+
+  if (isCertRevoked) {
+    PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
+           ("NSSCertDBTrustDomain: certificate is in blocklist"));
+    return Result::ERROR_REVOKED_CERTIFICATE;
+  }
+
   // XXX: CERT_GetCertTrust seems to be abusing SECStatus as a boolean, where
   // SECSuccess means that there is a trust record and SECFailure means there
   // is not a trust record. I looked at NSS's internal uses of
   // CERT_GetCertTrust, and all that code uses the result as a boolean meaning
   // "We have a trust record."
   CERTCertTrust trust;
   if (CERT_GetCertTrust(candidateCert.get(), &trust) == SECSuccess) {
     uint32_t flags = SEC_GET_TRUST_FLAGS(&trust, mCertDBTrustType);
@@ -352,37 +374,16 @@ NSSCertDBTrustDomain::CheckRevocation(En
   // Since this affects EV there is no reason why DV should be more strict
   // so all intermediatates are allowed to have OCSP responses up to one year
   // old.
   uint16_t maxOCSPLifetimeInDays = 10;
   if (endEntityOrCA == EndEntityOrCA::MustBeCA) {
     maxOCSPLifetimeInDays = 365;
   }
 
-  if (!mCertBlocklist) {
-    return Result::FATAL_ERROR_LIBRARY_FAILURE;
-  }
-
-  bool isCertRevoked;
-  nsresult nsrv = mCertBlocklist->IsCertRevoked(
-                    certID.issuer.UnsafeGetData(),
-                    certID.issuer.GetLength(),
-                    certID.serialNumber.UnsafeGetData(),
-                    certID.serialNumber.GetLength(),
-                    &isCertRevoked);
-  if (NS_FAILED(nsrv)) {
-    return Result::FATAL_ERROR_LIBRARY_FAILURE;
-  }
-
-  if (isCertRevoked) {
-    PR_LOG(gCertVerifierLog, PR_LOG_DEBUG,
-           ("NSSCertDBTrustDomain: certificate is in blocklist"));
-    return Result::ERROR_REVOKED_CERTIFICATE;
-  }
-
   // If we have a stapled OCSP response then the verification of that response
   // determines the result unless the OCSP response is expired. We make an
   // exception for expired responses because some servers, nginx in particular,
   // are known to serve expired responses due to bugs.
   // We keep track of the result of verifying the stapled response but don't
   // immediately return failure if the response has expired.
   //
   // We only set the OCSP stapling status if we're validating the end-entity