bug 1294580 - prevent end-entity certificates from being their own trust anchors r=Cykesiopka
authorDavid Keeler <dkeeler@mozilla.com>
Fri, 24 Feb 2017 12:32:41 -0800
changeset 345093 df65d15b648daef67f1a76987c21f4fe9b23bdb7
parent 345092 012f622e6db538ca428ebbd12a219b71c4275d16
child 345094 9afe051a090cfb55a2dca1a541fea0a883ecf119
push id31431
push usercbook@mozilla.com
push dateTue, 28 Feb 2017 10:23:16 +0000
treeherdermozilla-central@682ee6f48027 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCykesiopka
bugs1294580
milestone54.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
bug 1294580 - prevent end-entity certificates from being their own trust anchors r=Cykesiopka MozReview-Commit-ID: KaZaFG8AWwl
security/certverifier/CertVerifier.cpp
security/certverifier/NSSCertDBTrustDomain.cpp
security/manager/ssl/tests/unit/test_cert_trust.js
--- 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);
 }