Allow Cast certificates to have serial numbers greater than 20 bytes.

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 8a4d6eb400e66f7a74603af852f37fc2dda38720..228f15c8660c57b265ae793aa5253355049eaed6 100644
--- a/components/cast_certificate/cast_cert_validator.cc
+++ b/components/cast_certificate/cast_cert_validator.cc
@@ -28,6 +28,22 @@
 namespace cast_certificate {
 namespace {
 
+// Returns the parsing options used for Cast certificates.
+net::ParseCertificateOptions GetCertParsingOptions() {
+  net::ParseCertificateOptions options;
+
+  // Some cast intermediate certificates contain serial numbers that are
+  // 21 octets long, and might also not use valid DER encoding for an
+  // INTEGER (non-minimal encoding).
+  //
+  // Allow these sorts of serial numbers.
+  //
+  // TODO(eroman): At some point in the future this workaround will no longer be
+  // necessary. Should revisit this for removal in 2017 if not earlier.
+  options.allow_invalid_serial_numbers = true;
+  return options;
+}
+
 // -------------------------------------------------------------------------
 // Cast trust anchors.
 // -------------------------------------------------------------------------
@@ -56,17 +72,18 @@ class CastTrustStore {
   friend struct base::DefaultSingletonTraits<CastTrustStore>;
 
   CastTrustStore() {
-    // Initialize the trust store with two root certificates.
+    AddAnchor(kCastRootCaDer);
+    AddAnchor(kEurekaRootCaDer);
+  }
+
+  // Adds a trust anchor given a DER-encoded certificate from static
+  // storage.
+  template <size_t N>
+  void AddAnchor(const uint8_t (&data)[N]) {
     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);
+            data, N, net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE,
+            GetCertParsingOptions());

[mattm, 2016/06/20 23:42:12]

Since these are statically included and verified by tests, we don't need to
relax the parsing on the roots right?

[eroman, 2016/06/20 23:45:45]

That is correct.

I added it anyway for consistency, but I can safely remove these if you prefer.

[mattm, 2016/06/20 23:55:04]

I'd prefer limiting the places we allow exceptions over consistency, though this
isn't a huge deal.

[eroman, 2016/06/21 00:19:31]

Done.

     CHECK(root);
     store_.AddTrustedCertificate(std::move(root));
   }
@@ -277,7 +294,7 @@ bool VerifyDeviceCert(const std::vector<std::string>& certs,
     if (!net::ParsedCertificate::CreateAndAddToVector(
             reinterpret_cast<const uint8_t*>(cert_der.data()), cert_der.size(),
             net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE,
-            &input_chain)) {
+            GetCertParsingOptions(), &input_chain)) {
       return false;
     }
   }
@@ -309,8 +326,8 @@ std::unique_ptr<CertVerificationContext> CertVerificationContextImplForTest(
 bool AddTrustAnchorForTest(const uint8_t* data, size_t length) {
   scoped_refptr<net::ParsedCertificate> anchor(
       net::ParsedCertificate::CreateFromCertificateData(
-          data, length,
-          net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE));
+          data, length, net::ParsedCertificate::DataSource::EXTERNAL_REFERENCE,
+          GetCertParsingOptions()));
   if (!anchor)
     return false;
   CastTrustStore::Get().AddTrustedCertificate(std::move(anchor));