bug 1294580 - prevent end-entity certificates from being their own trust anchors r?Cykesiopka
MozReview-Commit-ID: KaZaFG8AWwl
--- a/security/certverifier/CertVerifier.cpp
+++ b/security/certverifier/CertVerifier.cpp
@@ -287,18 +287,17 @@ CertVerifier::VerifyCertificateTranspare
CERTCertListNode* endEntityNode = CERT_LIST_HEAD(builtChain);
if (!endEntityNode || CERT_LIST_END(endEntityNode, builtChain)) {
return Result::FATAL_ERROR_INVALID_ARGS;
}
CERTCertListNode* issuerNode = CERT_LIST_NEXT(endEntityNode);
if (!issuerNode || CERT_LIST_END(issuerNode, builtChain)) {
// Issuer certificate is required for SCT verification.
- // TODO(bug 1294580): change this to Result::FATAL_ERROR_INVALID_ARGS
- return Success;
+ return Result::FATAL_ERROR_INVALID_ARGS;
}
CERTCertificate* endEntity = endEntityNode->cert;
CERTCertificate* issuer = issuerNode->cert;
if (!endEntity || !issuer) {
return Result::FATAL_ERROR_INVALID_ARGS;
}
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -238,17 +238,21 @@ NSSCertDBTrustDomain::GetCertTrust(EndEn
== CERTDB_TERMINAL_RECORD) {
trustLevel = TrustLevel::ActivelyDistrusted;
return Success;
}
// For TRUST, we only use the CERTDB_TRUSTED_CA bit, because Gecko hasn't
// needed to consider end-entity certs to be their own trust anchors since
// Gecko implemented nsICertOverrideService.
- if (flags & CERTDB_TRUSTED_CA) {
+ // Of course, for this to work as expected, we need to make sure we're
+ // inquiring about the trust of a CA and not an end-entity. If an end-entity
+ // has the CERTDB_TRUSTED_CA bit set, Gecko does not consider it to be a
+ // trust anchor; it must inherit its trust.
+ if (flags & CERTDB_TRUSTED_CA && endEntityOrCA == EndEntityOrCA::MustBeCA) {
if (policy.IsAnyPolicy()) {
trustLevel = TrustLevel::TrustAnchor;
return Success;
}
if (CertIsAuthoritativeForEVPolicy(candidateCert, policy)) {
trustLevel = TrustLevel::TrustAnchor;
return Success;
}
--- a/security/manager/ssl/tests/unit/test_cert_trust.js
+++ b/security/manager/ssl/tests/unit/test_cert_trust.js
@@ -204,14 +204,36 @@ function run_test() {
setup_basic_trusts(ca_cert, int_cert);
test_ca_distrust(ee_cert, int_cert, false);
// Reset trust to default ("inherit trust")
setCertTrust(ca_cert, ",,");
setCertTrust(int_cert, ",,");
- // It turns out that if an end-entity certificate is manually trusted, it can
- // be the root of its own verified chain. This will be removed in bug 1294580.
- setCertTrust(ee_cert, "C,,");
+ // If an end-entity certificate is manually trusted, it may not be the root of
+ // its own verified chain. In general this will cause "unknown issuer" errors
+ // unless a CA trust anchor can be found.
+ setCertTrust(ee_cert, "CTu,CTu,CTu");
+ checkCertErrorGeneric(certdb, ee_cert, SEC_ERROR_UNKNOWN_ISSUER,
+ certificateUsageSSLServer);
+ checkCertErrorGeneric(certdb, ee_cert, SEC_ERROR_UNKNOWN_ISSUER,
+ certificateUsageSSLClient);
+ checkCertErrorGeneric(certdb, ee_cert, SEC_ERROR_UNKNOWN_ISSUER,
+ certificateUsageEmailSigner);
+ checkCertErrorGeneric(certdb, ee_cert, SEC_ERROR_UNKNOWN_ISSUER,
+ certificateUsageEmailRecipient);
+ checkCertErrorGeneric(certdb, ee_cert, SEC_ERROR_UNKNOWN_ISSUER,
+ certificateUsageObjectSigner);
+
+ // Now make a CA trust anchor available.
+ setCertTrust(ca_cert, "CTu,CTu,CTu");
checkCertErrorGeneric(certdb, ee_cert, PRErrorCodeSuccess,
certificateUsageSSLServer);
+ checkCertErrorGeneric(certdb, ee_cert, PRErrorCodeSuccess,
+ certificateUsageSSLClient);
+ checkCertErrorGeneric(certdb, ee_cert, PRErrorCodeSuccess,
+ certificateUsageEmailSigner);
+ checkCertErrorGeneric(certdb, ee_cert, PRErrorCodeSuccess,
+ certificateUsageEmailRecipient);
+ checkCertErrorGeneric(certdb, ee_cert, PRErrorCodeSuccess,
+ certificateUsageObjectSigner);
}