Add new ParsedCertificate class, move TrustStore to own file.

components/cast_certificate/cast_cert_validator.cc

Index: components/cast_certificate/cast_cert_validator.cc
diff --git a/components/cast_certificate/cast_cert_validator.cc b/components/cast_certificate/cast_cert_validator.cc
index b83ad541174068c04236a0af64260beda24bc4c5..37989500d4e279ebf9f6367f664874b6c568e98a 100644
--- a/components/cast_certificate/cast_cert_validator.cc
+++ b/components/cast_certificate/cast_cert_validator.cc
@@ -17,8 +17,10 @@
 #include "net/cert/internal/extended_key_usage.h"
 #include "net/cert/internal/parse_certificate.h"
 #include "net/cert/internal/parse_name.h"
+#include "net/cert/internal/parsed_certificate.h"
 #include "net/cert/internal/signature_algorithm.h"
 #include "net/cert/internal/signature_policy.h"
+#include "net/cert/internal/trust_store.h"
 #include "net/cert/internal/verify_certificate_chain.h"
 #include "net/cert/internal/verify_signed_data.h"
 #include "net/der/input.h"
@@ -55,10 +57,18 @@ class CastTrustStore {
 
   CastTrustStore() {
     // Initialize the trust store with two root certificates.
-    CHECK(store_.AddTrustedCertificateWithoutCopying(kCastRootCaDer,
-                                                     sizeof(kCastRootCaDer)));
-    CHECK(store_.AddTrustedCertificateWithoutCopying(kEurekaRootCaDer,
-                                                     sizeof(kEurekaRootCaDer)));
+    scoped_refptr<net::ParsedCertificate> root =
+        net::ParsedCertificate::CreateFromCertificateData(
+            kCastRootCaDer, sizeof(kCastRootCaDer),
+            net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE);
+    CHECK(root);
+    store_.AddTrustedCertificate(std::move(root));
+
+    root = net::ParsedCertificate::CreateFromCertificateData(
+        kEurekaRootCaDer, sizeof(kEurekaRootCaDer),
+        net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE);
+    CHECK(root);
+    store_.AddTrustedCertificate(std::move(root));
   }
 
   net::TrustStore store_;
@@ -167,50 +177,27 @@ bool HasClientAuth(const std::vector<net::der::Input>& ekus) {
 // Checks properties on the target certificate.
 //
 //   * The Key Usage must include Digital Signature
-//   * THe Extended Key Usage must includ TLS Client Auth
+//   * The Extended Key Usage must include TLS Client Auth
 //   * May have the policy 1.3.6.1.4.1.11129.2.5.2 to indicate it
 //     is an audio-only device.
 WARN_UNUSED_RESULT bool CheckTargetCertificate(
-    const net::der::Input& cert_der,
+    const net::ParsedCertificate* cert,
     std::unique_ptr<CertVerificationContext>* context,
     CastDeviceCertPolicy* policy) {
-  // TODO(eroman): Simplify this. The certificate chain verification
-  // function already parses this stuff, awkward to re-do it here.
-
-  net::der::Input tbs_certificate_tlv;
-  net::der::Input signature_algorithm_tlv;
-  net::der::BitString signature_value;
-  if (!net::ParseCertificate(cert_der, &tbs_certificate_tlv,
-                             &signature_algorithm_tlv, &signature_value))
-    return false;
-
-  net::ParsedTbsCertificate tbs;
-  if (!net::ParseTbsCertificate(tbs_certificate_tlv, &tbs))
-    return false;
-
-  // Get the extensions.
-  if (!tbs.has_extensions)
-    return false;
-  ExtensionsMap extensions;
-  if (!net::ParseExtensions(tbs.extensions_tlv, &extensions))
-    return false;
-
-  net::der::Input extension_value;
-
   // Get the Key Usage extension.
-  if (!GetExtensionValue(extensions, net::KeyUsageOid(), &extension_value))
-    return false;
-  net::der::BitString key_usage;
-  if (!net::ParseKeyUsage(extension_value, &key_usage))
+  if (!cert->has_key_usage())
     return false;
 
   // Ensure Key Usage contains digitalSignature.
-  if (!key_usage.AssertsBit(net::KEY_USAGE_BIT_DIGITAL_SIGNATURE))
+  if (!cert->key_usage().AssertsBit(net::KEY_USAGE_BIT_DIGITAL_SIGNATURE))
     return false;
 
   // Get the Extended Key Usage extension.
-  if (!GetExtensionValue(extensions, net::ExtKeyUsageOid(), &extension_value))
+  net::der::Input extension_value;
+  if (!GetExtensionValue(cert->unparsed_extensions(), net::ExtKeyUsageOid(),
+                         &extension_value)) {
     return false;
+  }
   std::vector<net::der::Input> ekus;
   if (!net::ParseEKUExtension(extension_value, &ekus))
     return false;
@@ -221,8 +208,8 @@ WARN_UNUSED_RESULT bool CheckTargetCertificate(
 
   // Check for an optional audio-only policy extension.
   *policy = CastDeviceCertPolicy::NONE;
-  if (GetExtensionValue(extensions, net::CertificatePoliciesOid(),
-                        &extension_value)) {
+  if (GetExtensionValue(cert->unparsed_extensions(),
+                        net::CertificatePoliciesOid(), &extension_value)) {
     std::vector<net::der::Input> policies;
     if (!net::ParseCertificatePoliciesExtension(extension_value, &policies))
       return false;
@@ -236,10 +223,11 @@ WARN_UNUSED_RESULT bool CheckTargetCertificate(
 
   // Get the Common Name for the certificate.
   std::string common_name;
-  if (!GetCommonNameFromSubject(tbs.subject_tlv, &common_name))
+  if (!GetCommonNameFromSubject(cert->tbs().subject_tlv, &common_name))
     return false;
 
-  context->reset(new CertVerificationContextImpl(tbs.spki_tlv, common_name));
+  context->reset(
+      new CertVerificationContextImpl(cert->tbs().spki_tlv, common_name));
   return true;
 }
 
@@ -256,6 +244,20 @@ net::der::GeneralizedTime ConvertExplodedTime(
   return result;
 }
 
+class ScopedCheckUnreferencedCerts {
+ public:
+  explicit ScopedCheckUnreferencedCerts(
+      std::vector<scoped_refptr<net::ParsedCertificate>>* certs)
+      : certs_(certs) {}
+  ~ScopedCheckUnreferencedCerts() {
+    for (const auto& cert : *certs_)
+      DCHECK(cert->HasOneRef());
+  }
+
+ private:
+  std::vector<scoped_refptr<net::ParsedCertificate>>* certs_;
+};
+
 }  // namespace
 
 bool VerifyDeviceCert(const std::vector<std::string>& certs,
@@ -263,10 +265,22 @@ bool VerifyDeviceCert(const std::vector<std::string>& certs,
                       std::unique_ptr<CertVerificationContext>* context,
                       CastDeviceCertPolicy* policy) {
   // The underlying verification function expects a sequence of
-  // der::Input, so wrap the data in it (cheap).
-  std::vector<net::der::Input> input_chain;
-  for (const auto& cert : certs)
-    input_chain.push_back(net::der::Input(&cert));
+  // ParsedCertificate.
+  std::vector<scoped_refptr<net::ParsedCertificate>> input_chain;
+  // Verify that nothing saves a reference to the input certs, since the backing
+  // data will go out of scope when the function finishes.
+  ScopedCheckUnreferencedCerts ref_checker(&input_chain);
+
+  for (const auto& cert_der : certs) {
+    // No reference to the ParsedCertificate is kept past the end of this
+    // function, so using EXTERNAL_REFERENCE here is safe.
+    if (!net::ParsedCertificate::CreateAndAddToVector(
+            reinterpret_cast<const uint8_t*>(cert_der.data()), cert_der.size(),
+            net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE,
+            &input_chain)) {
+      return false;
+    }
+  }
 
   // Use a signature policy compatible with Cast's PKI.
   auto signature_policy = CreateCastSignaturePolicy();
@@ -281,7 +295,7 @@ bool VerifyDeviceCert(const std::vector<std::string>& certs,
 
   // Check properties of the leaf certificate (key usage, policy), and construct
   // a CertVerificationContext that uses its public key.
-  return CheckTargetCertificate(input_chain[0], context, policy);
+  return CheckTargetCertificate(input_chain[0].get(), context, policy);
 }
 
 std::unique_ptr<CertVerificationContext> CertVerificationContextImplForTest(
@@ -293,8 +307,14 @@ std::unique_ptr<CertVerificationContext> CertVerificationContextImplForTest(
 }
 
 bool AddTrustAnchorForTest(const uint8_t* data, size_t length) {

[sheretov, 2016/06/03 22:08:02]

This looks strange to me.  The old code was adding the passed-in cert to the
trust store.  The new code ignores the arguments, and instead adds the
hard-coded root cert to the trust store.  However, kCastRootCaDer should have
already been in the trust store, based on logic in
CastTrustStore::CastTrustStore().  So this entire function looks like a no-op.

[eroman, 2016/06/03 22:10:27]

Agreed, this is wrong.
I should have noticed during review.

[mattm, 2016/06/03 22:26:12]

Eek, thanks. Fixed.

-  return CastTrustStore::Get().AddTrustedCertificateWithoutCopying(data,
-                                                                   length);
+  scoped_refptr<net::ParsedCertificate> anchor(
+      net::ParsedCertificate::CreateFromCertificateData(
+          kCastRootCaDer, sizeof(kCastRootCaDer),
+          net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE));
+  if (!anchor)
+    return false;
+  CastTrustStore::Get().AddTrustedCertificate(std::move(anchor));
+  return true;
 }
 
 }  // namespace cast_certificate