Bug 891066, Part 6: Move SSL server cert verification logic to security/certverifier, r=cviecco
authorBrian Smith <brian@briansmith.org>
Mon, 08 Jul 2013 16:30:59 -0700
changeset 181301 3e3ddb3ce8d331b9898c04e1bb90764738366edc
parent 181300 aaad90a5936fe7c70754712365d4b06338dd43f3
child 181302 f09555644f77bb593d607eb51ba418d1da2e2f64
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscviecco
bugs891066
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 891066, Part 6: Move SSL server cert verification logic to security/certverifier, r=cviecco
security/certverifier/CertVerifier.cpp
security/certverifier/CertVerifier.h
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/manager/ssl/src/SSLServerCertVerification.cpp
security/manager/ssl/src/nsCertOverrideService.cpp
security/manager/ssl/src/nsNSSCertificate.cpp
security/manager/ssl/src/nsNSSCertificate.h
security/manager/ssl/src/nsNSSCertificateDB.cpp
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -5,19 +5,21 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CertVerifier.h"
 
 #include <stdint.h>
 
 #include "insanity/pkixtypes.h"
 #include "ExtendedValidation.h"
+#include "NSSCertDBTrustDomain.h"
 #include "cert.h"
 #include "secerr.h"
 #include "prerror.h"
+#include "sslerr.h"
 
 // ScopedXXX in this file are insanity::pkix::ScopedXXX, not
 // mozilla::ScopedXXX.
 using namespace insanity::pkix;
 using namespace mozilla::psm;
 
 #ifdef PR_LOGGING
 extern PRLogModuleInfo* gPIPNSSLog;
@@ -448,9 +450,58 @@ pkix_done:
         &cvout[validationChainLocation].value.pointer.chain);
     }
   }
 
   return rv;
 #endif
 }
 
+SECStatus
+CertVerifier::VerifySSLServerCert(CERTCertificate* peerCert,
+                                  PRTime time,
+                     /*optional*/ void* pinarg,
+                                  const char* hostname,
+                                  bool saveIntermediatesInPermanentDatabase,
+                 /*optional out*/ insanity::pkix::ScopedCERTCertList* certChainOut,
+                 /*optional out*/ SECOidTag* evOidPolicy)
+{
+  PR_ASSERT(peerCert);
+  // XXX: PR_ASSERT(pinarg)
+  PR_ASSERT(hostname);
+  PR_ASSERT(hostname[0]);
+
+  if (certChainOut) {
+    *certChainOut = nullptr;
+  }
+  if (evOidPolicy) {
+    *evOidPolicy = SEC_OID_UNKNOWN;
+  }
+
+  if (!hostname || !hostname[0]) {
+    PR_SetError(SSL_ERROR_BAD_CERT_DOMAIN, 0);
+    return SECFailure;
+  }
+
+  ScopedCERTCertList validationChain;
+  SECStatus rv = VerifyCert(peerCert, certificateUsageSSLServer, time,
+                            pinarg, 0, &validationChain, evOidPolicy);
+  if (rv != SECSuccess) {
+    return rv;
+  }
+
+  rv = CERT_VerifyCertName(peerCert, hostname);
+  if (rv != SECSuccess) {
+    return rv;
+  }
+
+  if (saveIntermediatesInPermanentDatabase) {
+    SaveIntermediateCerts(validationChain);
+  }
+
+  if (certChainOut) {
+    *certChainOut = validationChain.release();
+  }
+
+  return SECSuccess;
+}
+
 } } // namespace mozilla::psm
--- a/security/certverifier/CertVerifier.h
+++ b/security/certverifier/CertVerifier.h
@@ -27,16 +27,26 @@ public:
                        const SECCertificateUsage usage,
                        const PRTime time,
                        void* pinArg,
                        const Flags flags = 0,
       /*optional out*/ insanity::pkix::ScopedCERTCertList* validationChain = nullptr,
       /*optional out*/ SECOidTag* evOidPolicy = nullptr ,
       /*optional out*/ CERTVerifyLog* verifyLog = nullptr);
 
+  SECStatus VerifySSLServerCert(
+                    CERTCertificate* peerCert,
+                    PRTime time,
+       /*optional*/ void* pinarg,
+                    const char* hostname,
+                    bool saveIntermediatesInPermanentDatabase = false,
+   /*optional out*/ insanity::pkix::ScopedCERTCertList* certChainOut = nullptr,
+   /*optional out*/ SECOidTag* evOidPolicy = nullptr);
+
+
   enum implementation_config {
     classic = 0,
 #ifndef NSS_NO_LIBPKIX
     libpkix = 1,
 #endif
   };
 
   enum missing_cert_download_config { missing_cert_download_off = 0, missing_cert_download_on };
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -4,20 +4,22 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "NSSCertDBTrustDomain.h"
 
 #include <stdint.h>
 
 #include "insanity/ScopedPtr.h"
-#include "cert.h"
+#include "certdb.h"
 #include "nss.h"
 #include "ocsp.h"
+#include "pk11pub.h"
 #include "prerror.h"
+#include "prmem.h"
 #include "prprf.h"
 #include "secerr.h"
 #include "secmod.h"
 
 using namespace insanity::pkix;
 
 namespace mozilla { namespace psm {
 
@@ -155,9 +157,105 @@ SetClassicOCSPBehavior(CertVerifier::ocs
 
   int OCSPTimeoutSeconds = 3;
   if (strict == CertVerifier::ocsp_strict) {
     OCSPTimeoutSeconds = 10;
   }
   CERT_SetOCSPTimeout(OCSPTimeoutSeconds);
 }
 
+char*
+DefaultServerNicknameForCert(CERTCertificate* cert)
+{
+  char* nickname = nullptr;
+  int count;
+  bool conflict;
+  char* servername = nullptr;
+
+  servername = CERT_GetCommonName(&cert->subject);
+  if (!servername) {
+    // Certs without common names are strange, but they do exist...
+    // Let's try to use another string for the nickname
+    servername = CERT_GetOrgUnitName(&cert->subject);
+    if (!servername) {
+      servername = CERT_GetOrgName(&cert->subject);
+      if (!servername) {
+        servername = CERT_GetLocalityName(&cert->subject);
+        if (!servername) {
+          servername = CERT_GetStateName(&cert->subject);
+          if (!servername) {
+            servername = CERT_GetCountryName(&cert->subject);
+            if (!servername) {
+              // We tried hard, there is nothing more we can do.
+              // A cert without any names doesn't really make sense.
+              return nullptr;
+            }
+          }
+        }
+      }
+    }
+  }
+
+  count = 1;
+  while (1) {
+    if (count == 1) {
+      nickname = PR_smprintf("%s", servername);
+    }
+    else {
+      nickname = PR_smprintf("%s #%d", servername, count);
+    }
+    if (!nickname) {
+      break;
+    }
+
+    conflict = SEC_CertNicknameConflict(nickname, &cert->derSubject,
+                                        cert->dbhandle);
+    if (!conflict) {
+      break;
+    }
+    PR_Free(nickname);
+    count++;
+  }
+  PR_FREEIF(servername);
+  return nickname;
+}
+
+void
+SaveIntermediateCerts(const ScopedCERTCertList& certList)
+{
+  if (!certList) {
+    return;
+  }
+
+  bool isEndEntity = true;
+  for (CERTCertListNode* node = CERT_LIST_HEAD(certList);
+        !CERT_LIST_END(node, certList);
+        node = CERT_LIST_NEXT(node)) {
+    if (isEndEntity) {
+      // Skip the end-entity; we only want to store intermediates
+      isEndEntity = false;
+      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;
+    }
+
+    // We have found a signer cert that we want to remember.
+    char* nickname = DefaultServerNicknameForCert(node->cert);
+    if (nickname && *nickname) {
+      ScopedPtr<PK11SlotInfo, PK11_FreeSlot> slot(PK11_GetInternalKeySlot());
+      if (slot) {
+        PK11_ImportCert(slot.get(), node->cert, CK_INVALID_HANDLE,
+                        nickname, false);
+      }
+    }
+    PR_FREEIF(nickname);
+  }
+}
+
 } } // namespace mozilla::psm
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -22,16 +22,24 @@ extern const char BUILTIN_ROOTS_MODULE_D
 //
 // The modNameUTF8 parameter should usually be
 // BUILTIN_ROOTS_MODULE_DEFAULT_NAME.
 SECStatus LoadLoadableRoots(/*optional*/ const char* dir,
                             const char* modNameUTF8);
 
 void UnloadLoadableRoots(const char* modNameUTF8);
 
+// Controls the OCSP fetching behavior of the classic verification mode. In the
+// classic mode, the OCSP fetching behavior is set globally instead of per
+// validation.
 void
 SetClassicOCSPBehavior(CertVerifier::ocsp_download_config enabled,
                        CertVerifier::ocsp_strict_config strict,
                        CertVerifier::ocsp_get_config get);
 
+// Caller must free the result with PR_Free
+char* DefaultServerNicknameForCert(CERTCertificate* cert);
+
+void SaveIntermediateCerts(const insanity::pkix::ScopedCERTCertList& certList);
+
 } } // namespace mozilla::psm
 
 #endif // mozilla_psm__NSSCertDBTrustDomain_h
--- a/security/manager/ssl/src/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/src/SSLServerCertVerification.cpp
@@ -89,21 +89,23 @@
 // thread since they may do I/O, because many parts of nsNSSSocketInfo (the
 // subclass of TransportSecurityInfo used when validating certificates during
 // an SSL handshake) and the PSM NSS I/O layer are not thread-safe, and because
 // we need the event to interrupt the PR_Poll that may waiting for I/O on the
 // socket for which we are validating the cert.
 
 #include "SSLServerCertVerification.h"
 
+#include <cstring>
+
 #include "insanity/pkixtypes.h"
-
 #include "CertVerifier.h"
 #include "CryptoTask.h"
 #include "ExtendedValidation.h"
+#include "NSSCertDBTrustDomain.h"
 #include "nsIBadCertListener2.h"
 #include "nsICertOverrideService.h"
 #include "nsISiteSecurityService.h"
 #include "nsNSSComponent.h"
 #include "nsNSSCleaner.h"
 #include "nsRecentBadCerts.h"
 #include "nsNSSIOLayer.h"
 #include "nsNSSShutDown.h"
@@ -649,43 +651,16 @@ SSLServerCertVerificationJob::SSLServerC
   , mInfoObject(infoObject)
   , mCert(CERT_DupCertificate(cert))
   , mProviderFlags(providerFlags)
   , mJobStartTime(TimeStamp::Now())
   , mStapledOCSPResponse(SECITEM_DupItem(stapledOCSPResponse))
 {
 }
 
-SECStatus
-PSM_SSL_PKIX_AuthCertificate(CertVerifier& certVerifier,
-                             CERTCertificate* peerCert,
-                             nsIInterfaceRequestor* pinarg,
-                             const char* hostname,
-                             insanity::pkix::ScopedCERTCertList* validationChain,
-                             SECOidTag* evOidPolicy)
-{
-    SECStatus rv = certVerifier.VerifyCert(peerCert, certificateUsageSSLServer,
-                                           PR_Now(), pinarg, 0,
-                                           validationChain, evOidPolicy);
-
-    if (rv == SECSuccess) {
-        // cert is OK. This is the client side of an SSL connection.
-        // Now check the name field in the cert against the desired hostname.
-        // NB: This is our only defense against Man-In-The-Middle (MITM) attacks!
-        if (hostname && hostname[0])
-            rv = CERT_VerifyCertName(peerCert, hostname);
-        else
-            rv = SECFailure;
-        if (rv != SECSuccess)
-            PORT_SetError(SSL_ERROR_BAD_CERT_DOMAIN);
-    }
-
-    return rv;
-}
-
 // This function assumes that we will only use the SPDY connection coalescing
 // feature on connections where we have negotiated SPDY using NPN. If we ever
 // talk SPDY without having negotiated it with SPDY, this code will give wrong
 // and perhaps unsafe results.
 //
 // Returns SECSuccess on the initial handshake of all connections, on
 // renegotiations for any connections where we did not negotiate SPDY, or on any
 // SPDY connection where the server's certificate did not change.
@@ -749,16 +724,19 @@ BlockServerCertChangeForSpdy(nsNSSSocket
   return SECFailure;
 }
 
 SECStatus
 AuthCertificate(CertVerifier& certVerifier, TransportSecurityInfo* infoObject,
                 CERTCertificate* cert, SECItem* stapledOCSPResponse,
                 uint32_t providerFlags)
 {
+  MOZ_ASSERT(infoObject);
+  MOZ_ASSERT(cert);
+
   SECStatus rv;
   if (stapledOCSPResponse) {
     CERTCertDBHandle* handle = CERT_GetDefaultCertDB();
     rv = CERT_CacheOCSPResponseFromSideChannel(handle, cert, PR_Now(),
                                                stapledOCSPResponse,
                                                infoObject);
     if (rv != SECSuccess) {
       // Due to buggy servers that will staple expired OCSP responses
@@ -797,22 +775,27 @@ AuthCertificate(CertVerifier& certVerifi
     if (!certVerifier.mOCSPDownloadEnabled) {
       reasonsForNotFetching |= 2;
     }
 
     Telemetry::Accumulate(Telemetry::SSL_OCSP_MAY_FETCH,
                           reasonsForNotFetching);
   }
 
+  // We want to avoid storing any intermediate cert information when browsing
+  // in private, transient contexts.
+  bool saveIntermediates =
+    !(providerFlags & nsISocketProvider::NO_PERMANENT_STORAGE);
 
   insanity::pkix::ScopedCERTCertList certList;
   SECOidTag evOidPolicy;
-  rv = PSM_SSL_PKIX_AuthCertificate(certVerifier, cert, infoObject,
-                                    infoObject->GetHostNameRaw(),
-                                    &certList, &evOidPolicy);
+  rv = certVerifier.VerifySSLServerCert(cert, PR_Now(), infoObject,
+                                        infoObject->GetHostNameRaw(),
+                                        saveIntermediates, nullptr,
+                                        &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;
 
@@ -820,57 +803,17 @@ AuthCertificate(CertVerifier& certVerifi
     if( rv == SECSuccess ){
       nsc = nsNSSCertificate::Create(cert, &evOidPolicy);
     }
     else {
       nsc = nsNSSCertificate::Create(cert);
     }
   }
 
-  if (rv == SECSuccess && certList) {
-    // 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->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;
-        }
-
-        // We have found a signer cert that we want to remember.
-        char* nickname = nsNSSCertificate::defaultServerNickname(node->cert);
-        if (nickname && *nickname) {
-          // There is a suspicion that there is some thread safety issues
-          // in PK11_importCert and the mutex is a way to serialize until
-          // this issue has been cleared.
-          MutexAutoLock PK11Mutex(*gSSLVerificationPK11Mutex);
-          ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
-          if (slot) {
-            PK11_ImportCert(slot, node->cert, CK_INVALID_HANDLE,
-                            nickname, false);
-          }
-        }
-        PR_FREEIF(nickname);
-      }
-    }
-
+  if (rv == SECSuccess) {
     // 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);
     }
 
--- a/security/manager/ssl/src/nsCertOverrideService.cpp
+++ b/security/manager/ssl/src/nsCertOverrideService.cpp
@@ -3,16 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsCertOverrideService.h"
 
 #include "insanity/pkixtypes.h"
 #include "nsIX509Cert.h"
+#include "NSSCertDBTrustDomain.h"
 #include "nsNSSCertificate.h"
 #include "nsNSSCertHelper.h"
 #include "nsCRT.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "nsStreamUtils.h"
 #include "nsNetUtil.h"
 #include "nsILineInputStream.h"
 #include "nsIObserver.h"
@@ -453,17 +454,17 @@ nsCertOverrideService::RememberValidityO
   nsCOMPtr<nsIX509Cert2> cert2 = do_QueryInterface(aCert);
   if (!cert2)
     return NS_ERROR_FAILURE;
 
   insanity::pkix::ScopedCERTCertificate nsscert(cert2->GetCert());
   if (!nsscert)
     return NS_ERROR_FAILURE;
 
-  char* nickname = nsNSSCertificate::defaultServerNickname(nsscert.get());
+  char* nickname = DefaultServerNicknameForCert(nsscert.get());
   if (!aTemporary && nickname && *nickname)
   {
     ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
     if (!slot) {
       PR_Free(nickname);
       return NS_ERROR_FAILURE;
     }
   
--- a/security/manager/ssl/src/nsNSSCertificate.cpp
+++ b/security/manager/ssl/src/nsNSSCertificate.cpp
@@ -1417,73 +1417,16 @@ nsNSSCertificate::SaveSMimeProfile()
     return NS_ERROR_NOT_AVAILABLE;
 
   if (SECSuccess != CERT_SaveSMimeProfile(mCert.get(), nullptr, nullptr))
     return NS_ERROR_FAILURE;
   else
     return NS_OK;
 }
 
-
-char* nsNSSCertificate::defaultServerNickname(CERTCertificate* cert)
-{
-  nsNSSShutDownPreventionLock locker;
-  char* nickname = nullptr;
-  int count;
-  bool conflict;
-  char* servername = nullptr;
-
-  servername = CERT_GetCommonName(&cert->subject);
-  if (!servername) {
-    // Certs without common names are strange, but they do exist...
-    // Let's try to use another string for the nickname
-    servername = CERT_GetOrgUnitName(&cert->subject);
-    if (!servername) {
-      servername = CERT_GetOrgName(&cert->subject);
-      if (!servername) {
-        servername = CERT_GetLocalityName(&cert->subject);
-        if (!servername) {
-          servername = CERT_GetStateName(&cert->subject);
-          if (!servername) {
-            servername = CERT_GetCountryName(&cert->subject);
-            if (!servername) {
-              // We tried hard, there is nothing more we can do.
-              // A cert without any names doesn't really make sense.
-              return nullptr;
-            }
-          }
-        }
-      }
-    }
-  }
-
-  count = 1;
-  while (1) {
-    if (count == 1) {
-      nickname = PR_smprintf("%s", servername);
-    }
-    else {
-      nickname = PR_smprintf("%s #%d", servername, count);
-    }
-    if (!nickname) {
-      break;
-    }
-
-    conflict = SEC_CertNicknameConflict(nickname, &cert->derSubject,
-                                        cert->dbhandle);
-    if (!conflict) {
-      break;
-    }
-    PR_Free(nickname);
-    count++;
-  }
-  PR_FREEIF(servername);
-  return nickname;
-}
-
 #ifndef NSS_NO_LIBPKIX
 
 nsresult
 nsNSSCertificate::hasValidEVOidTag(SECOidTag& resultOidTag, bool& validEV)
 {
   nsNSSShutDownPreventionLock locker;
   if (isAlreadyShutDown())
     return NS_ERROR_NOT_AVAILABLE;
--- a/security/manager/ssl/src/nsNSSCertificate.h
+++ b/security/manager/ssl/src/nsNSSCertificate.h
@@ -48,20 +48,16 @@ public:
   virtual ~nsNSSCertificate();
   nsresult FormatUIStrings(const nsAutoString& nickname,
                            nsAutoString& nickWithSerial,
                            nsAutoString& details);
   static nsNSSCertificate* Create(CERTCertificate*cert = nullptr,
                                   SECOidTag* evOidPolicy = nullptr);
   static nsNSSCertificate* ConstructFromDER(char* certDER, int derLen);
 
-  // It is the responsibility of the caller of this method to free the returned
-  // string using PR_Free.
-  static char* defaultServerNickname(CERTCertificate* cert);
-
 private:
   insanity::pkix::ScopedCERTCertificate mCert;
   bool             mPermDelete;
   uint32_t         mCertType;
   nsresult CreateASN1Struct(nsIASN1Object** aRetVal);
   nsresult CreateTBSCertificateASN1Struct(nsIASN1Sequence** retSequence,
                                           nsINSSComponent* nssComponent);
   nsresult GetSortableDate(PRTime aTime, nsAString& _aSortableDate);
--- a/security/manager/ssl/src/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/src/nsNSSCertificateDB.cpp
@@ -7,16 +7,18 @@
 // only exported so PSM can use it for this specific purpose.
 #define CERT_AddTempCertToPerm __CERT_AddTempCertToPerm
 
 #include "nsNSSComponent.h"
 #include "nsNSSCertificateDB.h"
 
 #include "CertVerifier.h"
 #include "ExtendedValidation.h"
+#include "NSSCertDBTrustDomain.h"
+#include "insanity/pkixtypes.h"
 #include "nsNSSComponent.h"
 #include "mozilla/Base64.h"
 #include "nsCOMPtr.h"
 #include "nsNSSCertificate.h"
 #include "nsNSSHelper.h"
 #include "nsNSSCertHelper.h"
 #include "nsNSSCertCache.h"
 #include "nsCRT.h"
@@ -691,17 +693,17 @@ nsNSSCertificateDB::ImportServerCertific
     nsrv = NS_ERROR_FAILURE;
     goto loser;
   }
 
   for ( i = 0; i < numcerts; i++ ) {
     rawCerts[i] = &certCollection->rawCerts[i];
   }
 
-  serverNickname = nsNSSCertificate::defaultServerNickname(cert.get());
+  serverNickname = DefaultServerNicknameForCert(cert.get());
   srv = CERT_ImportCerts(CERT_GetDefaultCertDB(), certUsageSSLServer,
              numcerts, rawCerts, nullptr, true, false,
              serverNickname);
   PR_FREEIF(serverNickname);
   if ( srv != SECSuccess ) {
     nsrv = NS_ERROR_FAILURE;
     goto loser;
   }