author | David Keeler <dkeeler@mozilla.com> |
Thu, 11 May 2017 16:41:12 -0700 | |
changeset 360179 | 600b709c2634cfd9d57f0eefd56af8d1200791c9 |
parent 360178 | 913491f988da3253c033962a9d1c39e294d624ba |
child 360180 | d11aec13b38e0837c07d0906c1ab506e162b2a2f |
push id | 31871 |
push user | ryanvm@gmail.com |
push date | Tue, 23 May 2017 22:02:07 +0000 |
treeherder | mozilla-central@545ffce30eac [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | Cykesiopka, jcj |
bugs | 1364159, 731478 |
milestone | 55.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
|
--- 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