Bug 712363, Part 5: Add back synchronous cert validation for Thunderbird, r=honzab,a=Standard8 for checkin to comm specific relbranch COMM110_2012022905_RELBRANCH
authorBrian Smith <bsmith@mozilla.com>
Tue, 31 Jan 2012 08:05:11 -0800
branchCOMM110_2012022905_RELBRANCH
changeset 85512 6aeea2de9c0df7222f5b0b8db65e3d3bc2333676
parent 85511 eda4af2d01dfec21dbadd08e5b99bcc4b474b26f
child 85513 432b5385d14cff5890fee76503a65c7217702541
push id1
push usersledru@mozilla.com
push dateThu, 04 Dec 2014 17:57:20 +0000
reviewershonzab, Standard8
bugs712363
milestone11.0
Bug 712363, Part 5: Add back synchronous cert validation for Thunderbird, r=honzab,a=Standard8 for checkin to comm specific relbranch
security/manager/ssl/src/SSLServerCertVerification.cpp
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -156,16 +156,17 @@ extern PRLogModuleInfo* gPIPNSSLog;
 #endif
 
 namespace mozilla { namespace psm {
 
 namespace {
 
 NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
 
+NSSCleanupAutoPtrClass(CERTCertificate, CERT_DestroyCertificate)
 NSSCleanupAutoPtrClass_WithParam(PRArenaPool, PORT_FreeArena, FalseParam, false)
 
 // do not use a nsCOMPtr to avoid static initializer/destructor
 nsIThreadPool * gCertVerificationThreadPool = nsnull;
 } // unnamed namespace
 
 // Called when the socket transport thread starts, to initialize the SSL cert
 // verification thread pool. By tying the thread pool startup/shutdown directly
@@ -229,16 +230,17 @@ public:
   SSLServerCertVerificationResult(nsNSSSocketInfo & socketInfo,
                                   PRErrorCode errorCode,
                                   SSLErrorMessageType errorMessageType = 
                                       PlainErrorMessage);
 
   void Dispatch();
 private:
   const nsRefPtr<nsNSSSocketInfo> mSocketInfo;
+public:
   const PRErrorCode mErrorCode;
   const SSLErrorMessageType mErrorMessageType;
 };
 
 class CertErrorRunnable : public SyncRunnableBase
 {
  public:
   CertErrorRunnable(const void * fdForLogging,
@@ -254,17 +256,17 @@ class CertErrorRunnable : public SyncRun
       mCollectedErrors(collectedErrors),
       mErrorCodeTrust(errorCodeTrust),
       mErrorCodeMismatch(errorCodeMismatch),
       mErrorCodeExpired(errorCodeExpired)
   {
   }
 
   virtual void RunOnTargetThread();
-  nsCOMPtr<nsIRunnable> mResult; // out
+  nsRefPtr<SSLServerCertVerificationResult> mResult; // out
 private:
   SSLServerCertVerificationResult* CheckCertOverrides();
   
   const void * const mFdForLogging; // may become an invalid pointer; do not dereference
   const nsCOMPtr<nsIX509Cert> mCert;
   const nsRefPtr<nsNSSSocketInfo> mInfoObject;
   const PRErrorCode mDefaultErrorCodeToReport;
   const PRUint32 mCollectedErrors;
@@ -598,19 +600,16 @@ private:
   NS_DECL_NSIRUNNABLE
 
   // Must be called only on the socket transport thread
   SSLServerCertVerificationJob(const void * fdForLogging,
                                nsNSSSocketInfo & socketInfo, 
                                CERTCertificate & cert);
   ~SSLServerCertVerificationJob();
 
-  // Runs on one of the background threads
-  SECStatus AuthCertificate(const nsNSSShutDownPreventionLock & proofOfLock);
-
   const void * const mFdForLogging;
   const nsRefPtr<nsNSSSocketInfo> mSocketInfo;
   CERTCertificate * const mCert;
 };
 
 SSLServerCertVerificationJob::SSLServerCertVerificationJob(
     const void * fdForLogging, nsNSSSocketInfo & socketInfo,
     CERTCertificate & cert)
@@ -622,18 +621,17 @@ SSLServerCertVerificationJob::SSLServerC
 
 SSLServerCertVerificationJob::~SSLServerCertVerificationJob()
 {
   CERT_DestroyCertificate(mCert);
 }
 
 SECStatus
 PSM_SSL_PKIX_AuthCertificate(CERTCertificate *peerCert, void * pinarg,
-                             const char * hostname,
-                             const nsNSSShutDownPreventionLock & /*proofOfLock*/)
+                             const char * hostname)
 {
     SECStatus          rv;
     
     if (!nsNSSComponent::globalConstFlagUsePKIXVerification) {
         rv = CERT_VerifyCertNow(CERT_GetDefaultCertDB(), peerCert, true,
                                 certUsageSSLServer, pinarg);
     }
     else {
@@ -817,29 +815,28 @@ 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
-SSLServerCertVerificationJob::AuthCertificate(
-  nsNSSShutDownPreventionLock const & nssShutdownPreventionLock)
+AuthCertificate(nsNSSSocketInfo * socketInfo, CERTCertificate * cert)
 {
-  if (BlockServerCertChangeForSpdy(mSocketInfo, mCert) != SECSuccess)
+  if (BlockServerCertChangeForSpdy(socketInfo, cert) != SECSuccess)
     return SECFailure;
 
-  if (mCert->serialNumber.data &&
-      mCert->issuerName &&
-      !strcmp(mCert->issuerName, 
+  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 = mCert->serialNumber.data;
-    unsigned int server_cert_comparison_len = mCert->serialNumber.len;
+    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;
     }
@@ -861,51 +858,50 @@ SSLServerCertVerificationJob::AuthCertif
       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 = PSM_SSL_PKIX_AuthCertificate(mCert, mSocketInfo,
-                                              mSocketInfo->GetHostName(),
-                                              nssShutdownPreventionLock);
+  SECStatus rv = PSM_SSL_PKIX_AuthCertificate(cert, socketInfo,
+                                              socketInfo->GetHostName());
 
   // We want to remember the CA certs in the temp db, so that the application can find the
   // complete chain at any time it might need it.
   // But we keep only those CA certs in the temp db, that we didn't already know.
 
-  nsRefPtr<nsSSLStatus> status = mSocketInfo->SSLStatus();
+  nsRefPtr<nsSSLStatus> status = socketInfo->SSLStatus();
   nsRefPtr<nsNSSCertificate> nsc;
 
   if (!status || !status->mServerCert) {
-    nsc = nsNSSCertificate::Create(mCert);
+    nsc = nsNSSCertificate::Create(cert);
   }
 
   CERTCertList *certList = nsnull;
-  certList = CERT_GetCertChainFromCert(mCert, PR_Now(), certUsageSSLCA);
+  certList = CERT_GetCertChainFromCert(cert, PR_Now(), certUsageSSLCA);
   if (!certList) {
     rv = SECFailure;
   } else {
     PRErrorCode blacklistErrorCode;
     if (rv == SECSuccess) { // PSM_SSL_PKIX_AuthCertificate said "valid cert"
-      blacklistErrorCode = PSM_SSL_BlacklistDigiNotar(mCert, certList);
+      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(mCert, certList);
+      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) {
-      mSocketInfo->SetCertIssuerBlacklisted();
+      socketInfo->SetCertIssuerBlacklisted();
       PORT_SetError(blacklistErrorCode);
       rv = SECFailure;
     }
   }
 
   if (rv == SECSuccess) {
     if (nsc) {
       bool dummyIsEV;
@@ -923,17 +919,17 @@ SSLServerCertVerificationJob::AuthCertif
         continue;
       }
 
       if (node->cert->isperm) {
         // We don't need to remember certs already stored in perm db.
         continue;
       }
         
-      if (node->cert == mCert) {
+      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) {
@@ -951,29 +947,29 @@ SSLServerCertVerificationJob::AuthCertif
       CERT_DestroyCertList(certList);
     }
 
     // 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();
-      mSocketInfo->SetSSLStatus(status);
+      socketInfo->SetSSLStatus(status);
     }
 
     if (rv == SECSuccess) {
       // Certificate verification succeeded delete any potential record
       // of certificate error bits.
       nsSSLIOLayerHelpers::mHostsWithCertErrors->RememberCertHasError(
-        mSocketInfo, nsnull, rv);
+        socketInfo, nsnull, rv);
     }
     else {
       // Certificate verification failed, update the status' bits.
       nsSSLIOLayerHelpers::mHostsWithCertErrors->LookupCertErrorBits(
-        mSocketInfo, status);
+        socketInfo, status);
     }
 
     if (status && !status->mServerCert) {
       status->mServerCert = nsc;
       PR_LOG(gPIPNSSLog, PR_LOG_DEBUG,
              ("AuthCertificate setting NEW cert %p\n", status->mServerCert.get()));
     }
   }
@@ -1034,17 +1030,17 @@ SSLServerCertVerificationJob::Run()
 
   nsNSSShutDownPreventionLock nssShutdownPrevention;
   if (mSocketInfo->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(nssShutdownPrevention);
+    SECStatus rv = AuthCertificate(mSocketInfo, mCert);
     if (rv == SECSuccess) {
       nsRefPtr<SSLServerCertVerificationResult> restart 
         = new SSLServerCertVerificationResult(*mSocketInfo, 0);
       restart->Dispatch();
       return NS_OK;
     }
 
     error = PR_GetError();
@@ -1114,24 +1110,101 @@ AuthCertificateHook(void *arg, PRFileDes
   NS_ASSERTION(!isServer, "AuthCertificateHook: isServer unexpectedly true");
 
   if (!checkSig || isServer) {
       PR_SetError(PR_INVALID_STATE_ERROR, 0);
       return SECFailure;
   }
       
   CERTCertificate *serverCert = SSL_PeerCertificate(fd);
+  CERTCertificateCleaner serverCertCleaner(serverCert);
 
   nsNSSSocketInfo *socketInfo = static_cast<nsNSSSocketInfo*>(arg);
-  SECStatus rv = SSLServerCertVerificationJob::Dispatch(
+
+  bool onSTSThread;
+  nsresult nrv;
+  nsCOMPtr<nsIEventTarget> sts
+    = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &nrv);
+  if (NS_SUCCEEDED(nrv)) {
+    nrv = sts->IsOnCurrentThread(&onSTSThread);
+  }
+
+  if (NS_FAILED(nrv)) {
+    NS_ERROR("Could not get STS service or IsOnCurrentThread failed");
+    PR_SetError(PR_UNKNOWN_ERROR, 0);
+    return SECFailure;
+  }
+  
+  if (onSTSThread) {
+    // We *must* do certificate verification on a background thread because
+    // we need the socket transport thread to be free for our OCSP requests,
+    // and we *want* to do certificate verification on a background thread
+    // because of the performance benefits of doing so.
+    SECStatus rv = SSLServerCertVerificationJob::Dispatch(
                         static_cast<const void *>(fd), socketInfo, serverCert);
+    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);
+  if (rv == SECSuccess) {
+    return SECSuccess;
+  }
 
-  CERT_DestroyCertificate(serverCert);
+  PRErrorCode error = PR_GetError();
+  if (error != 0) {
+    nsRefPtr<CertErrorRunnable> runnable = CreateCertErrorRunnable(
+                    error, socketInfo, serverCert,
+                    static_cast<const void *>(fd));
+    if (!runnable) {
+      // CreateCertErrorRunnable sets a new error code when it fails
+      error = PR_GetError();
+    } else {
+      // We have to return SECSuccess or SECFailure based on the result of the
+      // override processing, so we must block this thread waiting for it. The
+      // CertErrorRunnable will NOT dispatch the result at all, since we passed
+      // false for CreateCertErrorRunnable's async parameter
+      nrv = runnable->DispatchToMainThreadAndWait();
+      if (NS_FAILED(nrv)) {
+        NS_ERROR("Failed to dispatch CertErrorRunnable");
+        PR_SetError(PR_INVALID_STATE_ERROR, 0);
+        return SECFailure;
+      }
 
-  return rv;
+      if (!runnable->mResult) {
+        NS_ERROR("CertErrorRunnable did not set result");
+        PR_SetError(PR_INVALID_STATE_ERROR, 0);
+        return SECFailure;
+      }
+
+      if (runnable->mResult->mErrorCode == 0) {
+        return SECSuccess; // cert error override occurred.
+      }
+
+      // We must call SetCanceled here to set the error message type
+      // in case it isn't PlainErrorMessage, which is what we would
+      // default to if we just called
+      // PR_SetError(runnable->mResult->mErrorCode, 0) and returned
+      // SECFailure without doing this.
+      socketInfo->SetCanceled(runnable->mResult->mErrorCode,
+                              runnable->mResult->mErrorMessageType);
+      error = runnable->mResult->mErrorCode;
+    }
+  }
+
+  if (error == 0) {
+    NS_ERROR("error code not set");
+    error = PR_UNKNOWN_ERROR;
+  }
+
+  PR_SetError(error, 0);
+  return SECFailure;
 }
 
 SSLServerCertVerificationResult::SSLServerCertVerificationResult(
         nsNSSSocketInfo & socketInfo, PRErrorCode errorCode,
         SSLErrorMessageType errorMessageType)
   : mSocketInfo(&socketInfo)
   , mErrorCode(errorCode)
   , mErrorMessageType(errorMessageType)