Certificate path builder for new certificate verification library

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 6a81c84d562b56aa48c2fcbcc3e7d9e1e312578b..fea130b33b0b24d7bc61d38fdad84c1e6eed544a 100644
--- a/components/cast_certificate/cast_cert_validator.cc
+++ b/components/cast_certificate/cast_cert_validator.cc
@@ -13,15 +13,16 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/memory/singleton.h"
+#include "net/cert/internal/cert_issuer_source_static.h"
 #include "net/cert/internal/certificate_policies.h"
 #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/path_builder.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"
 
@@ -245,20 +246,6 @@ 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_;
-};
-
 // Returns the parsing options used for Cast certificates.
 net::ParseCertificateOptions GetCertParsingOptions() {
   net::ParseCertificateOptions options;
@@ -281,22 +268,29 @@ bool VerifyDeviceCert(const std::vector<std::string>& certs,
                       const base::Time::Exploded& time,
                       std::unique_ptr<CertVerificationContext>* context,
                       CastDeviceCertPolicy* policy) {
-  // The underlying verification function expects a sequence of
-  // 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(),
+  if (certs.empty())
+    return false;
+
+  // No reference to these ParsedCertificates is kept past the end of this
+  // function, so using EXTERNAL_REFERENCE here is safe.
+  scoped_refptr<net::ParsedCertificate> target_cert(
+      net::ParsedCertificate::CreateFromCertificateData(
+          reinterpret_cast<const uint8_t*>(certs[0].data()), certs[0].size(),
+          net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE,
+          GetCertParsingOptions()));
+  if (!target_cert)
+    return false;
+
+  net::CertIssuerSourceStatic intermediate_cert_issuer_source;
+  for (size_t i = 1; i < certs.size(); ++i) {

[eroman, 2016/06/27 19:58:52]

optional: How about starting at i=0 and handle the target cert in here too? It
avoids duplicating the ugly calls to CreateFromCertificateData(), and the return
false above.

[mattm, 2016/06/27 23:45:44]

Done.

+    scoped_refptr<net::ParsedCertificate> cert(
+        net::ParsedCertificate::CreateFromCertificateData(
+            reinterpret_cast<const uint8_t*>(certs[i].data()), certs[i].size(),
             net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE,
-            GetCertParsingOptions(), &input_chain)) {
+            GetCertParsingOptions()));
+    if (!cert)
       return false;
-    }
+    intermediate_cert_issuer_source.AddCert(cert);
   }
 
   // Use a signature policy compatible with Cast's PKI.
@@ -304,15 +298,19 @@ bool VerifyDeviceCert(const std::vector<std::string>& certs,
 
   // Do RFC 5280 compatible certificate verification using the two Cast

[eroman, 2016/06/27 19:58:52]

Should this comment be expanded to mention path building?

[mattm, 2016/06/27 23:45:45]

Done.

   // trust anchors and Cast signature policy.
-  if (!net::VerifyCertificateChain(input_chain, CastTrustStore::Get(),
-                                   signature_policy.get(),
-                                   ConvertExplodedTime(time), nullptr)) {
+  net::CertPathBuilder::Result result;
+  net::CertPathBuilder path_builder(target_cert.get(), &CastTrustStore::Get(),
+                                    signature_policy.get(),
+                                    ConvertExplodedTime(time), &result);
+  path_builder.AddCertIssuerSource(&intermediate_cert_issuer_source);
+  net::CompletionStatus rv = path_builder.Run(base::Closure());
+  DCHECK_EQ(rv, net::CompletionStatus::SYNC);
+  if (!result.is_success())
     return false;
-  }
 
   // Check properties of the leaf certificate (key usage, policy), and construct
   // a CertVerificationContext that uses its public key.
-  return CheckTargetCertificate(input_chain[0].get(), context, policy);
+  return CheckTargetCertificate(target_cert.get(), context, policy);
 }
 
 std::unique_ptr<CertVerificationContext> CertVerificationContextImplForTest(