Add X509Certificate::Verify stubs for Mac and Linux....

Add X509Certificate::Verify stubs for Mac and Linux.
They do nothing but return ERR_NOT_IMPLEMENTED.

In SSLClientSocketWin, call
X509Certificate::CreateFromHandle only once and store
the result in the server_cert_ member.

Add the CertVerifyResult::Reset method to clear all
members.

R=eroman
BUG=3592
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=9272

[wtc, 2009-02-05 01:59:14]

eroman: please review the whole CL.

abarth: you only need to review the comments I added to
x509_certificate.h.  The caller of
X509Certificate::CreateFromHandle, etc. must store the
returned pointer in a scoped_refptr to acquire a reference
to the returned object, otherwise it would be using the
object without a reference to ensure object lifetime.

[eroman, 2009-02-05 03:11:32]

lgtm

http://codereview.chromium.org/21071/diff/17/21
File net/base/cert_verify_result.h (right):

http://codereview.chromium.org/21071/diff/17/21#newcode16
Line 16: has_md5_ca(false), has_md2_ca(false) {
Maybe just call Reset() in the constructor to avoid duplication?

http://codereview.chromium.org/21071/diff/17/20
File net/base/ssl_client_socket_win.cc (right):

http://codereview.chromium.org/21071/diff/17/20#newcode239
Line 239: if (server_cert_ == NULL)
style nit: how about "if (!server_cert_)" for consistency with other parts of
this file that have "if (server_cert_)"

http://codereview.chromium.org/21071/diff/17/20#newcode688
Line 688: return verifier_.Verify(server_cert_, hostname_,
Is there any danger in accessing a cert_handle from multiple threads?

This looks fine to me, but figured I should ask anyway since I don't know much
about PCCERT_CONTEXT. For example Verify accesses cert_handle_->hCertStore, so
thinking if it could be possible for hCertStore to be changing while Verify() is
reading it.

http://codereview.chromium.org/21071/diff/17/19
File net/base/x509_certificate_win.cc (right):

http://codereview.chromium.org/21071/diff/17/19#newcode168
Line 168: // *verify_result.  The caller MUST initialize verify_result->has_xxx
to
"verify_result->has_xxx" is a little weird.
maybe just say that verify_result should have been Reset() prior to calling us.

[abarth-chromium, 2009-02-05 05:38:50]

Yep.  Those comments are right.

[wtc, 2009-02-05 18:50:13]

Eric,

I made all the changes you suggested.  Please review the
new patch set.

http://codereview.chromium.org/21071/diff/17/20
File net/base/ssl_client_socket_win.cc (right):

http://codereview.chromium.org/21071/diff/17/20#newcode688
Line 688: return verifier_.Verify(server_cert_, hostname_,
On 2009/02/05 03:11:33, eroman wrote:
> Is there any danger in accessing a cert_handle from multiple threads?
> 
> This looks fine to me, but figured I should ask anyway since I don't know much
> about PCCERT_CONTEXT. For example Verify accesses cert_handle_->hCertStore, so
> thinking if it could be possible for hCertStore to be changing while Verify()
is
> reading it.

This is a good question.  I am afraid that the answer
depends on the underlying OS crypto library.  For Windows
CryptoAPI, the cert handle is PCCERT_CONTEXT, which is
const CERT_CONTEXT*.  So all the users of a cert handle
have read-only access to the CERT_CONTEXT structure.  In
particular, none of them can change cert_handle_->hCertStore
while another thread is using it.  So on Windows, it is
safe to access a cert handle from multiple threads.

[wtc, 2009-02-05 18:55:39]

http://codereview.chromium.org/21071/diff/17/20
File net/base/ssl_client_socket_win.cc (right):

http://codereview.chromium.org/21071/diff/17/20#newcode688
Line 688: return verifier_.Verify(server_cert_, hostname_,
On 2009/02/05 03:11:33, eroman wrote:
> Is there any danger in accessing a cert_handle from multiple threads?
> 
> This looks fine to me, but figured I should ask anyway since I don't know much
> about PCCERT_CONTEXT. For example Verify accesses cert_handle_->hCertStore, so
> thinking if it could be possible for hCertStore to be changing while Verify()
is
> reading it.

More thought: Since server_cert_ is now a
scoped_refptr<X509Certificate>, the cert verifier could
potentially access the X509Certificate object, rather
than just the OSCertHandle.

I wonder if we should eliminate that possibility by
passing the OSCertHandle instead to the cert verifier.
This would require changing X509Certificate::Verify to
be a static method that takes an OSCertHandle.

What do you think?  (If we're going to do this, I'd do
it in a follow-up CL.)