Bug 1580318 - Remove nsIX509CertList from verifyCertFinished r=keeler
authorSean Feng <sefeng@mozilla.com>
Thu, 07 Nov 2019 14:35:16 +0000
changeset 501098 cbaf22ddc3f3be784934118a3e5d18801211be2c
parent 501097 29e5b37c795db33a500c8ba37ac3fbb4be2ce9b6
child 501099 02538f9dceed8e940abfe9cc8eb56ffee1dab2ec
push id99987
push usersefeng@mozilla.com
push dateThu, 07 Nov 2019 14:46:24 +0000
treeherderautoland@cbaf22ddc3f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1580318
milestone72.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 1580318 - Remove nsIX509CertList from verifyCertFinished r=keeler Differential Revision: https://phabricator.services.mozilla.com/D44244
security/manager/pki/resources/content/pippki.js
security/manager/ssl/nsIX509CertDB.idl
security/manager/ssl/nsNSSCertificateDB.cpp
--- a/security/manager/pki/resources/content/pippki.js
+++ b/security/manager/pki/resources/content/pippki.js
@@ -293,15 +293,13 @@ function getBestChain(results) {
  *          certificate chain for the given usage, or null if there is none.
  */
 function getChainForUsage(results, usage) {
   for (let result of results) {
     if (
       certificateUsages[result.usageString] == usage &&
       result.errorCode == PRErrorCodeSuccess
     ) {
-      // The chain attribute here is generated by the VerifyCertAtTime API,
-      // and it is a nsIX509CertList, so we have to enumerate it.
-      return Array.from(result.chain.getEnumerator());
+      return result.chain;
     }
   }
   return null;
 }
--- a/security/manager/ssl/nsIX509CertDB.idl
+++ b/security/manager/ssl/nsIX509CertDB.idl
@@ -6,17 +6,16 @@
 
 #include "nsISupports.idl"
 
 interface nsIArray;
 interface nsIX509Cert;
 interface nsIFile;
 interface nsIInterfaceRequestor;
 interface nsIZipReader;
-interface nsIX509CertList;
 interface nsIInputStream;
 
 %{C++
 #define NS_X509CERTDB_CONTRACTID "@mozilla.org/security/x509certdb;1"
 %}
 
 typedef uint32_t AppTrustedRoot;
 
@@ -35,17 +34,17 @@ interface nsIOpenSignedAppFileCallback :
  * represents whether or not the end-entity certificate verified as EV.
  * If aPRErrorCode is non-zero, it represents the error encountered during
  * verification. aVerifiedChain is null in that case and aHasEVPolicy has no
  * meaning.
  */
 [scriptable, function, uuid(49e16fc8-efac-4f57-8361-956ef6b960a4)]
 interface nsICertVerificationCallback : nsISupports {
   void verifyCertFinished(in int32_t aPRErrorCode,
-                          in nsIX509CertList aVerifiedChain,
+                          in Array<nsIX509Cert> aVerifiedChain,
                           in bool aHasEVPolicy);
 };
 
 /**
  * This represents a service to access and manipulate
  * X.509 certificates stored in a database.
  */
 [scriptable, uuid(5c16cd9b-5a73-47f1-ab0f-11ede7495cce)]
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -1152,24 +1152,27 @@ nsNSSCertificateDB::GetCerts(nsTArray<Re
   return nsNSSCertificateDB::ConstructCertArrayFromUniqueCertList(certList,
                                                                   _retval);
 }
 
 nsresult VerifyCertAtTime(nsIX509Cert* aCert,
                           int64_t /*SECCertificateUsage*/ aUsage,
                           uint32_t aFlags, const nsACString& aHostname,
                           mozilla::pkix::Time aTime,
-                          nsIX509CertList** aVerifiedChain, bool* aHasEVPolicy,
+                          nsTArray<RefPtr<nsIX509Cert>>& aVerifiedChain,
+                          bool* aHasEVPolicy,
                           int32_t* /*PRErrorCode*/ _retval) {
   NS_ENSURE_ARG_POINTER(aCert);
   NS_ENSURE_ARG_POINTER(aHasEVPolicy);
-  NS_ENSURE_ARG_POINTER(aVerifiedChain);
   NS_ENSURE_ARG_POINTER(_retval);
 
-  *aVerifiedChain = nullptr;
+  if (!aVerifiedChain.IsEmpty()) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
   *aHasEVPolicy = false;
   *_retval = PR_UNKNOWN_ERROR;
 
   UniqueCERTCertificate nssCert(aCert->GetCert());
   if (!nssCert) {
     return NS_ERROR_INVALID_ARG;
   }
 
@@ -1198,26 +1201,30 @@ nsresult VerifyCertAtTime(nsIX509Cert* a
         nullptr,  // Assume no context
         aHostname.IsVoid() ? nullptr : flatHostname.get(), resultChain, aFlags,
         Nothing(),  // extraCertificates
         Nothing(),  // stapledOCSPResponse
         Nothing(),  // sctsFromTLSExtension
         OriginAttributes(), &evOidPolicy);
   }
 
-  nsCOMPtr<nsIX509CertList> nssCertList;
-  // This adopts the list
-  nssCertList = new nsNSSCertList(std::move(resultChain));
-  NS_ENSURE_TRUE(nssCertList, NS_ERROR_FAILURE);
+  if (result == mozilla::pkix::Success) {
+    nsresult rv = nsNSSCertificateDB::ConstructCertArrayFromUniqueCertList(
+        resultChain, aVerifiedChain);
+
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    if (evOidPolicy != SEC_OID_UNKNOWN) {
+      *aHasEVPolicy = true;
+    }
+  }
 
   *_retval = mozilla::pkix::MapResultToPRErrorCode(result);
-  if (result == mozilla::pkix::Success && evOidPolicy != SEC_OID_UNKNOWN) {
-    *aHasEVPolicy = true;
-  }
-  nssCertList.forget(aVerifiedChain);
 
   return NS_OK;
 }
 
 class VerifyCertAtTimeTask final : public CryptoTask {
  public:
   VerifyCertAtTimeTask(nsIX509Cert* aCert, int64_t aUsage, uint32_t aFlags,
                        const nsACString& aHostname, uint64_t aTime,
@@ -1225,49 +1232,48 @@ class VerifyCertAtTimeTask final : publi
       : mCert(aCert),
         mUsage(aUsage),
         mFlags(aFlags),
         mHostname(aHostname),
         mTime(aTime),
         mCallback(new nsMainThreadPtrHolder<nsICertVerificationCallback>(
             "nsICertVerificationCallback", aCallback)),
         mPRErrorCode(SEC_ERROR_LIBRARY_FAILURE),
-        mVerifiedCertList(nullptr),
         mHasEVPolicy(false) {}
 
  private:
   virtual nsresult CalculateResult() override {
     nsCOMPtr<nsIX509CertDB> certDB = do_GetService(NS_X509CERTDB_CONTRACTID);
     if (!certDB) {
       return NS_ERROR_FAILURE;
     }
     return VerifyCertAtTime(mCert, mUsage, mFlags, mHostname,
                             mozilla::pkix::TimeFromEpochInSeconds(mTime),
-                            getter_AddRefs(mVerifiedCertList), &mHasEVPolicy,
-                            &mPRErrorCode);
+                            mVerifiedCertList, &mHasEVPolicy, &mPRErrorCode);
   }
 
   virtual void CallCallback(nsresult rv) override {
     if (NS_FAILED(rv)) {
-      Unused << mCallback->VerifyCertFinished(SEC_ERROR_LIBRARY_FAILURE,
-                                              nullptr, false);
+      nsTArray<RefPtr<nsIX509Cert>> tmp;
+      Unused << mCallback->VerifyCertFinished(SEC_ERROR_LIBRARY_FAILURE, tmp,
+                                              false);
     } else {
       Unused << mCallback->VerifyCertFinished(mPRErrorCode, mVerifiedCertList,
                                               mHasEVPolicy);
     }
   }
 
   nsCOMPtr<nsIX509Cert> mCert;
   int64_t mUsage;
   uint32_t mFlags;
   nsCString mHostname;
   uint64_t mTime;
   nsMainThreadPtrHandle<nsICertVerificationCallback> mCallback;
   int32_t mPRErrorCode;
-  nsCOMPtr<nsIX509CertList> mVerifiedCertList;
+  nsTArray<RefPtr<nsIX509Cert>> mVerifiedCertList;
   bool mHasEVPolicy;
 };
 
 NS_IMETHODIMP
 nsNSSCertificateDB::AsyncVerifyCertAtTime(
     nsIX509Cert* aCert, int64_t /*SECCertificateUsage*/ aUsage, uint32_t aFlags,
     const nsACString& aHostname, uint64_t aTime,
     nsICertVerificationCallback* aCallback) {