Bug 683883 - Improve DigitNotarGate handling in PSM; r=rrelyea CLOSED TREE a=release CAMINO_2_1_B2_MINIBRANCH
authorKai Engert <kaie@kuix.de>
Fri, 02 Sep 2011 13:58:32 -0400
branchCAMINO_2_1_B2_MINIBRANCH
changeset 35180 0da0036974e10043039bd6631910b91a113637c7
parent 35179 7657929691712397b3f23b1ccd8273da6e0fe0c3
child 35181 660d9e3556b9b527a3348f6afa703a327ffb784f
push id1971
push useralqahira@ardisson.org
push dateThu, 08 Sep 2011 04:55:20 +0000
reviewersrrelyea, release
bugs683883
milestone1.9.2.22
Bug 683883 - Improve DigitNotarGate handling in PSM; r=rrelyea CLOSED TREE a=release
security/manager/ssl/src/nsNSSCallbacks.cpp
--- a/security/manager/ssl/src/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/src/nsNSSCallbacks.cpp
@@ -997,53 +997,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);
@@ -1100,28 +1136,37 @@ SECStatus PR_CALLBACK AuthCertificateCal
     nsRefPtr<nsSSLStatus> status = infoObject->SSLStatus();
     nsRefPtr<nsNSSCertificate> nsc;
 
     if (!status || !status->mServerCert) {
       nsc = new nsNSSCertificate(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 (SECSuccess == rv) {
       if (nsc) {
         PRBool dummyIsEV;
         nsc->GetIsExtendedValidation(&dummyIsEV); // the nsc object will cache the status
       }