Correctly handle SSL Client Authentication requests when connecting...

net/http/http_proxy_client_socket_pool.cc

Index: net/http/http_proxy_client_socket_pool.cc
===================================================================
--- net/http/http_proxy_client_socket_pool.cc	(revision 65205)
+++ net/http/http_proxy_client_socket_pool.cc	(working copy)
@@ -82,7 +82,8 @@
       resolver_(host_resolver),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           callback_(this, &HttpProxyConnectJob::OnIOComplete)),
-      using_spdy_(false) {
+      using_spdy_(false),
+      error_response_info_() {

[wtc, 2010/11/11 01:11:35]

Remove this.  We usually omit this kind of member initializer
that takes no argument.

[Ryan Hamilton, 2010/11/11 18:57:00]

Done.

 }
 
 HttpProxyConnectJob::~HttpProxyConnectJob() {}
@@ -207,9 +208,11 @@
 }
 
 int HttpProxyConnectJob::DoSSLConnectComplete(int result) {
-  // TODO(rch): enable support for client auth to the proxy
-  if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED)
-    return ERR_PROXY_AUTH_UNSUPPORTED;
+  if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
+    error_response_info_ = transport_socket_handle_->ssl_error_response_info();
+    DCHECK(error_response_info_.cert_request_info.get());
+    return ERR_SSL_CLIENT_AUTH_CERT_NEEDED;

[wtc, 2010/11/11 01:11:35]

You can just return 'result' now.

[Ryan Hamilton, 2010/11/11 18:57:00]

Done.

+  }
   if (IsCertificateError(result)) {
     if (params_->ssl_params()->load_flags() & LOAD_IGNORE_ALL_CERT_ERRORS)
       result = OK;
@@ -239,6 +242,7 @@
   // need to add a predicate to this if statement so we fall through
   // to the else case. (HttpProxyClientSocket currently acts as
   // a "trusted" SPDY proxy).
+  LOG(INFO) << "Connected to HTTPS proxy, using spdy: " << (using_spdy_ ? "yes" : "no");

[wtc, 2010/11/11 01:11:35]

Use VLOG(1) instead of LOG(INFO).

This line seems longer than 80 characters.

[Ryan Hamilton, 2010/11/11 18:57:00]

Removed this line.

   if (using_spdy_ && params_->tunnel())
     next_state_ = STATE_SPDY_PROXY_CREATE_STREAM;
   else
@@ -246,6 +250,13 @@
   return result;
 }
 
+void HttpProxyConnectJob::GetAdditionalErrorState(ClientSocketHandle * handle) {
+  if (error_response_info_.cert_request_info) {
+    handle->set_ssl_error_response_info(error_response_info_);
+    handle->set_is_ssl_error(true);
+  }
+}
+
 int HttpProxyConnectJob::DoSpdyProxyCreateStream() {
   DCHECK(using_spdy_);
   DCHECK(params_->tunnel());
@@ -256,9 +267,11 @@
   scoped_refptr<SpdySession> spdy_session;
   // It's possible that a session to the proxy has recently been created
   if (spdy_pool->HasSession(pair)) {
-    if (transport_socket_handle_->socket())
-      transport_socket_handle_->socket()->Disconnect();
-    transport_socket_handle_->Reset();
+    if (transport_socket_handle_.get()) {

[wtc, 2010/11/11 01:11:35]

Can you explain why you need the null pointer check for
transport_socket_handle_ here?

[Ryan Hamilton, 2010/11/11 18:57:00]

Yes.  If HttpProxyClientSocket::Connect() is called when a SPDY session to the
proxy already exists, then DoSSLConnect will jump to this method without
initializing the transport_socket_handle.

+      if (transport_socket_handle_->socket())
+        transport_socket_handle_->socket()->Disconnect();
+      transport_socket_handle_->Reset();
+    }
     spdy_session = spdy_pool->Get(pair, params_->spdy_settings(), net_log());
   } else {
     // Create a session direct to the proxy itself