Bug 914034 - Cache stapled OCSP responses on the cert verification thread. r=briansmith, a=akeybl
authorDavid Keeler <dkeeler@mozilla.com>
Wed, 02 Oct 2013 15:08:07 -0700
changeset 160625 e87c5779660a96ae2e0d68be665665b6e65c43b2
parent 160624 1444db55ea3c74c11f8667a24b9b210facac0751
child 160626 db3d9b76252d41c84a7c3a72c25ce8113dd674e2
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbriansmith, akeybl
bugs914034
milestone26.0a2
Bug 914034 - Cache stapled OCSP responses on the cert verification thread. r=briansmith, a=akeybl
security/manager/ssl/src/SSLServerCertVerification.cpp
security/manager/ssl/src/nsNSSCallbacks.cpp
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -608,40 +608,45 @@ private:
 
 class SSLServerCertVerificationJob : public nsRunnable
 {
 public:
   // Must be called only on the socket transport thread
   static SECStatus Dispatch(const void * fdForLogging,
                             TransportSecurityInfo * infoObject,
                             CERTCertificate * serverCert,
+                            SECItem * stapledOCSPResponse,
                             uint32_t providerFlags);
 private:
   NS_DECL_NSIRUNNABLE
 
   // Must be called only on the socket transport thread
   SSLServerCertVerificationJob(const void * fdForLogging,
                                TransportSecurityInfo * infoObject, 
                                CERTCertificate * cert,
+                               SECItem * stapledOCSPResponse,
                                uint32_t providerFlags);
   const void * const mFdForLogging;
   const RefPtr<TransportSecurityInfo> mInfoObject;
   const ScopedCERTCertificate mCert;
   const uint32_t mProviderFlags;
   const TimeStamp mJobStartTime;
+  const ScopedSECItem mStapledOCSPResponse;
 };
 
 SSLServerCertVerificationJob::SSLServerCertVerificationJob(
     const void * fdForLogging, TransportSecurityInfo * infoObject,
-    CERTCertificate * cert, uint32_t providerFlags)
+    CERTCertificate * cert, SECItem * stapledOCSPResponse,
+    uint32_t providerFlags)
   : mFdForLogging(fdForLogging)
   , mInfoObject(infoObject)
   , mCert(CERT_DupCertificate(cert))
   , mProviderFlags(providerFlags)
   , mJobStartTime(TimeStamp::Now())
+  , mStapledOCSPResponse(SECITEM_DupItem(stapledOCSPResponse))
 {
 }
 
 SECStatus
 PSM_SSL_PKIX_AuthCertificate(CERTCertificate *peerCert,
                              nsIInterfaceRequestor * pinarg,
                              const char * hostname,
                              CERTCertList **validationChain,
@@ -824,17 +829,17 @@ BlockServerCertChangeForSpdy(nsNSSSocket
   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,
-                uint32_t providerFlags)
+                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;
@@ -864,22 +869,31 @@ AuthCertificate(TransportSecurityInfo * 
       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) {
+      return rv;
+    }
+  }
+
   CERTCertList *verifyCertChain = nullptr;
   SECOidTag evOidPolicy;
-  SECStatus rv = PSM_SSL_PKIX_AuthCertificate(cert, infoObject,
-                                              infoObject->GetHostName(),
-                                              &verifyCertChain,
-                                              &evOidPolicy);
+  rv = PSM_SSL_PKIX_AuthCertificate(cert, infoObject, infoObject->GetHostName(),
+                                    &verifyCertChain, &evOidPolicy);
 
   // 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.
 
   RefPtr<nsSSLStatus> status(infoObject->SSLStatus());
   RefPtr<nsNSSCertificate> nsc;
 
@@ -983,28 +997,29 @@ AuthCertificate(TransportSecurityInfo * 
 
   return rv;
 }
 
 /*static*/ SECStatus
 SSLServerCertVerificationJob::Dispatch(const void * fdForLogging,
                                        TransportSecurityInfo * infoObject,
                                        CERTCertificate * serverCert,
+                                       SECItem * stapledOCSPResponse,
                                        uint32_t providerFlags)
 {
   // Runs on the socket transport thread
   if (!infoObject || !serverCert) {
     NS_ERROR("Invalid parameters for SSL server cert validation");
     PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
     return SECFailure;
   }
   
   RefPtr<SSLServerCertVerificationJob> job(
     new SSLServerCertVerificationJob(fdForLogging, infoObject, serverCert,
-                                     providerFlags));
+                                     stapledOCSPResponse, providerFlags));
 
   nsresult nrv;
   if (!gCertVerificationThreadPool) {
     nrv = NS_ERROR_NOT_INITIALIZED;
   } else {
     nrv = gCertVerificationThreadPool->Dispatch(job, NS_DISPATCH_NORMAL);
   }
   if (NS_FAILED(nrv)) {
@@ -1037,18 +1052,19 @@ SSLServerCertVerificationJob::Run()
   PRErrorCode error;
 
   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, mProviderFlags);
+    PR_SetError(0, 0);
+    SECStatus rv = AuthCertificate(mInfoObject, mCert, mStapledOCSPResponse,
+                                   mProviderFlags);
     if (rv == SECSuccess) {
       uint32_t interval = (uint32_t) ((TimeStamp::Now() - mJobStartTime).ToMilliseconds());
       Telemetry::ID telemetryID;
 #ifndef NSS_NO_LIBPKIX
       if(nsNSSComponent::globalConstFlagUsePKIXVerification){
         telemetryID = Telemetry::SSL_SUCCESFUL_CERT_VALIDATION_TIME_LIBPKIX;
       }
       else{
@@ -1159,34 +1175,16 @@ AuthCertificateHook(void *arg, PRFileDes
 
   ScopedCERTCertificate serverCert(SSL_PeerCertificate(fd));
 
   if (!checkSig || isServer || !socketInfo || !serverCert) {
       PR_SetError(PR_INVALID_STATE_ERROR, 0);
       return SECFailure;
   }
 
-  // This value of "now" is used both here for OCSP stapling and later
-  // when calling CreateCertErrorRunnable.
-  PRTime now = PR_Now();
-  // SSL_PeerStapledOCSPResponses will never return a non-empty response if
-  // OCSP stapling wasn't enabled because libssl wouldn't have let the server
-  // return a stapled OCSP response.
-  // We don't own this pointer.
-  const SECItemArray *csa = SSL_PeerStapledOCSPResponses(fd);
-  // we currently only support single stapled responses
-  if (csa && csa->len == 1) {
-      CERTCertDBHandle *handle = CERT_GetDefaultCertDB();
-      SECStatus cacheResult = CERT_CacheOCSPResponseFromSideChannel(
-          handle, serverCert, now, &csa->items[0], arg);
-      if (cacheResult != SECSuccess) {
-          return SECFailure;
-      }
-  }
-
   if (BlockServerCertChangeForSpdy(socketInfo, serverCert) != SECSuccess)
     return SECFailure;
 
   bool onSTSThread;
   nsresult nrv;
   nsCOMPtr<nsIEventTarget> sts
     = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &nrv);
   if (NS_SUCCEEDED(nrv)) {
@@ -1194,46 +1192,59 @@ AuthCertificateHook(void *arg, PRFileDes
   }
 
   if (NS_FAILED(nrv)) {
     NS_ERROR("Could not get STS service or IsOnCurrentThread failed");
     PR_SetError(PR_UNKNOWN_ERROR, 0);
     return SECFailure;
   }
 
+  // SSL_PeerStapledOCSPResponses will never return a non-empty response if
+  // OCSP stapling wasn't enabled because libssl wouldn't have let the server
+  // return a stapled OCSP response.
+  // We don't own these pointers.
+  const SECItemArray *csa = SSL_PeerStapledOCSPResponses(fd);
+  SECItem *stapledOCSPResponse = nullptr;
+  // we currently only support single stapled responses
+  if (csa && csa->len == 1) {
+    stapledOCSPResponse = &csa->items[0];
+  }
+
   uint32_t providerFlags = 0;
   socketInfo->GetProviderFlags(&providerFlags);
 
   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.
     socketInfo->SetCertVerificationWaiting();
     SECStatus rv = SSLServerCertVerificationJob::Dispatch(
-                           static_cast<const void *>(fd), socketInfo, serverCert,
-                           providerFlags);
+                     static_cast<const void *>(fd), socketInfo, serverCert,
+                     stapledOCSPResponse, 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, providerFlags);
+
+  SECStatus rv = AuthCertificate(socketInfo, serverCert, stapledOCSPResponse,
+                                 providerFlags);
   if (rv == SECSuccess) {
     return SECSuccess;
   }
 
   PRErrorCode error = PR_GetError();
   if (error != 0) {
     RefPtr<CertErrorRunnable> runnable(CreateCertErrorRunnable(
                     error, socketInfo, serverCert,
-                    static_cast<const void *>(fd), providerFlags, now));
+                    static_cast<const void *>(fd), providerFlags, PR_Now()));
     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
--- a/security/manager/ssl/src/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/src/nsNSSCallbacks.cpp
@@ -244,16 +244,40 @@ SECStatus nsNSSHttpRequestSession::trySe
                                                         const char **http_response_content_type, 
                                                         const char **http_response_headers, 
                                                         const char **http_response_data, 
                                                         uint32_t *http_response_data_len)
 {
   PR_LOG(gPIPNSSLog, PR_LOG_DEBUG,
          ("nsNSSHttpRequestSession::trySendAndReceiveFcn to %s\n", mURL.get()));
 
+  bool onSTSThread;
+  nsresult nrv;
+  nsCOMPtr<nsIEventTarget> sts
+    = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &nrv);
+  if (NS_FAILED(nrv)) {
+    NS_ERROR("Could not get STS service");
+    PR_SetError(PR_INVALID_STATE_ERROR, 0);
+    return SECFailure;
+  }
+
+  nrv = sts->IsOnCurrentThread(&onSTSThread);
+  if (NS_FAILED(nrv)) {
+    NS_ERROR("IsOnCurrentThread failed");
+    PR_SetError(PR_INVALID_STATE_ERROR, 0);
+    return SECFailure;
+  }
+
+  if (onSTSThread) {
+    NS_ERROR("nsNSSHttpRequestSession::trySendAndReceiveFcn called on socket "
+             "thread; this will not work.");
+    PR_SetError(PR_INVALID_STATE_ERROR, 0);
+    return SECFailure;
+  }
+
   const int max_retries = 2;
   int retry_count = 0;
   bool retryable_error = false;
   SECStatus result_sec_status = SECFailure;
 
   do
   {
     if (retry_count > 0)