From 98655ee8de9c160d6a3846645c90465f80b6d101 Mon Sep 17 00:00:00 2001 From: Dana Keeler Date: Tue, 11 Jun 2019 22:45:26 +0000 Subject: [PATCH] bug 1557092 - add fast path to avoid calling CERT_CreateSubjectList for most certificate verifications r=jcj,KevinJacobs Differential Revision: https://phabricator.services.mozilla.com/D34042 --- .../certverifier/NSSCertDBTrustDomain.cpp | 198 ++++++++++++------ security/certverifier/NSSCertDBTrustDomain.h | 4 +- security/nss.symbols | 1 + 3 files changed, 142 insertions(+), 61 deletions(-) diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp index 602270c60de4c..2302fb3707c5d 100644 --- a/security/certverifier/NSSCertDBTrustDomain.cpp +++ b/security/certverifier/NSSCertDBTrustDomain.cpp @@ -98,7 +98,37 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain( #endif mOCSPStaplingStatus(CertVerifier::OCSP_STAPLING_NEVER_CHECKED), mSCTListFromCertificate(), - mSCTListFromOCSPStapling() { + mSCTListFromOCSPStapling(), + mBuiltInRootsModule(SECMOD_FindModule(kRootModuleName)) { +} + +static Result FindRootsWithSubject(UniqueSECMODModule& rootsModule, + SECItem subject, + /*out*/ Vector>& roots) { + MOZ_ASSERT(rootsModule); + for (int slotIndex = 0; slotIndex < rootsModule->slotCount; slotIndex++) { + CERTCertificateList* rawResults = nullptr; + if (PK11_FindRawCertsWithSubject(rootsModule->slots[slotIndex], &subject, + &rawResults) != SECSuccess) { + return Result::FATAL_ERROR_LIBRARY_FAILURE; + } + // rawResults == nullptr means we didn't find any matching certificates + if (!rawResults) { + continue; + } + UniqueCERTCertificateList results(rawResults); + for (int certIndex = 0; certIndex < results->len; certIndex++) { + Vector root; + if (!root.append(results->certs[certIndex].data, + results->certs[certIndex].len)) { + return Result::FATAL_ERROR_NO_MEMORY; + } + if (!roots.append(std::move(root))) { + return Result::FATAL_ERROR_NO_MEMORY; + } + } + } + return Success; } // A self-signed issuer certificate should never be necessary in order to build @@ -110,7 +140,8 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain( // certificate extensions we support are restrictive rather than additive in // terms of the rest of the chain (for example, we don't support policy mapping // and we ignore any SCT information in intermediates). -bool NSSCertDBTrustDomain::ShouldSkipSelfSignedNonTrustAnchor(Input certDER) { +static bool ShouldSkipSelfSignedNonTrustAnchor(TrustDomain& trustDomain, + Input certDER) { BackCert cert(certDER, EndEntityOrCA::MustBeCA, nullptr); if (cert.Init() != Success) { return false; // turn any failures into "don't skip trying this cert" @@ -120,8 +151,8 @@ bool NSSCertDBTrustDomain::ShouldSkipSelfSignedNonTrustAnchor(Input certDER) { return false; } TrustLevel trust; - if (GetCertTrust(EndEntityOrCA::MustBeCA, CertPolicyId::anyPolicy, certDER, - trust) != Success) { + if (trustDomain.GetCertTrust(EndEntityOrCA::MustBeCA, CertPolicyId::anyPolicy, + certDER, trust) != Success) { return false; } // If the trust for this certificate is anything other than "inherit", we want @@ -132,11 +163,11 @@ bool NSSCertDBTrustDomain::ShouldSkipSelfSignedNonTrustAnchor(Input certDER) { uint8_t digestBuf[MAX_DIGEST_SIZE_IN_BYTES]; pkix::der::PublicKeyAlgorithm publicKeyAlg; SignedDigest signature; - if (DigestSignedData(*this, cert.GetSignedData(), digestBuf, publicKeyAlg, - signature) != Success) { + if (DigestSignedData(trustDomain, cert.GetSignedData(), digestBuf, + publicKeyAlg, signature) != Success) { return false; } - if (VerifySignedDigest(*this, publicKeyAlg, signature, + if (VerifySignedDigest(trustDomain, publicKeyAlg, signature, cert.GetSubjectPublicKeyInfo()) != Success) { return false; } @@ -145,10 +176,49 @@ bool NSSCertDBTrustDomain::ShouldSkipSelfSignedNonTrustAnchor(Input certDER) { return true; } +static Result CheckCandidates(TrustDomain& trustDomain, + TrustDomain::IssuerChecker& checker, + Vector& candidates, + Input* nameConstraintsInputPtr, bool& keepGoing) { + for (Input candidate : candidates) { + if (ShouldSkipSelfSignedNonTrustAnchor(trustDomain, candidate)) { + continue; + } + Result rv = checker.Check(candidate, nameConstraintsInputPtr, keepGoing); + if (rv != Success) { + return rv; + } + if (!keepGoing) { + return Success; + } + } + + return Success; +} + Result NSSCertDBTrustDomain::FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time) { - Vector rootCandidates; - Vector intermediateCandidates; + SECItem encodedIssuerNameItem = UnsafeMapInputToSECItem(encodedIssuerName); + // Handle imposed name constraints, if any. + ScopedAutoSECItem nameConstraints; + Input nameConstraintsInput; + Input* nameConstraintsInputPtr = nullptr; + SECStatus srv = + CERT_GetImposedNameConstraints(&encodedIssuerNameItem, &nameConstraints); + if (srv == SECSuccess) { + if (nameConstraintsInput.Init(nameConstraints.data, nameConstraints.len) != + Success) { + return Result::FATAL_ERROR_LIBRARY_FAILURE; + } + nameConstraintsInputPtr = &nameConstraintsInput; + } else if (PR_GetError() != SEC_ERROR_EXTENSION_NOT_FOUND) { + return Result::FATAL_ERROR_LIBRARY_FAILURE; + } + + // First try all relevant certificates known to Gecko, which avoids calling + // CERT_CreateSubjectCertList, because that can be expensive. + Vector geckoRootCandidates; + Vector geckoIntermediateCandidates; #ifdef MOZ_NEW_CERT_STORAGE if (!mCertStorage) { @@ -171,17 +241,69 @@ Result NSSCertDBTrustDomain::FindIssuer(Input encodedIssuerName, continue; // probably too big } // Currently we're only expecting intermediate certificates in cert storage. - if (!intermediateCandidates.append(certDER)) { + if (!geckoIntermediateCandidates.append(certDER)) { return Result::FATAL_ERROR_NO_MEMORY; } } #endif - SECItem encodedIssuerNameItem = UnsafeMapInputToSECItem(encodedIssuerName); + // We might not have this module if e.g. we're on a Linux distribution that + // does something unexpected. + Vector> builtInRoots; + if (mBuiltInRootsModule) { + Result rv = FindRootsWithSubject(mBuiltInRootsModule, encodedIssuerNameItem, + builtInRoots); + if (rv != Success) { + return rv; + } + for (const auto& root : builtInRoots) { + Input rootInput; + rv = rootInput.Init(root.begin(), root.length()); + if (rv != Success) { + continue; // probably too big + } + if (!geckoRootCandidates.append(rootInput)) { + return Result::FATAL_ERROR_NO_MEMORY; + } + } + } else { + MOZ_LOG(gCertVerifierLog, LogLevel::Debug, + ("NSSCertDBTrustDomain::FindIssuer: no built-in roots module")); + } + + for (const auto& thirdPartyRootInput : mThirdPartyRootInputs) { + if (!geckoRootCandidates.append(thirdPartyRootInput)) { + return Result::FATAL_ERROR_NO_MEMORY; + } + } + + for (const auto& thirdPartyIntermediateInput : + mThirdPartyIntermediateInputs) { + if (!geckoIntermediateCandidates.append(thirdPartyIntermediateInput)) { + return Result::FATAL_ERROR_NO_MEMORY; + } + } + + // Try all root certs first and then all (presumably) intermediates. + if (!geckoRootCandidates.appendAll(geckoIntermediateCandidates)) { + return Result::FATAL_ERROR_NO_MEMORY; + } + + bool keepGoing = true; + Result result = CheckCandidates(*this, checker, geckoRootCandidates, + nameConstraintsInputPtr, keepGoing); + if (result != Success) { + return result; + } + if (!keepGoing) { + return Success; + } // NSS seems not to differentiate between "no potential issuers found" and // "there was an error trying to retrieve the potential issuers." We assume // there was no error if CERT_CreateSubjectCertList returns nullptr. + Vector nssRootCandidates; + Vector nssIntermediateCandidates; UniqueCERTCertList candidates(CERT_CreateSubjectCertList( nullptr, CERT_GetDefaultCertDB(), &encodedIssuerNameItem, 0, false)); if (candidates) { @@ -193,66 +315,22 @@ Result NSSCertDBTrustDomain::FindIssuer(Input encodedIssuerName, continue; // probably too big } if (n->cert->isRoot) { - if (!rootCandidates.append(certDER)) { + if (!nssRootCandidates.append(certDER)) { return Result::FATAL_ERROR_NO_MEMORY; } } else { - if (!intermediateCandidates.append(certDER)) { + if (!nssIntermediateCandidates.append(certDER)) { return Result::FATAL_ERROR_NO_MEMORY; } } } } - - for (const auto& thirdPartyRootInput : mThirdPartyRootInputs) { - if (!rootCandidates.append(thirdPartyRootInput)) { - return Result::FATAL_ERROR_NO_MEMORY; - } - } - - for (const auto& thirdPartyIntermediateInput : - mThirdPartyIntermediateInputs) { - if (!intermediateCandidates.append(thirdPartyIntermediateInput)) { - return Result::FATAL_ERROR_NO_MEMORY; - } - } - - // Handle imposed name constraints, if any. - ScopedAutoSECItem nameConstraints; - Input nameConstraintsInput; - Input* nameConstraintsInputPtr = nullptr; - SECStatus srv = - CERT_GetImposedNameConstraints(&encodedIssuerNameItem, &nameConstraints); - if (srv == SECSuccess) { - if (nameConstraintsInput.Init(nameConstraints.data, nameConstraints.len) != - Success) { - return Result::FATAL_ERROR_LIBRARY_FAILURE; - } - nameConstraintsInputPtr = &nameConstraintsInput; - } else if (PR_GetError() != SEC_ERROR_EXTENSION_NOT_FOUND) { - return Result::FATAL_ERROR_LIBRARY_FAILURE; - } - - // Try all root certs first and then all (presumably) intermediates. - if (!rootCandidates.appendAll(intermediateCandidates)) { + if (!nssRootCandidates.appendAll(nssIntermediateCandidates)) { return Result::FATAL_ERROR_NO_MEMORY; } - for (Input candidate : rootCandidates) { - if (ShouldSkipSelfSignedNonTrustAnchor(candidate)) { - continue; - } - bool keepGoing; - Result rv = checker.Check(candidate, nameConstraintsInputPtr, keepGoing); - if (rv != Success) { - return rv; - } - if (!keepGoing) { - return Success; - } - } - - return Success; + return CheckCandidates(*this, checker, nssRootCandidates, + nameConstraintsInputPtr, keepGoing); } Result NSSCertDBTrustDomain::GetCertTrust(EndEntityOrCA endEntityOrCA, diff --git a/security/certverifier/NSSCertDBTrustDomain.h b/security/certverifier/NSSCertDBTrustDomain.h index 47055f67c4ea8..3fba79a4168bf 100644 --- a/security/certverifier/NSSCertDBTrustDomain.h +++ b/security/certverifier/NSSCertDBTrustDomain.h @@ -240,7 +240,6 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain { Result HandleOCSPFailure(const Result cachedResponseResult, const Result stapledOCSPResponseResult, const Result error); - bool ShouldSkipSelfSignedNonTrustAnchor(mozilla::pkix::Input certDER); const SECTrustType mCertDBTrustType; const OCSPFetching mOCSPFetching; @@ -272,6 +271,9 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain { // Certificate Transparency data extracted during certificate verification UniqueSECItem mSCTListFromCertificate; UniqueSECItem mSCTListFromOCSPStapling; + + // The built-in roots module, if available. + UniqueSECMODModule mBuiltInRootsModule; }; } // namespace psm diff --git a/security/nss.symbols b/security/nss.symbols index 4a0869a2118fa..8a4913db2e190 100644 --- a/security/nss.symbols +++ b/security/nss.symbols @@ -318,6 +318,7 @@ PK11_FindCertsFromNickname PK11_FindKeyByAnyCert PK11_FindKeyByDERCert PK11_FindKeyByKeyID +PK11_FindRawCertsWithSubject PK11_FindSlotByName PK11_FindSlotsByNames PK11_FreeSlot