Add CertErrors* parameter to the main Certificate parsing functions.

This just updates the interface and callers. Actually emitting errors
in the underlying implementation will be a different CL.

BUG=634443

Review-Url: https://codereview.chromium.org/2327973002
Cr-Commit-Position: refs/heads/master@{#418586}

components/cast_certificate/cast_cert_validator.cc

@@ -67,11 +67,11 @@ class CastTrustStore {
   // storage.
   template <size_t N>
   void AddAnchor(const uint8_t (&data)[N]) {
+    net::CertErrors errors;
     scoped_refptr<net::ParsedCertificate> cert =
-        net::ParsedCertificate::CreateFromCertificateData(
-            data, N, net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE,
-            {});
-    CHECK(cert);
+        net::ParsedCertificate::CreateWithoutCopyingUnsafe(data, N, {},
+                                                           &errors);
+    CHECK(cert) << errors.ToDebugString();
     // Enforce pathlen constraints and policies defined on the root certificate.
     scoped_refptr<net::TrustAnchor> anchor =
         net::TrustAnchor::CreateFromCertificateWithConstraints(std::move(cert));
@@ -255,7 +255,7 @@ net::ParseCertificateOptions GetCertParsingOptions() {
   return options;
 }
 
-// Verifies a cast device certficate given a chain of DER-encoded certificates.
+// Verifies a cast device certificate given a chain of DER-encoded certificates.
 bool VerifyDeviceCert(const std::vector<std::string>& certs,
                       const base::Time& time,
                       std::unique_ptr<CertVerificationContext>* context,
@@ -266,16 +266,13 @@ bool VerifyDeviceCert(const std::vector<std::string>& certs,
   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.
+  net::CertErrors errors;
   scoped_refptr<net::ParsedCertificate> target_cert;
   net::CertIssuerSourceStatic intermediate_cert_issuer_source;
   for (size_t i = 0; i < certs.size(); ++i) {
-    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()));
+    scoped_refptr<net::ParsedCertificate> cert(net::ParsedCertificate::Create(
+        certs[i], GetCertParsingOptions(), &errors));
+    // TODO(eroman): Propagate/log these parsing errors.
     if (!cert)
       return false;
 

components/cast_certificate/cast_cert_validator_unittest.cc

@@ -5,6 +5,7 @@
 #include "components/cast_certificate/cast_cert_validator.h"
 
 #include "components/cast_certificate/cast_cert_validator_test_helpers.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/parsed_certificate.h"
 #include "net/cert/internal/trust_store_in_memory.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -79,9 +80,10 @@ void RunTest(TestResult expected_result,
       ASSERT_FALSE(certs.empty());
 
       // Parse the root certificate of the chain.
+      net::CertErrors errors;
       scoped_refptr<net::ParsedCertificate> root =
-          net::ParsedCertificate::CreateFromCertificateCopy(certs.back(), {});
-      ASSERT_TRUE(root);
+          net::ParsedCertificate::Create(certs.back(), {}, &errors);
+      ASSERT_TRUE(root) << errors.ToDebugString();
 
       // Remove it from the chain.
       certs.pop_back();

components/cast_certificate/cast_crl.cc

@@ -12,6 +12,7 @@
 #include "base/memory/singleton.h"
 #include "components/cast_certificate/proto/revocation.pb.h"
 #include "crypto/sha2.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/parse_certificate.h"
 #include "net/cert/internal/parsed_certificate.h"
 #include "net/cert/internal/path_builder.h"
@@ -23,8 +24,8 @@
 #include "net/cert/x509_certificate.h"
 #include "net/der/encode_values.h"
 #include "net/der/input.h"
-#include "net/der/parser.h"
 #include "net/der/parse_values.h"
+#include "net/der/parser.h"
 
 namespace cast_certificate {
 namespace {
@@ -62,11 +63,11 @@ class CastCRLTrustStore {
 
   CastCRLTrustStore() {
     // Initialize the trust store with the root certificate.
+    net::CertErrors errors;
     scoped_refptr<net::ParsedCertificate> cert =
-        net::ParsedCertificate::CreateFromCertificateData(
-            kCastCRLRootCaDer, sizeof(kCastCRLRootCaDer),
-            net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE, {});
-    CHECK(cert);
+        net::ParsedCertificate::CreateWithoutCopyingUnsafe(
+            kCastCRLRootCaDer, sizeof(kCastCRLRootCaDer), {}, &errors);
+    CHECK(cert) << errors.ToDebugString();
     // Enforce pathlen constraints and policies defined on the root certificate.
     scoped_refptr<net::TrustAnchor> anchor =
         net::TrustAnchor::CreateFromCertificateWithConstraints(std::move(cert));
@@ -107,13 +108,12 @@ bool VerifyCRL(const Crl& crl,
                net::TrustStore* trust_store,
                net::der::GeneralizedTime* overall_not_after) {
   // Verify the trust of the CRL authority.
+  net::CertErrors parse_errors;
   scoped_refptr<net::ParsedCertificate> parsed_cert =
-      net::ParsedCertificate::CreateFromCertificateData(
-          reinterpret_cast<const uint8_t*>(crl.signer_cert().data()),
-          crl.signer_cert().size(),
-          net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE, {});
+      net::ParsedCertificate::Create(crl.signer_cert(), {}, &parse_errors);
   if (parsed_cert == nullptr) {
-    VLOG(2) << "CRL - Issuer certificate parsing failed.";
+    VLOG(2) << "CRL - Issuer certificate parsing failed:\n"
+            << parse_errors.ToDebugString();
     return false;
   }
 
@@ -125,13 +125,13 @@ bool VerifyCRL(const Crl& crl,
   auto signature_policy = CreateCastSignaturePolicy();
   std::unique_ptr<net::SignatureAlgorithm> signature_algorithm_type =
       net::SignatureAlgorithm::CreateRsaPkcs1(net::DigestAlgorithm::Sha256);
-  net::CertErrors errors;
+  net::CertErrors verify_errors;
   if (!VerifySignedData(*signature_algorithm_type,
                         net::der::Input(&crl.tbs_crl()),
                         signature_value_bit_string, parsed_cert->tbs().spki_tlv,
-                        signature_policy.get(), &errors)) {
-    // TODO(634443): Dump the error information.
-    VLOG(2) << "CRL - Signature verification failed.";
+                        signature_policy.get(), &verify_errors)) {
+    VLOG(2) << "CRL - Signature verification failed:\n"
+            << verify_errors.ToDebugString();
     return false;
   }
 

components/cast_certificate/cast_crl_unittest.cc

@@ -7,6 +7,7 @@
 #include "components/cast_certificate/cast_cert_validator_test_helpers.h"
 #include "components/cast_certificate/cast_crl.h"
 #include "components/cast_certificate/proto/test_suite.pb.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/trust_store_in_memory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -21,9 +22,10 @@ std::unique_ptr<net::TrustStoreInMemory> CreateTrustStoreFromFile(
   const auto trusted_test_roots =
       cast_certificate::testing::ReadCertificateChainFromFile(path);
   for (const auto& trusted_root : trusted_test_roots) {
+    net::CertErrors errors;
     scoped_refptr<net::ParsedCertificate> cert(
-        net::ParsedCertificate::CreateFromCertificateCopy(trusted_root, {}));
-    EXPECT_TRUE(cert);
+        net::ParsedCertificate::Create(trusted_root, {}, &errors));
+    EXPECT_TRUE(cert) << errors.ToDebugString();
     scoped_refptr<net::TrustAnchor> anchor =
         net::TrustAnchor::CreateFromCertificateWithConstraints(std::move(cert));
     trust_store->AddTrustAnchor(std::move(anchor));

net/cert/internal/cert_issuer_source_aia.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "net/cert/cert_net_fetcher.h"
+#include "net/cert/internal/cert_errors.h"
 #include "url/gurl.h"
 
 namespace net {
@@ -86,11 +87,13 @@ void AiaRequest::OnFetchCompleted(Error error,
     // TODO(mattm): Avoid copying bytes. Change the CertNetFetcher and
     // ParsedCertificate interface to allow passing through ownership of the
     // bytes.
-    if (!ParsedCertificate::CreateAndAddToVector(
-            fetched_bytes.data(), fetched_bytes.size(),
-            ParsedCertificate::DataSource::INTERNAL_COPY, {}, &results_)) {
-      // TODO(mattm): propagate error info.
-      LOG(ERROR) << "Error parsing AIA data";
+    CertErrors errors;
+    if (!ParsedCertificate::CreateAndAddToVector(fetched_bytes.data(),
+                                                 fetched_bytes.size(), {},
+                                                 &results_, &errors)) {
+      // TODO(crbug.com/634443): propagate error info.
+      LOG(ERROR) << "Error parsing cert retrieved from AIA:\n"
+                 << errors.ToDebugString();
     }
   }
   // If the client is waiting for results, need to run callback if:

net/cert/internal/cert_issuer_source_aia_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "net/cert/cert_net_fetcher.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/parsed_certificate.h"
 #include "net/cert/internal/test_helpers.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -38,9 +39,13 @@ using ::testing::StrictMock;
                   "CERTIFICATE", &der);
   if (!r)
     return r;
-  *result = ParsedCertificate::CreateFromCertificateCopy(der, {});
-  if (!*result)
-    return ::testing::AssertionFailure() << "CreateFromCertificateCopy failed";
+  CertErrors errors;
+  *result = ParsedCertificate::Create(der, {}, &errors);
+  if (!*result) {
+    return ::testing::AssertionFailure()
+           << "ParsedCertificate::Create() failed:\n"
+           << errors.ToDebugString();
+  }
   return ::testing::AssertionSuccess();
 }
 

net/cert/internal/cert_issuer_source_static_unittest.cc

@@ -5,6 +5,7 @@
 #include "net/cert/internal/cert_issuer_source_static.h"
 
 #include "base/bind.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/parsed_certificate.h"
 #include "net/cert/internal/test_helpers.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -36,9 +37,13 @@ void NotCalled(CertIssuerSource::Request* req) {
                   "CERTIFICATE", &der);
   if (!r)
     return r;
-  *result = ParsedCertificate::CreateFromCertificateCopy(der, {});
-  if (!*result)
-    return ::testing::AssertionFailure() << "CreateFromCertificateCopy failed";
+  CertErrors errors;
+  *result = ParsedCertificate::Create(der, {}, &errors);
+  if (!*result) {
+    return ::testing::AssertionFailure()
+           << "ParsedCertificate::Create() failed:\n"
+           << errors.ToDebugString();
+  }
   return ::testing::AssertionSuccess();
 }
 

net/cert/internal/parse_certificate.cc

@@ -170,7 +170,10 @@ bool VerifySerialNumber(const der::Input& value) {
 bool ParseCertificate(const der::Input& certificate_tlv,
                       der::Input* out_tbs_certificate_tlv,
                       der::Input* out_signature_algorithm_tlv,
-                      der::BitString* out_signature_value) {
+                      der::BitString* out_signature_value,
+                      CertErrors* out_errors) {
+  // TODO(crbug.com/634443): Fill |out_errors| (which may be null) with error
+  // information.
   der::Parser parser(certificate_tlv);
 
   //   Certificate  ::=  SEQUENCE  {

net/cert/internal/parse_certificate.h

@@ -17,6 +17,7 @@
 
 namespace net {
 
+class CertErrors;
 struct ParsedTbsCertificate;
 
 // Returns true if the given serial number (CertificateSerialNumber in RFC 5280)
@@ -55,14 +56,17 @@ struct NET_EXPORT ParseCertificateOptions {
 };
 
 // Parses a DER-encoded "Certificate" as specified by RFC 5280. Returns true on
-// success and sets the results in the |out_*| parameters.
+// success and sets the results in the |out_*| parameters. On both the failure
+// and success case, if |out_errors| was non-null it may contain extra error
+// information.
 //
 // Note that on success the out parameters alias data from the input
 // |certificate_tlv|.  Hence the output values are only valid as long as
 // |certificate_tlv| remains valid.
 //
-// On failure the out parameters have an undefined state. Some of them may have
-// been updated during parsing, whereas others may not have been changed.
+// On failure the out parameters have an undefined state, except for
+// out_errors. Some of them may have been updated during parsing, whereas
+// others may not have been changed.
 //
 // The out parameters represent each field of the Certificate SEQUENCE:
 //       Certificate  ::=  SEQUENCE  {
@@ -91,8 +95,8 @@ struct NET_EXPORT ParseCertificateOptions {
 NET_EXPORT bool ParseCertificate(const der::Input& certificate_tlv,
                                  der::Input* out_tbs_certificate_tlv,
                                  der::Input* out_signature_algorithm_tlv,
-                                 der::BitString* out_signature_value)
-    WARN_UNUSED_RESULT;
+                                 der::BitString* out_signature_value,
+                                 CertErrors* out_errors) WARN_UNUSED_RESULT;
 
 // Parses a DER-encoded "TBSCertificate" as specified by RFC 5280. Returns true
 // on success and sets the results in |out|. Certain invalid inputs may

net/cert/internal/parse_certificate_fuzzer.cc

@@ -6,6 +6,7 @@
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/certificate_policies.h"
 #include "net/cert/internal/extended_key_usage.h"
 #include "net/cert/internal/name_constraints.h"
@@ -32,8 +33,9 @@ void ParseCertificateForFuzzer(const der::Input& in) {
   der::Input tbs_certificate_tlv;
   der::Input signature_algorithm_tlv;
   der::BitString signature_value;
+  CertErrors errors;
   if (!ParseCertificate(in, &tbs_certificate_tlv, &signature_algorithm_tlv,
-                        &signature_value))
+                        &signature_value, &errors))
     return;
   std::unique_ptr<SignatureAlgorithm> sig_alg(
       SignatureAlgorithm::CreateFromDer(signature_algorithm_tlv));

net/cert/internal/parse_certificate_unittest.cc

@@ -5,6 +5,7 @@
 #include "net/cert/internal/parse_certificate.h"
 
 #include "base/strings/stringprintf.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/test_helpers.h"
 #include "net/der/input.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -49,7 +50,8 @@ void EnsureParsingCertificateSucceeds(const std::string& file_name) {
   der::Input signature_algorithm_tlv;
   der::BitString signature_value;
   ASSERT_TRUE(ParseCertificate(der::Input(&data), &tbs_certificate_tlv,
-                               &signature_algorithm_tlv, &signature_value));
+                               &signature_algorithm_tlv, &signature_value,
+                               nullptr));
 
   // Ensure that the parsed certificate matches expectations.
   EXPECT_EQ(0, signature_value.unused_bits());
@@ -73,8 +75,12 @@ void EnsureParsingCertificateFails(const std::string& file_name) {
   der::Input tbs_certificate_tlv;
   der::Input signature_algorithm_tlv;
   der::BitString signature_value;
+  CertErrors errors;
   ASSERT_FALSE(ParseCertificate(der::Input(&data), &tbs_certificate_tlv,
-                                &signature_algorithm_tlv, &signature_value));
+                                &signature_algorithm_tlv, &signature_value,
+                                &errors));
+  // TODO(crbug.com/634443): Verify |errors| to make sure it failed for the
+  //                         expected reason.
 }
 
 // Tests parsing a Certificate.

net/cert/internal/parse_ocsp_unittest.cc

@@ -59,10 +59,11 @@ OCSPFailure ParseOCSP(const std::string& file_name) {
   der::BitString cert_signature_value;
   if (!ParseCertificate(ca_input, &issuer_tbs_certificate_tlv,
                         &issuer_signature_algorithm_tlv,
-                        &issuer_signature_value))
+                        &issuer_signature_value, nullptr))
     return PARSE_CERT;
   if (!ParseCertificate(cert_input, &cert_tbs_certificate_tlv,
-                        &cert_signature_algorithm_tlv, &cert_signature_value))
+                        &cert_signature_algorithm_tlv, &cert_signature_value,
+                        nullptr))
     return PARSE_CERT;
   OCSPResponse parsed_ocsp;
   OCSPResponseData parsed_ocsp_data;

net/cert/internal/parsed_certificate.cc

@@ -24,11 +24,61 @@ WARN_UNUSED_RESULT bool GetSequenceValue(const der::Input& tlv,
 ParsedCertificate::ParsedCertificate() {}
 ParsedCertificate::~ParsedCertificate() {}
 
-scoped_refptr<ParsedCertificate> ParsedCertificate::CreateFromCertificateData(
+scoped_refptr<ParsedCertificate> ParsedCertificate::Create(
+    const uint8_t* data,
+    size_t length,
+    const ParseCertificateOptions& options,
+    CertErrors* errors) {
+  return CreateInternal(data, length, DataSource::INTERNAL_COPY, options,
+                        errors);
+}
+
+scoped_refptr<ParsedCertificate> ParsedCertificate::Create(
+    const base::StringPiece& data,
+    const ParseCertificateOptions& options,
+    CertErrors* errors) {
+  return ParsedCertificate::Create(
+      reinterpret_cast<const uint8_t*>(data.data()), data.size(), options,
+      errors);
+}
+
+bool ParsedCertificate::CreateAndAddToVector(
+    const uint8_t* data,
+    size_t length,
+    const ParseCertificateOptions& options,
+    ParsedCertificateList* chain,
+    CertErrors* errors) {
+  scoped_refptr<ParsedCertificate> cert(Create(data, length, options, errors));
+  if (!cert)
+    return false;
+  chain->push_back(std::move(cert));
+  return true;
+}
+
+bool ParsedCertificate::CreateAndAddToVector(
+    const base::StringPiece& data,
+    const ParseCertificateOptions& options,
+    ParsedCertificateList* chain,
+    CertErrors* errors) {
+  return CreateAndAddToVector(reinterpret_cast<const uint8_t*>(data.data()),
+                              data.size(), options, chain, errors);
+}
+
+scoped_refptr<ParsedCertificate> ParsedCertificate::CreateWithoutCopyingUnsafe(
+    const uint8_t* data,
+    size_t length,
+    const ParseCertificateOptions& options,
+    CertErrors* errors) {
+  return CreateInternal(data, length, DataSource::EXTERNAL_REFERENCE, options,
+                        errors);
+}
+
+scoped_refptr<ParsedCertificate> ParsedCertificate::CreateInternal(
     const uint8_t* data,
     size_t length,
     DataSource source,
-    const ParseCertificateOptions& options) {
+    const ParseCertificateOptions& options,
+    CertErrors* errors) {
   scoped_refptr<ParsedCertificate> result(new ParsedCertificate);
 
   switch (source) {
@@ -44,7 +94,7 @@ scoped_refptr<ParsedCertificate> ParsedCertificate::CreateFromCertificateData(
 
   if (!ParseCertificate(result->cert_, &result->tbs_certificate_tlv_,
                         &result->signature_algorithm_tlv_,
-                        &result->signature_value_)) {
+                        &result->signature_value_, errors)) {
     return nullptr;
   }
 
@@ -149,26 +199,4 @@ scoped_refptr<ParsedCertificate> ParsedCertificate::CreateFromCertificateData(
   return result;
 }
 
-scoped_refptr<ParsedCertificate> ParsedCertificate::CreateFromCertificateCopy(
-    const base::StringPiece& data,
-    const ParseCertificateOptions& options) {
-  return ParsedCertificate::CreateFromCertificateData(
-      reinterpret_cast<const uint8_t*>(data.data()), data.size(),
-      DataSource::INTERNAL_COPY, options);
-}
-
-bool ParsedCertificate::CreateAndAddToVector(
-    const uint8_t* data,
-    size_t length,
-    DataSource source,
-    const ParseCertificateOptions& options,
-    ParsedCertificateList* chain) {
-  scoped_refptr<ParsedCertificate> cert(
-      CreateFromCertificateData(data, length, source, options));
-  if (!cert)
-    return false;
-  chain->push_back(std::move(cert));
-  return true;
-}
-
 }  // namespace net

net/cert/internal/parsed_certificate.h

@@ -20,6 +20,7 @@ struct GeneralNames;
 class NameConstraints;
 class ParsedCertificate;
 class SignatureAlgorithm;
+class CertErrors;
 
 using ParsedCertificateList = std::vector<scoped_refptr<ParsedCertificate>>;
 
@@ -36,40 +37,67 @@ class NET_EXPORT ParsedCertificate
   // Map from OID to ParsedExtension.
   using ExtensionsMap = std::map<der::Input, ParsedExtension>;
 
-  // The certificate data for may either be owned internally (INTERNAL_COPY) or
-  // owned externally (EXTERNAL_REFERENCE). When it is owned internally the data
-  // is held by |cert_data_|
-  enum class DataSource {
-    INTERNAL_COPY,
-    EXTERNAL_REFERENCE,
-  };
-
   // Creates a ParsedCertificate given a DER-encoded Certificate. Returns
   // nullptr on failure. Failure will occur if the standard certificate fields
   // and supported extensions cannot be parsed.
   //
-  // The provided certificate data is either copied, or aliased, depending on
-  // the value of |source|. See the comments for DataSource for details.
-  static scoped_refptr<ParsedCertificate> CreateFromCertificateData(
+  // The provided certificate data is copied, so |data| needn't remain valid
+  // after this call.
+  //
+  // On either success or failure, if |errors| is non-null it may have error
+  // information added to it.
+  static scoped_refptr<ParsedCertificate> Create(
       const uint8_t* data,
       size_t length,
-      DataSource source,
-      const ParseCertificateOptions& options);
+      const ParseCertificateOptions& options,
+      CertErrors* errors);
 
-  // Creates a ParsedCertificate and appends it to |chain|. Returns true if the
-  // certificate was successfully parsed and added. If false is return, |chain|
-  // is unmodified.
+  // Overload that takes a StringPiece.
+  static scoped_refptr<ParsedCertificate> Create(
+      const base::StringPiece& data,
+      const ParseCertificateOptions& options,
+      CertErrors* errors);
+
+  // Creates a ParsedCertificate by copying the provided |data|, and appends it
+  // to |chain|. Returns true if the certificate was successfully parsed and
+  // added. If false is return, |chain| is unmodified.
+  //
+  // On either success or failure, if |errors| is non-null it may have error
+  // information added to it.
   static bool CreateAndAddToVector(
       const uint8_t* data,
       size_t length,
-      DataSource source,
       const ParseCertificateOptions& options,
-      std::vector<scoped_refptr<net::ParsedCertificate>>* chain);
+      std::vector<scoped_refptr<net::ParsedCertificate>>* chain,
+      CertErrors* errors);
 
-  // Creates a ParsedCertificate, copying the data from |data|.
-  static scoped_refptr<ParsedCertificate> CreateFromCertificateCopy(
+  // Overload that takes a StringPiece.
+  static bool CreateAndAddToVector(
       const base::StringPiece& data,
-      const ParseCertificateOptions& options);
+      const ParseCertificateOptions& options,
+      std::vector<scoped_refptr<net::ParsedCertificate>>* chain,
+      CertErrors* errors);
+
+  // Like Create() this builds a ParsedCertificate given a DER-encoded
+  // Certificate and returns nullptr on failure.
+  //
+  // However a copy of |data| is NOT made.
+  //
+  // This is a dangerous way to create as ParsedCertificate and should only be
+  // used with care when saving a copy is really worth it, or the data is known
+  // to come from static storage (and hence remain valid for entire life of
+  // process).
+  //
+  // ParsedCertificate is reference counted, so it is easy to extend the life
+  // and and end up with a ParsedCertificate referencing feed memory.
+  //
+  // On either success or failure, if |errors| is non-null it may have error
+  // information added to it.
+  static scoped_refptr<ParsedCertificate> CreateWithoutCopyingUnsafe(
+      const uint8_t* data,
+      size_t length,
+      const ParseCertificateOptions& options,
+      CertErrors* errors);
 
   // Returns the DER-encoded certificate data for this cert.
   const der::Input& der_cert() const { return cert_; }
@@ -180,10 +208,25 @@ class NET_EXPORT ParsedCertificate
   }
 
  private:
+  // The certificate data for may either be owned internally (INTERNAL_COPY) or
+  // owned externally (EXTERNAL_REFERENCE). When it is owned internally the data
+  // is held by |cert_data_|
+  enum class DataSource {
+    INTERNAL_COPY,
+    EXTERNAL_REFERENCE,
+  };
+
   friend class base::RefCountedThreadSafe<ParsedCertificate>;
   ParsedCertificate();
   ~ParsedCertificate();
 
+  static scoped_refptr<ParsedCertificate> CreateInternal(
+      const uint8_t* data,
+      size_t length,
+      DataSource source,
+      const ParseCertificateOptions& options,
+      CertErrors* errors);
+
   // The backing store for the certificate data. This is only applicable when
   // the ParsedCertificate was initialized using DataSource::INTERNAL_COPY.
   std::vector<uint8_t> cert_data_;

net/cert/internal/path_builder_pkits_unittest.cc

@@ -58,9 +58,10 @@ class PathBuilderPkitsTestDelegate {
     }
     ParsedCertificateList certs;
     for (const std::string& der : cert_ders) {
-      certs.push_back(ParsedCertificate::CreateFromCertificateCopy(der, {}));
-      if (!certs.back()) {
-        ADD_FAILURE() << "ParsedCertificate::CreateFromCertificateCopy failed";
+      CertErrors errors;
+      if (!ParsedCertificate::CreateAndAddToVector(der, {}, &certs, &errors)) {
+        ADD_FAILURE() << "ParseCertificate::CreateAndAddToVector() failed:\n"
+                      << errors.ToDebugString();
         return false;
       }
     }

net/cert/internal/path_builder_unittest.cc

@@ -121,9 +121,13 @@ class AsyncCertIssuerSourceStatic : public CertIssuerSource {
       "net/data/ssl/certificates/" + file_name, "CERTIFICATE", &der);
   if (!r)
     return r;
-  *result = ParsedCertificate::CreateFromCertificateCopy(der, {});
-  if (!*result)
-    return ::testing::AssertionFailure() << "CreateFromCertificateCopy failed";
+  CertErrors errors;
+  *result = ParsedCertificate::Create(der, {}, &errors);
+  if (!*result) {
+    return ::testing::AssertionFailure()
+           << "ParseCertificate::Create() failed:\n"
+           << errors.ToDebugString();
+  }
   return ::testing::AssertionSuccess();
 }
 
@@ -941,8 +945,8 @@ TEST_F(PathBuilderKeyRolloverTest,
 TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) {
   // Create a separate copy of oldintermediate.
   scoped_refptr<ParsedCertificate> oldintermediate_dupe(
-      ParsedCertificate::CreateFromCertificateCopy(
-          oldintermediate_->der_cert().AsStringPiece(), {}));
+      ParsedCertificate::Create(oldintermediate_->der_cert().AsStringPiece(),
+                                {}, nullptr));
 
   // Only newroot is a trusted root.
   TrustStoreInMemory trust_store;
@@ -1002,9 +1006,8 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediates) {
 // SPKI as a TrustAnchor.
 TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) {
   // Create a separate copy of newroot.
-  scoped_refptr<ParsedCertificate> newroot_dupe(
-      ParsedCertificate::CreateFromCertificateCopy(
-          newroot_->der_cert().AsStringPiece(), {}));
+  scoped_refptr<ParsedCertificate> newroot_dupe(ParsedCertificate::Create(
+      newroot_->der_cert().AsStringPiece(), {}, nullptr));
 
   // Only newroot is a trusted root.
   TrustStoreInMemory trust_store;
@@ -1233,8 +1236,8 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {
 
   // Second async batch: return a different copy of oldintermediate_ again.
   scoped_refptr<ParsedCertificate> oldintermediate_dupe(
-      ParsedCertificate::CreateFromCertificateCopy(
-          oldintermediate_->der_cert().AsStringPiece(), {}));
+      ParsedCertificate::Create(oldintermediate_->der_cert().AsStringPiece(),
+                                {}, nullptr));
   EXPECT_CALL(*target_issuers_req, GetNext(_))
       .WillOnce(DoAll(SetArgPointee<0>(oldintermediate_dupe),
                       Return(CompletionStatus::SYNC)))

net/cert/internal/test_helpers.cc

@@ -8,6 +8,7 @@
 #include "base/base_paths.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/pem_tokenizer.h"
 #include "net/der/parser.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -141,19 +142,17 @@ void ReadVerifyCertChainTestFromFile(const std::string& file_name,
     const std::string& block_data = pem_tokenizer.data();
 
     if (block_type == kCertificateHeader) {
-      ASSERT_TRUE(net::ParsedCertificate::CreateAndAddToVector(
-          reinterpret_cast<const uint8_t*>(block_data.data()),
-          block_data.size(), net::ParsedCertificate::DataSource::INTERNAL_COPY,
-          {}, chain));
+      CertErrors errors;
+      ASSERT_TRUE(net::ParsedCertificate::CreateAndAddToVector(block_data, {},
+                                                               chain, &errors))
+          << errors.ToDebugString();
     } else if (block_type == kTrustAnchorUnconstrained ||
                block_type == kTrustAnchorConstrained) {
       ASSERT_FALSE(*trust_anchor) << "Duplicate trust anchor";
+      CertErrors errors;
       scoped_refptr<ParsedCertificate> root =
-          net::ParsedCertificate::CreateFromCertificateData(
-              reinterpret_cast<const uint8_t*>(block_data.data()),
-              block_data.size(),
-              net::ParsedCertificate::DataSource::INTERNAL_COPY, {});
-      ASSERT_TRUE(root);
+          net::ParsedCertificate::Create(block_data, {}, &errors);
+      ASSERT_TRUE(root) << errors.ToDebugString();
       *trust_anchor =
           block_type == kTrustAnchorUnconstrained
               ? TrustAnchor::CreateFromCertificateNoConstraints(std::move(root))

net/cert/internal/trust_store_nss.cc

@@ -13,6 +13,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/task_runner.h"
 #include "crypto/nss_util.h"
+#include "net/cert/internal/cert_errors.h"
 #include "net/cert/internal/parsed_certificate.h"
 
 // TODO(mattm): structure so that supporting ChromeOS multi-profile stuff is
@@ -56,13 +57,13 @@ void GetAnchors(const scoped_refptr<ParsedCertificate>& cert,
     if ((SEC_GET_TRUST_FLAGS(&trust, trust_type) & ca_trust) != ca_trust)
       continue;
 
-    scoped_refptr<ParsedCertificate> anchor_cert =
-        ParsedCertificate::CreateFromCertificateData(
-            node->cert->derCert.data, node->cert->derCert.len,
-            ParsedCertificate::DataSource::INTERNAL_COPY, {});
+    CertErrors errors;
+    scoped_refptr<ParsedCertificate> anchor_cert = ParsedCertificate::Create(
+        node->cert->derCert.data, node->cert->derCert.len, {}, &errors);
     if (!anchor_cert) {
-      // TODO(mattm): return errors better.
-      LOG(ERROR) << "error parsing issuer certificate";
+      // TODO(crbug.com/634443): return errors better.
+      LOG(ERROR) << "Error parsing issuer certificate:\n"
+                 << errors.ToDebugString();
       continue;
     }
 

net/cert/internal/verify_certificate_chain_pkits_unittest.cc

@@ -56,12 +56,11 @@ class VerifyCertificateChainPkitsTestDelegate {
     // PKITS lists chains from trust anchor to target, VerifyCertificateChain
     // takes them starting with the target and not including the trust anchor.
     std::vector<scoped_refptr<net::ParsedCertificate>> input_chain;
+    CertErrors errors;
     for (auto i = cert_ders.rbegin(); i != cert_ders.rend(); ++i) {
-      if (!net::ParsedCertificate::CreateAndAddToVector(
-              reinterpret_cast<const uint8_t*>(i->data()), i->size(),
-              net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE, {},
-              &input_chain)) {
-        ADD_FAILURE() << "cert failed to parse";
+      if (!net::ParsedCertificate::CreateAndAddToVector(*i, {}, &input_chain,
+                                                        &errors)) {
+        ADD_FAILURE() << "Cert failed to parse:\n" << errors.ToDebugString();
         return false;
       }
     }
@@ -76,7 +75,6 @@ class VerifyCertificateChainPkitsTestDelegate {
     der::GeneralizedTime time = {2011, 4, 15, 0, 0, 0};
 
     //  TODO(crbug.com/634443): Test errors on failure?
-    CertErrors errors;
     return VerifyCertificateChain(input_chain, trust_anchor.get(),
                                   &signature_policy, time, &errors);
   }

net/cert/x509_util.cc

@@ -127,7 +127,8 @@ bool ParseCertificateSandboxed(const base::StringPiece& certificate,
   der::Input cert_data(certificate);
   der::Input tbs_cert, signature_alg;
   der::BitString signature_value;
-  if (!ParseCertificate(cert_data, &tbs_cert, &signature_alg, &signature_value))
+  if (!ParseCertificate(cert_data, &tbs_cert, &signature_alg, &signature_value,
+                        nullptr))
     return false;
 
   ParsedTbsCertificate parsed_tbs_cert;

net/cert/x509_util_openssl.cc

@@ -322,7 +322,7 @@ bool GetTLSServerEndPointChannelBinding(const X509Certificate& certificate,
   der::BitString signature_value;
   if (!ParseCertificate(der::Input(&der_encoded_certificate),
                         &tbs_certificate_tlv, &signature_algorithm_tlv,
-                        &signature_value))
+                        &signature_value, nullptr))
     return false;
 
   std::unique_ptr<SignatureAlgorithm> signature_algorithm =

net/tools/cert_verify_tool/verify_using_path_builder.cc

@@ -152,6 +152,21 @@ void PrintResultPath(const net::CertPathBuilder::ResultPath* result_path,
   }
 }
 
+scoped_refptr<net::ParsedCertificate> ParseCertificate(const CertInput& input) {
+  net::CertErrors errors;
+  scoped_refptr<net::ParsedCertificate> cert =
+      net::ParsedCertificate::Create(input.der_cert, {}, &errors);
+  if (!cert) {
+    PrintCertError("ERROR: ParsedCertificate failed:", input);
+    std::cout << errors.ToDebugString() << "\n";
+  }
+
+  // TODO(crbug.com/634443): Print errors if there are any on success too (i.e.
+  //                         warnings).
+
+  return cert;
+}
+
 }  // namespace
 
 // Verifies |target_der_cert| using CertPathBuilder.
@@ -170,12 +185,8 @@ bool VerifyUsingPathBuilder(
   net::TrustStoreInMemory trust_store_in_memory;
   trust_store.AddTrustStoreSynchronousOnly(&trust_store_in_memory);
   for (const auto& der_cert : root_der_certs) {
-    scoped_refptr<net::ParsedCertificate> cert =
-        net::ParsedCertificate::CreateFromCertificateCopy(der_cert.der_cert,
-                                                          {});
-    if (!cert)
-      PrintCertError("ERROR: ParsedCertificate failed:", der_cert);
-    else {
+    scoped_refptr<net::ParsedCertificate> cert = ParseCertificate(der_cert);
+    if (cert) {
       trust_store_in_memory.AddTrustAnchor(
           net::TrustAnchor::CreateFromCertificateNoConstraints(cert));
     }
@@ -194,22 +205,15 @@ bool VerifyUsingPathBuilder(
 
   net::CertIssuerSourceStatic intermediate_cert_issuer_source;
   for (const auto& der_cert : intermediate_der_certs) {
-    scoped_refptr<net::ParsedCertificate> cert =
-        net::ParsedCertificate::CreateFromCertificateCopy(der_cert.der_cert,
-                                                          {});
-    if (!cert)
-      PrintCertError("ERROR: ParsedCertificate failed:", der_cert);
-    else
+    scoped_refptr<net::ParsedCertificate> cert = ParseCertificate(der_cert);
+    if (cert)
       intermediate_cert_issuer_source.AddCert(cert);
   }
 
   scoped_refptr<net::ParsedCertificate> target_cert =
-      net::ParsedCertificate::CreateFromCertificateCopy(
-          target_der_cert.der_cert, {});
-  if (!target_cert) {
-    PrintCertError("ERROR: ParsedCertificate failed:", target_der_cert);
+      ParseCertificate(target_der_cert);
+  if (!target_cert)
     return false;
-  }
 
   // Verify the chain.
   net::SimpleSignaturePolicy signature_policy(2048);