Use test root cert once we can not find root cert in system cert store on Android when running un...

Use test root cert once we can not find root cert in system cert store on Android when running unit tests.

Many SSL tests need to install a root cert on device to verify the certificate sent from server during SSL handshake.
This change is to make sure that the cert verification function can use the test root cert during unit tests on Android.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=120760

[joth, 2012-02-02 19:41:24]

LGTM - with the caveats in the TODOs and as discussed before.

[commit-bot: I haz the power, 2012-02-04 08:58:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jnd@chromium.org/9313004/1

[commit-bot: I haz the power, 2012-02-04 08:58:45]

Presubmit check for 9313004-1 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for: net/base/x509_certificate_openssl.cc

[Johnny(Jianning) Ding, 2012-02-04 09:04:37]

On 2012/02/02 10:41:13, Johnny(Jianning) Ding wrote:

Hi Wan-Teh, would you please take a look per request of OWNER rule.

[wtc, 2012-02-06 20:56:29]

Patch Set 1 LGTM.  Please note my last comment (marked with
"IMPORTANT") below.

http://codereview.chromium.org/9313004/diff/1/net/base/x509_certificate_opens...
File net/base/x509_certificate_openssl.cc (right):

http://codereview.chromium.org/9313004/diff/1/net/base/x509_certificate_opens...
net/base/x509_certificate_openssl.cc:370: // Returns true we have verification
result in |verify_result| from Android

Nit: add "if" after "Returns true".

Nit: delete the blank line on line 374.

http://codereview.chromium.org/9313004/diff/1/net/base/x509_certificate_opens...
net/base/x509_certificate_openssl.cc:609: AppendPublicKeyHashes(ctx.get(),
&verify_result->public_key_hashes);

IMPORTANT: should we move this AppendPublicKeyHashes call
to line 611 so that it will also apply to the Android
Trust Manager case?

Similarly, the GetCertChainInfo call on line 606 probably
should also be used for the Android Trust Manager case.

[Johnny(Jianning) Ding, 2012-02-07 04:57:12]

http://codereview.chromium.org/9313004/diff/1/net/base/x509_certificate_opens...
File net/base/x509_certificate_openssl.cc (right):

http://codereview.chromium.org/9313004/diff/1/net/base/x509_certificate_opens...
net/base/x509_certificate_openssl.cc:370: // Returns true we have verification
result in |verify_result| from Android
On 2012/02/06 20:56:30, wtc wrote:
> 
> Nit: add "if" after "Returns true".
> 
> Nit: delete the blank line on line 374.

Done.

http://codereview.chromium.org/9313004/diff/1/net/base/x509_certificate_opens...
net/base/x509_certificate_openssl.cc:609: AppendPublicKeyHashes(ctx.get(),
&verify_result->public_key_hashes);
Currently the verified cert chain info doesn't return by 
android::VerifyX509CertChain.
We should have another bug to track the task of adding verified cert chain info
and public key hashes for Android platform.

@Joth, sounds good?

On 2012/02/06 20:56:30, wtc wrote:
> 
> IMPORTANT: should we move this AppendPublicKeyHashes call
> to line 611 so that it will also apply to the Android
> Trust Manager case?
> 
> Similarly, the GetCertChainInfo call on line 606 probably
> should also be used for the Android Trust Manager case.

[commit-bot: I haz the power, 2012-02-07 09:13:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jnd@chromium.org/9313004/10001

[commit-bot: I haz the power, 2012-02-07 10:43:08]

Change committed as 120760