Bug 683883 - Improve DigitNotarGate handling in PSM; r=rrelyea GECKO70_2011083111_RELBRANCH
authorKai Engert <kaie@kuix.de>
Fri, 02 Sep 2011 13:58:32 -0400
branchGECKO70_2011083111_RELBRANCH
changeset 73106 4e3c2f671ef20f9821d758c2bbb8a80fd15e26dd
parent 73105 731b7bc62da34ca9c4ced8394addc0f92f7a61c4
child 73107 66c5548f6b8a20bfc8f41919743c028d4c449c56
push id198
push usereakhgari@mozilla.com
push dateFri, 02 Sep 2011 18:30:53 +0000
treeherdermozilla-beta@d19291e470ab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrrelyea
bugs683883
milestone7.0
Bug 683883 - Improve DigitNotarGate handling in PSM; r=rrelyea
security/manager/ssl/src/nsNSSCallbacks.cpp
--- a/security/manager/ssl/src/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/src/nsNSSCallbacks.cpp
@@ -1031,53 +1031,89 @@ static struct nsSerialBinaryBlacklistEnt
   { 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
 };
 
-// Bug 682927: Do not trust any DigiNotar-issued certificates.
-// We do this check after normal certificate validation because we do not
-// want to override a "revoked" OCSP response.
+// 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", PR_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)
 {
   PRBool isDigiNotarIssuedCert = PR_FALSE;
 
   for (CERTCertListNode *node = CERT_LIST_HEAD(serverCertChain);
        !CERT_LIST_END(node, serverCertChain);
        node = CERT_LIST_NEXT(node)) {
     if (!node->cert->issuerName)
       continue;
 
+    // If it's one of the "Staat der Nederlanden Root"s, then don't blacklist.
+    // Compare names, and ensure it's a self-signed root.
+    if ((!strcmp(node->cert->issuerName,
+                "CN=Staat der Nederlanden Root CA,O=Staat der Nederlanden,C=NL") ||
+         !strcmp(node->cert->issuerName,
+                "CN=Staat der Nederlanden Root CA - G2,O=Staat der Nederlanden,C=NL")) &&
+        SECITEM_ItemsAreEqual(&node->cert->derIssuer,&node->cert->derSubject)
+        ) {
+      // keep as valid
+      return 0;
+    }
+
     if (strstr(node->cert->issuerName, "CN=DigiNotar")) {
       isDigiNotarIssuedCert = PR_TRUE;
-      // Do not let the user override the error if the cert was
-      // chained from the "DigiNotar Root CA" cert and the cert was issued
-      // within the time window in which we think the mis-issuance(s) occurred.
-      if (strstr(node->cert->issuerName, "CN=DigiNotar Root CA")) {
-        PRTime cutoff = 0, notBefore = 0, notAfter = 0;
-        PRStatus status = PR_ParseTimeString("01-JUL-2011 00:00", PR_TRUE, &cutoff);
-        NS_ASSERTION(status == PR_SUCCESS, "PR_ParseTimeString failed");
-        if (status != PR_SUCCESS ||
-           CERT_GetCertTimes(serverCert, &notBefore, &notAfter) != SECSuccess ||
-           notBefore >= cutoff) {
-          return SEC_ERROR_REVOKED_CERTIFICATE;
-        }
-      }
     }
   }
 
-  if (isDigiNotarIssuedCert)
-    return SEC_ERROR_UNTRUSTED_ISSUER; // user can override this
-  else
-    return 0; // No DigiNotor cert => carry on as normal
+  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;
 }
 
 
 SECStatus PR_CALLBACK AuthCertificateCallback(void* client_data, PRFileDesc* fd,
                                               PRBool checksig, PRBool isServer) {
   nsNSSShutDownPreventionLock locker;
 
   CERTCertificate *serverCert = SSL_PeerCertificate(fd);
@@ -1133,28 +1169,37 @@ SECStatus PR_CALLBACK AuthCertificateCal
     nsRefPtr<nsSSLStatus> status = infoObject->SSLStatus();
     nsRefPtr<nsNSSCertificate> nsc;
 
     if (!status || !status->mServerCert) {
       nsc = nsNSSCertificate::Create(serverCert);
     }
 
     CERTCertList *certList = nsnull;
-    if (rv == SECSuccess) {
-      certList = CERT_GetCertChainFromCert(serverCert, PR_Now(), certUsageSSLCA);
-      if (!certList) {
+    certList = CERT_GetCertChainFromCert(serverCert, PR_Now(), certUsageSSLCA);
+    if (!certList) {
+      rv = SECFailure;
+    } else {
+      PRErrorCode blacklistErrorCode;
+      if (rv == SECSuccess) { // PSM_SSL_PKIX_AuthCertificate said "valid cert"
+        blacklistErrorCode = PSM_SSL_BlacklistDigiNotar(serverCert, 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(serverCert, 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;
-      } else {
-        PRErrorCode blacklistErrorCode = PSM_SSL_BlacklistDigiNotar(serverCert,
-                                                                    certList);
-        if (blacklistErrorCode != 0) {
-          infoObject->SetCertIssuerBlacklisted();
-          PORT_SetError(blacklistErrorCode);
-          rv = SECFailure;
-        }
       }
     }
 
     if (rv == SECSuccess) {
       if (nsc) {
         PRBool dummyIsEV;
         nsc->GetIsExtendedValidation(&dummyIsEV); // the nsc object will cache the status
       }