Handle TLS-intolerant servers by retrying with TLS 1.0

turned off.

R=darin
BUG=3001
Review URL: http://codereview.chromium.org/5617

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@3017 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 93 insertions, 19 deletions

diff --git a/net/base/client_socket_factory.cc b/net/base/client_socket_factory.cc
index fb5c182..43df0e6 100644
--- a/net/base/client_socket_factory.cc
+++ b/net/base/client_socket_factory.cc
@@ -22,9 +22,11 @@ class DefaultClientSocketFactory : public ClientSocketFactory {
 
   virtual ClientSocket* CreateSSLClientSocket(
       ClientSocket* transport_socket,
-      const std::string& hostname) {
+      const std::string& hostname,
+      int protocol_version_mask) {
 #if defined(OS_WIN)
-    return new SSLClientSocket(transport_socket, hostname);
+    return new SSLClientSocket(transport_socket, hostname,
+                               protocol_version_mask);
 #else
     // TODO(pinkerton): turn on when we port SSL socket from win32
     NOTIMPLEMENTED();
diff --git a/net/base/client_socket_factory.h b/net/base/client_socket_factory.h
index 9823ac5..ed371a3 100644
--- a/net/base/client_socket_factory.h
+++ b/net/base/client_socket_factory.h
@@ -21,9 +21,13 @@ class ClientSocketFactory {
   virtual ClientSocket* CreateTCPClientSocket(
       const AddressList& addresses) = 0;
 
+  // protocol_version_mask is a bitmask that specifies which versions of the
+  // SSL protocol (SSL 2.0, SSL 3.0, and TLS 1.0) should be enabled.  The bit
+  // flags are defined in net/base/ssl_client_socket.h.
   virtual ClientSocket* CreateSSLClientSocket(
       ClientSocket* transport_socket,
-      const std::string& hostname) = 0;
+      const std::string& hostname,
+      int protocol_version_mask) = 0;
 
   // Returns the default ClientSocketFactory.
   static ClientSocketFactory* GetDefaultFactory();
diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index e146c7a..ca3d0c0 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -77,6 +77,9 @@ NET_ERROR(SSL_CLIENT_AUTH_CERT_NEEDED, -110)
 // A tunnel connection through the proxy could not be established.
 NET_ERROR(TUNNEL_CONNECTION_FAILED, -111)
 
+// No SSL protocol versions are enabled.
+NET_ERROR(NO_SSL_VERSIONS_ENABLED, -112)
+
 // Certificate error codes
 //
 // The values of certificate error codes must be consecutive.
diff --git a/net/base/ssl_client_socket.cc b/net/base/ssl_client_socket.cc
index 3229c86..3d576af 100644
--- a/net/base/ssl_client_socket.cc
+++ b/net/base/ssl_client_socket.cc
@@ -87,11 +87,13 @@ static int MapNetErrorToCertStatus(int error) {
 static const int kRecvBufferSize = (5 + 16*1024 + 64);
 
 SSLClientSocket::SSLClientSocket(ClientSocket* transport_socket,
-                                 const std::string& hostname)
+                                 const std::string& hostname,
+                                 int protocol_version_mask)
 #pragma warning(suppress: 4355)
     : io_callback_(this, &SSLClientSocket::OnIOComplete),
       transport_(transport_socket),
       hostname_(hostname),
+      protocol_version_mask_(protocol_version_mask),
       user_callback_(NULL),
       user_buf_(NULL),
       user_buf_len_(0),
@@ -327,10 +329,19 @@ int SSLClientSocket::DoConnectComplete(int result) {
   SCHANNEL_CRED schannel_cred = {0};
   schannel_cred.dwVersion = SCHANNEL_CRED_VERSION;
 
-  // TODO(wtc): This should be configurable.  Hardcoded to do SSL 3.0 and
-  // TLS 1.0 for now.  The default (0) means Schannel selects the protocol.
-  // The global system registry settings take precedence over this value.
-  schannel_cred.grbitEnabledProtocols = SP_PROT_SSL3TLS1;
+  // The global system registry settings take precedence over the value of
+  // schannel_cred.grbitEnabledProtocols.
+  schannel_cred.grbitEnabledProtocols = 0;
+  if (protocol_version_mask_ & SSL2)
+    schannel_cred.grbitEnabledProtocols |= SP_PROT_SSL2;
+  if (protocol_version_mask_ & SSL3)
+    schannel_cred.grbitEnabledProtocols |= SP_PROT_SSL3;
+  if (protocol_version_mask_ & TLS1)
+    schannel_cred.grbitEnabledProtocols |= SP_PROT_TLS1;
+  // The default (0) means Schannel selects the protocol, rather than no
+  // protocols are selected.  So we have to fail here.
+  if (schannel_cred.grbitEnabledProtocols == 0)
+    return ERR_NO_SSL_VERSIONS_ENABLED;
 
   // The default session lifetime is 36000000 milliseconds (ten hours).  Set
   // schannel_cred.dwSessionLifespan to change the number of milliseconds that
@@ -818,7 +829,8 @@ int SSLClientSocket::VerifyServerCert() {
   chain_para.RequestedUsage.Usage.cUsageIdentifier = 0;
   chain_para.RequestedUsage.Usage.rgpszUsageIdentifier = NULL;  // LPSTR*
   PCCERT_CHAIN_CONTEXT chain_context;
-  // TODO(wtc): for now, always check revocation.
+  // TODO(wtc): for now, always check revocation.  If we don't want to check
+  // revocation, use the CERT_CHAIN_REVOCATION_CHECK_CACHE_ONLY flag.
   if (!CertGetCertificateChain(
            NULL,  // default chain engine, HCCE_CURRENT_USER
            server_cert_,
diff --git a/net/base/ssl_client_socket.h b/net/base/ssl_client_socket.h
index 5146105..bd72928 100644
--- a/net/base/ssl_client_socket.h
+++ b/net/base/ssl_client_socket.h
@@ -30,10 +30,20 @@ class SSLInfo;
 //
 class SSLClientSocket : public ClientSocket {
  public:
+  enum {
+    SSL2 = 1 << 0,
+    SSL3 = 1 << 1,
+    TLS1 = 1 << 2
+  };
+
   // Takes ownership of the transport_socket, which may already be connected.
   // The given hostname will be compared with the name(s) in the server's
-  // certificate during the SSL handshake.
-  SSLClientSocket(ClientSocket* transport_socket, const std::string& hostname);
+  // certificate during the SSL handshake.  protocol_version_mask is a bitwise
+  // OR of SSL2, SSL3, and TLS1 that specifies which versions of the SSL
+  // protocol should be enabled.
+  SSLClientSocket(ClientSocket* transport_socket,
+                  const std::string& hostname,
+                  int protocol_version_mask);
   ~SSLClientSocket();
 
   // ClientSocket methods:
@@ -72,6 +82,7 @@ class SSLClientSocket : public ClientSocket {
   CompletionCallbackImpl<SSLClientSocket> io_callback_;
   scoped_ptr<ClientSocket> transport_;
   std::string hostname_;
+  int protocol_version_mask_;
 
   CompletionCallback* user_callback_;
 
diff --git a/net/base/ssl_client_socket_unittest.cc b/net/base/ssl_client_socket_unittest.cc
index b29a762..2aba7ab 100644
--- a/net/base/ssl_client_socket_unittest.cc
+++ b/net/base/ssl_client_socket_unittest.cc
@@ -14,6 +14,9 @@
 
 namespace {
 
+const unsigned int kDefaultSSLVersionMask = net::SSLClientSocket::SSL3 |
+                                            net::SSLClientSocket::TLS1;
+
 class SSLClientSocketTest : public testing::Test {
 };
 
@@ -31,7 +34,8 @@ TEST_F(SSLClientSocketTest, DISABLED_Connect) {
   int rv = resolver.Resolve(hostname, 443, &addr, NULL);
   EXPECT_EQ(net::OK, rv);
 
-  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname);
+  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname,
+                            kDefaultSSLVersionMask);
 
   EXPECT_FALSE(sock.IsConnected());
 
@@ -62,7 +66,8 @@ TEST_F(SSLClientSocketTest, DISABLED_Read) {
   rv = callback.WaitForResult();
   EXPECT_EQ(rv, net::OK);
 
-  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname);
+  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname,
+                            kDefaultSSLVersionMask);
 
   rv = sock.Connect(&callback);
   if (rv != net::OK) {
@@ -105,7 +110,8 @@ TEST_F(SSLClientSocketTest, DISABLED_Read_SmallChunks) {
   int rv = resolver.Resolve(hostname, 443, &addr, NULL);
   EXPECT_EQ(rv, net::OK);
 
-  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname);
+  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname,
+                            kDefaultSSLVersionMask);
 
   rv = sock.Connect(&callback);
   if (rv != net::OK) {
@@ -148,7 +154,8 @@ TEST_F(SSLClientSocketTest, DISABLED_Read_Interrupted) {
   int rv = resolver.Resolve(hostname, 443, &addr, NULL);
   EXPECT_EQ(rv, net::OK);
 
-  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname);
+  net::SSLClientSocket sock(new net::TCPClientSocket(addr), hostname,
+                            kDefaultSSLVersionMask);
 
   rv = sock.Connect(&callback);
   if (rv != net::OK) {
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index c391dae..de3539317 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -58,6 +58,12 @@ HttpNetworkTransaction::HttpNetworkTransaction(HttpNetworkSession* session,
       read_buf_(NULL),
       read_buf_len_(0),
       next_state_(STATE_NONE) {
+#if defined(OS_WIN)
+  // TODO(wtc): Use SSL settings (bug 3003).
+  ssl_version_mask_ = SSLClientSocket::SSL3 | SSLClientSocket::TLS1;
+#else
+  ssl_version_mask_ = 0;  // A dummy value so that the code compiles.
+#endif
 }
 
 void HttpNetworkTransaction::Destroy() {
@@ -483,7 +489,8 @@ int HttpNetworkTransaction::DoConnect() {
   // If we are using a direct SSL connection, then go ahead and create the SSL
   // wrapper socket now.  Otherwise, we need to first issue a CONNECT request.
   if (using_ssl_ && !using_tunnel_)
-    s = socket_factory_->CreateSSLClientSocket(s, request_->url.host());
+    s = socket_factory_->CreateSSLClientSocket(s, request_->url.host(),
+                                               ssl_version_mask_);
 
   connection_.set_socket(s);
   return connection_.socket()->Connect(&io_callback_);
@@ -497,6 +504,8 @@ int HttpNetworkTransaction::DoConnectComplete(int result) {
     next_state_ = STATE_WRITE_HEADERS;
     if (using_tunnel_)
       establishing_tunnel_ = true;
+  } else if (result == ERR_SSL_PROTOCOL_ERROR) {
+    result = HandleSSLHandshakeError(result);
   } else {
     result = ReconsiderProxyAfterError(result);
   }
@@ -508,7 +517,8 @@ int HttpNetworkTransaction::DoSSLConnectOverTunnel() {
 
   // Add a SSL socket on top of our existing transport socket.
   ClientSocket* s = connection_.release_socket();
-  s = socket_factory_->CreateSSLClientSocket(s, request_->url.host());
+  s = socket_factory_->CreateSSLClientSocket(s, request_->url.host(),
+                                             ssl_version_mask_);
   connection_.set_socket(s);
   return connection_.socket()->Connect(&io_callback_);
 }
@@ -517,8 +527,11 @@ int HttpNetworkTransaction::DoSSLConnectOverTunnelComplete(int result) {
   if (IsCertificateError(result))
     result = HandleCertificateError(result);
 
-  if (result == OK)
+  if (result == OK) {
     next_state_ = STATE_WRITE_HEADERS;
+  } else if (result == ERR_SSL_PROTOCOL_ERROR) {
+    result = HandleSSLHandshakeError(result);
+  }
   return result;
 }
 
@@ -874,6 +887,20 @@ int HttpNetworkTransaction::HandleCertificateError(int error) {
   return error;
 }
 
+int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
+#if defined(OS_WIN)
+  if (ssl_version_mask_ & SSLClientSocket::TLS1) {
+    // This could be a TLS-intolerant server.  Turn off TLS 1.0 and retry.
+    ssl_version_mask_ &= ~SSLClientSocket::TLS1;
+    connection_.set_socket(NULL);
+    connection_.Reset();
+    next_state_ = STATE_INIT_CONNECTION;
+    error = OK;
+  }
+#endif
+  return error;
+}
+
 // 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
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index 2414050..bbbfb74 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -84,6 +84,11 @@ class HttpNetworkTransaction : public HttpTransaction {
   // returns the same error code.
   int HandleCertificateError(int error);
 
+  // Called to possibly recover from an SSL handshake error.  Sets next_state_
+  // and returns OK if recovering from the error.  Otherwise, the same error
+  // code is returned.
+  int HandleSSLHandshakeError(int error);
+
   // Called to possibly recover from the given error.  Sets next_state_ and
   // returns OK if recovering from the error.  Otherwise, the same error code
   // is returned.
@@ -181,6 +186,8 @@ class HttpNetworkTransaction : public HttpTransaction {
   // the real request/response of the transaction.
   bool establishing_tunnel_;
 
+  int ssl_version_mask_;
+
   std::string request_headers_;
   size_t request_headers_bytes_sent_;
   scoped_ptr<UploadDataStream> request_body_stream_;
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 8cfe348..b0d1964 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -158,7 +158,8 @@ class MockClientSocketFactory : public net::ClientSocketFactory {
   }
   virtual net::ClientSocket* CreateSSLClientSocket(
       net::ClientSocket* transport_socket,
-      const std::string& hostname) {
+      const std::string& hostname,
+      int protocol_version_mask) {
     return NULL;
   }
 };