Add a key purpose parameter to Certificate PathBuilder.

Add a key purpose parameter to Certificate PathBuilder.

In this CL it is used to verify the Extended Key Usage.
(A subsequent CL will add Key Usage checks.)

BUG=634442
Review-Url: https://codereview.chromium.org/2800993002
Cr-Commit-Position: refs/heads/master@{#463714}
Committed: https://chromium.googlesource.com/chromium/src/+/5d358265af599429828297cab308b27fc4f1e8aa

[eroman, 2017-04-06 01:58:36]

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

[commit-bot: I haz the power, 2017-04-06 01:59:07]

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, 2017-04-06 03:27:43]

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

[commit-bot: I haz the power, 2017-04-06 03:27:44]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[eroman, 2017-04-06 04:31:57]

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

[eroman, 2017-04-06 04:36:38]

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

[commit-bot: I haz the power, 2017-04-06 04:37:00]

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, 2017-04-06 05:36:52]

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

[commit-bot: I haz the power, 2017-04-06 05:36:53]

Dry run: This issue passed the CQ dry run.

[mattm, 2017-04-06 22:16:03]

https://codereview.chromium.org/2800993002/diff/20001/net/data/verify_certifi...
File
net/data/verify_certificate_chain_unittest/generate-target-restricts-eku-fail.py
(right):

https://codereview.chromium.org/2800993002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/generate-target-restricts-eku-fail.py:25:
key_purpose = common.DEFAULT_KEY_PURPOSE
specify server_auth explicitly?

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
File net/cert/internal/verify_certificate_chain.cc (right):

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
net/cert/internal/verify_certificate_chain.cc:450: if
(!trust_anchor.enforces_constraints())
Should it be checked on the trust anchor if it has enforces_constraints on?

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
File net/cert/internal/verify_certificate_chain.h (right):

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
net/cert/internal/verify_certificate_chain.h:28: KEY_PURPOSE_ANY,
nit: With enum class I think it's fine to leave off the KEY_PURPOSE_ prefix
since you always need to say KeyPurpose:: anyway.

[eroman, 2017-04-07 00:39:40]

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
File net/cert/internal/verify_certificate_chain.cc (right):

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
net/cert/internal/verify_certificate_chain.cc:450: if
(!trust_anchor.enforces_constraints())
On 2017/04/06 22:16:03, mattm wrote:
> Should it be checked on the trust anchor if it has enforces_constraints on?

Good point! Yes it should. I will update for this.

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
File net/cert/internal/verify_certificate_chain.h (right):

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
net/cert/internal/verify_certificate_chain.h:28: KEY_PURPOSE_ANY,
On 2017/04/06 22:16:03, mattm wrote:
> nit: With enum class I think it's fine to leave off the KEY_PURPOSE_ prefix
> since you always need to say KeyPurpose:: anyway.

I ran into a couple of problems with macro conflicts, which led me to these
verbose names :(

I don't remember the specific names that collided, but weird things happened on
Windows and Mac.

I can play with these some more and see if I can find something better. If I
could use the kCamelCase that would be ideal, but I don't think that is
consistent with our style guide...

[ryanchung, 2017-04-07 02:02:46]

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

[ryanchung, 2017-04-07 02:02:47]

https://codereview.chromium.org/2800993002/diff/40001/components/cast_certifi...
File components/cast_certificate/cast_cert_validator.cc (right):

https://codereview.chromium.org/2800993002/diff/40001/components/cast_certifi...
components/cast_certificate/cast_cert_validator.cc:180: // TODO(delete before
landing): Doug/Cast OWNERs: the behavior after my
These changes look fine.
I guess the clientAuth EKU check isn't all that important since we're not using
it for TLS here.
We have not created any ICAs with EKUs.

So, this change looks good.

[mattm, 2017-04-07 02:56:37]

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
File net/cert/internal/verify_certificate_chain.h (right):

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
net/cert/internal/verify_certificate_chain.h:28: KEY_PURPOSE_ANY,
On 2017/04/07 00:39:40, eroman wrote:
> On 2017/04/06 22:16:03, mattm wrote:
> > nit: With enum class I think it's fine to leave off the KEY_PURPOSE_ prefix
> > since you always need to say KeyPurpose:: anyway.
> 
> I ran into a couple of problems with macro conflicts, which led me to these
> verbose names :(
> 
> I don't remember the specific names that collided, but weird things happened
on
> Windows and Mac.
> 
> I can play with these some more and see if I can find something better. If I
> could use the kCamelCase that would be ideal, but I don't think that is
> consistent with our style guide...

Ugh, macro conflicts. Don't worry about it too much then, if you wanna just
stick with these names that's fine too.

[eroman, 2017-04-07 22:10:42]

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

[commit-bot: I haz the power, 2017-04-07 22:11:33]

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

[eroman, 2017-04-07 22:13:08]

https://codereview.chromium.org/2800993002/diff/20001/net/data/verify_certifi...
File
net/data/verify_certificate_chain_unittest/generate-target-restricts-eku-fail.py
(right):

https://codereview.chromium.org/2800993002/diff/20001/net/data/verify_certifi...
net/data/verify_certificate_chain_unittest/generate-target-restricts-eku-fail.py:25:
key_purpose = common.DEFAULT_KEY_PURPOSE
On 2017/04/06 22:16:03, mattm wrote:
> specify server_auth explicitly?

Done.

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
File net/cert/internal/verify_certificate_chain.cc (right):

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
net/cert/internal/verify_certificate_chain.cc:450: if
(!trust_anchor.enforces_constraints())
On 2017/04/07 00:39:40, eroman wrote:
> On 2017/04/06 22:16:03, mattm wrote:
> > Should it be checked on the trust anchor if it has enforces_constraints on?
> 
> Good point! Yes it should. I will update for this.

Done.

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
File net/cert/internal/verify_certificate_chain.h (right):

https://codereview.chromium.org/2800993002/diff/40001/net/cert/internal/verif...
net/cert/internal/verify_certificate_chain.h:28: KEY_PURPOSE_ANY,
On 2017/04/07 02:56:36, mattm wrote:
> On 2017/04/07 00:39:40, eroman wrote:
> > On 2017/04/06 22:16:03, mattm wrote:
> > > nit: With enum class I think it's fine to leave off the KEY_PURPOSE_
prefix
> > > since you always need to say KeyPurpose:: anyway.
> > 
> > I ran into a couple of problems with macro conflicts, which led me to these
> > verbose names :(
> > 
> > I don't remember the specific names that collided, but weird things happened
> on
> > Windows and Mac.
> > 
> > I can play with these some more and see if I can find something better. If I
> > could use the kCamelCase that would be ideal, but I don't think that is
> > consistent with our style guide...
> 
> Ugh, macro conflicts. Don't worry about it too much then, if you wanna just
> stick with these names that's fine too.

Checking with trybots now, I can definitely settle on something shorter.
I remember ANY was a problem so I have changed that to ANY_EKU.

[commit-bot: I haz the power, 2017-04-07 22:32:28]

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

[commit-bot: I haz the power, 2017-04-07 22:32:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mattm, 2017-04-07 23:22:38]

lgtm

[eroman, 2017-04-07 23:42:38]

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

[commit-bot: I haz the power, 2017-04-07 23:43:02]

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

[eroman, 2017-04-07 23:44:15]

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

[eroman, 2017-04-07 23:44:16]

+dougsteed for OWNERS approval on the first two files (cast_certificate/*)

I explained the change in more detail in a comment there.

[eroman, 2017-04-07 23:59:31]

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

[commit-bot: I haz the power, 2017-04-07 23:59:58]

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

[ryanchung, 2017-04-08 00:22:49]

Description was changed from

==========
Add a key purpose parameter to Certificate PathBuilder.

In this CL it is used to verify the Extended Key Usage.
(A subsequent CL will add Key Usage checks.)

BUG=634442
==========

to

==========
Add a key purpose parameter to Certificate PathBuilder.

In this CL it is used to verify the Extended Key Usage.
(A subsequent CL will add Key Usage checks.)

BUG=634442
==========

[ryanchung, 2017-04-08 00:22:49]

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

[commit-bot: I haz the power, 2017-04-08 01:04:12]

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

[commit-bot: I haz the power, 2017-04-08 01:04:13]

Dry run: This issue passed the CQ dry run.

[eroman, 2017-04-10 22:26:24]

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

[commit-bot: I haz the power, 2017-04-10 22:27:02]

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, 2017-04-11 00:41:39]

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

[commit-bot: I haz the power, 2017-04-11 00:41:40]

Dry run: This issue passed the CQ dry run.

[eroman, 2017-04-11 15:52:47]

ping Doug for OWNERS

[dougsteed, 2017-04-11 16:08:42]

lgtm

[eroman, 2017-04-11 16:13:40]

The CQ bit was checked by eroman@chromium.org

[eroman, 2017-04-11 16:13:40]

The patchset sent to the CQ was uploaded after l-g-t-m from mattm@chromium.org,
dougsteed@chromium.org
Link to the patchset: https://codereview.chromium.org/2800993002/#ps160001
(title: "Remove temporary review comment")

[commit-bot: I haz the power, 2017-04-11 16:14:07]

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-04-11 16:17:45]

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

[commit-bot: I haz the power, 2017-04-11 16:17:46]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[eroman, 2017-04-11 16:17:51]

The CQ bit was unchecked by eroman@chromium.org

[eroman, 2017-04-11 17:08:01]

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

[commit-bot: I haz the power, 2017-04-11 17:08:50]

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, 2017-04-11 18:18:25]

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

[commit-bot: I haz the power, 2017-04-11 18:18:26]

Dry run: This issue passed the CQ dry run.

[eroman, 2017-04-11 18:19:19]

The CQ bit was checked by eroman@chromium.org

[eroman, 2017-04-11 18:19:19]

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

[commit-bot: I haz the power, 2017-04-11 18:20:15]

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-04-11 19:13:55]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1491934759331060,
"parent_rev": "d914e02c97cb940507d6662b1b9a744d39b69a96", "commit_rev":
"5d358265af599429828297cab308b27fc4f1e8aa"}

[commit-bot: I haz the power, 2017-04-11 19:14:54]

Description was changed from

==========
Add a key purpose parameter to Certificate PathBuilder.

In this CL it is used to verify the Extended Key Usage.
(A subsequent CL will add Key Usage checks.)

BUG=634442
==========

to

==========
Add a key purpose parameter to Certificate PathBuilder.

In this CL it is used to verify the Extended Key Usage.
(A subsequent CL will add Key Usage checks.)

BUG=634442

Review-Url: https://codereview.chromium.org/2800993002
Cr-Commit-Position: refs/heads/master@{#463714}
Committed:
https://chromium.googlesource.com/chromium/src/+/5d358265af599429828297cab308...
==========

[commit-bot: I haz the power, 2017-04-11 19:14:55]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/5d358265af599429828297cab308...