Certificate path builder for new certificate verification library

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 "net/cert/internal/name_constraints.h"
 #include "net/cert/internal/parse_certificate.h"
 #include "net/cert/internal/signature_algorithm.h"
 #include "net/cert/internal/signature_policy.h"
 #include "net/cert/internal/verify_name_match.h"
 #include "net/cert/internal/verify_signed_data.h"
 #include "net/der/input.h"
 #include "net/der/parser.h"
 
 namespace net {
 
 namespace {
 
 // Map from OID to ParsedExtension.
 using ExtensionsMap = std::map<der::Input, ParsedExtension>;
 
 // Describes all parsed properties of a certificate that are relevant for
 // certificate verification.
 struct FullyParsedCert {
-  ParsedCertificate cert;
-  ParsedTbsCertificate tbs;
+  // XXX better naming.. this results in lots of non-obvious cert.cert->foo code
+  const CertThing* cert;
+
+  // XXX should some of this be moved into CertThing?
 
   std::unique_ptr<SignatureAlgorithm> signature_algorithm;
 
   // Standard extensions that were parsed.
   bool has_basic_constraints = false;
   ParsedBasicConstraints basic_constraints;
 
   bool has_key_usage = false;
   der::BitString key_usage;
 
@@ skipping 31 matching lines @@
   }
   return true;
 }
 
 WARN_UNUSED_RESULT bool GetSequenceValue(const der::Input& tlv,
                                          der::Input* value) {
   der::Parser parser(tlv);
   return parser.ReadTag(der::kSequence, value) && !parser.HasMore();
 }
 
+// XXX update doc
 // Parses an X.509 Certificate fully (including the TBSCertificate and
 // standard extensions), saving all the properties to |out_|.
-WARN_UNUSED_RESULT bool FullyParseCertificate(const der::Input& cert_tlv,
+WARN_UNUSED_RESULT bool FullyParseCertificate(const CertThing& cert,
                                               FullyParsedCert* out) {
-  // Parse the outer Certificate.
-  if (!ParseCertificate(cert_tlv, &out->cert))
-    return false;
+  // XXX move more of this into CertThing?
+
+  out->cert = &cert;
 
   // Parse the signature algorithm contained in the Certificate (there is
   // another one in the TBSCertificate, which is checked later by
   // VerifySignatureAlgorithmsMatch)
-  out->signature_algorithm =
-      SignatureAlgorithm::CreateFromDer(out->cert.signature_algorithm_tlv);
+  out->signature_algorithm = SignatureAlgorithm::CreateFromDer(
+      cert.parsed_cert().signature_algorithm_tlv);
   if (!out->signature_algorithm)
     return false;
 
-  // Parse the TBSCertificate.
-  if (!ParseTbsCertificate(out->cert.tbs_certificate_tlv, &out->tbs))
-    return false;
-
   // Reset state relating to extensions (which may not get overwritten). This is
   // just a precaution, since in practice |out| will already be default
   // initialize.
   out->has_basic_constraints = false;
   out->has_key_usage = false;
   out->unconsumed_extensions.clear();
   out->subject_alt_names.reset();
   out->has_name_constraints = false;
 
   // Parse the standard X.509 extensions and remove them from
   // |unconsumed_extensions|.
-  if (out->tbs.has_extensions) {
+  if (cert.parsed_tbs().has_extensions) {
     // ParseExtensions() ensures there are no duplicates, and maps the (unique)
     // OID to the extension value.
-    if (!ParseExtensions(out->tbs.extensions_tlv, &out->unconsumed_extensions))
+    if (!ParseExtensions(cert.parsed_tbs().extensions_tlv,
+                         &out->unconsumed_extensions))
       return false;
 
     ParsedExtension extension;
 
     // Basic constraints.
     if (ConsumeExtension(BasicConstraintsOid(), &out->unconsumed_extensions,
                          &extension)) {
       out->has_basic_constraints = true;
       if (!ParseBasicConstraints(extension.value, &out->basic_constraints))
         return false;
@@ skipping 15 matching lines @@
       out->subject_alt_names = GeneralNames::CreateFromDer(extension.value);
       if (!out->subject_alt_names)
         return false;
       // RFC 5280 section 4.1.2.6:
       // If subject naming information is present only in the subjectAltName
       // extension (e.g., a key bound only to an email address or URI), then the
       // subject name MUST be an empty sequence and the subjectAltName extension
       // MUST be critical.
       if (!extension.critical) {
         der::Input subject_value;
-        if (!GetSequenceValue(out->tbs.subject_tlv, &subject_value))
+        if (!GetSequenceValue(cert.parsed_tbs().subject_tlv, &subject_value))
           return false;
         if (subject_value.Length() == 0)
           return false;
       }
     }
 
     // Name constraints.
     if (ConsumeExtension(NameConstraintsOid(), &out->unconsumed_extensions,
                          &out->name_constraints_extension)) {
       out->has_name_constraints = true;
@@ skipping 25 matching lines @@
 //
 //    A certificate is self-issued if the same DN appears in the subject
 //    and issuer fields (the two DNs are the same if they match according
 //    to the rules specified in Section 7.1).  In general, the issuer and
 //    subject of the certificates that make up a path are different for
 //    each certificate.  However, a CA may issue a certificate to itself to
 //    support key rollover or changes in certificate policies.  These
 //    self-issued certificates are not counted when evaluating path length
 //    or name constraints.
 WARN_UNUSED_RESULT bool IsSelfIssued(const FullyParsedCert& cert) {
-  return NameMatches(cert.tbs.subject_tlv, cert.tbs.issuer_tlv);
+  return cert.cert->normalized_subject() == cert.cert->normalized_issuer();
 }
 
 // Returns true if |cert| is valid at time |time|.
 //
 // The certificate's validity requirements are described by RFC 5280 section
 // 4.1.2.5:
 //
 //    The validity period for a certificate is the period of time from
 //    notBefore through notAfter, inclusive.
 WARN_UNUSED_RESULT bool VerifyTimeValidity(const FullyParsedCert& cert,
                                            const der::GeneralizedTime time) {
-  return !(time < cert.tbs.validity_not_before) &&
-         !(cert.tbs.validity_not_after < time);
+  return !(time < cert.cert->parsed_tbs().validity_not_before) &&
+         !(cert.cert->parsed_tbs().validity_not_after < time);
 }
 
 // Returns true if |signature_algorithm_tlv| is a valid algorithm encoding for
 // RSA with SHA1.
 WARN_UNUSED_RESULT bool IsRsaWithSha1SignatureAlgorithm(
     const der::Input& signature_algorithm_tlv) {
   std::unique_ptr<SignatureAlgorithm> algorithm =
       SignatureAlgorithm::CreateFromDer(signature_algorithm_tlv);
 
   return algorithm &&
@@ skipping 15 matching lines @@
 //
 // The spec is not explicit about what "the same algorithm identifier" means.
 // Our interpretation is that the two DER-encoded fields must be byte-for-byte
 // identical.
 //
 // In practice however there are certificates which use different encodings for
 // specifying RSA with SHA1 (different OIDs). This is special-cased for
 // compatibility sake.
 WARN_UNUSED_RESULT bool VerifySignatureAlgorithmsMatch(
     const FullyParsedCert& cert) {
-  const der::Input& alg1_tlv = cert.cert.signature_algorithm_tlv;
-  const der::Input& alg2_tlv = cert.tbs.signature_algorithm_tlv;
+  const der::Input& alg1_tlv = cert.cert->parsed_cert().signature_algorithm_tlv;
+  const der::Input& alg2_tlv = cert.cert->parsed_tbs().signature_algorithm_tlv;
 
   // Ensure that the two DER-encoded signature algorithms are byte-for-byte
   // equal, but make a compatibility concession for RSA with SHA1.
   return alg1_tlv == alg2_tlv || (IsRsaWithSha1SignatureAlgorithm(alg1_tlv) &&
                                   IsRsaWithSha1SignatureAlgorithm(alg2_tlv));
 }
 
 // This function corresponds to RFC 5280 section 6.1.3's "Basic Certificate
 // Processing" procedure.
 //
 // |skip_issuer_checks| controls whether the function will skip:
 //   - Checking that |cert|'s signature using |working_spki|
-//   - Checkinging that |cert|'s issuer matches |working_issuer_name|
+//   - Checkinging that |cert|'s issuer matches |working_normalized_issuer_name|
 // This should be set to true only when verifying a trusted root certificate.
 WARN_UNUSED_RESULT bool BasicCertificateProcessing(
     const FullyParsedCert& cert,
     bool is_target_cert,
     bool skip_issuer_checks,
     const SignaturePolicy* signature_policy,
     const der::GeneralizedTime& time,
     const der::Input& working_spki,
-    const der::Input& working_issuer_name,
+    const der::Input& working_normalized_issuer_name,
     const std::vector<std::unique_ptr<NameConstraints>>&
         name_constraints_list) {
   // Check that the signature algorithms in Certificate vs TBSCertificate
   // match. This isn't part of RFC 5280 section 6.1.3, but is mandated by
   // sections 4.1.1.2 and 4.1.2.3.
   if (!VerifySignatureAlgorithmsMatch(cert))
     return false;
 
   // Verify the digital signature using the previous certificate's key (RFC
   // 5280 section 6.1.3 step a.1).
   if (!skip_issuer_checks) {
-    if (!VerifySignedData(
-            *cert.signature_algorithm, cert.cert.tbs_certificate_tlv,
-            cert.cert.signature_value, working_spki, signature_policy)) {
+    if (!VerifySignedData(*cert.signature_algorithm,
+                          cert.cert->parsed_cert().tbs_certificate_tlv,
+                          cert.cert->parsed_cert().signature_value,
+                          working_spki, signature_policy)) {
       return false;
     }
   }
 
   // Check the time range for the certificate's validity, ensuring it is valid
   // at |time|.
   // (RFC 5280 section 6.1.3 step a.2)
   if (!VerifyTimeValidity(cert, time))
     return false;
 
   // TODO(eroman): Check revocation (RFC 5280 section 6.1.3 step a.3)
 
   // Verify the certificate's issuer name matches the issuing certificate's
   // subject name. (RFC 5280 section 6.1.3 step a.4)
   if (!skip_issuer_checks) {
-    if (!NameMatches(cert.tbs.issuer_tlv, working_issuer_name))
+    if (der::Input(&cert.cert->normalized_issuer()) !=
+        working_normalized_issuer_name)
       return false;
   }
 
   // Name constraints (RFC 5280 section 6.1.3 step b & c)
   // If certificate i is self-issued and it is not the final certificate in the
   // path, skip this step for certificate i.
   if (!name_constraints_list.empty() &&
       (!IsSelfIssued(cert) || is_target_cert)) {
+    // XXX used normalized_subject here
     der::Input subject_value;
-    if (!GetSequenceValue(cert.tbs.subject_tlv, &subject_value))
+    if (!GetSequenceValue(cert.cert->parsed_tbs().subject_tlv, &subject_value))
       return false;
     for (const auto& nc : name_constraints_list) {
       if (!nc->IsPermittedCert(subject_value, cert.subject_alt_names.get()))
         return false;
     }
   }
 
   // TODO(eroman): Steps d-f are omitted, as policy constraints are not yet
   // implemented.
 
   return true;
 }
 
 // This function corresponds to RFC 5280 section 6.1.4's "Preparation for
 // Certificate i+1" procedure. |cert| is expected to be an intermediary.
 WARN_UNUSED_RESULT bool PrepareForNextCertificate(
     const FullyParsedCert& cert,
     size_t* max_path_length_ptr,
     der::Input* working_spki,
-    der::Input* working_issuer_name,
+    der::Input* working_normalized_issuer_name,
     std::vector<std::unique_ptr<NameConstraints>>* name_constraints_list) {
   // TODO(eroman): Steps a-b are omitted, as policy constraints are not yet
   // implemented.
 
   // From RFC 5280 section 6.1.4 step c:
   //
-  //    Assign the certificate subject name to working_issuer_name.
-  *working_issuer_name = cert.tbs.subject_tlv;
+  //    Assign the certificate subject name to working_normalized_issuer_name.
+  *working_normalized_issuer_name =
+      der::Input(&cert.cert->normalized_subject());
 
   // From RFC 5280 section 6.1.4 step d:
   //
   //    Assign the certificate subjectPublicKey to working_public_key.
-  *working_spki = cert.tbs.spki_tlv;
+  *working_spki = cert.cert->parsed_tbs().spki_tlv;
 
   // Note that steps e and f are omitted as they are handled by
   // the assignment to |working_spki| above. See the definition
   // of |working_spki|.
 
   // From RFC 5280 section 6.1.4 step g:
   if (cert.has_name_constraints) {
     std::unique_ptr<NameConstraints> name_constraints(
         NameConstraints::CreateFromDer(
             cert.name_constraints_extension.value,
@@ skipping 133 matching lines @@
   // 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.
   if (!VerifyTargetCertHasConsistentCaBits(cert))
     return false;
 
   return true;
 }
 
 }  // namespace
 
-TrustAnchor::TrustAnchor() {}
-TrustAnchor::~TrustAnchor() {}
+CertThing::CertThing() {}
+CertThing::~CertThing() {}
 
-std::unique_ptr<TrustAnchor> TrustAnchor::CreateFromCertificateData(
+std::unique_ptr<CertThing> CertThing::CreateFromCertificateData(
     const uint8_t* data,
     size_t length,
     DataSource source) {
-  std::unique_ptr<TrustAnchor> result(new TrustAnchor);
+  std::unique_ptr<CertThing> result(new CertThing);
 
   switch (source) {
     case DataSource::INTERNAL_COPY:
       result->cert_data_.assign(data, data + length);
       result->cert_ =
           der::Input(result->cert_data_.data(), result->cert_data_.size());
       break;
     case DataSource::EXTERNAL_REFERENCE:
       result->cert_ = der::Input(data, length);
       break;
   }
 
   // Parse the certificate to get its name.
-  ParsedCertificate cert;
-  if (!ParseCertificate(result->cert(), &cert))
+  if (!ParseCertificate(result->cert_, &result->parsed_cert_))
     return nullptr;
 
-  ParsedTbsCertificate tbs;
-  if (!ParseTbsCertificate(cert.tbs_certificate_tlv, &tbs))
+  if (!ParseTbsCertificate(result->parsed_cert_.tbs_certificate_tlv,
+                           &result->parsed_tbs_))
     return nullptr;
 
-  result->name_ = tbs.subject_tlv;
-
-  // TODO(eroman): If adding a self-signed certificate, check that its
-  // signature is correct? This check will not otherwise be done during
-  // verification.
+  der::Input subject_value;
+  if (!GetSequenceValue(result->parsed_tbs_.subject_tlv, &subject_value) ||
+      !NormalizeName(subject_value, &result->normalized_subject_))
+    return nullptr;
+  der::Input issuer_value;
+  if (!GetSequenceValue(result->parsed_tbs_.issuer_tlv, &issuer_value) ||
+      !NormalizeName(issuer_value, &result->normalized_issuer_))
+    return nullptr;
 
   return result;
 }
 
-bool TrustAnchor::MatchesName(const der::Input& name) const {
-  return NameMatches(name, name_);
+std::unique_ptr<CertThing> CertThing::CreateFromCertificateCopy(
+    const base::StringPiece& data) {
+  return CertThing::CreateFromCertificateData(
+      reinterpret_cast<const uint8_t*>(data.data()), data.size(),
+      DataSource::INTERNAL_COPY);
+}
+
+std::unique_ptr<CertThing> CertThing::Clone() const {
+  // XXX Should it always INTERNAL_COPY, or should it continue with what the
+  // previous had?.. lifetimes could get weird with that. (Eg, someone created a
+  // Cert, passed it to a function they could gaurantee it would outlive, but
+  // that function internall makes a clone and returns it as part of the results
+  // or error info..)
+  /*std::unique_ptr<CertThing>
+  result(CreateFromCertificateData(cert_.UnsafeData(), cert_.Length(),
+  CertThing::DataSource::INTERNAL_COPY));
+  // If it parsed to make this object, it should never fail to parse again.
+  CHECK(result);
+  return result;*/
+
+  std::unique_ptr<CertThing> result(new CertThing);
+
+  result->cert_data_.assign(
+      reinterpret_cast<const char*>(cert_.UnsafeData()),
+      reinterpret_cast<const char*>(cert_.UnsafeData()) + cert_.Length());
+  result->cert_ =
+      der::Input(result->cert_data_.data(), result->cert_data_.size());
+
+  result->parsed_cert_ = ParsedCertificate(parsed_cert_, cert_, result->cert_);
+  result->parsed_tbs_ = ParsedTbsCertificate(parsed_tbs_, cert_, result->cert_);
+
+  result->normalized_subject_ = normalized_subject_;
+  result->normalized_issuer_ = normalized_issuer_;
+
+  // XXX clean up
+  /*result->parsed_cert_.tbs_certificate_tlv = der::Input(
+      result->cert_.UnsafeData() +
+          (parsed_cert_.tbs_certificate_tlv.UnsafeData() - cert_.UnsafeData()),
+      parsed_cert_.tbs_certificate_tlv.Length());*/
+
+  return result;
+}
+
+bool CertThing::MatchesName(const der::Input& name) const {
+  return NameMatches(name, parsed_tbs_.subject_tlv);  // XXX don't re-normalize
 }
 
 TrustStore::TrustStore() {}
 TrustStore::~TrustStore() {}
 
 void TrustStore::Clear() {
   anchors_.clear();
 }
 
 bool TrustStore::AddTrustedCertificate(const uint8_t* data, size_t length) {
+  // TODO(eroman): If adding a self-signed certificate, check that its
+  // signature is correct? This check will not otherwise be done during
+  // verification.
   return AddTrustedCertificate(data, length,
-                               TrustAnchor::DataSource::INTERNAL_COPY);
+                               CertThing::DataSource::INTERNAL_COPY);
 }
 
 bool TrustStore::AddTrustedCertificate(const base::StringPiece& data) {
   return AddTrustedCertificate(reinterpret_cast<const uint8_t*>(data.data()),
                                data.size());
 }
 
 bool TrustStore::AddTrustedCertificateWithoutCopying(const uint8_t* data,
                                                      size_t length) {
   return AddTrustedCertificate(data, length,
-                               TrustAnchor::DataSource::EXTERNAL_REFERENCE);
+                               CertThing::DataSource::EXTERNAL_REFERENCE);
 }
 
-const TrustAnchor* TrustStore::FindTrustAnchorByName(
+const CertThing* TrustStore::FindTrustAnchorByName(
     const der::Input& name) const {
   for (const auto& anchor : anchors_) {
     if (anchor->MatchesName(name)) {
       return anchor.get();
     }
   }
   return nullptr;
 }
 
+std::vector<const CertThing*> TrustStore::FindTrustAnchorsByNormalizedName(
+    const std::string& normalized_name) const {
+  std::vector<const CertThing*> result;
+  for (const auto& anchor : anchors_) {
+    if (anchor->normalized_subject() == normalized_name) {
+      result.push_back(anchor.get());
+    }
+  }
+  return result;
+}
+
 bool TrustStore::IsTrustedCertificate(const der::Input& cert_der) const {
   for (const auto& anchor : anchors_) {
-    if (anchor->cert() == cert_der)
+    if (anchor->der_cert() == cert_der)
       return true;
   }
   return false;
 }
 
 bool TrustStore::AddTrustedCertificate(const uint8_t* data,
                                        size_t length,
-                                       TrustAnchor::DataSource source) {
-  auto anchor = TrustAnchor::CreateFromCertificateData(data, length, source);
+                                       CertThing::DataSource source) {
+  auto anchor = CertThing::CreateFromCertificateData(data, length, source);
   if (!anchor)
     return false;
   anchors_.push_back(std::move(anchor));
   return true;
 }
 
-// TODO(eroman): Move this into existing anonymous namespace.
-namespace {
-
 // This implementation is structured to mimic the description of certificate
 // path verification given by RFC 5280 section 6.1.
 //
 // Unlike RFC 5280, the trust anchor is specified as the root certificate in
 // the chain. This root certificate is assumed to be trusted, and neither its
 // signature nor issuer name are verified. (It needn't be self-signed).
 bool VerifyCertificateChainAssumingTrustedRoot(
-    const std::vector<der::Input>& certs_der,
+    const std::vector<std::unique_ptr<CertThing>>& certs,
     // The trust store is only used for assertions.
     const TrustStore& trust_store,
     const SignaturePolicy* signature_policy,
     const der::GeneralizedTime& time) {
   // An empty chain is necessarily invalid.
-  if (certs_der.empty())
+  if (certs.empty())
     return false;
 
   // IMPORTANT: the assumption being made is that the root certificate in
   // the given path is the trust anchor (and has already been verified as
   // such).
-  DCHECK(trust_store.IsTrustedCertificate(certs_der.back()));
+  DCHECK(trust_store.IsTrustedCertificate(certs.back()->der_cert()));  // XXX
 
   // 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<std::unique_ptr<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;
 
-  // |working_issuer_name| corresponds with the same named variable in RFC 5280
-  // section 6.1.2:
+  // |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_issuer_name;
+  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_der.size();
+  size_t max_path_length = certs.size();
 
   // Iterate over all the certificates in the reverse direction: starting from
   // the 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    :  Trust anchor.
   //   * i=N-1  :  Target certificate.
-  for (size_t i = 0; i < certs_der.size(); ++i) {
-    const size_t index_into_certs_der = certs_der.size() - i - 1;
+  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_der == 0;
+    const bool is_target_cert = index_into_certs == 0;
 
     // |is_trust_anchor| is true if the current certificate is the trust
     // anchor. This certificate is implicitly trusted.
     const bool is_trust_anchor = i == 0;
 
     // Parse the current certificate into |cert|.
     FullyParsedCert cert;
-    const der::Input& cert_der = certs_der[index_into_certs_der];
-    if (!FullyParseCertificate(cert_der, &cert))
+    const CertThing& cert_thing = *certs[index_into_certs];
+    if (!FullyParseCertificate(cert_thing, &cert))
       return false;
 
     // 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"
-    if (!BasicCertificateProcessing(
-            cert, is_target_cert, is_trust_anchor, signature_policy, time,
-            working_spki, working_issuer_name, name_constraints_list)) {
+    if (!BasicCertificateProcessing(cert, is_target_cert, is_trust_anchor,
+                                    signature_policy, time, working_spki,
+                                    working_normalized_issuer_name,
+                                    name_constraints_list)) {
       return false;
     }
     if (!is_target_cert) {
       if (!PrepareForNextCertificate(cert, &max_path_length, &working_spki,
-                                     &working_issuer_name,
+                                     &working_normalized_issuer_name,
                                      &name_constraints_list)) {
         return false;
       }
     } else {
       if (!WrapUp(cert))
         return false;
     }
   }
 
   // TODO(eroman): RFC 5280 forbids duplicate certificates per section 6.1:
   //
   //    A certificate MUST NOT appear more than once in a prospective
   //    certification path.
 
   return true;
 }
 
+// TODO(eroman): Move this into existing anonymous namespace.
+namespace {
+
 // TODO(eroman): This function is a temporary hack in the absence of full
 // path building. It may insert 1 certificate at the root of the
 // chain to ensure that the path's root certificate is a trust anchor.
 //
 // Beyond this no other verification is done on the chain. The caller is
 // responsible for verifying the subsequent chain's correctness.
 WARN_UNUSED_RESULT bool BuildSimplePathToTrustAnchor(
     const std::vector<der::Input>& certs_der,
     const TrustStore& trust_store,
     std::vector<der::Input>* certs_der_trusted_root) {
@@ skipping 14 matching lines @@
   ParsedCertificate cert;
   ParsedTbsCertificate tbs;
   if (!ParseCertificate(certs_der.back(), &cert) ||
       !ParseTbsCertificate(cert.tbs_certificate_tlv, &tbs)) {
     return false;
   }
 
   auto trust_anchor = trust_store.FindTrustAnchorByName(tbs.issuer_tlv);
   if (!trust_anchor)
     return false;
-  certs_der_trusted_root->push_back(trust_anchor->cert());
+  certs_der_trusted_root->push_back(trust_anchor->der_cert());
   return true;
 }
 
 }  // namespace
 
 bool VerifyCertificateChain(const std::vector<der::Input>& certs_der,
                             const TrustStore& trust_store,
                             const SignaturePolicy* signature_policy,
                             const der::GeneralizedTime& time) {
   // Modify the certificate chain so that its root is a trusted certificate.
   std::vector<der::Input> certs_der_trusted_root;
   if (!BuildSimplePathToTrustAnchor(certs_der, trust_store,
                                     &certs_der_trusted_root)) {
     return false;
   }
 
+  std::vector<std::unique_ptr<CertThing>> certs_trusted_root;
+  for (const auto& cert_der : certs_der_trusted_root) {
+    std::unique_ptr<CertThing> cert(CertThing::CreateFromCertificateData(
+        cert_der.UnsafeData(), cert_der.Length(),
+        CertThing::DataSource::EXTERNAL_REFERENCE));
+    if (!cert)
+      return false;
+    certs_trusted_root.push_back(std::move(cert));
+  }
+
   // Verify the chain.
   return VerifyCertificateChainAssumingTrustedRoot(
-      certs_der_trusted_root, trust_store, signature_policy, time);
+      certs_trusted_root, trust_store, signature_policy, time);
 }
 
 }  // namespace net