bug 1400913 - back out the functionality changes from bug 1364159 (but keep the test) r=jcj
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 18 Sep 2017 10:28:58 -0700
changeset 381564 11b46d2109c423c10ad890282acd9da08c00a409
parent 381563 6835fc2e493bf7af55ee65b48153e80be2f0ee33
child 381565 fd4481cbbd2958bd2b6339f1bb58ab8ef33b11bf
push id32532
push userarchaeopteryx@coole-files.de
push dateTue, 19 Sep 2017 09:08:57 +0000
treeherdermozilla-central@7c12af6fd620 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcj
bugs1400913, 1364159
milestone57.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 1400913 - back out the functionality changes from bug 1364159 (but keep the test) r=jcj Bug 1364159 introduced an optimization that attempted to avoid reading from the user's cached certificate database as much as possible when building a verified certificate chain. Unfortunately this had the side-effect of not preferring root certificates in path building, which can result in unnecessarily long chains (which rather defeats the purpose, since it means more signature verifications). This patch reverts the functionality changes from that bug but keeps the test that was added (the test didn't directly test the functionality changes - it's more of a check that path building will query the cached certificate db when necessary). MozReview-Commit-ID: I56THTLUytH
security/certverifier/CertVerifier.cpp
security/certverifier/CertVerifier.h
security/certverifier/NSSCertDBTrustDomain.cpp
security/certverifier/NSSCertDBTrustDomain.h
security/manager/ssl/SSLServerCertVerification.cpp
security/manager/ssl/nsNSSCallbacks.cpp
security/manager/ssl/nsNSSCertificate.cpp
security/manager/ssl/nsNSSCertificateDB.cpp
security/manager/ssl/nsNSSIOLayer.cpp
security/manager/ssl/nsSiteSecurityService.cpp
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -444,17 +444,16 @@ CertVerifier::SHA1ModeMoreRestrictiveTha
 
 static const unsigned int MIN_RSA_BITS = 2048;
 static const unsigned int MIN_RSA_BITS_WEAK = 1024;
 
 Result
 CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage,
                          Time time, void* pinArg, const char* hostname,
                  /*out*/ UniqueCERTCertList& builtChain,
-            /*optional*/ UniqueCERTCertList* peerCertChain,
             /*optional*/ const Flags flags,
             /*optional*/ const SECItem* stapledOCSPResponseSECItem,
             /*optional*/ const SECItem* sctsFromTLSSECItem,
             /*optional*/ const OriginAttributes& originAttributes,
         /*optional out*/ SECOidTag* evOidPolicy,
         /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus,
         /*optional out*/ KeySizeStatus* keySizeStatus,
         /*optional out*/ SHA1ModeResult* sha1ModeResult,
@@ -548,18 +547,17 @@ CertVerifier::VerifyCert(CERTCertificate
                                        mOCSPCache, pinArg, ocspGETConfig,
                                        mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                        mCertShortLifetimeInDays,
                                        pinningDisabled, MIN_RSA_BITS_WEAK,
                                        ValidityCheckingMode::CheckingOff,
                                        SHA1Mode::Allowed,
                                        NetscapeStepUpPolicy::NeverMatch,
                                        originAttributes,
-                                       builtChain, peerCertChain, nullptr,
-                                       nullptr);
+                                       builtChain, nullptr, nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::digitalSignature,
                           KeyPurposeId::id_kp_clientAuth,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       break;
     }
 
@@ -623,18 +621,18 @@ CertVerifier::VerifyCert(CERTCertificate
 
         NSSCertDBTrustDomain
           trustDomain(trustSSL, evOCSPFetching,
                       mOCSPCache, pinArg, ocspGETConfig,
                       mOCSPTimeoutSoft, mOCSPTimeoutHard,
                       mCertShortLifetimeInDays, mPinningMode, MIN_RSA_BITS,
                       ValidityCheckingMode::CheckForEV,
                       sha1ModeConfigurations[i], mNetscapeStepUpPolicy,
-                      originAttributes, builtChain, peerCertChain,
-                      pinningTelemetryInfo, hostname);
+                      originAttributes, builtChain, pinningTelemetryInfo,
+                      hostname);
         rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time,
                                           KeyUsage::digitalSignature,// (EC)DHE
                                           KeyUsage::keyEncipherment, // RSA
                                           KeyUsage::keyAgreement,    // (EC)DH
                                           KeyPurposeId::id_kp_serverAuth,
                                           evPolicy, stapledOCSPResponse,
                                           ocspStaplingStatus);
         if (rv == Success &&
@@ -713,18 +711,17 @@ CertVerifier::VerifyCert(CERTCertificate
                                            mOCSPCache, pinArg, ocspGETConfig,
                                            mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                            mCertShortLifetimeInDays,
                                            mPinningMode, keySizeOptions[i],
                                            ValidityCheckingMode::CheckingOff,
                                            sha1ModeConfigurations[j],
                                            mNetscapeStepUpPolicy,
                                            originAttributes, builtChain,
-                                           peerCertChain, pinningTelemetryInfo,
-                                           hostname);
+                                           pinningTelemetryInfo, hostname);
           rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time,
                                             KeyUsage::digitalSignature,//(EC)DHE
                                             KeyUsage::keyEncipherment,//RSA
                                             KeyUsage::keyAgreement,//(EC)DH
                                             KeyPurposeId::id_kp_serverAuth,
                                             CertPolicyId::anyPolicy,
                                             stapledOCSPResponse,
                                             ocspStaplingStatus);
@@ -777,36 +774,36 @@ CertVerifier::VerifyCert(CERTCertificate
     case certificateUsageSSLCA: {
       NSSCertDBTrustDomain trustDomain(trustSSL, defaultOCSPFetching,
                                        mOCSPCache, pinArg, ocspGETConfig,
                                        mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                        mCertShortLifetimeInDays,
                                        pinningDisabled, MIN_RSA_BITS_WEAK,
                                        ValidityCheckingMode::CheckingOff,
                                        SHA1Mode::Allowed, mNetscapeStepUpPolicy,
-                                       originAttributes, builtChain,
-                                       peerCertChain, nullptr, nullptr);
+                                       originAttributes, builtChain, nullptr,
+                                       nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeCA, KeyUsage::keyCertSign,
                           KeyPurposeId::id_kp_serverAuth,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       break;
     }
 
     case certificateUsageEmailSigner: {
       NSSCertDBTrustDomain trustDomain(trustEmail, defaultOCSPFetching,
                                        mOCSPCache, pinArg, ocspGETConfig,
                                        mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                        mCertShortLifetimeInDays,
                                        pinningDisabled, MIN_RSA_BITS_WEAK,
                                        ValidityCheckingMode::CheckingOff,
                                        SHA1Mode::Allowed,
                                        NetscapeStepUpPolicy::NeverMatch,
-                                       originAttributes, builtChain,
-                                       peerCertChain, nullptr, nullptr);
+                                       originAttributes, builtChain, nullptr,
+                                       nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::digitalSignature,
                           KeyPurposeId::id_kp_emailProtection,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       if (rv == Result::ERROR_INADEQUATE_KEY_USAGE) {
         rv = BuildCertChain(trustDomain, certDER, time,
                             EndEntityOrCA::MustBeEndEntity,
@@ -824,18 +821,18 @@ CertVerifier::VerifyCert(CERTCertificate
       NSSCertDBTrustDomain trustDomain(trustEmail, defaultOCSPFetching,
                                        mOCSPCache, pinArg, ocspGETConfig,
                                        mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                        mCertShortLifetimeInDays,
                                        pinningDisabled, MIN_RSA_BITS_WEAK,
                                        ValidityCheckingMode::CheckingOff,
                                        SHA1Mode::Allowed,
                                        NetscapeStepUpPolicy::NeverMatch,
-                                       originAttributes, builtChain,
-                                       peerCertChain, nullptr, nullptr);
+                                       originAttributes, builtChain, nullptr,
+                                       nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::keyEncipherment, // RSA
                           KeyPurposeId::id_kp_emailProtection,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       if (rv == Result::ERROR_INADEQUATE_KEY_USAGE) {
         rv = BuildCertChain(trustDomain, certDER, time,
                             EndEntityOrCA::MustBeEndEntity,
@@ -850,18 +847,18 @@ CertVerifier::VerifyCert(CERTCertificate
       NSSCertDBTrustDomain trustDomain(trustObjectSigning, defaultOCSPFetching,
                                        mOCSPCache, pinArg, ocspGETConfig,
                                        mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                        mCertShortLifetimeInDays,
                                        pinningDisabled, MIN_RSA_BITS_WEAK,
                                        ValidityCheckingMode::CheckingOff,
                                        SHA1Mode::Allowed,
                                        NetscapeStepUpPolicy::NeverMatch,
-                                       originAttributes, builtChain,
-                                       peerCertChain, nullptr, nullptr);
+                                       originAttributes, builtChain, nullptr,
+                                       nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::digitalSignature,
                           KeyPurposeId::id_kp_codeSigning,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       break;
     }
 
@@ -885,32 +882,32 @@ CertVerifier::VerifyCert(CERTCertificate
 
       NSSCertDBTrustDomain sslTrust(trustSSL, defaultOCSPFetching, mOCSPCache,
                                     pinArg, ocspGETConfig, mOCSPTimeoutSoft,
                                     mOCSPTimeoutHard, mCertShortLifetimeInDays,
                                     pinningDisabled, MIN_RSA_BITS_WEAK,
                                     ValidityCheckingMode::CheckingOff,
                                     SHA1Mode::Allowed,
                                     NetscapeStepUpPolicy::NeverMatch,
-                                    originAttributes, builtChain, peerCertChain,
-                                    nullptr, nullptr);
+                                    originAttributes, builtChain, nullptr,
+                                    nullptr);
       rv = BuildCertChain(sslTrust, certDER, time, endEntityOrCA,
                           keyUsage, eku, CertPolicyId::anyPolicy,
                           stapledOCSPResponse);
       if (rv == Result::ERROR_UNKNOWN_ISSUER) {
         NSSCertDBTrustDomain emailTrust(trustEmail, defaultOCSPFetching,
                                         mOCSPCache, pinArg, ocspGETConfig,
                                         mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                         mCertShortLifetimeInDays,
                                         pinningDisabled, MIN_RSA_BITS_WEAK,
                                         ValidityCheckingMode::CheckingOff,
                                         SHA1Mode::Allowed,
                                         NetscapeStepUpPolicy::NeverMatch,
-                                        originAttributes, builtChain,
-                                        peerCertChain, nullptr, nullptr);
+                                        originAttributes, builtChain, nullptr,
+                                        nullptr);
         rv = BuildCertChain(emailTrust, certDER, time, endEntityOrCA,
                             keyUsage, eku, CertPolicyId::anyPolicy,
                             stapledOCSPResponse);
         if (rv == Result::ERROR_UNKNOWN_ISSUER) {
           NSSCertDBTrustDomain objectSigningTrust(trustObjectSigning,
                                                   defaultOCSPFetching,
                                                   mOCSPCache, pinArg,
                                                   ocspGETConfig,
@@ -918,18 +915,17 @@ CertVerifier::VerifyCert(CERTCertificate
                                                   mOCSPTimeoutHard,
                                                   mCertShortLifetimeInDays,
                                                   pinningDisabled,
                                                   MIN_RSA_BITS_WEAK,
                                                   ValidityCheckingMode::CheckingOff,
                                                   SHA1Mode::Allowed,
                                                   NetscapeStepUpPolicy::NeverMatch,
                                                   originAttributes, builtChain,
-                                                  peerCertChain, nullptr,
-                                                  nullptr);
+                                                  nullptr, nullptr);
           rv = BuildCertChain(objectSigningTrust, certDER, time,
                               endEntityOrCA, keyUsage, eku,
                               CertPolicyId::anyPolicy, stapledOCSPResponse);
         }
       }
 
       break;
     }
@@ -948,17 +944,16 @@ CertVerifier::VerifyCert(CERTCertificate
 Result
 CertVerifier::VerifySSLServerCert(const UniqueCERTCertificate& peerCert,
                      /*optional*/ const SECItem* stapledOCSPResponse,
                      /*optional*/ const SECItem* sctsFromTLS,
                                   Time time,
                      /*optional*/ void* pinarg,
                                   const nsACString& hostname,
                           /*out*/ UniqueCERTCertList& builtChain,
-                     /*optional*/ UniqueCERTCertList* peerCertChain,
                      /*optional*/ bool saveIntermediatesInPermanentDatabase,
                      /*optional*/ Flags flags,
                      /*optional*/ const OriginAttributes& originAttributes,
                  /*optional out*/ SECOidTag* evOidPolicy,
                  /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus,
                  /*optional out*/ KeySizeStatus* keySizeStatus,
                  /*optional out*/ SHA1ModeResult* sha1ModeResult,
                  /*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo,
@@ -975,20 +970,20 @@ CertVerifier::VerifySSLServerCert(const 
   if (hostname.IsEmpty()) {
     return Result::ERROR_BAD_CERT_DOMAIN;
   }
 
   // CreateCertErrorRunnable assumes that CheckCertHostname is only called
   // if VerifyCert succeeded.
   Result rv = VerifyCert(peerCert.get(), certificateUsageSSLServer, time,
                          pinarg, PromiseFlatCString(hostname).get(), builtChain,
-                         peerCertChain, flags,
-                         stapledOCSPResponse, sctsFromTLS, originAttributes,
-                         evOidPolicy, ocspStaplingStatus, keySizeStatus,
-                         sha1ModeResult, pinningTelemetryInfo, ctInfo);
+                         flags, stapledOCSPResponse, sctsFromTLS,
+                         originAttributes, evOidPolicy, ocspStaplingStatus,
+                         keySizeStatus, sha1ModeResult, pinningTelemetryInfo,
+                         ctInfo);
   if (rv != Success) {
     return rv;
   }
 
   Input peerCertInput;
   rv = peerCertInput.Init(peerCert->derCert.data, peerCert->derCert.len);
   if (rv != Success) {
     return rv;
--- a/security/certverifier/CertVerifier.h
+++ b/security/certverifier/CertVerifier.h
@@ -111,32 +111,26 @@ public:
   enum OCSPStaplingStatus {
     OCSP_STAPLING_NEVER_CHECKED = 0,
     OCSP_STAPLING_GOOD = 1,
     OCSP_STAPLING_NONE = 2,
     OCSP_STAPLING_EXPIRED = 3,
     OCSP_STAPLING_INVALID = 4,
   };
 
-  // As an optimization, a pointer to the certificate chain sent by the peer
-  // may be specified as peerCertChain. This can prevent NSSCertDBTrustDomain
-  // from calling CERT_CreateSubjectCertList to find potential issuers, which
-  // can be expensive.
-  //
   // *evOidPolicy == SEC_OID_UNKNOWN means the cert is NOT EV
   // Only one usage per verification is supported.
   mozilla::pkix::Result VerifyCert(
                     CERTCertificate* cert,
                     SECCertificateUsage usage,
                     mozilla::pkix::Time time,
                     void* pinArg,
                     const char* hostname,
             /*out*/ UniqueCERTCertList& builtChain,
-    /*optional in*/ UniqueCERTCertList* peerCertChain = nullptr,
-    /*optional in*/ Flags flags = 0,
+                    Flags flags = 0,
     /*optional in*/ const SECItem* stapledOCSPResponse = nullptr,
     /*optional in*/ const SECItem* sctsFromTLS = nullptr,
     /*optional in*/ const OriginAttributes& originAttributes =
                       OriginAttributes(),
    /*optional out*/ SECOidTag* evOidPolicy = nullptr,
    /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus = nullptr,
    /*optional out*/ KeySizeStatus* keySizeStatus = nullptr,
    /*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr,
@@ -146,17 +140,16 @@ public:
   mozilla::pkix::Result VerifySSLServerCert(
                     const UniqueCERTCertificate& peerCert,
        /*optional*/ const SECItem* stapledOCSPResponse,
        /*optional*/ const SECItem* sctsFromTLS,
                     mozilla::pkix::Time time,
        /*optional*/ void* pinarg,
                     const nsACString& hostname,
             /*out*/ UniqueCERTCertList& builtChain,
-       /*optional*/ UniqueCERTCertList* peerCertChain = nullptr,
        /*optional*/ bool saveIntermediatesInPermanentDatabase = false,
        /*optional*/ Flags flags = 0,
        /*optional*/ const OriginAttributes& originAttributes =
                       OriginAttributes(),
    /*optional out*/ SECOidTag* evOidPolicy = nullptr,
    /*optional out*/ OCSPStaplingStatus* ocspStaplingStatus = nullptr,
    /*optional out*/ KeySizeStatus* keySizeStatus = nullptr,
    /*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr,
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -55,17 +55,16 @@ NSSCertDBTrustDomain::NSSCertDBTrustDoma
                                            uint32_t certShortLifetimeInDays,
                                            CertVerifier::PinningMode pinningMode,
                                            unsigned int minRSABits,
                                            ValidityCheckingMode validityCheckingMode,
                                            CertVerifier::SHA1Mode sha1Mode,
                                            NetscapeStepUpPolicy netscapeStepUpPolicy,
                                            const OriginAttributes& originAttributes,
                                            UniqueCERTCertList& builtChain,
-                              /*optional*/ UniqueCERTCertList* peerCertChain,
                               /*optional*/ PinningTelemetryInfo* pinningTelemetryInfo,
                               /*optional*/ const char* hostname)
   : mCertDBTrustType(certDBTrustType)
   , mOCSPFetching(ocspFetching)
   , mOCSPCache(ocspCache)
   , mPinArg(pinArg)
   , mOCSPGetConfig(ocspGETConfig)
   , mOCSPTimeoutSoft(ocspTimeoutSoft)
@@ -73,17 +72,16 @@ NSSCertDBTrustDomain::NSSCertDBTrustDoma
   , mCertShortLifetimeInDays(certShortLifetimeInDays)
   , mPinningMode(pinningMode)
   , mMinRSABits(minRSABits)
   , mValidityCheckingMode(validityCheckingMode)
   , mSHA1Mode(sha1Mode)
   , mNetscapeStepUpPolicy(netscapeStepUpPolicy)
   , mOriginAttributes(originAttributes)
   , mBuiltChain(builtChain)
-  , mPeerCertChain(peerCertChain)
   , mPinningTelemetryInfo(pinningTelemetryInfo)
   , mHostname(hostname)
   , mCertBlocklist(do_GetService(NS_CERTBLOCKLIST_CONTRACTID))
   , mOCSPStaplingStatus(CertVerifier::OCSP_STAPLING_NEVER_CHECKED)
   , mSCTListFromCertificate()
   , mSCTListFromOCSPStapling()
 {
 }
@@ -138,113 +136,30 @@ FindIssuerInner(const UniqueCERTCertList
     if (!keepGoing) {
       break;
     }
   }
 
   return Success;
 }
 
-// Remove from newCandidates any CERTCertificates in alreadyTried.
-// alreadyTried is likely to be small or empty.
-static void
-RemoveCandidatesAlreadyTried(UniqueCERTCertList& newCandidates,
-                             const UniqueCERTCertList& alreadyTried)
-{
-  for (const CERTCertListNode* triedNode = CERT_LIST_HEAD(alreadyTried);
-       !CERT_LIST_END(triedNode, alreadyTried);
-       triedNode = CERT_LIST_NEXT(triedNode)) {
-    CERTCertListNode* newNode = CERT_LIST_HEAD(newCandidates);
-    while (!CERT_LIST_END(newNode, newCandidates)) {
-      CERTCertListNode* savedNode = CERT_LIST_NEXT(newNode);
-      if (CERT_CompareCerts(triedNode->cert, newNode->cert)) {
-        CERT_RemoveCertListNode(newNode);
-      }
-      newNode = savedNode;
-    }
-  }
-}
-
-// Add to matchingCandidates any CERTCertificates from candidatesIn that have a
-// DER-encoded subject name equal to the given subject name.
-static Result
-AddMatchingCandidates(UniqueCERTCertList& matchingCandidates,
-                      const UniqueCERTCertList& candidatesIn,
-                      Input subjectName)
-{
-  for (const CERTCertListNode* node = CERT_LIST_HEAD(candidatesIn);
-       !CERT_LIST_END(node, candidatesIn); node = CERT_LIST_NEXT(node)) {
-    Input candidateSubjectName;
-    Result rv = candidateSubjectName.Init(node->cert->derSubject.data,
-                                          node->cert->derSubject.len);
-    if (rv != Success) {
-      continue; // probably just too big - continue processing other candidates
-    }
-    if (InputsAreEqual(candidateSubjectName, subjectName)) {
-      UniqueCERTCertificate certDuplicate(CERT_DupCertificate(node->cert));
-      if (!certDuplicate) {
-        return Result::FATAL_ERROR_NO_MEMORY;
-      }
-      SECStatus srv = CERT_AddCertToListTail(matchingCandidates.get(),
-                                             certDuplicate.get());
-      if (srv != SECSuccess) {
-        return MapPRErrorCodeToResult(PR_GetError());
-      }
-      // matchingCandidates now owns certDuplicate
-      Unused << certDuplicate.release();
-    }
-  }
-  return Success;
-}
-
 Result
 NSSCertDBTrustDomain::FindIssuer(Input encodedIssuerName,
                                  IssuerChecker& checker, Time)
 {
-  // If the peer certificate chain was specified, try to use it before falling
-  // back to CERT_CreateSubjectCertList.
-  bool keepGoing;
-  UniqueCERTCertList peerCertChainCandidates(CERT_NewCertList());
-  if (!peerCertChainCandidates) {
-    return Result::FATAL_ERROR_NO_MEMORY;
-  }
-  if (mPeerCertChain) {
-    // Build a candidate list that consists only of certificates with a subject
-    // matching the issuer we're looking for.
-    Result rv = AddMatchingCandidates(peerCertChainCandidates, *mPeerCertChain,
-                                      encodedIssuerName);
-    if (rv != Success) {
-      return rv;
-    }
-    rv = FindIssuerInner(peerCertChainCandidates, true, encodedIssuerName,
-                         checker, keepGoing);
-    if (rv != Success) {
-      return rv;
-    }
-    if (keepGoing) {
-      rv = FindIssuerInner(peerCertChainCandidates, false, encodedIssuerName,
-                           checker, keepGoing);
-      if (rv != Success) {
-        return rv;
-      }
-    }
-    if (!keepGoing) {
-      return Success;
-    }
-  }
   // TODO: NSS seems to be ambiguous between "no potential issuers found" and
   // "there was an error trying to retrieve the potential issuers."
   SECItem encodedIssuerNameItem = UnsafeMapInputToSECItem(encodedIssuerName);
   UniqueCERTCertList
     candidates(CERT_CreateSubjectCertList(nullptr, CERT_GetDefaultCertDB(),
                                           &encodedIssuerNameItem, 0,
                                           false));
   if (candidates) {
-    RemoveCandidatesAlreadyTried(candidates, peerCertChainCandidates);
     // First, try all the root certs; then try all the non-root certs.
+    bool keepGoing;
     Result rv = FindIssuerInner(candidates, true, encodedIssuerName, checker,
                                 keepGoing);
     if (rv != Success) {
       return rv;
     }
     if (keepGoing) {
       rv = FindIssuerInner(candidates, false, encodedIssuerName, checker,
                            keepGoing);
--- a/security/certverifier/NSSCertDBTrustDomain.h
+++ b/security/certverifier/NSSCertDBTrustDomain.h
@@ -84,17 +84,16 @@ public:
                        uint32_t certShortLifetimeInDays,
                        CertVerifier::PinningMode pinningMode,
                        unsigned int minRSABits,
                        ValidityCheckingMode validityCheckingMode,
                        CertVerifier::SHA1Mode sha1Mode,
                        NetscapeStepUpPolicy netscapeStepUpPolicy,
                        const OriginAttributes& originAttributes,
                        UniqueCERTCertList& builtChain,
-          /*optional*/ UniqueCERTCertList* peerCertChain = nullptr,
           /*optional*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr,
           /*optional*/ const char* hostname = nullptr);
 
   virtual Result FindIssuer(mozilla::pkix::Input encodedIssuerName,
                             IssuerChecker& checker,
                             mozilla::pkix::Time time) override;
 
   virtual Result GetCertTrust(mozilla::pkix::EndEntityOrCA endEntityOrCA,
@@ -194,17 +193,16 @@ private:
   const uint32_t mCertShortLifetimeInDays;
   CertVerifier::PinningMode mPinningMode;
   const unsigned int mMinRSABits;
   ValidityCheckingMode mValidityCheckingMode;
   CertVerifier::SHA1Mode mSHA1Mode;
   NetscapeStepUpPolicy mNetscapeStepUpPolicy;
   const OriginAttributes& mOriginAttributes;
   UniqueCERTCertList& mBuiltChain; // non-owning
-  UniqueCERTCertList* mPeerCertChain; // non-owning
   PinningTelemetryInfo* mPinningTelemetryInfo;
   const char* mHostname; // non-owning - only used for pinning checks
   nsCOMPtr<nsICertBlocklist> mCertBlocklist;
   CertVerifier::OCSPStaplingStatus mOCSPStaplingStatus;
   // Certificate Transparency data extracted during certificate verification
   UniqueSECItem mSCTListFromCertificate;
   UniqueSECItem mSCTListFromOCSPStapling;
 };
--- a/security/manager/ssl/SSLServerCertVerification.cpp
+++ b/security/manager/ssl/SSLServerCertVerification.cpp
@@ -1407,19 +1407,18 @@ AuthCertificate(CertVerifier& certVerifi
       !infoObject->SharedState().IsOCSPMustStapleEnabled()) {
     flags |= CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
   }
 
   Result rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse,
                                                sctsFromTLSExtension, time,
                                                infoObject,
                                                infoObject->GetHostName(),
-                                               certList, &peerCertChain,
-                                               saveIntermediates, flags,
-                                               infoObject->
+                                               certList, saveIntermediates,
+                                               flags, infoObject->
                                                       GetOriginAttributes(),
                                                &evOidPolicy,
                                                &ocspStaplingStatus,
                                                &keySizeStatus, &sha1ModeResult,
                                                &pinningTelemetryInfo,
                                                &certificateTransparencyInfo);
 
   uint32_t evStatus = (rv != Success) ? 0                   // 0 = Failure
--- a/security/manager/ssl/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/nsNSSCallbacks.cpp
@@ -1185,23 +1185,16 @@ DetermineEVAndCTStatusAndSetNewCert(RefP
   }
 
   UniqueCERTCertificate cert(SSL_PeerCertificate(fd));
   MOZ_ASSERT(cert, "SSL_PeerCertificate failed in TLS handshake callback?");
   if (!cert) {
     return;
   }
 
-  UniqueCERTCertList peerCertChain(SSL_PeerCertificateChain(fd));
-  MOZ_ASSERT(peerCertChain,
-             "SSL_PeerCertificateChain failed in TLS handshake callback?");
-  if (!peerCertChain) {
-    return;
-  }
-
   RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
   MOZ_ASSERT(certVerifier,
              "Certificate verifier uninitialized in TLS handshake callback?");
   if (!certVerifier) {
     return;
   }
 
   // We don't own these pointers.
@@ -1233,17 +1226,16 @@ DetermineEVAndCTStatusAndSetNewCert(RefP
   mozilla::pkix::Result rv = certVerifier->VerifySSLServerCert(
     cert,
     stapledOCSPResponse,
     sctsFromTLSExtension,
     mozilla::pkix::Now(),
     infoObject,
     infoObject->GetHostName(),
     unusedBuiltChain,
-    &peerCertChain,
     saveIntermediates,
     flags,
     infoObject->GetOriginAttributes(),
     &evOidPolicy,
     nullptr, // OCSP stapling telemetry
     nullptr, // key size telemetry
     nullptr, // SHA-1 telemetry
     nullptr, // pinning telemetry
--- a/security/manager/ssl/nsNSSCertificate.cpp
+++ b/security/manager/ssl/nsNSSCertificate.cpp
@@ -658,17 +658,16 @@ nsNSSCertificate::GetChain(nsIArray** _r
 
   UniqueCERTCertList nssChain;
   // We want to test all usages, but we start with server because most of the
   // time Firefox users care about server certs.
   if (certVerifier->VerifyCert(mCert.get(), certificateUsageSSLServer, now,
                                nullptr, /*XXX fixme*/
                                nullptr, /* hostname */
                                nssChain,
-                               nullptr, // no peerCertChain
                                CertVerifier::FLAG_LOCAL_ONLY)
         != mozilla::pkix::Success) {
     nssChain = nullptr;
     // keep going
   }
 
   // This is the whitelist of all non-SSLServer usages that are supported by
   // verifycert.
@@ -683,17 +682,16 @@ nsNSSCertificate::GetChain(nsIArray** _r
        usage = usage << 1) {
     if ((usage & otherUsagesToTest) == 0) {
       continue;
     }
     if (certVerifier->VerifyCert(mCert.get(), usage, now,
                                  nullptr, /*XXX fixme*/
                                  nullptr, /*hostname*/
                                  nssChain,
-                                 nullptr, // no peerCertChain
                                  CertVerifier::FLAG_LOCAL_ONLY)
           != mozilla::pkix::Success) {
       nssChain = nullptr;
       // keep going
     }
   }
 
   if (!nssChain) {
--- a/security/manager/ssl/nsNSSCertificateDB.cpp
+++ b/security/manager/ssl/nsNSSCertificateDB.cpp
@@ -1423,29 +1423,27 @@ VerifyCertAtTime(nsIX509Cert* aCert,
   if (!aHostname.IsVoid() && aUsage == certificateUsageSSLServer) {
     result = certVerifier->VerifySSLServerCert(nssCert,
                                                nullptr, // stapledOCSPResponse
                                                nullptr, // sctsFromTLSExtension
                                                aTime,
                                                nullptr, // Assume no context
                                                aHostname,
                                                resultChain,
-                                               nullptr, // no peerCertChain
                                                false, // don't save intermediates
                                                aFlags,
                                                OriginAttributes(),
                                                &evOidPolicy);
   } else {
     const nsCString& flatHostname = PromiseFlatCString(aHostname);
     result = certVerifier->VerifyCert(nssCert.get(), aUsage, aTime,
                                       nullptr, // Assume no context
                                       aHostname.IsVoid() ? nullptr
                                                          : flatHostname.get(),
                                       resultChain,
-                                      nullptr, // no peerCertChain
                                       aFlags,
                                       nullptr, // stapledOCSPResponse
                                       nullptr, // sctsFromTLSExtension
                                       OriginAttributes(),
                                       &evOidPolicy);
   }
 
   nsCOMPtr<nsIX509CertList> nssCertList;
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -506,17 +506,16 @@ nsNSSSocketInfo::IsAcceptableForHost(con
   mozilla::pkix::Result result =
     certVerifier->VerifySSLServerCert(nssCert,
                                       nullptr, // stapledOCSPResponse
                                       nullptr, // sctsFromTLSExtension
                                       mozilla::pkix::Now(),
                                       nullptr, // pinarg
                                       hostname,
                                       unusedBuiltChain,
-                                      nullptr, // no peerCertChain
                                       false, // save intermediates
                                       flags);
   if (result != mozilla::pkix::Success) {
     return NS_OK;
   }
 
   // All tests pass
   *_retval = true;
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -1138,17 +1138,16 @@ nsSiteSecurityService::ProcessPKPHeader(
   CertVerifier::Flags flags = CertVerifier::FLAG_LOCAL_ONLY |
                               CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
   if (certVerifier->VerifySSLServerCert(nssCert,
                                         nullptr, // stapledOCSPResponse
                                         nullptr, // sctsFromTLSExtension
                                         now, nullptr, // pinarg
                                         host, // hostname
                                         certList,
-                                        nullptr, // no peerCertChain
                                         false, // don't store intermediates
                                         flags,
                                         aOriginAttributes)
         != mozilla::pkix::Success) {
     return NS_ERROR_FAILURE;
   }
 
   CERTCertListNode* rootNode = CERT_LIST_TAIL(certList);