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 175502 3e3ddb3ce8d331b9898c04e1bb90764738366edc
parent 175501 aaad90a5936fe7c70754712365d4b06338dd43f3
child 175503 f09555644f77bb593d607eb51ba418d1da2e2f64
push idunknown
push userunknown
push dateunknown
reviewerscviecco
bugs891066
milestone29.0a1
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;
   }