Bug 733454: Remove hard-coded blocklisting in PSM for Comodo and DigiNotar, r=cviecco
authorBrian Smith <bsmith@mozilla.com>
Mon, 01 Jul 2013 11:39:16 -0700
changeset 165289 4c4220bf9e14
parent 165288 25ee011845e7
child 165290 aaad90a5936f
push id26084
push usercbook@mozilla.com
push dateMon, 27 Jan 2014 12:08:26 +0000
treeherdermozilla-central@e6c9677b89d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscviecco
bugs733454
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 733454: Remove hard-coded blocklisting in PSM for Comodo and DigiNotar, r=cviecco
security/manager/ssl/src/SSLServerCertVerification.cpp
security/manager/ssl/src/TransportSecurityInfo.cpp
security/manager/ssl/src/TransportSecurityInfo.h
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -526,21 +526,16 @@ CreateCertErrorRunnable(CertVerifier& ce
   //                     possible failure.
 
   PRErrorCode errorCodeMismatch = 0;
   PRErrorCode errorCodeTrust = 0;
   PRErrorCode errorCodeExpired = 0;
 
   uint32_t collected_errors = 0;
 
-  if (infoObject->IsCertIssuerBlacklisted()) {
-    collected_errors |= nsICertOverrideService::ERROR_UNTRUSTED;
-    errorCodeTrust = defaultErrorCodeToReport;
-  }
-
   // Check the name field against the desired hostname.
   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)
@@ -681,101 +676,16 @@ PSM_SSL_PKIX_AuthCertificate(CertVerifie
             rv = SECFailure;
         if (rv != SECSuccess)
             PORT_SetError(SSL_ERROR_BAD_CERT_DOMAIN);
     }
 
     return rv;
 }
 
-struct nsSerialBinaryBlacklistEntry
-{
-  unsigned int len;
-  const char* binary_serial;
-};
-
-// bug 642395
-static struct nsSerialBinaryBlacklistEntry myUTNBlacklistEntries[] = {
-  { 17, "\x00\x92\x39\xd5\x34\x8f\x40\xd1\x69\x5a\x74\x54\x70\xe1\xf2\x3f\x43" },
-  { 17, "\x00\xd8\xf3\x5f\x4e\xb7\x87\x2b\x2d\xab\x06\x92\xe3\x15\x38\x2f\xb0" },
-  { 16, "\x72\x03\x21\x05\xc5\x0c\x08\x57\x3d\x8e\xa5\x30\x4e\xfe\xe8\xb0" },
-  { 17, "\x00\xb0\xb7\x13\x3e\xd0\x96\xf9\xb5\x6f\xae\x91\xc8\x74\xbd\x3a\xc0" },
-  { 16, "\x39\x2a\x43\x4f\x0e\x07\xdf\x1f\x8a\xa3\x05\xde\x34\xe0\xc2\x29" },
-  { 16, "\x3e\x75\xce\xd4\x6b\x69\x30\x21\x21\x88\x30\xae\x86\xa8\x2a\x71" },
-  { 17, "\x00\xe9\x02\x8b\x95\x78\xe4\x15\xdc\x1a\x71\x0a\x2b\x88\x15\x44\x47" },
-  { 17, "\x00\xd7\x55\x8f\xda\xf5\xf1\x10\x5b\xb2\x13\x28\x2b\x70\x77\x29\xa3" },
-  { 16, "\x04\x7e\xcb\xe9\xfc\xa5\x5f\x7b\xd0\x9e\xae\x36\xe1\x0c\xae\x1e" },
-  { 17, "\x00\xf5\xc8\x6a\xf3\x61\x62\xf1\x3a\x64\xf5\x4f\x6d\xc9\x58\x7c\x06" },
-  { 0, 0 } // end marker
-};
-
-// Call this if we have already decided that a cert should be treated as INVALID,
-// in order to check if we to worsen the error to REVOKED.
-PRErrorCode
-PSM_SSL_DigiNotarTreatAsRevoked(CERTCertificate* serverCert,
-                                CERTCertList* serverCertChain)
-{
-  // If any involved cert was issued by DigiNotar,
-  // and serverCert was issued after 01-JUL-2011,
-  // then worsen the error to revoked.
-
-  PRTime cutoff = 0;
-  PRStatus status = PR_ParseTimeString("01-JUL-2011 00:00", true, &cutoff);
-  if (status != PR_SUCCESS) {
-    NS_ASSERTION(status == PR_SUCCESS, "PR_ParseTimeString failed");
-    // be safe, assume it's afterwards, keep going
-  } else {
-    PRTime notBefore = 0, notAfter = 0;
-    if (CERT_GetCertTimes(serverCert, &notBefore, &notAfter) == SECSuccess &&
-           notBefore < cutoff) {
-      // no worsening for certs issued before the cutoff date
-      return 0;
-    }
-  }
-
-  for (CERTCertListNode* node = CERT_LIST_HEAD(serverCertChain);
-       !CERT_LIST_END(node, serverCertChain);
-       node = CERT_LIST_NEXT(node)) {
-    if (node->cert->issuerName &&
-        strstr(node->cert->issuerName, "CN=DigiNotar")) {
-      return SEC_ERROR_REVOKED_CERTIFICATE;
-    }
-  }
-
-  return 0;
-}
-
-// Call this only if a cert has been reported by NSS as VALID
-PRErrorCode
-PSM_SSL_BlacklistDigiNotar(CERTCertificate* serverCert,
-                           CERTCertList* serverCertChain)
-{
-  bool isDigiNotarIssuedCert = false;
-
-  for (CERTCertListNode* node = CERT_LIST_HEAD(serverCertChain);
-       !CERT_LIST_END(node, serverCertChain);
-       node = CERT_LIST_NEXT(node)) {
-    if (!node->cert->issuerName)
-      continue;
-
-    if (strstr(node->cert->issuerName, "CN=DigiNotar")) {
-      isDigiNotarIssuedCert = true;
-      break;
-    }
-  }
-
-  if (isDigiNotarIssuedCert) {
-    // let's see if we want to worsen the error code to revoked.
-    PRErrorCode revoked_code = PSM_SSL_DigiNotarTreatAsRevoked(serverCert, serverCertChain);
-    return (revoked_code != 0) ? revoked_code : SEC_ERROR_UNTRUSTED_ISSUER;
-  }
-
-  return 0;
-}
-
 // This function assumes that we will only use the SPDY connection coalescing
 // feature on connections where we have negotiated SPDY using NPN. If we ever
 // talk SPDY without having negotiated it with SPDY, this code will give wrong
 // and perhaps unsafe results.
 //
 // Returns SECSuccess on the initial handshake of all connections, on
 // renegotiations for any connections where we did not negotiate SPDY, or on any
 // SPDY connection where the server's certificate did not change.
@@ -839,54 +749,16 @@ BlockServerCertChangeForSpdy(nsNSSSocket
   return SECFailure;
 }
 
 SECStatus
 AuthCertificate(CertVerifier& certVerifier, TransportSecurityInfo* infoObject,
                 CERTCertificate* cert, SECItem* stapledOCSPResponse,
                 uint32_t providerFlags)
 {
-  if (cert->serialNumber.data &&
-      cert->issuerName &&
-      !strcmp(cert->issuerName,
-        "CN=UTN-USERFirst-Hardware,OU=http://www.usertrust.com,O=The USERTRUST Network,L=Salt Lake City,ST=UT,C=US")) {
-
-    unsigned char* server_cert_comparison_start = cert->serialNumber.data;
-    unsigned int server_cert_comparison_len = cert->serialNumber.len;
-
-    while (server_cert_comparison_len) {
-      if (*server_cert_comparison_start != 0)
-        break;
-
-      ++server_cert_comparison_start;
-      --server_cert_comparison_len;
-    }
-
-    nsSerialBinaryBlacklistEntry* walk = myUTNBlacklistEntries;
-    for ( ; walk && walk->len; ++walk) {
-
-      unsigned char* locked_cert_comparison_start = (unsigned char*)walk->binary_serial;
-      unsigned int locked_cert_comparison_len = walk->len;
-
-      while (locked_cert_comparison_len) {
-        if (*locked_cert_comparison_start != 0)
-          break;
-
-        ++locked_cert_comparison_start;
-        --locked_cert_comparison_len;
-      }
-
-      if (server_cert_comparison_len == locked_cert_comparison_len &&
-          !memcmp(server_cert_comparison_start, locked_cert_comparison_start, locked_cert_comparison_len)) {
-        PR_SetError(SEC_ERROR_REVOKED_CERTIFICATE, 0);
-        return SECFailure;
-      }
-    }
-  }
-
   SECStatus rv;
   if (stapledOCSPResponse) {
     CERTCertDBHandle* handle = CERT_GetDefaultCertDB();
     rv = CERT_CacheOCSPResponseFromSideChannel(handle, cert, PR_Now(),
                                                stapledOCSPResponse,
                                                infoObject);
     if (rv != SECSuccess) {
       // Due to buggy servers that will staple expired OCSP responses
@@ -949,40 +821,17 @@ AuthCertificate(CertVerifier& certVerifi
     }
     else {
       nsc = nsNSSCertificate::Create(cert);
     }
   }
 
   ScopedCERTCertList certList(verifyCertChain);
 
-  if (!certList) {
-    rv = SECFailure;
-  } else {
-    PRErrorCode blacklistErrorCode;
-    if (rv == SECSuccess) { // PSM_SSL_PKIX_AuthCertificate said "valid cert"
-      blacklistErrorCode = PSM_SSL_BlacklistDigiNotar(cert, certList);
-    } else { // PSM_SSL_PKIX_AuthCertificate said "invalid cert"
-      PRErrorCode savedErrorCode = PORT_GetError();
-      // Check if we want to worsen the error code to "revoked".
-      blacklistErrorCode = PSM_SSL_DigiNotarTreatAsRevoked(cert, certList);
-      if (blacklistErrorCode == 0) {
-        // we don't worsen the code, let's keep the original error code from NSS
-        PORT_SetError(savedErrorCode);
-      }
-    }
-
-    if (blacklistErrorCode != 0) {
-      infoObject->SetCertIssuerBlacklisted();
-      PORT_SetError(blacklistErrorCode);
-      rv = SECFailure;
-    }
-  }
-
-  if (rv == SECSuccess) {
+  if (rv == SECSuccess && certList) {
     // We want to avoid storing any intermediate cert information when browsing
     // in private, transient contexts.
     if (!(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE)) {
       for (CERTCertListNode* node = CERT_LIST_HEAD(certList);
            !CERT_LIST_END(node, certList);
            node = CERT_LIST_NEXT(node)) {
 
         if (node->cert->slot) {
--- a/security/manager/ssl/src/TransportSecurityInfo.cpp
+++ b/security/manager/ssl/src/TransportSecurityInfo.cpp
@@ -38,18 +38,17 @@ namespace mozilla { namespace psm {
 
 TransportSecurityInfo::TransportSecurityInfo()
   : mMutex("TransportSecurityInfo::mMutex"),
     mSecurityState(nsIWebProgressListener::STATE_IS_INSECURE),
     mSubRequestsBrokenSecurity(0),
     mSubRequestsNoSecurity(0),
     mErrorCode(0),
     mErrorMessageType(PlainErrorMessage),
-    mPort(0),
-    mIsCertIssuerBlacklisted(false)
+    mPort(0)
 {
 }
 
 TransportSecurityInfo::~TransportSecurityInfo()
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return;
--- a/security/manager/ssl/src/TransportSecurityInfo.h
+++ b/security/manager/ssl/src/TransportSecurityInfo.h
@@ -68,23 +68,16 @@ public:
   void SetCanceled(PRErrorCode errorCode,
                    ::mozilla::psm::SSLErrorMessageType errorMessageType);
   
   /* Set SSL Status values */
   nsresult SetSSLStatus(nsSSLStatus *aSSLStatus);
   nsSSLStatus* SSLStatus() { return mSSLStatus; }
   void SetStatusErrorBits(nsIX509Cert & cert, uint32_t collected_errors);
 
-  bool IsCertIssuerBlacklisted() const {
-    return mIsCertIssuerBlacklisted;
-  }
-  void SetCertIssuerBlacklisted() {
-    mIsCertIssuerBlacklisted = true;
-  }
-
 private:
   mutable ::mozilla::Mutex mMutex;
 
 protected:
   nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
 
 private:
   uint32_t mSecurityState;
@@ -97,17 +90,16 @@ private:
   nsresult formatErrorMessage(::mozilla::MutexAutoLock const & proofOfLock, 
                               PRErrorCode errorCode,
                               ::mozilla::psm::SSLErrorMessageType errorMessageType,
                               bool wantsHtml, bool suppressPort443, 
                               nsString &result);
 
   int32_t mPort;
   nsXPIDLCString mHostName;
-  PRErrorCode mIsCertIssuerBlacklisted;
 
   /* SSL Status */
   mozilla::RefPtr<nsSSLStatus> mSSLStatus;
 
   virtual void virtualDestroyNSSReference();
   void destructorSafeDestroyNSSReference();
 };