Correctly handle SSL Client Authentication requests when connecting...

net/socket/ssl_client_socket_pool.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/socket/ssl_client_socket_pool.h"
 
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "net/base/net_errors.h"
 #include "net/base/ssl_cert_request_info.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/socks_client_socket_pool.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/ssl_host_info.h"
 #include "net/socket/tcp_client_socket_pool.h"
 
 namespace net {
 
 SSLSocketParams::SSLSocketParams(
     const scoped_refptr<TCPSocketParams>& tcp_params,
     const scoped_refptr<SOCKSSocketParams>& socks_params,
     const scoped_refptr<HttpProxySocketParams>& http_proxy_params,
     ProxyServer::Scheme proxy,
     const std::string& hostname,
+    uint16 port,
     const SSLConfig& ssl_config,
     int load_flags,
     bool force_spdy_over_ssl,
     bool want_spdy_over_npn)
     : tcp_params_(tcp_params),
       http_proxy_params_(http_proxy_params),
       socks_params_(socks_params),
       proxy_(proxy),
       hostname_(hostname),
+      port_(port),
       ssl_config_(ssl_config),
       load_flags_(load_flags),
       force_spdy_over_ssl_(force_spdy_over_ssl),
       want_spdy_over_npn_(want_spdy_over_npn) {
   switch (proxy_) {
     case ProxyServer::SCHEME_DIRECT:
       DCHECK(tcp_params_.get() != NULL);
       DCHECK(http_proxy_params_.get() == NULL);
       DCHECK(socks_params_.get() == NULL);
       break;
@@ skipping 194 matching lines @@
   transport_socket_handle_.reset(new ClientSocketHandle());
   scoped_refptr<HttpProxySocketParams> http_proxy_params =
       params_->http_proxy_params();
   return transport_socket_handle_->Init(
       group_name(), http_proxy_params,
       http_proxy_params->destination().priority(), &callback_,
       http_proxy_pool_, net_log());
 }
 
 int SSLConnectJob::DoTunnelConnectComplete(int result) {
-  ClientSocket* socket = transport_socket_handle_->socket();
-  HttpProxyClientSocket* tunnel_socket =
-      static_cast<HttpProxyClientSocket*>(socket);
-
+  // Extract the information needed to prompt for the proxy client auth.
+  // so that when ClientSocketPoolBaseHelper calls |GetAdditionalErrorState|,
+  // we can easily set the state.
+  if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED)

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

Add curly braces.  This is required by the Style Guide, if
any branch in an if statement has curly braces.

Please rewrite the first comment block so that it applies to
both branches and delete the second comment block.

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

Done.

+    error_response_info_ = transport_socket_handle_->ssl_error_response_info();
   // Extract the information needed to prompt for the proxy authentication.
   // so that when ClientSocketPoolBaseHelper calls |GetAdditionalErrorState|,
   // we can easily set the state.
-  if (result == ERR_PROXY_AUTH_REQUESTED)
+  else if (result == ERR_PROXY_AUTH_REQUESTED) {
+    ClientSocket* socket = transport_socket_handle_->socket();
+    HttpProxyClientSocket* tunnel_socket =
+        static_cast<HttpProxyClientSocket*>(socket);
     error_response_info_ = *tunnel_socket->GetResponseInfo();
-
+  }
   if (result < 0)
     return result;
 
   next_state_ = STATE_SSL_CONNECT;
   return result;
 }
 
 void SSLConnectJob::GetAdditionalErrorState(ClientSocketHandle * handle) {
   // Headers in |error_response_info_| indicate a proxy tunnel setup
   // problem. See DoTunnelConnectComplete.
   if (error_response_info_.headers) {
     handle->set_pending_http_proxy_connection(
         transport_socket_handle_.release());
   }
   handle->set_ssl_error_response_info(error_response_info_);
   if (!ssl_connect_start_time_.is_null())
     handle->set_is_ssl_error(true);
 }
 
 int SSLConnectJob::DoSSLConnect() {
   next_state_ = STATE_SSL_CONNECT_COMPLETE;
   // Reset the timeout to just the time allowed for the SSL handshake.
   ResetTimer(base::TimeDelta::FromSeconds(kSSLHandshakeTimeoutInSeconds));
   ssl_connect_start_time_ = base::TimeTicks::Now();
 
   ssl_socket_.reset(client_socket_factory_->CreateSSLClientSocket(
         transport_socket_handle_.release(), params_->hostname(),
+        params_->port(),

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

Nit: merge these two lines.

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

Done.

         params_->ssl_config(), ssl_host_info_.release(),
         dnsrr_resolver_));
   return ssl_socket_->Connect(&callback_);
 }
 
 int SSLConnectJob::DoSSLConnectComplete(int result) {
   SSLClientSocket::NextProtoStatus status =
       SSLClientSocket::kNextProtoUnsupported;
   std::string proto;
   // GetNextProto will fail and and trigger a NOTREACHED if we pass in a socket
@@ skipping 15 matching lines @@
       ssl_socket_->set_was_spdy_negotiated(true);
     }
   }
   if (params_->want_spdy_over_npn() && !ssl_socket_->was_spdy_negotiated())
     return ERR_NPN_NEGOTIATION_FAILED;
 
   // Spdy might be turned on by default, or it might be over npn.
   bool using_spdy = params_->force_spdy_over_ssl() ||
       params_->want_spdy_over_npn();
 
+  if (ProxyServer::SCHEME_HTTPS)
+    LOG(INFO) << "Connected to host through proxy, over spdy: " << (using_spdy ? "yes" : "no");

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

Is this line longer than 80 characters?

Replace LOG(INFO) by VLOG(1).

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

Sorry, removed the line.

+
   if (result == OK ||
       ssl_socket_->IgnoreCertError(result, params_->load_flags())) {
     DCHECK(ssl_connect_start_time_ != base::TimeTicks());
     base::TimeDelta connect_duration =
         base::TimeTicks::Now() - ssl_connect_start_time_;
     if (using_spdy) {
       UMA_HISTOGRAM_CUSTOM_TIMES("Net.SpdyConnectionLatency",
                                  connect_duration,
                                  base::TimeDelta::FromMilliseconds(1),
                                  base::TimeDelta::FromMinutes(10),
@@ skipping 174 matching lines @@
       list->Append(http_proxy_pool_->GetInfoAsValue("http_proxy_pool",
                                                     "http_proxy_pool",
                                                     true));
     }
     dict->Set("nested_pools", list);
   }
   return dict;
 }
 
 }  // namespace net