Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Bug 712363, Part 5: Add back synchronous cert validation for Thunderb…

…ird, r=honzab,a=Standard8 for checkin to comm specific relbranch

--HG--
branch : COMM110_2012020102_RELBRANCH
extra : transplant_source : %D9%EA%B2%2C%E3zX%9E-%3D%EDj%91%005%0CO%16%F8H
  • Loading branch information...
commit 597249254f8ad3aa31692626da0557ba450d9e4f 1 parent 16d8bf8
Brian Smith authored January 31, 2012
135  security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -161,6 +161,7 @@ namespace {
161 161
 
162 162
 NS_DEFINE_CID(kNSSComponentCID, NS_NSSCOMPONENT_CID);
163 163
 
  164
+NSSCleanupAutoPtrClass(CERTCertificate, CERT_DestroyCertificate)
164 165
 NSSCleanupAutoPtrClass_WithParam(PRArenaPool, PORT_FreeArena, FalseParam, false)
165 166
 
166 167
 // do not use a nsCOMPtr to avoid static initializer/destructor
@@ -234,6 +235,7 @@ class SSLServerCertVerificationResult : public nsRunnable
234 235
   void Dispatch();
235 236
 private:
236 237
   const nsRefPtr<nsNSSSocketInfo> mSocketInfo;
  238
+public:
237 239
   const PRErrorCode mErrorCode;
238 240
   const SSLErrorMessageType mErrorMessageType;
239 241
 };
@@ -259,7 +261,7 @@ class CertErrorRunnable : public SyncRunnableBase
259 261
   }
260 262
 
261 263
   virtual void RunOnTargetThread();
262  
-  nsCOMPtr<nsIRunnable> mResult; // out
  264
+  nsRefPtr<SSLServerCertVerificationResult> mResult; // out
263 265
 private:
264 266
   SSLServerCertVerificationResult* CheckCertOverrides();
265 267
   
@@ -603,9 +605,6 @@ class SSLServerCertVerificationJob : public nsRunnable
603 605
                                CERTCertificate & cert);
604 606
   ~SSLServerCertVerificationJob();
605 607
 
606  
-  // Runs on one of the background threads
607  
-  SECStatus AuthCertificate(const nsNSSShutDownPreventionLock & proofOfLock);
608  
-
609 608
   const void * const mFdForLogging;
610 609
   const nsRefPtr<nsNSSSocketInfo> mSocketInfo;
611 610
   CERTCertificate * const mCert;
@@ -627,8 +626,7 @@ SSLServerCertVerificationJob::~SSLServerCertVerificationJob()
627 626
 
628 627
 SECStatus
629 628
 PSM_SSL_PKIX_AuthCertificate(CERTCertificate *peerCert, void * pinarg,
630  
-                             const char * hostname,
631  
-                             const nsNSSShutDownPreventionLock & /*proofOfLock*/)
  629
+                             const char * hostname)
632 630
 {
633 631
     SECStatus          rv;
634 632
     
@@ -822,19 +820,18 @@ BlockServerCertChangeForSpdy(nsNSSSocketInfo *infoObject,
822 820
 }
823 821
 
824 822
 SECStatus
825  
-SSLServerCertVerificationJob::AuthCertificate(
826  
-  nsNSSShutDownPreventionLock const & nssShutdownPreventionLock)
  823
+AuthCertificate(nsNSSSocketInfo * socketInfo, CERTCertificate * cert)
827 824
 {
828  
-  if (BlockServerCertChangeForSpdy(mSocketInfo, mCert) != SECSuccess)
  825
+  if (BlockServerCertChangeForSpdy(socketInfo, cert) != SECSuccess)
829 826
     return SECFailure;
830 827
 
831  
-  if (mCert->serialNumber.data &&
832  
-      mCert->issuerName &&
833  
-      !strcmp(mCert->issuerName, 
  828
+  if (cert->serialNumber.data &&
  829
+      cert->issuerName &&
  830
+      !strcmp(cert->issuerName, 
834 831
         "CN=UTN-USERFirst-Hardware,OU=http://www.usertrust.com,O=The USERTRUST Network,L=Salt Lake City,ST=UT,C=US")) {
835 832
 
836  
-    unsigned char *server_cert_comparison_start = mCert->serialNumber.data;
837  
-    unsigned int server_cert_comparison_len = mCert->serialNumber.len;
  833
+    unsigned char *server_cert_comparison_start = cert->serialNumber.data;
  834
+    unsigned int server_cert_comparison_len = cert->serialNumber.len;
838 835
 
839 836
     while (server_cert_comparison_len) {
840 837
       if (*server_cert_comparison_start != 0)
@@ -866,33 +863,32 @@ SSLServerCertVerificationJob::AuthCertificate(
866 863
     }
867 864
   }
868 865
 
869  
-  SECStatus rv = PSM_SSL_PKIX_AuthCertificate(mCert, mSocketInfo,
870  
-                                              mSocketInfo->GetHostName(),
871  
-                                              nssShutdownPreventionLock);
  866
+  SECStatus rv = PSM_SSL_PKIX_AuthCertificate(cert, socketInfo,
  867
+                                              socketInfo->GetHostName());
872 868
 
873 869
   // We want to remember the CA certs in the temp db, so that the application can find the
874 870
   // complete chain at any time it might need it.
875 871
   // But we keep only those CA certs in the temp db, that we didn't already know.
876 872
 
877  
-  nsRefPtr<nsSSLStatus> status = mSocketInfo->SSLStatus();
  873
+  nsRefPtr<nsSSLStatus> status = socketInfo->SSLStatus();
878 874
   nsRefPtr<nsNSSCertificate> nsc;
879 875
 
880 876
   if (!status || !status->mServerCert) {
881  
-    nsc = nsNSSCertificate::Create(mCert);
  877
+    nsc = nsNSSCertificate::Create(cert);
882 878
   }
883 879
 
884 880
   CERTCertList *certList = nsnull;
885  
-  certList = CERT_GetCertChainFromCert(mCert, PR_Now(), certUsageSSLCA);
  881
+  certList = CERT_GetCertChainFromCert(cert, PR_Now(), certUsageSSLCA);
886 882
   if (!certList) {
887 883
     rv = SECFailure;
888 884
   } else {
889 885
     PRErrorCode blacklistErrorCode;
890 886
     if (rv == SECSuccess) { // PSM_SSL_PKIX_AuthCertificate said "valid cert"
891  
-      blacklistErrorCode = PSM_SSL_BlacklistDigiNotar(mCert, certList);
  887
+      blacklistErrorCode = PSM_SSL_BlacklistDigiNotar(cert, certList);
892 888
     } else { // PSM_SSL_PKIX_AuthCertificate said "invalid cert"
893 889
       PRErrorCode savedErrorCode = PORT_GetError();
894 890
       // Check if we want to worsen the error code to "revoked".
895  
-      blacklistErrorCode = PSM_SSL_DigiNotarTreatAsRevoked(mCert, certList);
  891
+      blacklistErrorCode = PSM_SSL_DigiNotarTreatAsRevoked(cert, certList);
896 892
       if (blacklistErrorCode == 0) {
897 893
         // we don't worsen the code, let's keep the original error code from NSS
898 894
         PORT_SetError(savedErrorCode);
@@ -900,7 +896,7 @@ SSLServerCertVerificationJob::AuthCertificate(
900 896
     }
901 897
       
902 898
     if (blacklistErrorCode != 0) {
903  
-      mSocketInfo->SetCertIssuerBlacklisted();
  899
+      socketInfo->SetCertIssuerBlacklisted();
904 900
       PORT_SetError(blacklistErrorCode);
905 901
       rv = SECFailure;
906 902
     }
@@ -928,7 +924,7 @@ SSLServerCertVerificationJob::AuthCertificate(
928 924
         continue;
929 925
       }
930 926
         
931  
-      if (node->cert == mCert) {
  927
+      if (node->cert == cert) {
932 928
         // We don't want to remember the server cert, 
933 929
         // the code that cares for displaying page info does this already.
934 930
         continue;
@@ -956,19 +952,19 @@ SSLServerCertVerificationJob::AuthCertificate(
956 952
     // to the caller that contains at least the cert and its status.
957 953
     if (!status) {
958 954
       status = new nsSSLStatus();
959  
-      mSocketInfo->SetSSLStatus(status);
  955
+      socketInfo->SetSSLStatus(status);
960 956
     }
961 957
 
962 958
     if (rv == SECSuccess) {
963 959
       // Certificate verification succeeded delete any potential record
964 960
       // of certificate error bits.
965 961
       nsSSLIOLayerHelpers::mHostsWithCertErrors->RememberCertHasError(
966  
-        mSocketInfo, nsnull, rv);
  962
+        socketInfo, nsnull, rv);
967 963
     }
968 964
     else {
969 965
       // Certificate verification failed, update the status' bits.
970 966
       nsSSLIOLayerHelpers::mHostsWithCertErrors->LookupCertErrorBits(
971  
-        mSocketInfo, status);
  967
+        socketInfo, status);
972 968
     }
973 969
 
974 970
     if (status && !status->mServerCert) {
@@ -1036,7 +1032,7 @@ SSLServerCertVerificationJob::Run()
1036 1032
     // Reset the error code here so we can detect if AuthCertificate fails to
1037 1033
     // set the error code if/when it fails.
1038 1034
     PR_SetError(0, 0); 
1039  
-    SECStatus rv = AuthCertificate(nssShutdownPrevention);
  1035
+    SECStatus rv = AuthCertificate(mSocketInfo, mCert);
1040 1036
     if (rv == SECSuccess) {
1041 1037
       nsRefPtr<SSLServerCertVerificationResult> restart 
1042 1038
         = new SSLServerCertVerificationResult(*mSocketInfo, 0);
@@ -1116,14 +1112,91 @@ AuthCertificateHook(void *arg, PRFileDesc *fd, PRBool checkSig, PRBool isServer)
1116 1112
   }
1117 1113
       
1118 1114
   CERTCertificate *serverCert = SSL_PeerCertificate(fd);
  1115
+  CERTCertificateCleaner serverCertCleaner(serverCert);
1119 1116
 
1120 1117
   nsNSSSocketInfo *socketInfo = static_cast<nsNSSSocketInfo*>(arg);
1121  
-  SECStatus rv = SSLServerCertVerificationJob::Dispatch(
  1118
+
  1119
+  bool onSTSThread;
  1120
+  nsresult nrv;
  1121
+  nsCOMPtr<nsIEventTarget> sts
  1122
+    = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &nrv);
  1123
+  if (NS_SUCCEEDED(nrv)) {
  1124
+    nrv = sts->IsOnCurrentThread(&onSTSThread);
  1125
+  }
  1126
+
  1127
+  if (NS_FAILED(nrv)) {
  1128
+    NS_ERROR("Could not get STS service or IsOnCurrentThread failed");
  1129
+    PR_SetError(PR_UNKNOWN_ERROR, 0);
  1130
+    return SECFailure;
  1131
+  }
  1132
+  
  1133
+  if (onSTSThread) {
  1134
+    // We *must* do certificate verification on a background thread because
  1135
+    // we need the socket transport thread to be free for our OCSP requests,
  1136
+    // and we *want* to do certificate verification on a background thread
  1137
+    // because of the performance benefits of doing so.
  1138
+    SECStatus rv = SSLServerCertVerificationJob::Dispatch(
1122 1139
                         static_cast<const void *>(fd), socketInfo, serverCert);
  1140
+    return rv;
  1141
+  }
  1142
+  
  1143
+  // We can't do certificate verification on a background thread, because the
  1144
+  // thread doing the network I/O may not interrupt its network I/O on receipt
  1145
+  // of our SSLServerCertVerificationResult event, and/or it might not even be
  1146
+  // a non-blocking socket.
  1147
+  SECStatus rv = AuthCertificate(socketInfo, serverCert);
  1148
+  if (rv == SECSuccess) {
  1149
+    return SECSuccess;
  1150
+  }
1123 1151
 
1124  
-  CERT_DestroyCertificate(serverCert);
  1152
+  PRErrorCode error = PR_GetError();
  1153
+  if (error != 0) {
  1154
+    nsRefPtr<CertErrorRunnable> runnable = CreateCertErrorRunnable(
  1155
+                    error, socketInfo, serverCert,
  1156
+                    static_cast<const void *>(fd));
  1157
+    if (!runnable) {
  1158
+      // CreateCertErrorRunnable sets a new error code when it fails
  1159
+      error = PR_GetError();
  1160
+    } else {
  1161
+      // We have to return SECSuccess or SECFailure based on the result of the
  1162
+      // override processing, so we must block this thread waiting for it. The
  1163
+      // CertErrorRunnable will NOT dispatch the result at all, since we passed
  1164
+      // false for CreateCertErrorRunnable's async parameter
  1165
+      nrv = runnable->DispatchToMainThreadAndWait();
  1166
+      if (NS_FAILED(nrv)) {
  1167
+        NS_ERROR("Failed to dispatch CertErrorRunnable");
  1168
+        PR_SetError(PR_INVALID_STATE_ERROR, 0);
  1169
+        return SECFailure;
  1170
+      }
1125 1171
 
1126  
-  return rv;
  1172
+      if (!runnable->mResult) {
  1173
+        NS_ERROR("CertErrorRunnable did not set result");
  1174
+        PR_SetError(PR_INVALID_STATE_ERROR, 0);
  1175
+        return SECFailure;
  1176
+      }
  1177
+
  1178
+      if (runnable->mResult->mErrorCode == 0) {
  1179
+        return SECSuccess; // cert error override occurred.
  1180
+      }
  1181
+
  1182
+      // We must call SetCanceled here to set the error message type
  1183
+      // in case it isn't PlainErrorMessage, which is what we would
  1184
+      // default to if we just called
  1185
+      // PR_SetError(runnable->mResult->mErrorCode, 0) and returned
  1186
+      // SECFailure without doing this.
  1187
+      socketInfo->SetCanceled(runnable->mResult->mErrorCode,
  1188
+                              runnable->mResult->mErrorMessageType);
  1189
+      error = runnable->mResult->mErrorCode;
  1190
+    }
  1191
+  }
  1192
+
  1193
+  if (error == 0) {
  1194
+    NS_ERROR("error code not set");
  1195
+    error = PR_UNKNOWN_ERROR;
  1196
+  }
  1197
+
  1198
+  PR_SetError(error, 0);
  1199
+  return SECFailure;
1127 1200
 }
1128 1201
 
1129 1202
 SSLServerCertVerificationResult::SSLServerCertVerificationResult(

0 notes on commit 5972492

Please sign in to comment.
Something went wrong with that request. Please try again.