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

components/cast_certificate/cast_cert_validator.cc

 // Copyright 2014 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 "components/cast_certificate/cast_cert_validator.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 10 matching lines @@
 #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"
 
 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.
 // -------------------------------------------------------------------------
 
 // There are two trusted roots for Cast certificate chains:
 //
 //   (1) CN=Cast Root CA    (kCastRootCaDer)
 //   (2) CN=Eureka Root CA  (kEurekaRootCaDer)
 //
 // These constants are defined by the files included next:
 
 #include "components/cast_certificate/cast_root_ca_cert_der-inc.h"
 #include "components/cast_certificate/eureka_root_ca_der-inc.h"
 
 // Singleton for the Cast trust store.
 class CastTrustStore {
  public:
   static CastTrustStore* GetInstance() {
     return base::Singleton<CastTrustStore,
                            base::LeakySingletonTraits<CastTrustStore>>::get();
   }
 
   static net::TrustStore& Get() { return GetInstance()->store_; }
 
  private:
   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));
   }
 
   net::TrustStore store_;
   DISALLOW_COPY_AND_ASSIGN(CastTrustStore);
 };
 
 using ExtensionsMap = std::map<net::der::Input, net::ParsedExtension>;
 
@@ skipping 190 matching lines @@
   // 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)) {
+            GetCertParsingOptions(), &input_chain)) {
       return false;
     }
   }
 
   // Use a signature policy compatible with Cast's PKI.
   auto signature_policy = CreateCastSignaturePolicy();
 
   // Do RFC 5280 compatible certificate verification using the two Cast
   // trust anchors and Cast signature policy.
   if (!net::VerifyCertificateChain(input_chain, CastTrustStore::Get(),
@@ skipping 11 matching lines @@
     const base::StringPiece& spki) {
   // Use a bogus CommonName, since this is just exposed for testing signature
   // verification by unittests.
   return base::WrapUnique(
       new CertVerificationContextImpl(net::der::Input(spki), "CommonName"));
 }
 
 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));
   return true;
 }
 
 }  // namespace cast_certificate