bug 943115 - return early in CreateCertErrorRunnable for non-overridable errors r=briansmith a=bajaj
authorDavid Keeler <dkeeler@mozilla.com>
Tue, 26 Nov 2013 13:49:47 -0800
changeset 167830 1786928ad7b2e6df873721cdc6d8812cfb44ae5a
parent 167829 f3e711fceae8a61dd8921c236ac481913acb332a
child 167831 171046c2495fe4d72b293af29079b22b0b9e38fc
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith, bajaj
bugs943115
milestone27.0
bug 943115 - return early in CreateCertErrorRunnable for non-overridable errors r=briansmith a=bajaj
security/manager/ssl/src/SSLServerCertVerification.cpp
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -436,32 +436,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;
   }
@@ -521,46 +552,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",