Allow the TrustStore interface to return matching intermediates, and identify distrusted certs.

net/cert/internal/verify_certificate_chain.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/internal/verify_certificate_chain.h"
 
 #include <memory>
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ skipping 19 matching lines @@
 // -----------------------------------------------
 // Errors/Warnings set by VerifyCertificateChain
 // -----------------------------------------------
 
 DEFINE_CERT_ERROR_ID(
     kSignatureAlgorithmMismatch,
     "Certificate.signatureAlgorithm != TBSCertificate.signature");
 DEFINE_CERT_ERROR_ID(kInvalidOrUnsupportedSignatureAlgorithm,
                      "Invalid or unsupported signature algorithm");
 DEFINE_CERT_ERROR_ID(kChainIsEmpty, "Chain is empty");
+DEFINE_CERT_ERROR_ID(kChainIsLength1,
+                     "TODO: Cannot verify a chain of length 1");
 DEFINE_CERT_ERROR_ID(kUnconsumedCriticalExtension,
                      "Unconsumed critical extension");
 DEFINE_CERT_ERROR_ID(
     kTargetCertInconsistentCaBits,
     "Target certificate looks like a CA but does not set all CA properties");
 DEFINE_CERT_ERROR_ID(kKeyCertSignBitNotSet, "keyCertSign bit is not set");
 DEFINE_CERT_ERROR_ID(kMaxPathLengthViolated, "max_path_length reached");
 DEFINE_CERT_ERROR_ID(kBasicConstraintsIndicatesNotCa,
                      "Basic Constraints indicates not a CA");
 DEFINE_CERT_ERROR_ID(kMissingBasicConstraints,
                      "Does not have Basic Constraints");
 DEFINE_CERT_ERROR_ID(kNotPermittedByNameConstraints,
                      "Not permitted by name constraints");
 DEFINE_CERT_ERROR_ID(kSubjectDoesNotMatchIssuer,
                      "subject does not match issuer");
 DEFINE_CERT_ERROR_ID(kVerifySignedDataFailed, "VerifySignedData failed");
 DEFINE_CERT_ERROR_ID(kSignatureAlgorithmsDifferentEncoding,
                      "Certificate.signatureAlgorithm is encoded differently "
                      "than TBSCertificate.signature");
 DEFINE_CERT_ERROR_ID(kEkuLacksServerAuth,
                      "The extended key usage does not include server auth");
 DEFINE_CERT_ERROR_ID(kEkuLacksClientAuth,
                      "The extended key usage does not include client auth");
+DEFINE_CERT_ERROR_ID(kCertIsDistrusted, "Certificate is distrusted");
+DEFINE_CERT_ERROR_ID(kCertIsNotTrustAnchor,
+                     "Certificate is not a trust anchor");
 
 bool IsHandledCriticalExtensionOid(const der::Input& oid) {
   if (oid == BasicConstraintsOid())
     return true;
   // Key Usage is NOT processed for end-entity certificates (this is the
   // responsibility of callers), however it is considered "handled" here in
   // order to allow being marked as critical.
   if (oid == KeyUsageOid())
     return true;
   if (oid == ExtKeyUsageOid())
@@ skipping 359 matching lines @@
   VerifyNoUnconsumedCriticalExtensions(cert, errors);
 
   // TODO(eroman): Step g is omitted, as policy constraints are not yet
   // implemented.
 
   // The following check is NOT part of RFC 5280 6.1.5's "Wrap-Up Procedure",
   // however is implied by RFC 5280 section 4.2.1.9.
   VerifyTargetCertHasConsistentCaBits(cert, errors);
 }
 
-// Initializes the path validation algorithm given anchor constraints. This
-// follows the description in RFC 5937
-void ProcessTrustAnchorConstraints(
-    const TrustAnchor& trust_anchor,
+// Enforces trust anchor constraints compatibile with RFC 5937.
+//
+// Note that the anchor constraints are encoded via the attached certificate
+// itself.
+void ApplyTrustAnchorConstraints(
+    const ParsedCertificate& cert,
     KeyPurpose required_key_purpose,
     size_t* max_path_length_ptr,
     std::vector<const NameConstraints*>* name_constraints_list,
     CertErrors* errors) {
-  // In RFC 5937 the enforcement of anchor constraints is governed by the input
-  // enforceTrustAnchorConstraints to path validation. In our implementation
-  // this is always on, and enforcement is controlled solely by whether or not
-  // the trust anchor specified constraints.
-  if (!trust_anchor.enforces_constraints())
-    return;
-
-  // Anchor constraints are encoded via the attached certificate.
-  const ParsedCertificate& cert = *trust_anchor.cert();
-
   // This is not part of RFC 5937 nor RFC 5280, but matches the EKU handling
   // done for intermediates (described in Web PKI's Baseline Requirements).
   VerifyExtendedKeyUsage(cert, required_key_purpose, errors);
 
   // The following enforcements follow from RFC 5937 (primarily section 3.2):
 
   // Initialize name constraints initial-permitted/excluded-subtrees.
   if (cert.has_name_constraints())
     name_constraints_list->push_back(&cert.name_constraints());
 
@@ skipping 21 matching lines @@
     *max_path_length_ptr = cert.basic_constraints().path_len;
 
   // From RFC 5937 section 2:
   //
   //    Extensions may be marked critical or not critical.  When trust anchor
   //    constraints are enforced, clients MUST reject certification paths
   //    containing a trust anchor with unrecognized critical extensions.
   VerifyNoUnconsumedCriticalExtensions(cert, errors);
 }
 
+// Initializes the path validation algorithm given anchor constraints. This
+// follows the description in RFC 5937
+void ProcessRootCertificate(
+    const ParsedCertificate& cert,
+    const CertificateTrust& trust,
+    KeyPurpose required_key_purpose,
+    size_t* max_path_length_ptr,
+    std::vector<const NameConstraints*>* name_constraints_list,
+    der::Input* working_spki,
+    der::Input* working_normalized_issuer_name,
+    CertErrors* errors) {
+  switch (trust.type) {
+    case CertificateTrustType::UNSPECIFIED:
+      // Doesn't chain to a trust anchor - implicitly distrusted
+      errors->AddError(kCertIsNotTrustAnchor);
+      return;

[mattm, 2017/04/28 20:26:47]

working_spki and working_normalized_issuer_name aren't initialized in the
non-trusted cases even though it looks like VerifyCertificateChain continues to
try to evaluate the rest of the chain. Won't this lead to spurious
CertPathErrors?

[eroman, 2017/04/28 21:48:04]

Good point.

I don't currently have tests for those cases, and need to follow-up with tests
at least for the distrust case (unspecified is probably not reachable in
practice). I would like to simplify the test format before sticking in the
distrust stuff, since I think this will work better layering on top of the
changes in https://codereview.chromium.org/2805213004/  (which can be further
simplified now)

I will go ahead and initialize the subject and spki for all cases for now.

+    case CertificateTrustType::DISTRUSTED:
+      // Chains to an actively distrusted certificate.
+      errors->AddError(kCertIsDistrusted);
+      return;
+    case CertificateTrustType::TRUSTED_ANCHOR:
+    case CertificateTrustType::TRUSTED_ANCHOR_WITH_CONSTRAINTS:
+      // Chains to a trust anchor - use its SPKI and subject when verifying the
+      // next certificate.
+      *working_spki = cert.tbs().spki_tlv;
+      *working_normalized_issuer_name = cert.normalized_subject();
+
+      // If the trust anchor has constraints, enforce them.
+      if (trust.type == CertificateTrustType::TRUSTED_ANCHOR_WITH_CONSTRAINTS) {
+        ApplyTrustAnchorConstraints(cert, required_key_purpose,
+                                    max_path_length_ptr, name_constraints_list,
+                                    errors);
+      }
+      return;
+  }
+}
+
+}  // namespace
+
 // This implementation is structured to mimic the description of certificate
 // path verification given by RFC 5280 section 6.1.
-void VerifyCertificateChainNoReturnValue(
-    const ParsedCertificateList& certs,
-    const TrustAnchor* trust_anchor,
-    const SignaturePolicy* signature_policy,
-    const der::GeneralizedTime& time,
-    KeyPurpose required_key_purpose,
-    CertPathErrors* errors) {
-  DCHECK(trust_anchor);
+void VerifyCertificateChain(const ParsedCertificateList& certs,
+                            const CertificateTrust& last_cert_trust,
+                            const SignaturePolicy* signature_policy,
+                            const der::GeneralizedTime& time,
+                            KeyPurpose required_key_purpose,
+                            CertPathErrors* errors) {
   DCHECK(signature_policy);
   DCHECK(errors);
 
   // An empty chain is necessarily invalid.
   if (certs.empty()) {
     errors->GetOtherErrors()->AddError(kChainIsEmpty);
     return;
   }
 
+  // TODO(eroman): Verifying a trusted leaf certificate is not currently
+  // permitted.
+  if (certs.size() == 1) {
+    errors->GetOtherErrors()->AddError(kChainIsLength1);
+    return;
+  }
+
   // Will contain a NameConstraints for each previous cert in the chain which
   // had nameConstraints. This corresponds to the permitted_subtrees and
   // excluded_subtrees state variables from RFC 5280.
   std::vector<const NameConstraints*> name_constraints_list;
 
   // |working_spki| is an amalgamation of 3 separate variables from RFC 5280:
   //    * working_public_key
   //    * working_public_key_algorithm
   //    * working_public_key_parameters
   //
   // They are combined for simplicity since the signature verification takes an
   // SPKI, and the parameter inheritence is not applicable for the supported
   // key types.
   //
   // An approximate explanation of |working_spki| is this description from RFC
   // 5280 section 6.1.2:
   //
   //    working_public_key:  the public key used to verify the
   //    signature of a certificate.
-  der::Input working_spki = trust_anchor->spki();
+  der::Input working_spki;
 
   // |working_normalized_issuer_name| is the normalized value of the
   // working_issuer_name variable in RFC 5280 section 6.1.2:
   //
   //    working_issuer_name:  the issuer distinguished name expected
   //    in the next certificate in the chain.
-  der::Input working_normalized_issuer_name =
-      trust_anchor->normalized_subject();
+  der::Input working_normalized_issuer_name;
 
   // |max_path_length| corresponds with the same named variable in RFC 5280
   // section 6.1.2:
   //
   //    max_path_length:  this integer is initialized to n, is
   //    decremented for each non-self-issued certificate in the path,
   //    and may be reduced to the value in the path length constraint
   //    field within the basic constraints extension of a CA
   //    certificate.
   size_t max_path_length = certs.size();
 
-  // Apply any trust anchor constraints per RFC 5937.
+  // Iterate over all the certificates in the reverse direction: starting from
+  // the root certificate and progressing towards the target certificate.
   //
-  // TODO(eroman): Errors on the trust anchor are put into a certificate bucket
-  //               GetErrorsForCert(certs.size()). This is a bit magical, and
-  //               has some integration issues.
-  ProcessTrustAnchorConstraints(*trust_anchor, required_key_purpose,
-                                &max_path_length, &name_constraints_list,
-                                errors->GetErrorsForCert(certs.size()));
-
-  // Iterate over all the certificates in the reverse direction: starting from
-  // the certificate signed by trust anchor and progressing towards the target
-  // certificate.
-  //
-  // Note that |i| uses 0-based indexing whereas in RFC 5280 it is 1-based.
-  //
-  //   * i=0    :  Certificated signed by trust anchor.
-  //   * i=N-1  :  Target certificate.
+  //   * i=0               :  Root certificate (i.e. trust anchor)
+  //   * i=1               :  Certificated signed by the root certificate
+  //   * i=certs.size()-1  :  Target certificate.
   for (size_t i = 0; i < certs.size(); ++i) {
     const size_t index_into_certs = certs.size() - i - 1;
 
     // |is_target_cert| is true if the current certificate is the target
     // certificate being verified. The target certificate isn't necessarily an
     // end-entity certificate.
     const bool is_target_cert = index_into_certs == 0;
+    const bool is_root_cert = i == 0;
 
     const ParsedCertificate& cert = *certs[index_into_certs];
 
     // Output errors for the current certificate into an error bucket that is
     // associated with that certificate.
     CertErrors* cert_errors = errors->GetErrorsForCert(index_into_certs);
 
+    if (is_root_cert) {
+      ProcessRootCertificate(cert, last_cert_trust, required_key_purpose,
+                             &max_path_length, &name_constraints_list,
+                             &working_spki, &working_normalized_issuer_name,
+                             cert_errors);
+
+      // Don't do any other checks for root certificates.
+      continue;
+    }
+
     // Per RFC 5280 section 6.1:
     //  * Do basic processing for each certificate
     //  * If it is the last certificate in the path (target certificate)
     //     - Then run "Wrap up"
     //     - Otherwise run "Prepare for Next cert"
     BasicCertificateProcessing(cert, is_target_cert, signature_policy, time,
                                working_spki, working_normalized_issuer_name,
                                name_constraints_list, cert_errors);
 
     // The key purpose is checked not just for the end-entity certificate, but
@@ skipping 10 matching lines @@
       WrapUp(cert, cert_errors);
     }
   }
 
   // TODO(eroman): RFC 5280 forbids duplicate certificates per section 6.1:
   //
   //    A certificate MUST NOT appear more than once in a prospective
   //    certification path.
 }
 
-}  // namespace
-
-bool VerifyCertificateChain(const ParsedCertificateList& certs,
-                            const TrustAnchor* trust_anchor,
-                            const SignaturePolicy* signature_policy,
-                            const der::GeneralizedTime& time,
-                            KeyPurpose required_key_purpose,
-                            CertPathErrors* errors) {
-  // TODO(eroman): This function requires that |errors| is empty upon entry,
-  // which is not part of the API contract.
-  DCHECK(!errors->ContainsHighSeverityErrors());
-  VerifyCertificateChainNoReturnValue(certs, trust_anchor, signature_policy,
-                                      time, required_key_purpose, errors);
-  return !errors->ContainsHighSeverityErrors();
-}
-
 }  // namespace net