SSL fixes for sites with buggy DEFLATE support.

1) Keep a global set of known buggy hosts in memory.

   Previously we would fallback to SSLv3 with each connection. While I
   quite like the idea of making these buggy sites slow, it doesn't
   really help users and it feels silly.

2) Catch SSL decompression alerts while reading the initial data from the
   server and fallback so SSLv3.

   Since we added False Start, servers which believe that they support
   DEFLATE compression but fail at it are not detected as a handshake
   error. Rather they cause an error when reading the first byte of
   application level data.

BUG=41591
TEST=Navigate to https://ws.sso.post.ch/ without an SSL error

http://codereview.chromium.org/1585041

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

4 files changed, 147 insertions, 5 deletions

diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index d6d6a4b..c42549d 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -147,6 +147,12 @@ NET_ERROR(SSL_NO_RENEGOTIATION, -123)
 // due to a broken LSP.
 NET_ERROR(WINSOCK_UNEXPECTED_WRITTEN_BYTES, -124)
 
+// An SSL peer sent us a fatal decompression_failure alert. This typically
+// occurs when a peer selects DEFLATE compression in the mismaken belief that
+// it supports it.
+NET_ERROR(SSL_DECOMPRESSION_FAILURE_ALERT, -125)
+
+
 // Certificate error codes
 //
 // The values of certificate error codes must be consecutive.
@@ -362,6 +368,5 @@ NET_ERROR(NO_PRIVATE_KEY_FOR_CERT, -502)
 // An error adding to the OS certificate database (e.g. OS X Keychain).
 NET_ERROR(ADD_USER_CERT_FAILED, -503)
 
-//
 // The FTP PASV command failed.
 NET_ERROR(FTP_PASV_COMMAND_FAILED, -600)
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 0144316..64c4fd9 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -10,6 +10,7 @@
 #include "base/histogram.h"
 #include "base/scoped_ptr.h"
 #include "base/stats_counters.h"
+#include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/trace_event.h"
 #include "build/build_config.h"
@@ -48,6 +49,10 @@ namespace {
 const std::string* g_next_protos = NULL;
 bool g_use_alternate_protocols = false;
 
+// A set of host:port strings. These are servers which we have needed to back
+// off to SSLv3 for.
+std::set<std::string>* g_tls_intolerant_servers = NULL;
+
 void BuildRequestHeaders(const HttpRequestInfo* request_info,
                          const HttpRequestHeaders& authorization_headers,
                          const UploadDataStream* upload_data_stream,
@@ -245,6 +250,8 @@ HttpNetworkTransaction::HttpNetworkTransaction(HttpNetworkSession* session)
   session->ssl_config_service()->GetSSLConfig(&ssl_config_);
   if (g_next_protos)
     ssl_config_.next_protos = *g_next_protos;
+  if (!g_tls_intolerant_servers)
+    g_tls_intolerant_servers = new std::set<std::string>;
 }
 
 // static
@@ -845,6 +852,12 @@ int HttpNetworkTransaction::DoInitConnectionComplete(int result) {
 int HttpNetworkTransaction::DoSSLConnect() {
   next_state_ = STATE_SSL_CONNECT_COMPLETE;
 
+  if (ContainsKey(*g_tls_intolerant_servers, GetHostAndPort(request_->url))) {
+    LOG(WARNING) << "Falling back to SSLv3 because host is TLS intolerant: "
+                 << GetHostAndPort(request_->url);
+    ssl_config_.tls1_enabled = false;
+  }
+
   if (request_->load_flags & LOAD_VERIFY_EV_CERT)
     ssl_config_.verify_ev_cert = true;
 
@@ -1022,6 +1035,21 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
       result = HandleCertificateRequest(result);
       if (result == OK)
         return result;
+    } else if (result == ERR_SSL_DECOMPRESSION_FAILURE_ALERT &&
+               ssl_config_.tls1_enabled) {
+      // Some buggy servers select DEFLATE compression when offered and then
+      // fail to ever decompress anything. They will send a fatal alert telling
+      // us this. Normally we would pick this up during the handshake because
+      // our Finished message is compressed and we'll never get the server's
+      // Finished if it fails to process ours.
+      //
+      // However, with False Start, we'll believe that the handshake is
+      // complete as soon as we've /sent/ our Finished message. In this case,
+      // we only find out that the server is buggy here, when we try to read
+      // the initial reply.
+      g_tls_intolerant_servers->insert(GetHostAndPort(request_->url));
+      ResetConnectionAndRequestForResend();
+      return OK;
     }
   }
 
@@ -1522,13 +1550,12 @@ int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
   switch (error) {
     case ERR_SSL_PROTOCOL_ERROR:
     case ERR_SSL_VERSION_OR_CIPHER_MISMATCH:
+    case ERR_SSL_DECOMPRESSION_FAILURE_ALERT:
       if (ssl_config_.tls1_enabled) {
         // This could be a TLS-intolerant server or an SSL 3.0 server that
         // chose a TLS-only cipher suite.  Turn off TLS 1.0 and retry.
-        ssl_config_.tls1_enabled = false;
-        connection_->socket()->Disconnect();
-        connection_->Reset();
-        next_state_ = STATE_INIT_CONNECTION;
+        g_tls_intolerant_servers->insert(GetHostAndPort(request_->url));
+        ResetConnectionAndRequestForResend();
         error = OK;
       }
       break;
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 41408bd..28dfa8d 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -4886,4 +4886,112 @@ TEST_F(HttpNetworkTransactionTest, ResolveCanonicalName) {
   }
 }
 
+class TLSDecompressionFailureSocketDataProvider : public SocketDataProvider {
+ public:
+  TLSDecompressionFailureSocketDataProvider(bool fail_all)
+      : fail_all_(fail_all) {
+  }
+
+  virtual MockRead GetNextRead() {
+    if (fail_all_)
+      return MockRead(false /* async */, ERR_SSL_DECOMPRESSION_FAILURE_ALERT);
+
+    return MockRead(false /* async */,
+                    "HTTP/1.1 200 OK\r\nContent-Length: 3\r\n\r\nok.\r\n");
+  }
+
+  virtual MockWriteResult OnWrite(const std::string& data) {
+    return MockWriteResult(false /* async */, data.size());
+  }
+
+  void Reset() {
+  }
+
+ private:
+  const bool fail_all_;
+};
+
+// Test that we restart a connection when we see a decompression failure from
+// the peer during the handshake. (In the real world we'll restart with SSLv3
+// and we won't offer DEFLATE in that case.)
+TEST_F(HttpNetworkTransactionTest, RestartAfterTLSDecompressionFailure) {
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("https://tlsdecompressionfailure.example.com/");
+  request.load_flags = 0;
+
+  SessionDependencies session_deps;
+  TLSDecompressionFailureSocketDataProvider socket_data_provider1(
+      false /* fail all reads */);
+  TLSDecompressionFailureSocketDataProvider socket_data_provider2(false);
+  SSLSocketDataProvider ssl_socket_data_provider1(
+    false, ERR_SSL_DECOMPRESSION_FAILURE_ALERT);
+  SSLSocketDataProvider ssl_socket_data_provider2(false, OK);
+  session_deps.socket_factory.AddSocketDataProvider(&socket_data_provider1);
+  session_deps.socket_factory.AddSocketDataProvider(&socket_data_provider2);
+  session_deps.socket_factory.AddSSLSocketDataProvider(
+      &ssl_socket_data_provider1);
+  session_deps.socket_factory.AddSSLSocketDataProvider(
+      &ssl_socket_data_provider2);
+
+  scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+  scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session));
+  TestCompletionCallback callback;
+
+  int rv = trans->Start(&request, &callback, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  const HttpResponseInfo* response = trans->GetResponseInfo();
+  ASSERT_TRUE(response != NULL);
+  ASSERT_TRUE(response->headers != NULL);
+  EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+
+  std::string response_data;
+  ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
+  EXPECT_EQ("ok.", response_data);
+}
+
+// Test that we restart a connection if we get a decompression failure from the
+// peer while reading the first bytes from the connection. This occurs when the
+// peer cannot handle DEFLATE but we're using False Start, so we don't notice
+// in the handshake.
+TEST_F(HttpNetworkTransactionTest,
+       RestartAfterTLSDecompressionFailureWithFalseStart) {
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("https://tlsdecompressionfailure2.example.com/");
+  request.load_flags = 0;
+
+  SessionDependencies session_deps;
+  TLSDecompressionFailureSocketDataProvider socket_data_provider1(
+      true /* fail all reads */);
+  TLSDecompressionFailureSocketDataProvider socket_data_provider2(false);
+  SSLSocketDataProvider ssl_socket_data_provider1(false, OK);
+  SSLSocketDataProvider ssl_socket_data_provider2(false, OK);
+  session_deps.socket_factory.AddSocketDataProvider(&socket_data_provider1);
+  session_deps.socket_factory.AddSocketDataProvider(&socket_data_provider2);
+  session_deps.socket_factory.AddSSLSocketDataProvider(
+      &ssl_socket_data_provider1);
+  session_deps.socket_factory.AddSSLSocketDataProvider(
+      &ssl_socket_data_provider2);
+
+  scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+  scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session));
+  TestCompletionCallback callback;
+
+  int rv = trans->Start(&request, &callback, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  const HttpResponseInfo* response = trans->GetResponseInfo();
+  ASSERT_TRUE(response != NULL);
+  ASSERT_TRUE(response->headers != NULL);
+  EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+
+  std::string response_data;
+  ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
+  EXPECT_EQ("ok.", response_data);
+}
+
 }  // namespace net
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 35e48e9..7fdc424 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -189,6 +189,8 @@ int MapNSPRError(PRErrorCode err) {
     case SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT:
     case SSL_ERROR_ILLEGAL_PARAMETER_ALERT:
       return ERR_SSL_PROTOCOL_ERROR;
+    case SSL_ERROR_DECOMPRESSION_FAILURE_ALERT:
+      return ERR_SSL_DECOMPRESSION_FAILURE_ALERT;
 
     default: {
       if (IS_SSL_ERROR(err)) {