Skip to content

Commit

Permalink
bug 1557092 - add fast path to avoid calling CERT_CreateSubjectList f…
Browse files Browse the repository at this point in the history 
…or most certificate verifications r=jcj,KevinJacobs

Differential Revision: https://phabricator.services.mozilla.com/D34042
  • Loading branch information
@mozkeeler
mozkeeler committed Jun 11, 2019
1 parent 3cb0865 commit 98655ee
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 61 deletions.
198 changes: 138 additions & 60 deletions security/certverifier/NSSCertDBTrustDomain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vector<uint8_t>>& 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<uint8_t> 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
Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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;
}
Expand All @@ -145,10 +176,49 @@ bool NSSCertDBTrustDomain::ShouldSkipSelfSignedNonTrustAnchor(Input certDER) {
return true;
}

static Result CheckCandidates(TrustDomain& trustDomain,
TrustDomain::IssuerChecker& checker,
Vector<Input>& 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<Input> rootCandidates;
Vector<Input> 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<Input> geckoRootCandidates;
Vector<Input> geckoIntermediateCandidates;

#ifdef MOZ_NEW_CERT_STORAGE
if (!mCertStorage) {
Expand All @@ -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<Vector<uint8_t>> 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<Input> nssRootCandidates;
Vector<Input> nssIntermediateCandidates;
UniqueCERTCertList candidates(CERT_CreateSubjectCertList(
nullptr, CERT_GetDefaultCertDB(), &encodedIssuerNameItem, 0, false));
if (candidates) {
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion security/certverifier/NSSCertDBTrustDomain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions security/nss.symbols
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ PK11_FindCertsFromNickname
PK11_FindKeyByAnyCert
PK11_FindKeyByDERCert
PK11_FindKeyByKeyID
PK11_FindRawCertsWithSubject
PK11_FindSlotByName
PK11_FindSlotsByNames
PK11_FreeSlot
Expand Down

0 comments on commit 98655ee

Please sign in to comment.