Mac EV verification using Chrome methods rather than OS methods.

Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of OS EV checking, and those APIs stopped providing results on Sierra when we stopped passing the hostname to the OS verification (see crbug/621684). Additionally, it was impossible to unittest the old implementation. The new method is similar to what we do on other platforms, does not depend on OS handling, and can be unittested. Due to OS API limitations, the new method will be less efficient, but that additional inefficiency is limited to sites with EV certs.

BUG=655612
Committed: https://crrev.com/1a282f54af2ea79cf9aa0b78234e3b4053b4d73b
Cr-Commit-Position: refs/heads/master@{#431367}

[mattm, 2016-10-26 20:41:57]

Description was changed from

==========
*WIP* Mac EV verification using Chrome methods rather than OS methods.

BUG=
==========

to

==========
*WIP* Mac EV verification using Chrome methods rather than OS methods.

BUG=
==========

[mattm, 2016-10-26 20:43:21]

Patchset #1 (id:1) has been deleted

[mattm, 2016-11-01 01:57:12]

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

[mattm, 2016-11-01 01:57:25]

Description was changed from

==========
*WIP* Mac EV verification using Chrome methods rather than OS methods.

BUG=
==========

to

==========
*WIP* Mac EV verification using Chrome methods rather than OS methods.

BUG=655612
==========

[commit-bot: I haz the power, 2016-11-01 01:57:31]

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-11-01 02:30:23]

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

[commit-bot: I haz the power, 2016-11-01 02:30:24]

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_...)

[mattm, 2016-11-02 19:45:17]

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

[commit-bot: I haz the power, 2016-11-02 19:46:02]

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

[mattm, 2016-11-02 19:48:03]

Description was changed from

==========
*WIP* Mac EV verification using Chrome methods rather than OS methods.

BUG=655612
==========

to

==========
Mac EV verification using Chrome methods rather than OS methods.

BUG=655612
==========

[mattm, 2016-11-02 19:48:03]

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

[commit-bot: I haz the power, 2016-11-02 20:40:43]

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

[commit-bot: I haz the power, 2016-11-02 20:40:44]

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_...)

[mattm, 2016-11-07 23:30:09]

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

[commit-bot: I haz the power, 2016-11-07 23:30:46]

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-11-07 23:42:53]

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

[commit-bot: I haz the power, 2016-11-07 23:42:54]

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...)

[Ryan Sleevi, 2016-11-08 00:11:21]

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.cc (right):

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:55: #endif
Erk; this makes me super-nervous to extend the platform-specificness of here to
also include multiple encodings, and it'll make it hard to maintain that these
are encoded correctly. Also, - there's an additional one-byte overhead with the
representation used (e.g. lines 68) because now we also have to account for the
NULL byte.

Considering that we already pay the encoding overhead of the string
representation on Windows (vis-a-vis the rgPolicyInfo translation) or of
registering (vis-a-vis the EVRootCAMetadata ctor), I think converting the
der::Input int the string form should be an acceptable run-time performance
tradeoff that we can optimize later.

I just feel really awkward having to maintain and validate these two tables in
the future - even doing this review would take quite a while!

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:875: #elif defined(OS_MACOSX)
At this point, we should probably move these into _win, _mac, and _nss files fr
their implementation size.

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:959: // These are just stub functions for
platforms where we don't use this EV
And we'd delete these stubs (if possible).

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.h (right):

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.h:32: }
nit:
}  // namespace der

(see line 26)

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.h:84: #elif defined(OS_WIN)
Why not just extend this to be
#elif defined(OS_WIN) || defined(OS_MACOSX)

?

https://codereview.chromium.org/2456523003/diff/100001/net/cert/x509_util_mac.cc
File net/cert/x509_util_mac.cc (right):

https://codereview.chromium.org/2456523003/diff/100001/net/cert/x509_util_mac...
net/cert/x509_util_mac.cc:86: // On Sierra we can't disable network revocation
checking without also
nit: On Sierra, it's not possible to disable network revocation checking without
also breaking AIA.

(Reworded w/o the we)

https://codereview.chromium.org/2456523003/diff/100001/net/cert/x509_util_mac...
net/cert/x509_util_mac.cc:109: } else {
Don't } else after a return

if (base::mac::IsAtLeastOS10_12()) {
  ...
  return noErr;
}

if ...

[mattm, 2016-11-08 23:06:26]

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.cc (right):

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:55: #endif
On 2016/11/08 00:11:20, Ryan Sleevi wrote:
> Erk; this makes me super-nervous to extend the platform-specificness of here
to
> also include multiple encodings, and it'll make it hard to maintain that these
> are encoded correctly. Also, - there's an additional one-byte overhead with
the
> representation used (e.g. lines 68) because now we also have to account for
the
> NULL byte.
> 
> Considering that we already pay the encoding overhead of the string
> representation on Windows (vis-a-vis the rgPolicyInfo translation) or of
> registering (vis-a-vis the EVRootCAMetadata ctor), I think converting the
> der::Input int the string form should be an acceptable run-time performance
> tradeoff that we can optimize later.
> 
> I just feel really awkward having to maintain and validate these two tables in
> the future - even doing this review would take quite a while!

Ok, scrapped that.

There didn't seem to be a clean way to mimic the win code (doesn't seem great to
create a nid for every random policy we see so that it can be converted to txt),
so I went with a kinda hybrid approach. Similar to NSS but storing the DER oids
instead of tag/nid. Could have just gone with a nss-like approach storing nids
in the map/set but I guess I just don't like that this uses nids at all and
wanted to minimize dependency on that.

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:875: #elif defined(OS_MACOSX)
On 2016/11/08 00:11:20, Ryan Sleevi wrote:
> At this point, we should probably move these into _win, _mac, and _nss files
fr
> their implementation size.

I'll poke at that separately.

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:959: // These are just stub functions for
platforms where we don't use this EV
On 2016/11/08 00:11:20, Ryan Sleevi wrote:
> And we'd delete these stubs (if possible).

Acknowledged.

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.h (right):

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.h:32: }
On 2016/11/08 00:11:21, Ryan Sleevi wrote:
> nit:
> }  // namespace der
> 
> (see line 26)

Done.

https://codereview.chromium.org/2456523003/diff/100001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.h:84: #elif defined(OS_WIN)
On 2016/11/08 00:11:21, Ryan Sleevi wrote:
> Why not just extend this to be
> #elif defined(OS_WIN) || defined(OS_MACOSX)
> 
> ?

Acknowledged.

https://codereview.chromium.org/2456523003/diff/100001/net/cert/x509_util_mac.cc
File net/cert/x509_util_mac.cc (right):

https://codereview.chromium.org/2456523003/diff/100001/net/cert/x509_util_mac...
net/cert/x509_util_mac.cc:86: // On Sierra we can't disable network revocation
checking without also
On 2016/11/08 00:11:21, Ryan Sleevi wrote:
> nit: On Sierra, it's not possible to disable network revocation checking
without
> also breaking AIA.
> 
> (Reworded w/o the we)

Done.

https://codereview.chromium.org/2456523003/diff/100001/net/cert/x509_util_mac...
net/cert/x509_util_mac.cc:109: } else {
On 2016/11/08 00:11:21, Ryan Sleevi wrote:
> Don't } else after a return
> 
> if (base::mac::IsAtLeastOS10_12()) {
>   ...
>   return noErr;
> }
> 
> if ...

Done.

[mattm, 2016-11-08 23:06:29]

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

[commit-bot: I haz the power, 2016-11-08 23:07:18]

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

[Ryan Sleevi, 2016-11-08 23:54:08]

rsleevi@chromium.org changed reviewers:
+ davidben@chromium.org

[Ryan Sleevi, 2016-11-08 23:54:09]

Code LGTM w/ question for Davidben

We definitely want to document the "Why" subtlety in the CL description, since
you've got a lot of context here. Do you want to make a stab at it, or would it
help if I took a go?

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
File net/cert/cert_verify_proc_mac.cc (right):

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:308: *ev_policy_oid = std::string();
ev_policy_oid->clear(); is slightly more efficient ;)

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:323: &extension_value)) {
Since you braced here, should you brace 289-291 for consistency (I think so)

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:371: return
metadata->HasEVPolicyOID(weak_fingerprint, ev_policy_oid);
It seems like this check would be faster/a quicker short-circuit. Should we move
this before lines 349-362?

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:427: // |last_covered| contains the coverage
state of the leaf certificate.
Inconsistency with respect to || vs no ||. I'd drop the || (which is consistent
with 422 as well)

Or word the comment w/o mentioning the variable name

// Set to the coverage state of the previous certificate. As the certificates
are iterated over
// from root to leaf, at the end of the iteration, this indicates the coverage
state of the leaf certificate.


Similarly, you could reword error to

// Set to true if any errors are found, which will cause such chains to not be
treated as covered by the CRLSet.

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.cc (right):

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:737: }
@davidben to comment on if there's anything cleaner he wants to support.

[davidben, 2016-11-09 00:15:03]

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.cc (right):

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:15: #include <openssl/obj.h>
#include "third_party/boringssl/src/include/openssl/obj.h"

I hate how verbose it is, but I guess it's more consistent or something. :-/

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:737: }
On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> @davidben to comment on if there's anything cleaner he wants to support.

I'll probably have to tweak this slightly at some point, but don't worry about
it. I don't think I have a good answer for it right now. (End goal is to unship
the giant OID table from Chromium and OBJ_txt2* supports looking up by
human-readable name, so it depends on the OID table.)

This is probably shorter though:

bssl::UniquePtr<ASN1_OBJECT> obj(OBJ_txt2obj(policy));
if (!policy) {
  NOTREACHED();
  return std::string();
}

return std::string(reinterpret_cast<const char*>(obj->data), obj->length);

Note: I think you may need to include openssl/asn1.h to get the definition of
ASN1_OBJECT. I'll see about fixing that later since it's kind of ridiculous.

[davidben, 2016-11-09 00:15:55]

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.cc (right):

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:737: }
On 2016/11/09 00:15:03, davidben wrote:
> On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> > @davidben to comment on if there's anything cleaner he wants to support.
> 
> I'll probably have to tweak this slightly at some point, but don't worry about
> it. I don't think I have a good answer for it right now. (End goal is to
unship
> the giant OID table from Chromium and OBJ_txt2* supports looking up by
> human-readable name, so it depends on the OID table.)
> 
> This is probably shorter though:
> 
> bssl::UniquePtr<ASN1_OBJECT> obj(OBJ_txt2obj(policy));
> if (!policy) {
>   NOTREACHED();
>   return std::string();
> }
> 
> return std::string(reinterpret_cast<const char*>(obj->data), obj->length);
> 
> Note: I think you may need to include openssl/asn1.h to get the definition of
> ASN1_OBJECT. I'll see about fixing that later since it's kind of ridiculous.

Sorry, that should be if (!obj) {

And I guess you don't need/want the NOTREACHED() since the callers all check for
empty. Missed that bit.

[commit-bot: I haz the power, 2016-11-09 00:43:05]

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

[commit-bot: I haz the power, 2016-11-09 00:43:06]

Dry run: This issue passed the CQ dry run.

[mattm, 2016-11-10 02:42:34]

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

[commit-bot: I haz the power, 2016-11-10 02:42: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, 2016-11-10 02:51:15]

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

[commit-bot: I haz the power, 2016-11-10 02:51:16]

Dry run: 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...)

[mattm, 2016-11-10 03:09:48]

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

[commit-bot: I haz the power, 2016-11-10 03:10:06]

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-11-10 03:19:53]

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

[commit-bot: I haz the power, 2016-11-10 03:19:54]

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

[mattm, 2016-11-10 03:26:04]

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

[commit-bot: I haz the power, 2016-11-10 03:26:18]

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-11-10 04:44:25]

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

[commit-bot: I haz the power, 2016-11-10 04:44:25]

Dry run: This issue passed the CQ dry run.

[mattm, 2016-11-10 05:40:39]

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

[mattm, 2016-11-10 05:40:41]

+msw for chrome/browser/ui OWNERS

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
File net/cert/cert_verify_proc_mac.cc (right):

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:308: *ev_policy_oid = std::string();
On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> ev_policy_oid->clear(); is slightly more efficient ;)

Done.

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:323: &extension_value)) {
On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> Since you braced here, should you brace 289-291 for consistency (I think so)

Done.

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:371: return
metadata->HasEVPolicyOID(weak_fingerprint, ev_policy_oid);
On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> It seems like this check would be faster/a quicker short-circuit. Should we
move
> this before lines 349-362?

Done.

https://codereview.chromium.org/2456523003/diff/120001/net/cert/cert_verify_p...
net/cert/cert_verify_proc_mac.cc:427: // |last_covered| contains the coverage
state of the leaf certificate.
On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> Inconsistency with respect to || vs no ||. I'd drop the || (which is
consistent
> with 422 as well)
> 
> Or word the comment w/o mentioning the variable name
> 
> // Set to the coverage state of the previous certificate. As the certificates
> are iterated over
> // from root to leaf, at the end of the iteration, this indicates the coverage
> state of the leaf certificate.
> 
> 
> Similarly, you could reword error to
> 
> // Set to true if any errors are found, which will cause such chains to not be
> treated as covered by the CRLSet.

Done.

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
File net/cert/ev_root_ca_metadata.cc (right):

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:15: #include <openssl/obj.h>
On 2016/11/09 00:15:03, davidben wrote:
> #include "third_party/boringssl/src/include/openssl/obj.h"
> 
> I hate how verbose it is, but I guess it's more consistent or something. :-/

Done.

https://codereview.chromium.org/2456523003/diff/120001/net/cert/ev_root_ca_me...
net/cert/ev_root_ca_metadata.cc:737: }
On 2016/11/09 00:15:55, davidben wrote:
> On 2016/11/09 00:15:03, davidben wrote:
> > On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> > > @davidben to comment on if there's anything cleaner he wants to support.
> > 
> > I'll probably have to tweak this slightly at some point, but don't worry
about
> > it. I don't think I have a good answer for it right now. (End goal is to
> unship
> > the giant OID table from Chromium and OBJ_txt2* supports looking up by
> > human-readable name, so it depends on the OID table.)
> > 
> > This is probably shorter though:
> > 
> > bssl::UniquePtr<ASN1_OBJECT> obj(OBJ_txt2obj(policy));
> > if (!policy) {
> >   NOTREACHED();
> >   return std::string();
> > }
> > 
> > return std::string(reinterpret_cast<const char*>(obj->data), obj->length);
> > 
> > Note: I think you may need to include openssl/asn1.h to get the definition
of
> > ASN1_OBJECT. I'll see about fixing that later since it's kind of ridiculous.
> 
> Sorry, that should be if (!obj) {
> 
> And I guess you don't need/want the NOTREACHED() since the callers all check
for
> empty. Missed that bit.

Done.

[mattm, 2016-11-10 05:47:05]

Description was changed from

==========
Mac EV verification using Chrome methods rather than OS methods.

BUG=655612
==========

to

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private APIs, and those APIs stopped
providing results on Sierra. Additionally, it was impossible to unittest the old
implementation. The new method is similar to what we do on other platforms, does
not depend on OS handling, and can be unittested. Due to OS API limitations, the
new method will be less efficient, but that additional inefficiency is limited
to sites with EV certs.

BUG=655612
==========

[mattm, 2016-11-10 05:48:19]

On 2016/11/08 23:54:09, Ryan Sleevi wrote:
> Code LGTM w/ question for Davidben
> 
> We definitely want to document the "Why" subtlety in the CL description, since
> you've got a lot of context here. Do you want to make a stab at it, or would
it
> help if I took a go?

Almost forgot (I wish the description field could have first-class review
comments). Just updated description too.

[mattm, 2016-11-10 05:49:54]

Description was changed from

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private APIs, and those APIs stopped
providing results on Sierra. Additionally, it was impossible to unittest the old
implementation. The new method is similar to what we do on other platforms, does
not depend on OS handling, and can be unittested. Due to OS API limitations, the
new method will be less efficient, but that additional inefficiency is limited
to sites with EV certs.

BUG=655612
==========

to

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs, and those APIs
stopped providing results on Sierra. Additionally, it was impossible to unittest
the old implementation. The new method is similar to what we do on other
platforms, does not depend on OS handling, and can be unittested. Due to OS API
limitations, the new method will be less efficient, but that additional
inefficiency is limited to sites with EV certs.

BUG=655612
==========

[mattm, 2016-11-10 05:50:35]

Description was changed from

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs, and those APIs
stopped providing results on Sierra. Additionally, it was impossible to unittest
the old implementation. The new method is similar to what we do on other
platforms, does not depend on OS handling, and can be unittested. Due to OS API
limitations, the new method will be less efficient, but that additional
inefficiency is limited to sites with EV certs.

BUG=655612
==========

to

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of
OS EV checking, and those APIs stopped providing results on Sierra.
Additionally, it was impossible to unittest the old implementation. The new
method is similar to what we do on other platforms, does not depend on OS
handling, and can be unittested. Due to OS API limitations, the new method will
be less efficient, but that additional inefficiency is limited to sites with EV
certs.

BUG=655612
==========

[mattm, 2016-11-10 06:42:00]

Description was changed from

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of
OS EV checking, and those APIs stopped providing results on Sierra.
Additionally, it was impossible to unittest the old implementation. The new
method is similar to what we do on other platforms, does not depend on OS
handling, and can be unittested. Due to OS API limitations, the new method will
be less efficient, but that additional inefficiency is limited to sites with EV
certs.

BUG=655612
==========

to

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of
OS EV checking, and those APIs stopped providing results on Sierra when we
stopped passing the hostname to the OS verification (see crbug/621684).
Additionally, it was impossible to unittest the old implementation. The new
method is similar to what we do on other platforms, does not depend on OS
handling, and can be unittested. Due to OS API limitations, the new method will
be less efficient, but that additional inefficiency is limited to sites with EV
certs.

BUG=655612
==========

[msw, 2016-11-10 18:32:35]

c/b/ui rubber stamp lgtm

[mattm, 2016-11-10 21:45:01]

The CQ bit was checked by mattm@chromium.org

[mattm, 2016-11-10 21:45:01]

The patchset sent to the CQ was uploaded after l-g-t-m from rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/2456523003/#ps200001
(title: "compile fix")

[commit-bot: I haz the power, 2016-11-10 21:45:37]

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-11-10 21:50:28]

Description was changed from

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of
OS EV checking, and those APIs stopped providing results on Sierra when we
stopped passing the hostname to the OS verification (see crbug/621684).
Additionally, it was impossible to unittest the old implementation. The new
method is similar to what we do on other platforms, does not depend on OS
handling, and can be unittested. Due to OS API limitations, the new method will
be less efficient, but that additional inefficiency is limited to sites with EV
certs.

BUG=655612
==========

to

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of
OS EV checking, and those APIs stopped providing results on Sierra when we
stopped passing the hostname to the OS verification (see crbug/621684).
Additionally, it was impossible to unittest the old implementation. The new
method is similar to what we do on other platforms, does not depend on OS
handling, and can be unittested. Due to OS API limitations, the new method will
be less efficient, but that additional inefficiency is limited to sites with EV
certs.

BUG=655612
==========

[commit-bot: I haz the power, 2016-11-10 21:50:29]

Committed patchset #10 (id:200001)

[commit-bot: I haz the power, 2016-11-10 22:34:46]

Description was changed from

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of
OS EV checking, and those APIs stopped providing results on Sierra when we
stopped passing the hostname to the OS verification (see crbug/621684).
Additionally, it was impossible to unittest the old implementation. The new
method is similar to what we do on other platforms, does not depend on OS
handling, and can be unittested. Due to OS API limitations, the new method will
be less efficient, but that additional inefficiency is limited to sites with EV
certs.

BUG=655612
==========

to

==========
Mac EV verification using Chrome methods rather than OS methods.

Previously we were using some internal/private Mac OS APIs to get the results of
OS EV checking, and those APIs stopped providing results on Sierra when we
stopped passing the hostname to the OS verification (see crbug/621684).
Additionally, it was impossible to unittest the old implementation. The new
method is similar to what we do on other platforms, does not depend on OS
handling, and can be unittested. Due to OS API limitations, the new method will
be less efficient, but that additional inefficiency is limited to sites with EV
certs.

BUG=655612

Committed: https://crrev.com/1a282f54af2ea79cf9aa0b78234e3b4053b4d73b
Cr-Commit-Position: refs/heads/master@{#431367}
==========

[commit-bot: I haz the power, 2016-11-10 22:34:47]

Patchset 10 (id:??) landed as
https://crrev.com/1a282f54af2ea79cf9aa0b78234e3b4053b4d73b
Cr-Commit-Position: refs/heads/master@{#431367}