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

Allow Cast certificates to have serial numbers greater than 20 bytes, as well as non-minimal INTEGER encodings (invalid DER).

Previously such certificates were rejected, however there are some intermediate device certificates with serial numbers 21 bytes long, so make a temporary allowance.

BUG=621645
Committed: https://crrev.com/c15c91eed960673eff22f0bec50cdab4fd950824
Cr-Commit-Position: refs/heads/master@{#401019}

[eroman, 2016-06-20 23:12:01]

Description was changed from

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

Previously such certificates were rejected, however there are some intermediate
device certificates with serial numbers 21 bytes long, so make a temporary
allowance.

BUG=621645
==========

to

==========
Allow Cast certificates to have serial numbers greater than 20 bytes, as well as
non-minimal INTEGER encodings (invalid DER).

Previously such certificates were rejected, however there are some intermediate
device certificates with serial numbers 21 bytes long, so make a temporary
allowance.

BUG=621645
==========

[eroman, 2016-06-20 23:14:05]

eroman@chromium.org changed reviewers:
+ mattm@chromium.org, ryanchung@chromium.org

[eroman, 2016-06-20 23:14:06]

I expect to follow-up with more testing as part of //net unit-tests. 

For now the only testing of the |allow_invalid_serial_numbers=true| option is
from the Cast tests.

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

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
File components/cast_certificate/cast_cert_validator.cc (right):

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
components/cast_certificate/cast_cert_validator.cc:86: GetCertParsingOptions());
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]

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
File components/cast_certificate/cast_cert_validator.cc (right):

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
components/cast_certificate/cast_cert_validator.cc:86: GetCertParsingOptions());
On 2016/06/20 23:42:12, mattm wrote:
> Since these are statically included and verified by tests, we don't need to
> relax the parsing on the roots right?

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]

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
File components/cast_certificate/cast_cert_validator.cc (right):

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
components/cast_certificate/cast_cert_validator.cc:86: GetCertParsingOptions());
On 2016/06/20 23:45:45, eroman wrote:
> On 2016/06/20 23:42:12, mattm wrote:
> > Since these are statically included and verified by tests, we don't need to
> > relax the parsing on the roots right?
> 
> That is correct.
> 
> I added it anyway for consistency, but I can safely remove these if you
prefer.

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

[ryanchung, 2016-06-21 00:15:25]

Thanks for your help, Eric and Matt!
LGTM

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

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
File components/cast_certificate/cast_cert_validator.cc (right):

https://codereview.chromium.org/2079273004/diff/1/components/cast_certificate...
components/cast_certificate/cast_cert_validator.cc:86: GetCertParsingOptions());
On 2016/06/20 23:55:04, mattm wrote:
> On 2016/06/20 23:45:45, eroman wrote:
> > On 2016/06/20 23:42:12, mattm wrote:
> > > Since these are statically included and verified by tests, we don't need
to
> > > relax the parsing on the roots right?
> > 
> > That is correct.
> > 
> > I added it anyway for consistency, but I can safely remove these if you
> prefer.
> 
> I'd prefer limiting the places we allow exceptions over consistency, though
this
> isn't a huge deal.

Done.

[mattm, 2016-06-21 00:22:00]

lgtm

[eroman, 2016-06-21 00:23:06]

eroman@chromium.org changed reviewers:
+ sheretov@chromium.org

[eroman, 2016-06-21 00:23:06]

+sheretov@chromium.org for OWNERS approval.

[sheretov, 2016-06-21 16:02:51]

lgtm

[eroman, 2016-06-21 17:05:11]

The CQ bit was checked by eroman@chromium.org

[eroman, 2016-06-21 17:05:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
ryanchung@chromium.org
Link to the patchset: https://codereview.chromium.org/2079273004/#ps60001
(title: "remove allowance of invalid serial numbers for cast roots")

[commit-bot: I haz the power, 2016-06-21 17:05:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2079273004/60001

[commit-bot: I haz the power, 2016-06-21 17:10:47]

Description was changed from

==========
Allow Cast certificates to have serial numbers greater than 20 bytes, as well as
non-minimal INTEGER encodings (invalid DER).

Previously such certificates were rejected, however there are some intermediate
device certificates with serial numbers 21 bytes long, so make a temporary
allowance.

BUG=621645
==========

to

==========
Allow Cast certificates to have serial numbers greater than 20 bytes, as well as
non-minimal INTEGER encodings (invalid DER).

Previously such certificates were rejected, however there are some intermediate
device certificates with serial numbers 21 bytes long, so make a temporary
allowance.

BUG=621645
==========

[commit-bot: I haz the power, 2016-06-21 17:10:48]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-21 17:12:43]

Description was changed from

==========
Allow Cast certificates to have serial numbers greater than 20 bytes, as well as
non-minimal INTEGER encodings (invalid DER).

Previously such certificates were rejected, however there are some intermediate
device certificates with serial numbers 21 bytes long, so make a temporary
allowance.

BUG=621645
==========

to

==========
Allow Cast certificates to have serial numbers greater than 20 bytes, as well as
non-minimal INTEGER encodings (invalid DER).

Previously such certificates were rejected, however there are some intermediate
device certificates with serial numbers 21 bytes long, so make a temporary
allowance.

BUG=621645

Committed: https://crrev.com/c15c91eed960673eff22f0bec50cdab4fd950824
Cr-Commit-Position: refs/heads/master@{#401019}
==========

[commit-bot: I haz the power, 2016-06-21 17:12:44]

Patchset 4 (id:??) landed as
https://crrev.com/c15c91eed960673eff22f0bec50cdab4fd950824
Cr-Commit-Position: refs/heads/master@{#401019}