Bug 1592355 - Convert certList to raw array for Pins verification r=keeler

Differential Revision: https://phabricator.services.mozilla.com/D50967

security/certverifier/NSSCertDBTrustDomain.cpp

@@ -29,6 +29,7 @@
 #include "nsNSSCertHelper.h"
 #include "nsNSSCertValidity.h"
 #include "nsNSSCertificate.h"
+#include "nsNSSCertificateDB.h"
 #include "nsServiceManagerUtils.h"
 #include "nsThreadUtils.h"
 #include "nss.h"
@@ -1025,19 +1026,16 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& certArray, Time time,
   }
 
   // Modernization in-progress: Keep certList as a CERTCertList for storage into
-  // the mBuiltChain variable at the end, but let's use nsNSSCertList for the
+  // the mBuiltChain variable at the end.
-  // validity calculations.
+  nsTArray<RefPtr<nsIX509Cert>> nssCertList;
-  UniqueCERTCertList certListCopy = nsNSSCertList::DupCertList(certList);
+  nsresult nsrv = nsNSSCertificateDB::ConstructCertArrayFromUniqueCertList(
+      certList, nssCertList);
 
-  // This adopts the list
+  if (NS_FAILED(nsrv)) {
-  RefPtr<nsNSSCertList> nssCertList =
-      new nsNSSCertList(std::move(certListCopy));
-  if (!nssCertList) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
-
   nsCOMPtr<nsIX509Cert> rootCert;
-  nsresult nsrv = nssCertList->GetRootCertificate(rootCert);
+  nsrv = nsNSSCertificate::GetRootCertificate(nssCertList, rootCert);
   if (NS_FAILED(nsrv)) {
     return Result::FATAL_ERROR_LIBRARY_FAILURE;
   }
@@ -1082,10 +1080,11 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& certArray, Time time,
        (mDistrustedCAPolicy &
         DistrustedCAPolicy::DistrustSymantecRootsRegardlessOfDate))) {
     rootCert = nullptr;  // Clear the state for Segment...
-    nsCOMPtr<nsIX509CertList> intCerts;
+    nsTArray<RefPtr<nsIX509Cert>> intCerts;
     nsCOMPtr<nsIX509Cert> eeCert;
 
-    nsrv = nssCertList->SegmentCertificateChain(rootCert, intCerts, eeCert);
+    nsrv = nsNSSCertificate::SegmentCertificateChain(nssCertList, rootCert,
+                                                     intCerts, eeCert);
     if (NS_FAILED(nsrv)) {
       // This chain is supposed to be complete, so this is an error.
       return Result::ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED;

security/certverifier/TrustOverrideUtils.h

@@ -78,7 +78,7 @@ static bool CertMatchesStaticData(const CERTCertificate* cert,
 // "distrusted."
 template <size_t T>
 static nsresult CheckForSymantecDistrust(
-    const nsCOMPtr<nsIX509CertList>& intCerts,
+    const nsTArray<RefPtr<nsIX509Cert>>& intCerts,
     const nsCOMPtr<nsIX509Cert>& eeCert, const PRTime& permitAfterDate,
     const DataAndLength (&whitelist)[T],
     /* out */ bool& isDistrusted) {
@@ -113,21 +113,14 @@ static nsresult CheckForSymantecDistrust(
     }
   }
 
-  // Look for one of the intermediates to be in the whitelist
+  for (const auto& cert : intCerts) {
-  RefPtr<nsNSSCertList> intCertList = intCerts->GetCertList();
+    UniqueCERTCertificate nssCert(cert->GetCert());
-
-  return intCertList->ForEachCertificateInChain(
-      [&isDistrusted, &whitelist](nsCOMPtr<nsIX509Cert> aCert, bool aHasMore,
-                                  /* out */ bool& aContinue) {
-        // We need an owning handle when calling nsIX509Cert::GetCert().
-        UniqueCERTCertificate nssCert(aCert->GetCert());
     if (CertSPKIIsInList(nssCert.get(), whitelist)) {
-          // In the whitelist
       isDistrusted = false;
-          aContinue = false;
+      break;
+    }
   }
   return NS_OK;
-      });
 }
 
 #endif  // TrustOverrides_h

security/manager/ssl/PublicKeyPinningService.cpp

@@ -97,7 +97,7 @@ static nsresult EvalCert(const CERTCertificate* cert,
  * fingerprints from the given static fingerprints or the
  * dynamicFingerprints array, or to false otherwise.
  */
-static nsresult EvalChain(const RefPtr<nsNSSCertList>& certList,
+static nsresult EvalChain(const nsTArray<RefPtr<nsIX509Cert>>& certList,
                           const StaticFingerprints* fingerprints,
                           const nsTArray<nsCString>* dynamicFingerprints,
                           /*out*/ bool& certListIntersectsPinset) {
@@ -107,12 +107,8 @@ static nsresult EvalChain(const RefPtr<nsNSSCertList>& certList,
     return NS_ERROR_FAILURE;
   }
 
-  certList->ForEachCertificateInChain(
+  for (const auto& cert : certList) {
-      [&certListIntersectsPinset, &fingerprints, &dynamicFingerprints](
+    UniqueCERTCertificate nssCert(cert->GetCert());
-          nsCOMPtr<nsIX509Cert> aCert, bool aHasMore,
-          /* out */ bool& aContinue) {
-        // We need an owning handle when calling nsIX509Cert::GetCert().
-        UniqueCERTCertificate nssCert(aCert->GetCert());
     MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
             ("pkpin: certArray subject: '%s'\n", nssCert->subjectName));
     MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
@@ -122,12 +118,10 @@ static nsresult EvalChain(const RefPtr<nsNSSCertList>& certList,
     if (NS_FAILED(rv)) {
       return rv;
     }
-
     if (certListIntersectsPinset) {
-          aContinue = false;
+      break;
+    }
   }
-        return NS_OK;
-      });
 
   if (!certListIntersectsPinset) {
     MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
@@ -151,7 +145,7 @@ class TransportSecurityPreloadBinarySearchComparator {
 };
 
 nsresult PublicKeyPinningService::ChainMatchesPinset(
-    const RefPtr<nsNSSCertList>& certList,
+    const nsTArray<RefPtr<nsIX509Cert>>& certList,
     const nsTArray<nsCString>& aSHA256keys,
     /*out*/ bool& chainMatchesPinset) {
   return EvalChain(certList, nullptr, &aSHA256keys, chainMatchesPinset);
@@ -257,13 +251,13 @@ static nsresult FindPinningInformation(
 // subject public key info data in the list and the most relevant non-expired
 // pinset for the host or there is no pinning information for the host.
 static nsresult CheckPinsForHostname(
-    const RefPtr<nsNSSCertList>& certList, const char* hostname,
+    const nsTArray<RefPtr<nsIX509Cert>>& certList, const char* hostname,
     bool enforceTestMode, mozilla::pkix::Time time,
     const OriginAttributes& originAttributes,
     /*out*/ bool& chainHasValidPins,
     /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo) {
   chainHasValidPins = false;
-  if (!certList) {
+  if (certList.IsEmpty()) {
     return NS_ERROR_INVALID_ARG;
   }
   if (!hostname || hostname[0] == 0) {
@@ -328,7 +322,7 @@ static nsresult CheckPinsForHostname(
       // We only collect per-CA pinning statistics upon failures.
       if (!enforceTestModeResult) {
         nsCOMPtr<nsIX509Cert> rootCert;
-        rv = certList->GetRootCertificate(rootCert);
+        rv = nsNSSCertificate::GetRootCertificate(certList, rootCert);
         if (NS_FAILED(rv)) {
           return rv;
         }
@@ -358,13 +352,13 @@ static nsresult CheckPinsForHostname(
 }
 
 nsresult PublicKeyPinningService::ChainHasValidPins(
-    const RefPtr<nsNSSCertList>& certList, const char* hostname,
+    const nsTArray<RefPtr<nsIX509Cert>>& certList, const char* hostname,
     mozilla::pkix::Time time, bool enforceTestMode,
     const OriginAttributes& originAttributes,
     /*out*/ bool& chainHasValidPins,
     /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo) {
   chainHasValidPins = false;
-  if (!certList) {
+  if (certList.IsEmpty()) {
     return NS_ERROR_INVALID_ARG;
   }
   if (!hostname || hostname[0] == 0) {

security/manager/ssl/PublicKeyPinningService.h

@@ -34,7 +34,7 @@ class PublicKeyPinningService {
    * that would otherwise be valid for it
    */
   static nsresult ChainHasValidPins(
-      const RefPtr<nsNSSCertList>& certList, const char* hostname,
+      const nsTArray<RefPtr<nsIX509Cert>>& certList, const char* hostname,
       mozilla::pkix::Time time, bool enforceTestMode,
       const OriginAttributes& originAttributes,
       /*out*/ bool& chainHasValidPins,
@@ -44,7 +44,8 @@ class PublicKeyPinningService {
    * certificate list and the pins specified in the aSHA256keys array.
    * Values passed in are assumed to be in base64 encoded form.
    */
-  static nsresult ChainMatchesPinset(const RefPtr<nsNSSCertList>& certList,
+  static nsresult ChainMatchesPinset(
+      const nsTArray<RefPtr<nsIX509Cert>>& certList,
       const nsTArray<nsCString>& aSHA256keys,
       /*out*/ bool& chainMatchesPinset);
 

security/manager/ssl/nsNSSCallbacks.cpp

@@ -1138,23 +1138,16 @@ static void RebuildVerifiedCertificateInformation(PRFileDesc* fd,
 nsresult IsCertificateDistrustImminent(
     const nsTArray<RefPtr<nsIX509Cert>>& aCertArray,
     /* out */ bool& isDistrusted) {
-  nsCOMPtr<nsIX509CertList> tmpCertList;
+  if (aCertArray.IsEmpty()) {
-  nsresult rv = TransportSecurityInfo::ConvertCertArrayToCertList(
+    return NS_ERROR_INVALID_ARG;
-      aCertArray, getter_AddRefs(tmpCertList));
-
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (!tmpCertList) {
-    return NS_ERROR_INVALID_POINTER;
   }
 
   nsCOMPtr<nsIX509Cert> rootCert;
-  nsCOMPtr<nsIX509CertList> intCerts;
+  nsTArray<RefPtr<nsIX509Cert>> intCerts;
   nsCOMPtr<nsIX509Cert> eeCert;
 
-  RefPtr<nsNSSCertList> certList = tmpCertList->GetCertList();
+  nsresult rv = nsNSSCertificate::SegmentCertificateChain(aCertArray, rootCert,
-  rv = certList->SegmentCertificateChain(rootCert, intCerts, eeCert);
+                                                          intCerts, eeCert);
   if (NS_FAILED(rv)) {
     return rv;
   }

security/manager/ssl/nsNSSCallbacks.h

@@ -23,7 +23,6 @@ using mozilla::TimeDuration;
 using mozilla::Vector;
 
 class nsILoadGroup;
-class nsIX509CertList;
 
 char* PK11PasswordPrompt(PK11SlotInfo* slot, PRBool retry, void* arg);
 

security/manager/ssl/nsNSSCertificate.cpp

@@ -1124,6 +1124,59 @@ nsresult nsNSSCertList::GetRootCertificate(
   return NS_OK;
 }
 
+nsresult nsNSSCertificate::SegmentCertificateChain(
+    /* in */ const nsTArray<RefPtr<nsIX509Cert>>& aCertList,
+    /* out */ nsCOMPtr<nsIX509Cert>& aRoot,
+    /* out */ nsTArray<RefPtr<nsIX509Cert>>& aIntermediates,
+    /* out */ nsCOMPtr<nsIX509Cert>& aEndEntity) {
+  if (aRoot || aEndEntity) {
+    // All passed-in nsCOMPtrs should be empty for the state machine to work
+    return NS_ERROR_UNEXPECTED;
+  }
+
+  if (!aIntermediates.IsEmpty()) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  for (size_t i = 0; i < aCertList.Length(); ++i) {
+    const auto& cert = aCertList[i];
+    if (!aEndEntity) {
+      aEndEntity = cert;
+    } else if (i == aCertList.Length() - 1) {
+      aRoot = cert;
+    } else {
+      // One of (potentially many) intermediates
+      aIntermediates.AppendElement(cert);
+    }
+  }
+
+  if (!aRoot || !aEndEntity) {
+    // No self-signed (or empty) chains allowed
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  return NS_OK;
+}
+
+nsresult nsNSSCertificate::GetRootCertificate(
+    /* in */ const nsTArray<RefPtr<nsIX509Cert>>& aCertList,
+    /* out */ nsCOMPtr<nsIX509Cert>& aRoot) {
+  if (aRoot) {
+    return NS_ERROR_UNEXPECTED;
+  }
+  // If the list is empty, leave aRoot empty.
+  if (aCertList.IsEmpty()) {
+    return NS_ERROR_FAILURE;
+  }
+
+  nsCOMPtr<nsIX509Cert> cert(aCertList.LastElement());
+  aRoot = cert;
+  if (!aRoot) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  return NS_OK;
+}
+
 nsNSSCertListEnumerator::nsNSSCertListEnumerator(
     const std::vector<UniqueCERTCertificate>& certs) {
   mCerts.reserve(certs.size());

security/manager/ssl/nsNSSCertificate.h

@@ -49,6 +49,28 @@ class nsNSSCertificate final : public nsIX509Cert,
   static nsresult GetDbKey(const mozilla::UniqueCERTCertificate& cert,
                            nsACString& aDbKey);
 
+  // Split a certificate chain into the root, intermediates (if any), and end
+  // entity. This method does so blindly, assuming that the current list object
+  // is ordered [end entity, intermediates..., root]. If that isn't true, this
+  // method will return the certificates at the two ends without regard to the
+  // actual chain of trust. Callers are encouraged to check, if there's any
+  // doubt.
+  // Will return error if used on self-signed or empty chains.
+  // This method requires that all arguments be empty, notably the list
+  // `aIntermediates` must be empty.
+  static nsresult SegmentCertificateChain(
+      /* int */ const nsTArray<RefPtr<nsIX509Cert>>& aCertList,
+      /* out */ nsCOMPtr<nsIX509Cert>& aRoot,
+      /* out */ nsTArray<RefPtr<nsIX509Cert>>& aIntermediates,
+      /* out */ nsCOMPtr<nsIX509Cert>& aEndEntity);
+
+  // Obtain the root certificate of a certificate chain. This method does so
+  // blindly, as SegmentCertificateChain; the same restrictions apply. On an
+  // empty list, leaves aRoot empty and returns a failure.
+  static nsresult GetRootCertificate(
+      const nsTArray<RefPtr<nsIX509Cert>>& aCertList,
+      /* out */ nsCOMPtr<nsIX509Cert>& aRoot);
+
  private:
   virtual ~nsNSSCertificate();
 

security/manager/ssl/nsSiteSecurityService.cpp

@@ -1062,17 +1062,14 @@ nsresult nsSiteSecurityService::ProcessPKPHeader(
     return NS_ERROR_FAILURE;
   }
 
-  // This copy to produce an nsNSSCertList should also be removed in Bug
+  nsTArray<RefPtr<nsIX509Cert>> nssCertList;
-  // #1406854
+  rv = nsNSSCertificateDB::ConstructCertArrayFromUniqueCertList(certList,
-  nsCOMPtr<nsIX509CertList> x509CertList =
+                                                                nssCertList);
-      new nsNSSCertList(std::move(certList));
+  if (NS_FAILED(rv)) {
-  if (!x509CertList) {
     return rv;
   }
-
-  RefPtr<nsNSSCertList> nssCertList = x509CertList->GetCertList();
   nsCOMPtr<nsIX509Cert> rootCert;
-  rv = nssCertList->GetRootCertificate(rootCert);
+  rv = nsNSSCertificate::GetRootCertificate(nssCertList, rootCert);
   if (NS_FAILED(rv)) {
     return rv;
   }