Add tests for Cast certificate interpretation of policies.

Add tests for Cast certificate interpretation of policies.

Also changes the implementation to be more restrictive by looking at the whole chain rather than just the leaf.

Review-Url: https://codereview.chromium.org/2918233002
Cr-Commit-Position: refs/heads/master@{#482858}
Committed: https://chromium.googlesource.com/chromium/src/+/6a9b02bdc9938db94b6ff1ca41865093f30f1836

[eroman, 2017-06-02 22:56:30]

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

[eroman, 2017-06-02 22:56:30]

Please review carefully that this change is in fact

 (1) desired for Cast
 (2) will not break existing certificate chains

I am proposing a stricter check than before. I have given some sample chains
showing the before vs after results below.

An alternative (and my preference) would be to change the way that certificates
are issued, so these checks can be implemented more consistently with RFC 5280.
But I recognize that may not be an option for legacy support. For instance, it
would be great if audio+video was its own policy. Or another idea would be to
add a Policy Constraints with requireExplicitPolicy=0 to all top-level
audio-only intermediates (then policy non-conformance because a validation
failure)

In the example chains below, policies are shown inside {curlyBraces}:

-------------
Root  {}
Intermediate {}
Leaf {}

RESULT BEFORE: Success; policy=NONE
RESULT AFTER: Success; policy=NONE
-------------
Root  {}
Intermediate {audioOnly}
Leaf {}

RESULT BEFORE: Success; policy=NONE
RESULT AFTER: Failure
-------------
Root {}
Intermediate {anyPolicy}
Leaf {}

RESULT BEFORE: Success; policy=NONE
RESULT AFTER: Success; policy=NONE
-------------
Root {}
Intermediate {anyPolicy}
Leaf {audioOnly}

RESULT BEFORE: Success; policy=AUDIO_ONLY
RESULT AFTER: Success; policy=AUDIO_ONLY
-------------
Root {}
Intermediate {}
Leaf {audioOnly}

RESULT BEFORE: Success; policy=AUDIO_ONLY
RESULT AFTER: Failure
-------------
Root {}
Intermediate {audioOnly}
Leaf {audioOnly}

RESULT BEFORE: Success; policy=AUDIO_ONLY
RESULT AFTER: Success; policy=AUDIO_ONLY
-------------
Root {}
Intermediate {Foo, Bar, Baz}
Leaf {}

RESULT BEFORE: Success; policy=NONE
RESULT AFTER: Success; policy=NONE

[eroman, 2017-06-08 19:32:22]

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

[commit-bot: I haz the power, 2017-06-08 19:33:05]

Dry run: CQ is trying da patch.

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

[eroman, 2017-06-08 19:33:43]

Description was changed from

==========
Address a TODO regarding policies in Cast certificate verifier.

Rather than only checking the end-entity's policy, it check conformance
of the entire chain.
==========

to

==========
Add tests for Cast certificate interpretation of policies.

Also changes the implementation to be more restrictive by looking at the whole
chain rather than just the leaf.
==========

[eroman, 2017-06-08 19:39:41]

PTAL -- I have significantly changed this CL since patchset #1.

The new implementation should now match the understanding of Cast policies from
our email discussion. Chains are never rejected for inconsistent policies, they
are just downgraded to "audioOnly" (please confirm the test expectations).

I don't think separately histogramming is worthwhile anymore, since I am no
longer trying to make this use RFC 5280's approach.

[commit-bot: I haz the power, 2017-06-08 20:57:27]

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

[commit-bot: I haz the power, 2017-06-08 20:57:27]

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, 2017-06-20 17:15:07]

Ping

[ryanchung, 2017-06-20 21:04:27]

ryanchung@chromium.org changed reviewers:
+ ryanchung@chromium.org

[ryanchung, 2017-06-20 21:04:28]

this lgtm.

dougsteed for OWNERS.

[eroman, 2017-06-27 20:20:18]

Ping @dougsteed

[dougsteed, 2017-06-27 20:35:28]

lgtm

[eroman, 2017-06-27 20:39:23]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-06-27 20:39:42]

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, 2017-06-27 20:39:42]

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

[commit-bot: I haz the power, 2017-06-27 20:39:44]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[eroman, 2017-06-27 21:08:10]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-06-27 21:08:24]

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, 2017-06-27 21:08:25]

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

[commit-bot: I haz the power, 2017-06-27 21:08:26]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[ryanchung, 2017-06-27 23:56:06]

ryanchung@chromium.org changed reviewers:
+ halliwell@chromium.org

[ryanchung, 2017-06-27 23:56:07]

+halliwell for a /chromecast owner and committer

[halliwell, 2017-06-28 00:28:49]

On 2017/06/27 23:56:07, ryanchung wrote:
> +halliwell for a /chromecast owner and committer

rs-lgtm

[eroman, 2017-06-28 00:30:48]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2017-06-28 00:31:24]

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, 2017-06-28 02:39:21]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498609848173020,
"parent_rev": "487d29d5ead7e88b49a8e193bb1d7a3513e86d8e", "commit_rev":
"6a9b02bdc9938db94b6ff1ca41865093f30f1836"}

[commit-bot: I haz the power, 2017-06-28 02:39:36]

Description was changed from

==========
Add tests for Cast certificate interpretation of policies.

Also changes the implementation to be more restrictive by looking at the whole
chain rather than just the leaf.
==========

to

==========
Add tests for Cast certificate interpretation of policies.

Also changes the implementation to be more restrictive by looking at the whole
chain rather than just the leaf.

Review-Url: https://codereview.chromium.org/2918233002
Cr-Commit-Position: refs/heads/master@{#482858}
Committed:
https://chromium.googlesource.com/chromium/src/+/6a9b02bdc9938db94b6ff1ca4186...
==========

[commit-bot: I haz the power, 2017-06-28 02:39:38]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6a9b02bdc9938db94b6ff1ca4186...