Differentiate between VERIFY_FAILED and VERIFY_INCORRECT_KEY_USAGE.

Differentiate between VERIFY_FAILED and VERIFY_INCORRECT_KEY_USAGE.

VERIFY_FAILED means general failure to validate the X.509 chain at all,
which is not what we want when eKU is incorrectly.

BUG=233150
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=196500

[palmer, 2013-04-23 00:09:55]

I felt guilty leaving this for ppi to solve. :) Hopefully this CL is not off the
mark too badly. PTAL.

[ppi, 2013-04-23 14:48:37]

This looks great, thanks!

Could you also update the native unit test (CertVerifyProcTest.InvalidKeyUsage)
and remove the suppression (in
build/android/pylib/gtest/filter/net_unittests_disabled)? It looks like we just
need to remove the Android-specific part of:

#if defined(USE_OPENSSL)
  // This certificate has two errors: "invalid key usage" and "untrusted CA".
  // However, OpenSSL returns only one (the latter), and we can't detect
  // the other errors.
  EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, error);
#else
  EXPECT_EQ(ERR_CERT_INVALID, error);
  EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_INVALID);
#endif

[palmer, 2013-04-23 17:48:52]

I am not sure if I doctored the test correctly, but this is what I had to do to
get it to pass on my system.

[ppi, 2013-04-23 18:25:31]

lgtm, thanks!

[joth, 2013-04-23 18:27:20]

I added linux_redux try job - make sure this passes there too.

https://codereview.chromium.org/14358023/diff/7001/net/cert/cert_verify_proc_...
File net/cert/cert_verify_proc_unittest.cc (right):

https://codereview.chromium.org/14358023/diff/7001/net/cert/cert_verify_proc_...
net/cert/cert_verify_proc_unittest.cc:539: // the other errors.
think this will fail on linux redux (as the "can't detect" comment refers to
using full openssl, rather than the android flow)

so my guess is...

#if OPENSSL && !ANDROID
// can't detect other errors
EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, error);
#else
EXPECT_EQ(ERR_CERT_INVALID, error);
EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_INVALID);
#endif

[palmer, 2013-04-23 20:47:41]

https://codereview.chromium.org/14358023/diff/7001/net/cert/cert_verify_proc_...
File net/cert/cert_verify_proc_unittest.cc (right):

https://codereview.chromium.org/14358023/diff/7001/net/cert/cert_verify_proc_...
net/cert/cert_verify_proc_unittest.cc:539: // the other errors.
On 2013/04/23 18:27:21, joth wrote:
> think this will fail on linux redux (as the "can't detect" comment refers to
> using full openssl, rather than the android flow)
> 
> so my guess is...
> 
> #if OPENSSL && !ANDROID
> // can't detect other errors
> EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, error);
> #else
> EXPECT_EQ(ERR_CERT_INVALID, error);
> EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_INVALID);
> #endif

Done.

[commit-bot: I haz the power, 2013-04-23 22:25:48]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[palmer, 2013-04-23 22:29:40]

Hoping for some OWNERS approval: rsleevi for net/OWNERS, yfriedman for
net/android/OWNERS. Thanks!

[Yaron, 2013-04-24 00:51:34]

lgtm

[Ryan Sleevi, 2013-04-25 18:36:50]

net/ LGTM

https://codereview.chromium.org/14358023/diff/16001/net/cert/cert_verify_proc...
File net/cert/cert_verify_proc_android.cc (right):

https://codereview.chromium.org/14358023/diff/16001/net/cert/cert_verify_proc...
net/cert/cert_verify_proc_android.cc:45: verify_result->cert_status |=
CERT_STATUS_INVALID;
I've confirmed this matches how we map CERT_TRUST_IS_NOT_VALID_FOR_USAGE on
Windows.

[palmer, 2013-04-25 18:52:24]

On 2013/04/25 18:36:50, Ryan Sleevi wrote:
> net/ LGTM
> 
>
https://codereview.chromium.org/14358023/diff/16001/net/cert/cert_verify_proc...
> File net/cert/cert_verify_proc_android.cc (right):
> 
>
https://codereview.chromium.org/14358023/diff/16001/net/cert/cert_verify_proc...
> net/cert/cert_verify_proc_android.cc:45: verify_result->cert_status |=
> CERT_STATUS_INVALID;
> I've confirmed this matches how we map CERT_TRUST_IS_NOT_VALID_FOR_USAGE on
> Windows.

You are a gentleman and a scholar. Thanks!

[commit-bot: I haz the power, 2013-04-25 18:52:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/palmer@chromium.org/14358023/16001

[commit-bot: I haz the power, 2013-04-25 21:29:58]

Change committed as 196500