Remember if a user declines to provide a server with a client certificate

net/http/http_network_transaction.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_network_transaction.h"
 
 #include <set>
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ skipping 78 matching lines @@
       request_(NULL),
       headers_valid_(false),
       logged_response_time_(false),
       request_headers_(),
       read_buf_len_(0),
       next_state_(STATE_NONE),
       establishing_tunnel_(false) {
   session->ssl_config_service()->GetSSLConfig(&ssl_config_);
   if (session->http_stream_factory()->next_protos())
     ssl_config_.next_protos = *session->http_stream_factory()->next_protos();
-
 }
 
 HttpNetworkTransaction::~HttpNetworkTransaction() {
   if (stream_.get()) {
     HttpResponseHeaders* headers = GetResponseHeaders();
     // TODO(mbelshe): The stream_ should be able to compute whether or not the
     //                stream should be kept alive.  No reason to compute here
     //                and pass it in.
     bool try_to_keep_alive =
         next_state_ == STATE_NONE &&
@@ skipping 54 matching lines @@
 int HttpNetworkTransaction::RestartWithCertificate(
     X509Certificate* client_cert,
     CompletionCallback* callback) {
   // In HandleCertificateRequest(), we always tear down existing stream
   // requests to force a new connection.  So we shouldn't have one here.
   DCHECK(!stream_request_.get());
   DCHECK(!stream_.get());
   DCHECK_EQ(STATE_NONE, next_state_);
 
   ssl_config_.client_cert = client_cert;
-  if (client_cert) {
-    session_->ssl_client_auth_cache()->Add(GetHostAndPort(request_->url),
-                                           client_cert);
-  }
+  session_->ssl_client_auth_cache()->Add(GetHostAndPort(request_->url),
+                                         client_cert);
   ssl_config_.send_client_cert = true;
   // Reset the other member variables.
   // Note: this is necessary only with SSL renegotiation.
   ResetStateForRestart();
   next_state_ = STATE_CREATE_STREAM;
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     user_callback_ = callback;
   return rv;
 }
@@ skipping 782 matching lines @@
     // renegotiation.
     DCHECK(!stream_request_.get());
     stream_->Close(true);
     stream_.reset();
   }
 
   // The server is asking for a client certificate during the initial
   // handshake.
   stream_request_.reset();
 
   // If the user selected one of the certificate in client_certs for this

[agl, 2010/11/09 17:04:06]

I think this comment has some grammar issues. (Not your new stuff, the original
comment.)

[wtc, 2010/11/18 01:33:07]

Please fix my grammatical errors.  I guess we should say
"one of the certificates" (plural).  How about:
  If the user selected one of the certificates in client_certs
  or declined to provide one for this server before, use the
  past decision automatically.

-  // server before, use it automatically.
-  X509Certificate* client_cert = session_->ssl_client_auth_cache()->
-                                 Lookup(GetHostAndPort(request_->url));
+  // server before, or previously declined to provide one, use it
+  // automatically.
+  X509Certificate* client_cert = NULL;
+  bool found_cached_cert = session_->ssl_client_auth_cache()->Lookup(
+      GetHostAndPort(request_->url), &client_cert);
+  if (!found_cached_cert)
+    return error;
+
+  // If the user previously selected a specific certificate, as opposed to

[wtc, 2010/11/18 01:33:07]

Nit: remove
  If the user previously selected a specific certificate, as opposed to
  declining to provide one,

+  // declining to provide one, check that the certificate selected is still a
+  // certificate the server is likely to accept, based on the criteria it
+  // supplied in the CertificateRequest message.
   if (client_cert) {
     const std::vector<scoped_refptr<X509Certificate> >& client_certs =
         response_.cert_request_info->client_certs;
+    bool cert_still_valid = false;
     for (size_t i = 0; i < client_certs.size(); ++i) {
-      if (client_cert->fingerprint().Equals(client_certs[i]->fingerprint())) {
-        // TODO(davidben): Add a unit test which covers this path; we need to be
-        // able to send a legitimate certificate and also bypass/clear the
-        // SSL session cache.
-        ssl_config_.client_cert = client_cert;
-        ssl_config_.send_client_cert = true;
-        next_state_ = STATE_CREATE_STREAM;
-        // Reset the other member variables.
-        // Note: this is necessary only with SSL renegotiation.
-        ResetStateForRestart();
-        return OK;
+      if (X509Certificate::IsSameOSCert(client_cert->os_cert_handle(),

[wtc, 2010/11/18 01:33:07]

Use the new X509Certificate::Equals method:
  if (client_cert->Equals(client_certs[i])) {

+                                        client_certs[i]->os_cert_handle())) {
+        cert_still_valid = true;
+        break;
       }
     }
+
+    if (!cert_still_valid)
+      return error;
   }
-  return error;
+
+  // TODO(davidben): Add a unit test which covers this path; we need to be
+  // able to send a legitimate certificate and also bypass/clear the
+  // SSL session cache.
+  ssl_config_.client_cert = client_cert;
+  ssl_config_.send_client_cert = true;
+  next_state_ = STATE_CREATE_STREAM;
+  // Reset the other member variables.
+  // Note: this is necessary only with SSL renegotiation.
+  ResetStateForRestart();
+  return OK;
 }
 
 // This method determines whether it is safe to resend the request after an
 // IO error.  It can only be called in response to request header or body
 // write errors or response header read errors.  It should not be used in
 // other cases, such as a Connect error.
 int HttpNetworkTransaction::HandleIOError(int error) {
   switch (error) {
     // If we try to reuse a connection that the server is in the process of
     // closing, we may end up successfully writing out our request (or a
@@ skipping 158 matching lines @@
     default:
       return priority;
   }
 }
 
 
 
 #undef STATE_CASE
 
 }  // namespace net