Convert tests that parse an Extensions sequence to instead

Convert tests that parse an Extensions sequence to instead
parse an entire Certificate.

(Part of a series of changes for simplifying the parsing tests to be in
terms of just certificates for consistent entry point and expectations)

BUG=634443
Committed: https://crrev.com/c7d0f8db7ae70e9b68545deaf3bb2145b7366bd3
Cr-Commit-Position: refs/heads/master@{#420709}

[eroman, 2016-09-22 21:04:04]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 21:04:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2016-09-22 21:07:44]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 21:07:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2016-09-22 21:07:56]

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

[commit-bot: I haz the power, 2016-09-22 22:19:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 22:19:15]

Dry run: This issue passed the CQ dry run.

[mattm, 2016-09-23 01:54:32]

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
File net/cert/internal/parse_certificate_unittest.cc (left):

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
net/cert/internal/parse_certificate_unittest.cc:472: TEST(ParseExtensionsTest,
UnknownCritical) {
Were these tests supposed to be removed?

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
File net/cert/internal/parse_certificate_unittest.cc (right):

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
net/cert/internal/parse_certificate_unittest.cc:463: const uint8_t
kExpectedValue[] = {0xA0};
maybe dumb question, but why is it {a0} and not {05 a0}?

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
net/cert/internal/parse_certificate_unittest.cc:503:
EXPECT_TRUE(cert->has_basic_constraints());
repeated

[eroman, 2016-09-23 01:58:05]

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
File net/cert/internal/parse_certificate_unittest.cc (left):

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
net/cert/internal/parse_certificate_unittest.cc:472: TEST(ParseExtensionsTest,
UnknownCritical) {
On 2016/09/23 01:54:32, mattm wrote:
> Were these tests supposed to be removed?

Yes -- they are basically duplicates with extension_critical.pem and
extension_not_critical.pem (kind of happend when I changed those from basic
constraints to an unknown extension in the previous patch).

There are several other duplicate tests that were the result of testing
indepently at the various layers. Which I don't think are all that useful.

[eroman, 2016-09-23 02:14:10]

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
File net/cert/internal/parse_certificate_unittest.cc (right):

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
net/cert/internal/parse_certificate_unittest.cc:463: const uint8_t
kExpectedValue[] = {0xA0};
On 2016/09/23 01:54:32, mattm wrote:
> maybe dumb question, but why is it {a0} and not {05 a0}?

I will make this clearer in the code, and also add an assertion for the 0x05
(number of unused bits).

Explanation:

The "value" of the BIT string's DER is 05 0a

05 -- the number of unused bits
a0 -- the actual bits

So in other words, it is a bit-string of 3 bits, "101" (digitalSignature and
keyEncipherment I believe).

The comparison below is using BitString::bytes(), hence doesn't include the 5
counter.

I will improve the expectations to also check the first 3 bits directly.

Thanks for pointing out this confusion.

[eroman, 2016-09-23 02:20:50]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 02:21:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2016-09-23 02:21:39]

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
File net/cert/internal/parse_certificate_unittest.cc (right):

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
net/cert/internal/parse_certificate_unittest.cc:463: const uint8_t
kExpectedValue[] = {0xA0};
On 2016/09/23 02:14:10, eroman wrote:
> On 2016/09/23 01:54:32, mattm wrote:
> > maybe dumb question, but why is it {a0} and not {05 a0}?
> 
> I will make this clearer in the code, and also add an assertion for the 0x05
> (number of unused bits).
> 
> Explanation:
> 
> The "value" of the BIT string's DER is 05 0a
> 
> 05 -- the number of unused bits
> a0 -- the actual bits
> 
> So in other words, it is a bit-string of 3 bits, "101" (digitalSignature and
> keyEncipherment I believe).
> 
> The comparison below is using BitString::bytes(), hence doesn't include the 5
> counter.
> 
> I will improve the expectations to also check the first 3 bits directly.
> 
> Thanks for pointing out this confusion.

Done.

https://codereview.chromium.org/2361233002/diff/20001/net/cert/internal/parse...
net/cert/internal/parse_certificate_unittest.cc:503:
EXPECT_TRUE(cert->has_basic_constraints());
On 2016/09/23 01:54:32, mattm wrote:
> repeated

Done.

[mattm, 2016-09-23 02:39:41]

Thanks, lgtm

[commit-bot: I haz the power, 2016-09-23 03:00:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 03:00:33]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[eroman, 2016-09-23 05:50:37]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2016-09-23 05:50:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 05:50:44]

This CL has an open dependency (Issue 2359783003 Patch 80001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[eroman, 2016-09-23 17:06:41]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 17:07:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-23 18:33:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 18:33:54]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[eroman, 2016-09-23 18:56:30]

The CQ bit was checked by eroman@chromium.org

[eroman, 2016-09-23 18:56:31]

The patchset sent to the CQ was uploaded after l-g-t-m from mattm@chromium.org
Link to the patchset: https://codereview.chromium.org/2361233002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-23 18:56:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-23 20:09:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-23 20:10:48]

Description was changed from

==========
Convert tests that parse an Extensions sequence to instead
parse an entire Certificate.

(Part of a series of changes for simplifying the parsing tests to be in
terms of just certificates for consistent entry point and expectations)

BUG=634443
==========

to

==========
Convert tests that parse an Extensions sequence to instead
parse an entire Certificate.

(Part of a series of changes for simplifying the parsing tests to be in
terms of just certificates for consistent entry point and expectations)

BUG=634443

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

[commit-bot: I haz the power, 2016-09-23 20:10:50]

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