Return specific cert verification errors on Android

Return specific cert verification errors on Android

To implement CertVerifyProc on Android we refer to the Java 
side to query the platform trust managers. Currently the 
information we get from the platform is binary - each 
certificate chain is either identified as trusted or not, in 
which case we assume that this is due to not-trusted root.

This patch provides better granularity distinguishing the 
following cases: expired, not yet valid, incorrect (could not 
be parsed), not trusted root.

This allowed to reenable two net unittests:
- CertVerifyProcTest.ExtraneousMD5RootCert
- CertVerifyProcTest.IntermediateCARequireExplicitPolicy

The following net unittest had to be disabled as it joins 
the club of CertVerifyProc tests failing on bots with 
incorrect time/date settings:
- CertVerifyProcTest.InvalidKeyUsage

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

[ppi, 2013-02-12 17:56:05]

Could you guys have a look?

[Ryan Sleevi, 2013-02-12 18:14:54]

Could you look into why ExtraneousMD5RootCert succeeds? Based on the Android
implementation, this surprises me, and I'd like to make sure we understand why.

[ppi, 2013-02-12 18:25:57]

Only because the test does not >really< work on any platform now, see the
comment: "The certificate will expire on 2012-07-20. The test will still pass if
error == ERR_CERT_DATE_INVALID."

The change that this patch makes it that we are now aligned with other platforms
on that - ERR_CERT_DATE_INVALID is returned and the test "passes".

[Ryan Sleevi, 2013-02-13 01:41:38]

lgtm, mod nit

https://codereview.chromium.org/12212135/diff/7001/net/android/cert_verify_re...
File net/android/cert_verify_result_android_list.h (right):

https://codereview.chromium.org/12212135/diff/7001/net/android/cert_verify_re...
net/android/cert_verify_result_android_list.h:27:
CERT_VERIFY_RESULT_ANDROID(UNABLE_TO_PARSE, 4)
For the values here, it seems better if you used either the net_error_list style
numbering (that is: consistently negative values) or you used the cert_status
flag style numbering (that is, a single bit for each possible error).

[Philippe, 2013-02-13 10:00:43]

lgtm, thanks Przemek!

https://codereview.chromium.org/12212135/diff/7001/net/android/java/src/org/c...
File net/android/java/src/org/chromium/net/X509Util.java (right):

https://codereview.chromium.org/12212135/diff/7001/net/android/java/src/org/c...
net/android/java/src/org/chromium/net/X509Util.java:155: * Expired and not yet
valid certificates would be rejected by the trust managers, but the
Nit: I believe that we tend to use // style comments for the comments in
functions' body.

[ppi, 2013-02-13 12:34:55]

Thanks Ryan and Philippe!

I have addressed your remarks in patch set 2 and I intend to proceed with a
commit.

https://codereview.chromium.org/12212135/diff/7001/net/android/cert_verify_re...
File net/android/cert_verify_result_android_list.h (right):

https://codereview.chromium.org/12212135/diff/7001/net/android/cert_verify_re...
net/android/cert_verify_result_android_list.h:27:
CERT_VERIFY_RESULT_ANDROID(UNABLE_TO_PARSE, 4)
On 2013/02/13 01:41:39, Ryan Sleevi wrote:
> For the values here, it seems better if you used either the net_error_list
style
> numbering (that is: consistently negative values) or you used the cert_status
> flag style numbering (that is, a single bit for each possible error).

Thanks, I have moved to non-positive numbering (0 for OK, negative integers for
errors) in patch set 2.

https://codereview.chromium.org/12212135/diff/7001/net/android/java/src/org/c...
File net/android/java/src/org/chromium/net/X509Util.java (right):

https://codereview.chromium.org/12212135/diff/7001/net/android/java/src/org/c...
net/android/java/src/org/chromium/net/X509Util.java:155: * Expired and not yet
valid certificates would be rejected by the trust managers, but the
On 2013/02/13 10:00:43, Philippe wrote:
> Nit: I believe that we tend to use // style comments for the comments in
> functions' body.

Indeed that seems to be the case (and we save two lines per comment block).
Thanks, fixed in patch set 2.

[commit-bot: I haz the power, 2013-02-13 12:40:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ppi@chromium.org/12212135/4016

[commit-bot: I haz the power, 2013-02-13 19:13:52]

Change committed as 182280