Return the constructed certificate chain in X509Certificate::Verify()

net/socket/ssl_client_socket_mac.cc

Index: net/socket/ssl_client_socket_mac.cc
diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc
index 6a2f821ff3c94fd5ec5feb44ca4a018a262fbe53..926abcb9dfab221da5c62bc3ff0608e89cdddc0f 100644
--- a/net/socket/ssl_client_socket_mac.cc
+++ b/net/socket/ssl_client_socket_mac.cc
@@ -702,12 +702,12 @@ bool SSLClientSocketMac::SetSendBufferSize(int32 size) {
 
 void SSLClientSocketMac::GetSSLInfo(SSLInfo* ssl_info) {
   ssl_info->Reset();
-  if (!server_cert_) {

[wtc, 2011/07/26 00:16:35]

Does this mean we can set server_cert_ to NULL as soon as
we have verified it?

[Ryan Sleevi, 2011/07/26 00:44:15]

Not without further updates. The SSLClientSocket* currently compare
|server_cert_| with the new/incoming certificate on a renegotiation, to see if
it can skip re-verification.

However, your question has highlighted what may be a bug - I'll need to go back
and look carefully at this before committing/commenting.

The bug I'm looking at is at line 1245. We end up not updating |server_cert_| on
a renegotiation handshake if the end-entity certificates are the same. This
prevents a server from being able to change it's intermediates on the fly during
a renegotiation. I'm not sure how important this is, but it is supported by
Apache, so I believe it may be a bug.

If were to fix this bug to compare the entire old and new chains, then I don't
think we could safely swap using |->verified_cert| for |server_cert_| in that
check. This is because the |->verified_cert| chain may differ greatly from the
|server_cert_| chain, which would cause the chain mismatches to (potentially)
force a reverification.

I'm also thinking that perhaps SSLHostInfo should be tracking both the verified
chain and the original (server) chain. The original server chain is useful for
situations such as client certificate authentication (where the intermediates
sent by the server may be necessary in order to build a successful client auth
chain - http://crbug.com/47656 , http://crbug.com/47658), while the verified
chain would be used for display purposes. That's what I need to look into
further.

[wtc, 2011/07/26 01:37:54]

Those two bugs should be marked WontFix.  I remember we
discussed this issue before.  The certificates in the
server's Certificate message don't need to be used for
building the client certificate chain.  NSS uses them
to build the client certificate chain unintentionally.
It is an implementation quirk, not a feature.

NSS also uses the intermediate CA certificates sent by
server A to build the certificate chain for server B.
We should not need to emulate this behavior on Mac and
Windows.

[Ryan Sleevi, 2011/07/26 01:57:17]

I agree that we shouldn't emulate the server cert behaviour on Mac/Windows
(although, as an aside, we are currently doing it on Windows, due the
|cert_store()|), but I'm not sure I agree with the WontFix for the client cert.
It's not just Firefox that has the client cert behaviour.

We can discuss why on those issues, to see if there is a case to be made, but
for the time being I've restored the original behaviour, which is to set
|ssl_info->cert| = |server_cert_|.

The return of the verified chain in GetSSLInfo() can come in a later CL.

+  if (!server_cert_verify_result_.verified_cert) {
     NOTREACHED();
     return;
   }
 
-  ssl_info->cert = server_cert_;
+  ssl_info->cert = server_cert_verify_result_.verified_cert;
   ssl_info->cert_status = server_cert_verify_result_.cert_status;
   ssl_info->public_key_hashes = server_cert_verify_result_.public_key_hashes;
   ssl_info->is_issued_by_known_root =