bug 943115 - return early in CreateCertErrorRunnable for non-overridable errors r=briansmith
authorDavid Keeler <dkeeler@mozilla.com>
Tue, 26 Nov 2013 13:49:47 -0800
changeset 157626 e781364ed4f8ad008659a0076dd3f73ebabfd621
parent 157625 c6f465d6cfb04e96a5391ca776cff67eec86933c
child 157627 fc7b76d9da1ec1b461410baf1553687441c94b64
push id36792
push userdkeeler@mozilla.com
push dateTue, 26 Nov 2013 23:53:11 +0000
treeherdermozilla-inbound@fc7b76d9da1e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith
bugs943115
milestone28.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 943115 - return early in CreateCertErrorRunnable for non-overridable errors r=briansmith
security/manager/ssl/src/SSLServerCertVerification.cpp
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -428,32 +428,63 @@ CertErrorRunnable::RunOnTargetThread()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   mResult = CheckCertOverrides();
   
   MOZ_ASSERT(mResult);
 }
 
+// Converts a PRErrorCode into one of
+//   nsICertOverrideService::ERROR_UNTRUSTED,
+//   nsICertOverrideService::ERROR_MISMATCH,
+//   nsICertOverrideService::ERROR_TIME
+// if the given error code is an overridable error.
+// If it is not, then 0 is returned.
+uint32_t
+PRErrorCodeToOverrideType(PRErrorCode errorCode)
+{
+  switch (errorCode)
+  {
+    case SEC_ERROR_UNKNOWN_ISSUER:
+    case SEC_ERROR_CA_CERT_INVALID:
+    case SEC_ERROR_UNTRUSTED_ISSUER:
+    case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
+    case SEC_ERROR_UNTRUSTED_CERT:
+    case SEC_ERROR_INADEQUATE_KEY_USAGE:
+    case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED:
+      // We group all these errors as "cert not trusted"
+      return nsICertOverrideService::ERROR_UNTRUSTED;
+    case SSL_ERROR_BAD_CERT_DOMAIN:
+      return nsICertOverrideService::ERROR_MISMATCH;
+    case SEC_ERROR_EXPIRED_CERTIFICATE:
+      return nsICertOverrideService::ERROR_TIME;
+    default:
+      return 0;
+  }
+}
+
 // Returns null with the error code (PR_GetError()) set if it does not create
 // the CertErrorRunnable.
 CertErrorRunnable *
 CreateCertErrorRunnable(PRErrorCode defaultErrorCodeToReport,
                         TransportSecurityInfo * infoObject,
                         CERTCertificate * cert,
                         const void * fdForLogging,
                         uint32_t providerFlags,
                         PRTime now)
 {
   MOZ_ASSERT(infoObject);
   MOZ_ASSERT(cert);
   
-  // cert was revoked, don't do anything else
-  if (defaultErrorCodeToReport == SEC_ERROR_REVOKED_CERTIFICATE) {
-    PR_SetError(SEC_ERROR_REVOKED_CERTIFICATE, 0);
+  // We only allow overrides for certain errors. Return early if the error
+  // is not one of them. This is to avoid doing revocation fetching in the
+  // case of OCSP stapling and probably for other reasons.
+  if (PRErrorCodeToOverrideType(defaultErrorCodeToReport) == 0) {
+    PR_SetError(defaultErrorCodeToReport, 0);
     return nullptr;
   }
 
   if (defaultErrorCodeToReport == 0) {
     NS_ERROR("No error code set during certificate validation failure.");
     PR_SetError(PR_INVALID_STATE_ERROR, 0);
     return nullptr;
   }
@@ -513,46 +544,33 @@ CreateCertErrorRunnable(PRErrorCode defa
   if (CERT_VerifyCertName(cert, infoObject->GetHostNameRaw()) != SECSuccess) {
     collected_errors |= nsICertOverrideService::ERROR_MISMATCH;
     errorCodeMismatch = SSL_ERROR_BAD_CERT_DOMAIN;
   }
 
   CERTVerifyLogNode *i_node;
   for (i_node = verify_log->head; i_node; i_node = i_node->next)
   {
-    switch (i_node->error)
-    {
-      case SEC_ERROR_UNKNOWN_ISSUER:
-      case SEC_ERROR_CA_CERT_INVALID:
-      case SEC_ERROR_UNTRUSTED_ISSUER:
-      case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
-      case SEC_ERROR_UNTRUSTED_CERT:
-      case SEC_ERROR_INADEQUATE_KEY_USAGE:
-      case SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED:
-        // We group all these errors as "cert not trusted"
-        collected_errors |= nsICertOverrideService::ERROR_UNTRUSTED;
-        if (errorCodeTrust == SECSuccess) {
-          errorCodeTrust = i_node->error;
-        }
-        break;
-      case SSL_ERROR_BAD_CERT_DOMAIN:
-        collected_errors |= nsICertOverrideService::ERROR_MISMATCH;
-        if (errorCodeMismatch == SECSuccess) {
-          errorCodeMismatch = i_node->error;
-        }
-        break;
-      case SEC_ERROR_EXPIRED_CERTIFICATE:
-        collected_errors |= nsICertOverrideService::ERROR_TIME;
-        if (errorCodeExpired == SECSuccess) {
-          errorCodeExpired = i_node->error;
-        }
-        break;
-      default:
-        PR_SetError(i_node->error, 0);
-        return nullptr;
+    uint32_t overrideType = PRErrorCodeToOverrideType(i_node->error);
+    // If this isn't an overridable error, set the error and return.
+    if (overrideType == 0) {
+      PR_SetError(i_node->error, 0);
+      return nullptr;
+    }
+    collected_errors |= overrideType;
+    if (overrideType == nsICertOverrideService::ERROR_UNTRUSTED) {
+      errorCodeTrust = (errorCodeTrust == 0 ? i_node->error : errorCodeTrust);
+    } else if (overrideType == nsICertOverrideService::ERROR_MISMATCH) {
+      errorCodeMismatch = (errorCodeMismatch == 0 ? i_node->error
+                                                  : errorCodeMismatch);
+    } else if (overrideType == nsICertOverrideService::ERROR_TIME) {
+      errorCodeExpired = (errorCodeExpired == 0 ? i_node->error
+                                                : errorCodeExpired);
+    } else {
+      MOZ_CRASH("unexpected return value from PRErrorCodeToOverrideType");
     }
   }
 
   if (!collected_errors)
   {
     // This will happen when CERT_*Verify* only returned error(s) that are
     // not on our whitelist of overridable certificate errors.
     PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("[%p] !collected_errors: %d\n",