bug 1364159 - potentially avoid calling CERT_CreateSubjectCertList in NSSCertDBTrustDomain::FindIssuer r?Cykesiopka,jcj draft
authorDavid Keeler <dkeeler@mozilla.com>
Thu, 11 May 2017 16:41:12 -0700
changeset 582477 8fc1aa8e6f56957074dec91cb66f94d888758b0e
parent 582447 8f4d2d35cb317235f30f4e3738ad3df16d2f9f3f
child 629794 2ef5118f09f76b489b6f5201ece66f5067861ae9
push id60108
push userbmo:dkeeler@mozilla.com
push dateMon, 22 May 2017 18:02:03 +0000
reviewersCykesiopka, jcj
bugs1364159, 731478
milestone55.0a1
bug 1364159 - potentially avoid calling CERT_CreateSubjectCertList in NSSCertDBTrustDomain::FindIssuer r?Cykesiopka,jcj CERT_CreateSubjectCertList is not an inexpensive function call, since it enumerates the certificate database (i.e. reads from disk a lot). If we're verifying for a TLS handshake, however, we should already have in memory a certificate chain sent by the peer (there are some cases where we won't, such as session resumption (see bug 731478)). If we can, we should use those certificates before falling back to calling CERT_CreateSubjectCertList. MozReview-Commit-ID: ASjVGsELb1O
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
security/manager/ssl/tests/unit/bad_certs/ee-from-missing-intermediate.pem
security/manager/ssl/tests/unit/bad_certs/ee-from-missing-intermediate.pem.certspec
security/manager/ssl/tests/unit/bad_certs/moz.build
security/manager/ssl/tests/unit/moz.build
security/manager/ssl/tests/unit/test_missing_intermediate.js
security/manager/ssl/tests/unit/test_missing_intermediate/missing-intermediate.pem
security/manager/ssl/tests/unit/test_missing_intermediate/missing-intermediate.pem.certspec
security/manager/ssl/tests/unit/test_missing_intermediate/moz.build
security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
security/manager/ssl/tests/unit/xpcshell.ini
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -439,16 +439,17 @@ 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,
@@ -538,17 +539,18 @@ CertVerifier::VerifyCert(CERTCertificate
                                        mOCSPCache, pinArg, ocspGETConfig,
                                        mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                        mCertShortLifetimeInDays,
                                        pinningDisabled, MIN_RSA_BITS_WEAK,
                                        ValidityCheckingMode::CheckingOff,
                                        SHA1Mode::Allowed,
                                        NetscapeStepUpPolicy::NeverMatch,
                                        originAttributes,
-                                       builtChain, nullptr, nullptr);
+                                       builtChain, peerCertChain, nullptr,
+                                       nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::digitalSignature,
                           KeyPurposeId::id_kp_clientAuth,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       break;
     }
 
@@ -612,18 +614,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, pinningTelemetryInfo,
-                      hostname);
+                      originAttributes, builtChain, peerCertChain,
+                      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 &&
@@ -702,17 +704,18 @@ CertVerifier::VerifyCert(CERTCertificate
                                            mOCSPCache, pinArg, ocspGETConfig,
                                            mOCSPTimeoutSoft, mOCSPTimeoutHard,
                                            mCertShortLifetimeInDays,
                                            mPinningMode, keySizeOptions[i],
                                            ValidityCheckingMode::CheckingOff,
                                            sha1ModeConfigurations[j],
                                            mNetscapeStepUpPolicy,
                                            originAttributes, builtChain,
-                                           pinningTelemetryInfo, hostname);
+                                           peerCertChain, 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);
@@ -765,36 +768,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, nullptr,
-                                       nullptr);
+                                       originAttributes, builtChain,
+                                       peerCertChain, 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, nullptr,
-                                       nullptr);
+                                       originAttributes, builtChain,
+                                       peerCertChain, 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,
@@ -812,18 +815,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, nullptr,
-                                       nullptr);
+                                       originAttributes, builtChain,
+                                       peerCertChain, 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,
@@ -838,18 +841,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, nullptr,
-                                       nullptr);
+                                       originAttributes, builtChain,
+                                       peerCertChain, nullptr, nullptr);
       rv = BuildCertChain(trustDomain, certDER, time,
                           EndEntityOrCA::MustBeEndEntity,
                           KeyUsage::digitalSignature,
                           KeyPurposeId::id_kp_codeSigning,
                           CertPolicyId::anyPolicy, stapledOCSPResponse);
       break;
     }
 
@@ -873,32 +876,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, nullptr,
-                                    nullptr);
+                                    originAttributes, builtChain, peerCertChain,
+                                    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, nullptr,
-                                        nullptr);
+                                        originAttributes, builtChain,
+                                        peerCertChain, 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,
@@ -906,17 +909,18 @@ CertVerifier::VerifyCert(CERTCertificate
                                                   mOCSPTimeoutHard,
                                                   mCertShortLifetimeInDays,
                                                   pinningDisabled,
                                                   MIN_RSA_BITS_WEAK,
                                                   ValidityCheckingMode::CheckingOff,
                                                   SHA1Mode::Allowed,
                                                   NetscapeStepUpPolicy::NeverMatch,
                                                   originAttributes, builtChain,
-                                                  nullptr, nullptr);
+                                                  peerCertChain, nullptr,
+                                                  nullptr);
           rv = BuildCertChain(objectSigningTrust, certDER, time,
                               endEntityOrCA, keyUsage, eku,
                               CertPolicyId::anyPolicy, stapledOCSPResponse);
         }
       }
 
       break;
     }
@@ -935,16 +939,17 @@ CertVerifier::VerifyCert(CERTCertificate
 Result
 CertVerifier::VerifySSLServerCert(const UniqueCERTCertificate& peerCert,
                      /*optional*/ const SECItem* stapledOCSPResponse,
                      /*optional*/ const SECItem* sctsFromTLS,
                                   Time time,
                      /*optional*/ void* pinarg,
                                   const char* 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,
@@ -961,17 +966,17 @@ CertVerifier::VerifySSLServerCert(const 
 
   if (!hostname || !hostname[0]) {
     return Result::ERROR_BAD_CERT_DOMAIN;
   }
 
   // CreateCertErrorRunnable assumes that CheckCertHostname is only called
   // if VerifyCert succeeded.
   Result rv = VerifyCert(peerCert.get(), certificateUsageSSLServer, time,
-                         pinarg, hostname, builtChain, flags,
+                         pinarg, hostname, builtChain, peerCertChain, flags,
                          stapledOCSPResponse, sctsFromTLS, originAttributes,
                          evOidPolicy, ocspStaplingStatus, keySizeStatus,
                          sha1ModeResult, pinningTelemetryInfo, ctInfo);
   if (rv != Success) {
     return rv;
   }
 
   Input peerCertInput;
--- a/security/certverifier/CertVerifier.h
+++ b/security/certverifier/CertVerifier.h
@@ -110,26 +110,32 @@ 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,
-                    Flags flags = 0,
+    /*optional in*/ UniqueCERTCertList* peerCertChain = nullptr,
+    /*optional in*/ 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,
@@ -139,16 +145,17 @@ public:
   mozilla::pkix::Result VerifySSLServerCert(
                     const UniqueCERTCertificate& peerCert,
        /*optional*/ const SECItem* stapledOCSPResponse,
        /*optional*/ const SECItem* sctsFromTLS,
                     mozilla::pkix::Time time,
        /*optional*/ void* pinarg,
                     const char* 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
@@ -54,16 +54,17 @@ 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)
@@ -71,16 +72,17 @@ 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()
 {
 }
@@ -135,30 +137,113 @@ 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
@@ -83,16 +83,17 @@ 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,
@@ -192,16 +193,17 @@ 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
@@ -1399,18 +1399,19 @@ AuthCertificate(CertVerifier& certVerifi
       !infoObject->SharedState().IsOCSPMustStapleEnabled()) {
     flags |= CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
   }
 
   Result rv = certVerifier.VerifySSLServerCert(cert, stapledOCSPResponse,
                                                sctsFromTLSExtension, time,
                                                infoObject,
                                                infoObject->GetHostNameRaw(),
-                                               certList, saveIntermediates,
-                                               flags, infoObject->
+                                               certList, &peerCertChain,
+                                               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
@@ -1081,16 +1081,23 @@ 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.
@@ -1122,16 +1129,17 @@ DetermineEVAndCTStatusAndSetNewCert(RefP
   mozilla::pkix::Result rv = certVerifier->VerifySSLServerCert(
     cert,
     stapledOCSPResponse,
     sctsFromTLSExtension,
     mozilla::pkix::Now(),
     infoObject,
     infoObject->GetHostNameRaw(),
     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
@@ -659,16 +659,17 @@ 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,16 +684,17 @@ 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
@@ -1404,26 +1404,28 @@ VerifyCertAtTime(nsIX509Cert* aCert,
   if (!aHostname.IsVoid() && aUsage == certificateUsageSSLServer) {
     result = certVerifier->VerifySSLServerCert(nssCert,
                                                nullptr, // stapledOCSPResponse
                                                nullptr, // sctsFromTLSExtension
                                                aTime,
                                                nullptr, // Assume no context
                                                flatHostname.get(),
                                                resultChain,
+                                               nullptr, // no peerCertChain
                                                false, // don't save intermediates
                                                aFlags,
                                                OriginAttributes(),
                                                &evOidPolicy);
   } else {
     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
@@ -456,16 +456,17 @@ nsNSSSocketInfo::IsAcceptableForHost(con
   mozilla::pkix::Result result =
     certVerifier->VerifySSLServerCert(nssCert,
                                       nullptr, // stapledOCSPResponse
                                       nullptr, // sctsFromTLSExtension
                                       mozilla::pkix::Now(),
                                       nullptr, // pinarg
                                       hostnameFlat.get(),
                                       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
@@ -1073,16 +1073,17 @@ 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.get(), // 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);
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/bad_certs/ee-from-missing-intermediate.pem
@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIIC+zCCAeWgAwIBAgIUf+ecjIpXJoXZQMMGs/O52mUg1LEwCwYJKoZIhvcNAQEL
+MB8xHTAbBgNVBAMMFE1pc3NpbmcgSW50ZXJtZWRpYXRlMCIYDzIwMTUxMTI4MDAw
+MDAwWhgPMjAxODAyMDUwMDAwMDBaMCcxJTAjBgNVBAMMHGVlLWZyb20tbWlzc2lu
+Zy1pbnRlcm1lZGlhdGUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC6
+iFGoRI4W1kH9braIBjYQPTwT2erkNUq07PVoV2wke8HHJajg2B+9sZwGm24ahvJr
+4q9adWtqZHEIeqVap0WH9xzVJJwCfs1D/B5p0DggKZOrIMNJ5Nu5TMJrbA7tFYIP
+8X6taRqx0wI6iypB7qdw4A8Njf1mCyuwJJKkfbmIYXmQsVeQPdI7xeC4SB+oN9OI
+Q+8nFthVt2Zaqn4CkC86exCABiTMHGyXrZZhW7filhLAdTGjDJHdtMr3/K0dJdMJ
+77kXDqdo4bN7LyJvaeO0ipVhHe4m1iWdq5EITjbLHCQELL8Wiy/l8Y+ZFzG4s/5J
+I/pyUcQx1QOs2hgKNe2NAgMBAAGjJzAlMCMGA1UdEQQcMBqCCWxvY2FsaG9zdIIN
+Ki5leGFtcGxlLmNvbTALBgkqhkiG9w0BAQsDggEBAD9EKfDooyLfO+uzKGsP2Xby
+RUm0ynRPttC8eqg+7hCuq583DtdZU7VUsPjIwckDMF2dtbx8wWl0vDcSSRQ84SGW
+9+y3bqRggVeqTbcdN+y0Q643CB71wRQQt/pRwga8vLkMkDYke9APDDak62vKEaTP
+MHSB5fT+m0AtE0RnN6OWZgpI++dlGpFsGsspUuO3z7qtBI7u1jBv4tIg8NeOINRf
+waik5YQ3CEGz34G/PkdvkAY95gd2/oNdV86J1D0IMzKv+JPQsFIDaw8GHXFHCs6O
+x7heuiUSt8l4szi1AcalkzEZI3ExcKrKs0D5VtGZAVQLo6sUuhSF9gQvXkV8eMA=
+-----END CERTIFICATE-----
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/bad_certs/ee-from-missing-intermediate.pem.certspec
@@ -0,0 +1,3 @@
+issuer:Missing Intermediate
+subject:ee-from-missing-intermediate
+extension:subjectAlternativeName:localhost,*.example.com
--- a/security/manager/ssl/tests/unit/bad_certs/moz.build
+++ b/security/manager/ssl/tests/unit/bad_certs/moz.build
@@ -7,42 +7,43 @@
 # Temporarily disabled. See bug 1256495.
 #test_certificates = (
 #    'badSubjectAltNames.pem',
 #    'beforeEpoch.pem',
 #    'beforeEpochINT.pem',
 #    'beforeEpochIssuer.pem',
 #    'ca-used-as-end-entity.pem',
 #    'default-ee.pem',
+#    'ee-from-missing-intermediate.pem',
 #    'eeIssuedByNonCA.pem',
 #    'eeIssuedByV1Cert.pem',
 #    'emptyIssuerName.pem',
 #    'emptyNameCA.pem',
 #    'ev-test-intermediate.pem',
 #    'ev-test.pem',
 #    'evroot.pem',
 #    'expired-ee.pem',
 #    'expiredINT.pem',
 #    'expiredissuer.pem',
 #    'idn-certificate.pem',
 #    'inadequateKeySizeEE.pem',
 #    'inadequatekeyusage-ee.pem',
 #    'ipAddressAsDNSNameInSAN.pem',
 #    'md5signature-expired.pem',
 #    'md5signature.pem',
-#    'mismatchCN.pem',
 #    'mismatch-expired.pem',
 #    'mismatch-notYetValid.pem',
-#    'mismatch.pem',
 #    'mismatch-untrusted-expired.pem',
 #    'mismatch-untrusted.pem',
+#    'mismatch.pem',
+#    'mismatchCN.pem',
+#    'noValidNames.pem',
+#    'notYetValid.pem',
 #    'notYetValidINT.pem',
 #    'notYetValidIssuer.pem',
-#    'notYetValid.pem',
-#    'noValidNames.pem',
 #    'nsCertTypeCritical.pem',
 #    'nsCertTypeCriticalWithExtKeyUsage.pem',
 #    'nsCertTypeNotCritical.pem',
 #    'other-issuer-ee.pem',
 #    'other-test-ca.pem',
 #    'self-signed-EE-with-cA-true.pem',
 #    'selfsigned-inadequateEKU.pem',
 #    'selfsigned.pem',
--- a/security/manager/ssl/tests/unit/moz.build
+++ b/security/manager/ssl/tests/unit/moz.build
@@ -23,16 +23,17 @@ TEST_DIRS += [
     'test_certDB_import',
     'test_content_signing',
     'test_ct',
     'test_ev_certs',
     'test_getchain',
     'test_intermediate_basic_usage_constraints',
     'test_keysize',
     'test_keysize_ev',
+    'test_missing_intermediate',
     'test_name_constraints',
     'test_ocsp_fetch_method',
     'test_ocsp_url',
     'test_onecrl',
     'test_pinning_dynamic',
     'test_startcom_wosign',
     'test_validity',
 ]
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_missing_intermediate.js
@@ -0,0 +1,30 @@
+// -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
+// 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/.
+
+"use strict";
+
+// Tests that if a server does not send a complete certificate chain, we can
+// make use of cached intermediates to build a trust path.
+
+do_get_profile(); // must be called before getting nsIX509CertDB
+const certdb  = Cc["@mozilla.org/security/x509certdb;1"]
+                  .getService(Ci.nsIX509CertDB);
+
+function run_test() {
+  addCertFromFile(certdb, "bad_certs/test-ca.pem", "CTu,,");
+  add_tls_server_setup("BadCertServer", "bad_certs");
+  // If we don't know about the intermediate, we'll get an unknown issuer error.
+  add_connection_test("ee-from-missing-intermediate.example.com",
+                      SEC_ERROR_UNKNOWN_ISSUER);
+  add_test(() => {
+    addCertFromFile(certdb, "test_missing_intermediate/missing-intermediate.pem",
+                    ",,");
+    run_next_test();
+  });
+  // Now that we've cached the intermediate, the connection should succeed.
+  add_connection_test("ee-from-missing-intermediate.example.com",
+                      PRErrorCodeSuccess);
+  run_next_test();
+}
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_missing_intermediate/missing-intermediate.pem
@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIIC3DCCAcagAwIBAgIUX224QtJ2mJrowBNyubHeYknUAZ0wCwYJKoZIhvcNAQEL
+MBIxEDAOBgNVBAMMB1Rlc3QgQ0EwIhgPMjAxNTExMjgwMDAwMDBaGA8yMDE4MDIw
+NTAwMDAwMFowHzEdMBsGA1UEAwwUTWlzc2luZyBJbnRlcm1lZGlhdGUwggEiMA0G
+CSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC6iFGoRI4W1kH9braIBjYQPTwT2erk
+NUq07PVoV2wke8HHJajg2B+9sZwGm24ahvJr4q9adWtqZHEIeqVap0WH9xzVJJwC
+fs1D/B5p0DggKZOrIMNJ5Nu5TMJrbA7tFYIP8X6taRqx0wI6iypB7qdw4A8Njf1m
+CyuwJJKkfbmIYXmQsVeQPdI7xeC4SB+oN9OIQ+8nFthVt2Zaqn4CkC86exCABiTM
+HGyXrZZhW7filhLAdTGjDJHdtMr3/K0dJdMJ77kXDqdo4bN7LyJvaeO0ipVhHe4m
+1iWdq5EITjbLHCQELL8Wiy/l8Y+ZFzG4s/5JI/pyUcQx1QOs2hgKNe2NAgMBAAGj
+HTAbMAwGA1UdEwQFMAMBAf8wCwYDVR0PBAQDAgEGMAsGCSqGSIb3DQEBCwOCAQEA
+hOeRxGhMAYiPuIoqU79VoRw99fWG05N6NS8CDoT6gMUBFCDG77rR1g3nR/KzH2df
+eY0K4B6ARLSip8L3zbooFKexzbFo6bEGnJM/OTfojDprE+5nMDwMiIoQMswxzKd0
+9GnSispqPdvWojn5F++1EDv1VQXvjdkxt7iZLUxSJ59ktxGy1bqsnncslH2u/aPN
+gIdVfftPBjRikDilKmMRU1g+g8f3LLCv557D6icvSgMiB8p/i//RCwYqpUfYghcg
+EGU/vzq3746TTbkQvlgKjRS/h+J26atjgsGOVLOeETQurjgHKOj8ngllPrfKm52p
+DqVeWkpMVj4PptI7GVianw==
+-----END CERTIFICATE-----
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_missing_intermediate/missing-intermediate.pem.certspec
@@ -0,0 +1,4 @@
+issuer:Test CA
+subject:Missing Intermediate
+extension:basicConstraints:cA,
+extension:keyUsage:cRLSign,keyCertSign
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_missing_intermediate/moz.build
@@ -0,0 +1,19 @@
+# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
+# vim: set filetype=python:
+# 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/.
+
+# BadCertServer takes as an argument a path to a directory and loads
+# every key and certificate in it. We want to test what happens when a
+# server doesn't include an intermediate that is necessary to build a
+# complete trust path. The easiest way to do this right now is to put
+# the intermediate in a different directory, so that BadCertServer
+# doesn't know about it and can't send it in the TLS handshake.
+# Temporarily disabled. See bug 1256495.
+#test_certificates = (
+#    'missing-intermediate.pem',
+#)
+#
+#for test_certificate in test_certificates:
+#    GeneratedTestCertificate(test_certificate)
--- a/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
@@ -72,16 +72,17 @@ const BadCertHost sBadCertHosts[] =
   { "end-entity-issued-by-non-CA.example.com", "eeIssuedByNonCA" },
   { "inadequate-key-size-ee.example.com", "inadequateKeySizeEE" },
   { "badSubjectAltNames.example.com", "badSubjectAltNames" },
   { "ipAddressAsDNSNameInSAN.example.com", "ipAddressAsDNSNameInSAN" },
   { "noValidNames.example.com", "noValidNames" },
   { "bug413909.xn--hxajbheg2az3al.xn--jxalpdlp", "idn-certificate" },
   { "emptyissuername.example.com", "emptyIssuerName" },
   { "ev-test.example.com", "ev-test" },
+  { "ee-from-missing-intermediate.example.com", "ee-from-missing-intermediate" },
   { "localhost", "unknownissuer" },
   { nullptr, nullptr }
 };
 
 int32_t
 DoSNISocketConfigBySubjectCN(PRFileDesc* aFd, const SECItem* aSrvNameArr,
                              uint32_t aSrvNameArrSize)
 {
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -18,16 +18,17 @@ support-files =
   test_certviewer_invalid_oids/**
   test_content_signing/**
   test_ct/**
   test_ev_certs/**
   test_getchain/**
   test_intermediate_basic_usage_constraints/**
   test_keysize/**
   test_keysize_ev/**
+  test_missing_intermediate/**
   test_name_constraints/**
   test_ocsp_fetch_method/**
   test_ocsp_url/**
   test_onecrl/**
   test_pinning_dynamic/**
   test_sdr_preexisting/**
   test_signed_apps/**
   test_signed_dir/**
@@ -89,16 +90,18 @@ skip-if = toolkit == 'android'
 [test_js_cert_override_service.js]
 run-sequentially = hardcoded ports
 [test_keysize.js]
 [test_keysize_ev.js]
 run-sequentially = hardcoded ports
 [test_local_cert.js]
 [test_logoutAndTeardown.js]
 run-sequentially = hardcoded ports
+[test_missing_intermediate.js]
+run-sequentially = hardcoded ports
 [test_name_constraints.js]
 [test_nsCertType.js]
 run-sequentially = hardcoded ports
 [test_nsIX509Cert_utf8.js]
 [test_nsIX509CertValidity.js]
 [test_nss_shutdown.js]
 [test_ocsp_caching.js]
 run-sequentially = hardcoded ports