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 374120 df65d15b648daef67f1a76987c21f4fe9b23bdb7
parent 374119 012f622e6db538ca428ebbd12a219b71c4275d16
child 374121 9afe051a090cfb55a2dca1a541fea0a883ecf119
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCykesiopka
bugs1294580
milestone54.0a1
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);
 }