Bug 769288 - Part 2: Avoid storing intermediate cert data for private contexts. r=bsmith sr=mayhemer
☠☠ backed out by 553a3bcf1fe7 ☠ ☠
authorJosh Matthews <josh@joshmatthews.net>
Thu, 06 Dec 2012 22:05:34 -0500
changeset 115621 7b0fdedb4fa900e366f0b017dfba3864236ce8db
parent 115620 42b468942a84a4d5b3f941e520dd1f87fa7eab2e
child 115622 e10c24f10bd21d8887cfc96f521107ffb02ac783
push idunknown
push userunknown
push dateunknown
reviewersbsmith, mayhemer
bugs769288
milestone20.0a1
Bug 769288 - Part 2: Avoid storing intermediate cert data for private contexts. r=bsmith sr=mayhemer
security/manager/ssl/src/SSLServerCertVerification.cpp
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -830,17 +830,18 @@ BlockServerCertChangeForSpdy(nsNSSSocket
   // Report an error - changed cert is confirmed
   PR_LOG(gPIPNSSLog, PR_LOG_DEBUG,
          ("SPDY Refused to allow new cert during renegotiation\n"));
   PR_SetError(SSL_ERROR_RENEGOTIATION_NOT_ALLOWED, 0);
   return SECFailure;
 }
 
 SECStatus
-AuthCertificate(TransportSecurityInfo * infoObject, CERTCertificate * cert)
+AuthCertificate(TransportSecurityInfo * infoObject, CERTCertificate * cert,
+                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;
@@ -916,47 +917,51 @@ AuthCertificate(TransportSecurityInfo * 
 
   if (rv == SECSuccess) {
     if (nsc) {
       bool dummyIsEV;
       nsc->GetIsExtendedValidation(&dummyIsEV); // the nsc object will cache the status
     }
     
     nsCOMPtr<nsINSSComponent> nssComponent;
-      
-    for (CERTCertListNode *node = CERT_LIST_HEAD(certList);
-         !CERT_LIST_END(node, certList);
-         node = CERT_LIST_NEXT(node)) {
+
+    // 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) {
+          // This cert was found on a token, no need to remember it in the temp db.
+          continue;
+        }
 
-      if (node->cert->slot) {
-        // This cert was found on a token, no need to remember it in the temp db.
-        continue;
-      }
+        if (node->cert->isperm) {
+          // We don't need to remember certs already stored in perm db.
+          continue;
+        }
+
+        if (node->cert == cert) {
+          // We don't want to remember the server cert, 
+          // the code that cares for displaying page info does this already.
+          continue;
+        }
 
-      if (node->cert->isperm) {
-        // We don't need to remember certs already stored in perm db.
-        continue;
+        // We have found a signer cert that we want to remember.
+        char* nickname = nsNSSCertificate::defaultServerNickname(node->cert);
+        if (nickname && *nickname) {
+          ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+          if (slot) {
+            PK11_ImportCert(slot, node->cert, CK_INVALID_HANDLE, 
+                            nickname, false);
+          }
+        }
+        PR_FREEIF(nickname);
       }
-        
-      if (node->cert == cert) {
-        // We don't want to remember the server cert, 
-        // the code that cares for displaying page info does this already.
-        continue;
-      }
-
-      // We have found a signer cert that we want to remember.
-      char* nickname = nsNSSCertificate::defaultServerNickname(node->cert);
-      if (nickname && *nickname) {
-        ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
-        if (slot) {
-          PK11_ImportCert(slot, node->cert, CK_INVALID_HANDLE, 
-                          nickname, false);
-        }
-      }
-      PR_FREEIF(nickname);
     }
 
     // The connection may get terminated, for example, if the server requires
     // a client cert. Let's provide a minimal SSLStatus
     // to the caller that contains at least the cert and its status.
     if (!status) {
       status = new nsSSLStatus();
       infoObject->SetSSLStatus(status);
@@ -1038,17 +1043,17 @@ SSLServerCertVerificationJob::Run()
 
   nsNSSShutDownPreventionLock nssShutdownPrevention;
   if (mInfoObject->isAlreadyShutDown()) {
     error = SEC_ERROR_USER_CANCELLED;
   } else {
     // Reset the error code here so we can detect if AuthCertificate fails to
     // set the error code if/when it fails.
     PR_SetError(0, 0); 
-    SECStatus rv = AuthCertificate(mInfoObject, mCert);
+    SECStatus rv = AuthCertificate(mInfoObject, mCert, mProviderFlags);
     if (rv == SECSuccess) {
       RefPtr<SSLServerCertVerificationResult> restart(
         new SSLServerCertVerificationResult(mInfoObject, 0));
       restart->Dispatch();
       return NS_OK;
     }
 
     error = PR_GetError();
@@ -1163,17 +1168,17 @@ AuthCertificateHook(void *arg, PRFileDes
                            providerFlags);
     return rv;
   }
   
   // We can't do certificate verification on a background thread, because the
   // thread doing the network I/O may not interrupt its network I/O on receipt
   // of our SSLServerCertVerificationResult event, and/or it might not even be
   // a non-blocking socket.
-  SECStatus rv = AuthCertificate(socketInfo, serverCert);
+  SECStatus rv = AuthCertificate(socketInfo, serverCert, providerFlags);
   if (rv == SECSuccess) {
     return SECSuccess;
   }
 
   PRErrorCode error = PR_GetError();
   if (error != 0) {
     RefPtr<CertErrorRunnable> runnable(CreateCertErrorRunnable(
                     error, socketInfo, serverCert,