SocketStream should have independent state to wait for cert validation.

In current implementation, SocketStream remains the same state of FSM
when  SSLManager denies invalid cert. As a result, SocketSteram requests
cert verification to a UI thread infinitely.

BUG=129748, 130067
TEST=browser_test

Review URL: https://chromiumcodereview.appspot.com/10451063

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

5 files changed, 137 insertions, 94 deletions

diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index 0298cb4..a1536ea 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -522,7 +522,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoForward) {
 }
 
 // Visit a HTTP page which request WSS connection to a server providing invalid
-// certificate. Close the page while WSS connection waits for UI.
+// certificate. Close the page while WSS connection waits for SSLManager's
+// response from UI thread.
 IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndClose) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(https_server_expired_.Start());
@@ -542,7 +543,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndClose) {
       https_server_expired_.host_port_pair().port());
   GURL slaveUrl(slaveUrlPath);
 
-  // Create tabs and visit pages which create hanging WebSocket connections.
+  // Create tabs and visit pages which keep on creating wss connections.
   TabContentsWrapper* tabs[16];
   for (int i = 0; i < 16; ++i) {
     tabs[i] = browser()->AddSelectedTabWithURL(
@@ -550,8 +551,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndClose) {
   }
   browser()->SelectNextTab();
 
-  // Visit a page which causes a hanging WebSocket connection. After waiting
-  // for two round trip time, the title will be changed to 'PASS'.
+  // Visit a page which waits for one TLS handshake failure.
+  // The title will be changed to 'PASS'.
   ui_test_utils::NavigateToURL(browser(), masterUrl);
   const string16 result = watcher.WaitAndGetTitle();
   EXPECT_TRUE(LowerCaseEqualsASCII(result, "pass"));
diff --git a/chrome/test/data/ssl/wss_close.html b/chrome/test/data/ssl/wss_close.html
index 69c8940..0df5431 100644
--- a/chrome/test/data/ssl/wss_close.html
+++ b/chrome/test/data/ssl/wss_close.html
@@ -9,37 +9,18 @@ var queryBegin = href.lastIndexOf('?') + 1;
 var port = href.slice(queryBegin);
 var url = 'wss://localhost:' + port;
 
-function fail()
-{
-  // Set document title to 'FAIL'. The test observer catches this title changes
+// Do connection test.
+var ws = new WebSocket(url);
+ws.onopen = function() {
+  // Set document title to 'FAIL'. The test observer catches this title change
   // to know the result.
   document.title = 'FAIL';
 }
-// Do connection test.
-var ws = new WebSocket(url);
-
-ws.onopen = fail;
-ws.onclose = fail;
-ws.onerror = fail;
-
-// Use XHR to wait about two round trip time.
-var xhr = new XMLHttpRequest();
-xhr.onreadystatechange = function(){
-  if (this.readyState != this.DONE)
-    return;
-  var xhr2 = new XMLHttpRequest();
-  xhr2.onreadystatechange = function(){
-    if (this.readyState != this.DONE)
-      return;
-    document.title = 'PASS';
-  };
-  xhr2.open('GET', href);
-  xhr2.send();
-};
-xhr.open('GET', href);
-xhr.send();
-
-setTimeout(fail, 3000);
+ws.onclose = function() {
+  // Set document title to 'PASS'. The test observer catches this title change
+  // to know the result.
+  document.title = 'PASS';
+}
 
 </script>
 </head>
diff --git a/chrome/test/data/ssl/wss_close_slave.html b/chrome/test/data/ssl/wss_close_slave.html
index 70258a9..4ead9b5 100644
--- a/chrome/test/data/ssl/wss_close_slave.html
+++ b/chrome/test/data/ssl/wss_close_slave.html
@@ -7,7 +7,12 @@ var href = window.location.href;
 var queryBegin = href.lastIndexOf('?') + 1;
 var port = href.slice(queryBegin);
 var url = 'wss://localhost:' + port;
-var ws = new WebSocket(url);
+var ws;
+var cb = function() {
+  ws = new WebSocket(url);
+  ws.onclose = cb;
+}
+cb();
 </script>
 </head>
 </html>
diff --git a/net/socket_stream/socket_stream.cc b/net/socket_stream/socket_stream.cc
index 3a3c69a..46cb1e5 100644
--- a/net/socket_stream/socket_stream.cc
+++ b/net/socket_stream/socket_stream.cc
@@ -321,51 +321,6 @@ void SocketStream::Finish(int result) {
   Release();
 }
 
-int SocketStream::DidEstablishSSL(int result, SSLConfig* ssl_config) {
-  DCHECK(ssl_config);
-  if (IsCertificateError(result)) {
-    if (socket_->IsConnectedAndIdle()) {
-      result = HandleCertificateError(result);
-    } else {
-      // SSLClientSocket for Mac will report socket is not connected,
-      // if it returns cert verification error.  It didn't perform
-      // SSLHandshake yet.
-      // So, we should restart establishing connection with the
-      // certificate in allowed bad certificates in |ssl_config|.
-      // See also net/http/http_network_transaction.cc
-      //  HandleCertificateError() and RestartIgnoringLastError().
-      SSLClientSocket* ssl_socket =
-          static_cast<SSLClientSocket*>(socket_.get());
-      SSLInfo ssl_info;
-      ssl_socket->GetSSLInfo(&ssl_info);
-      if (ssl_info.cert == NULL ||
-          ssl_config->IsAllowedBadCert(ssl_info.cert, NULL)) {
-        // If we already have the certificate in the set of allowed bad
-        // certificates, we did try it and failed again, so we should not
-        // retry again: the connection should fail at last.
-        next_state_ = STATE_CLOSE;
-        return result;
-      }
-      // Add the bad certificate to the set of allowed certificates in the
-      // SSL config object.
-      SSLConfig::CertAndStatus bad_cert;
-      if (!X509Certificate::GetDEREncoded(ssl_info.cert->os_cert_handle(),
-                                          &bad_cert.der_cert)) {
-        next_state_ = STATE_CLOSE;
-        return result;
-      }
-      bad_cert.cert_status = ssl_info.cert_status;
-      ssl_config->allowed_bad_certs.push_back(bad_cert);
-      // Restart connection ignoring the bad certificate.
-      socket_->Disconnect();
-      socket_.reset();
-      next_state_ = STATE_TCP_CONNECT;
-      return OK;
-    }
-  }
-  return result;
-}
-
 int SocketStream::DidEstablishConnection() {
   if (!socket_.get() || !socket_->IsConnected()) {
     next_state_ = STATE_CLOSE;
@@ -509,6 +464,12 @@ void SocketStream::DoLoop(int result) {
       case STATE_SECURE_PROXY_CONNECT_COMPLETE:
         result = DoSecureProxyConnectComplete(result);
         break;
+      case STATE_SECURE_PROXY_HANDLE_CERT_ERROR:
+        result = DoSecureProxyHandleCertError(result);
+        break;
+      case STATE_SECURE_PROXY_HANDLE_CERT_ERROR_COMPLETE:
+        result = DoSecureProxyHandleCertErrorComplete(result);
+        break;
       case STATE_SSL_CONNECT:
         DCHECK_EQ(OK, result);
         result = DoSSLConnect();
@@ -516,6 +477,12 @@ void SocketStream::DoLoop(int result) {
       case STATE_SSL_CONNECT_COMPLETE:
         result = DoSSLConnectComplete(result);
         break;
+      case STATE_SSL_HANDLE_CERT_ERROR:
+        result = DoSSLHandleCertError(result);
+        break;
+      case STATE_SSL_HANDLE_CERT_ERROR_COMPLETE:
+        result = DoSSLHandleCertErrorComplete(result);
+        break;
       case STATE_READ_WRITE:
         result = DoReadWrite(result);
         break;
@@ -938,17 +905,39 @@ int SocketStream::DoSecureProxyConnect() {
 
 int SocketStream::DoSecureProxyConnectComplete(int result) {
   DCHECK_EQ(STATE_NONE, next_state_);
-  result = DidEstablishSSL(result, &proxy_ssl_config_);
+  if (IsCertificateError(result))
+    next_state_ = STATE_SECURE_PROXY_HANDLE_CERT_ERROR;
+  else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED || result == OK)
+    next_state_ = STATE_SECURE_PROXY_HANDLE_CERT_ERROR_COMPLETE;
+  else
+    next_state_ = STATE_CLOSE;
+  return result;
+}
+
+int SocketStream::DoSecureProxyHandleCertError(int result) {
+  DCHECK_EQ(STATE_NONE, next_state_);
+  DCHECK(IsCertificateError(result));
+  result = HandleCertificateError(result);
   if (result == ERR_IO_PENDING)
-    next_state_ = STATE_SECURE_PROXY_CONNECT_COMPLETE;
-  if (next_state_ != STATE_NONE)
-    return result;
+    next_state_ = STATE_SECURE_PROXY_HANDLE_CERT_ERROR_COMPLETE;
+  else
+    next_state_ = STATE_CLOSE;
+  return result;
+}
+
+int SocketStream::DoSecureProxyHandleCertErrorComplete(int result) {
+  DCHECK_EQ(STATE_NONE, next_state_);
+  // Reconnect with client authentication.
   if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED)
     return HandleCertificateRequest(result);
-  if (result == OK)
+
+  if (result == OK) {
+    if (!socket_->IsConnectedAndIdle())
+      return AllowCertErrorForReconnection(&proxy_ssl_config_);
     next_state_ = STATE_WRITE_TUNNEL_HEADERS;
-  else
+  } else {
     next_state_ = STATE_CLOSE;
+  }
   return result;
 }
 
@@ -970,19 +959,42 @@ int SocketStream::DoSSLConnect() {
 
 int SocketStream::DoSSLConnectComplete(int result) {
   DCHECK_EQ(STATE_NONE, next_state_);
-  result = DidEstablishSSL(result, &server_ssl_config_);
+  if (IsCertificateError(result))
+    next_state_ = STATE_SSL_HANDLE_CERT_ERROR;
+  else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED || result == OK)
+    next_state_ = STATE_SSL_HANDLE_CERT_ERROR_COMPLETE;
+  else
+    next_state_ = STATE_CLOSE;
+  return result;
+}
+
+int SocketStream::DoSSLHandleCertError(int result) {
+  DCHECK_EQ(STATE_NONE, next_state_);
+  DCHECK(IsCertificateError(result));
+  result = HandleCertificateError(result);
   if (result == ERR_IO_PENDING)
-    next_state_ = STATE_SSL_CONNECT_COMPLETE;
-  if (next_state_ != STATE_NONE)
-    return result;
+    next_state_ = STATE_SSL_HANDLE_CERT_ERROR_COMPLETE;
+  else
+    next_state_ = STATE_CLOSE;
+  return result;
+}
+
+int SocketStream::DoSSLHandleCertErrorComplete(int result) {
+  DCHECK_EQ(STATE_NONE, next_state_);
   // TODO(toyoshim): Upgrade to SPDY through TLS NPN extension if possible.
   // If we use HTTPS and this is the first connection to the SPDY server,
   // we should take care of TLS NPN extension here.
 
-  if (result == OK)
+  // Reconnect with client authentication.
+  if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED)
+    return HandleCertificateRequest(result);
+  if (result == OK) {
+    if (!socket_->IsConnectedAndIdle())
+      return AllowCertErrorForReconnection(&server_ssl_config_);
     result = DidEstablishConnection();
-  else
+  } else {
     next_state_ = STATE_CLOSE;
+  }
   return result;
 }
 
@@ -1114,9 +1126,6 @@ int SocketStream::HandleAuthChallenge(const HttpResponseHeaders* headers) {
 }
 
 int SocketStream::HandleCertificateRequest(int result) {
-  // TODO(toyoshim): We must support SSL client authentication for not only
-  // secure proxy but also secure server.
-
   if (proxy_ssl_config_.send_client_cert)
     // We already have performed SSL client authentication once and failed.
     return result;
@@ -1154,12 +1163,51 @@ int SocketStream::HandleCertificateRequest(int result) {
   if (!cert_still_valid)
     return result;
 
+  // TODO(toyoshim): To support SSL client authentication for not only secure
+  // proxy but also secure server, we must modify this function to take a
+  // SSLConfig* argument.
+  // http://crbug.com/63158 .
   proxy_ssl_config_.send_client_cert = true;
   proxy_ssl_config_.client_cert = client_cert;
   next_state_ = STATE_TCP_CONNECT;
   return OK;
 }
 
+int SocketStream::AllowCertErrorForReconnection(SSLConfig* ssl_config) {
+  DCHECK(ssl_config);
+  // The SSL handshake didn't finish, or the server closed the SSL connection.
+  // So, we should restart establishing connection with the certificate in
+  // allowed bad certificates in |ssl_config|.
+  // See also net/http/http_network_transaction.cc HandleCertificateError() and
+  // RestartIgnoringLastError().
+  SSLClientSocket* ssl_socket = static_cast<SSLClientSocket*>(socket_.get());
+  SSLInfo ssl_info;
+  ssl_socket->GetSSLInfo(&ssl_info);
+  if (ssl_info.cert == NULL ||
+      ssl_config->IsAllowedBadCert(ssl_info.cert, NULL)) {
+    // If we already have the certificate in the set of allowed bad
+    // certificates, we did try it and failed again, so we should not
+    // retry again: the connection should fail at last.
+    next_state_ = STATE_CLOSE;
+    return OK;
+  }
+  // Add the bad certificate to the set of allowed certificates in the
+  // SSL config object.
+  SSLConfig::CertAndStatus bad_cert;
+  if (!X509Certificate::GetDEREncoded(ssl_info.cert->os_cert_handle(),
+                                      &bad_cert.der_cert)) {
+    next_state_ = STATE_CLOSE;
+    return OK;
+  }
+  bad_cert.cert_status = ssl_info.cert_status;
+  ssl_config->allowed_bad_certs.push_back(bad_cert);
+  // Restart connection ignoring the bad certificate.
+  socket_->Disconnect();
+  socket_.reset();
+  next_state_ = STATE_TCP_CONNECT;
+  return OK;
+}
+
 void SocketStream::DoAuthRequired() {
   if (delegate_ && auth_info_.get())
     delegate_->OnAuthRequired(this, auth_info_.get());
diff --git a/net/socket_stream/socket_stream.h b/net/socket_stream/socket_stream.h
index f756969..35d2312 100644
--- a/net/socket_stream/socket_stream.h
+++ b/net/socket_stream/socket_stream.h
@@ -247,8 +247,12 @@ class NET_EXPORT SocketStream
     STATE_SOCKS_CONNECT_COMPLETE,
     STATE_SECURE_PROXY_CONNECT,
     STATE_SECURE_PROXY_CONNECT_COMPLETE,
+    STATE_SECURE_PROXY_HANDLE_CERT_ERROR,
+    STATE_SECURE_PROXY_HANDLE_CERT_ERROR_COMPLETE,
     STATE_SSL_CONNECT,
     STATE_SSL_CONNECT_COMPLETE,
+    STATE_SSL_HANDLE_CERT_ERROR,
+    STATE_SSL_HANDLE_CERT_ERROR_COMPLETE,
     STATE_READ_WRITE,
     STATE_AUTH_REQUIRED,
     STATE_CLOSE,
@@ -273,7 +277,6 @@ class NET_EXPORT SocketStream
   // notifications will be sent to delegate.
   void Finish(int result);
 
-  int DidEstablishSSL(int result, SSLConfig* ssl_config);
   int DidEstablishConnection();
   int DidReceiveData(int result);
   int DidSendData(int result);
@@ -300,8 +303,12 @@ class NET_EXPORT SocketStream
   int DoSOCKSConnectComplete(int result);
   int DoSecureProxyConnect();
   int DoSecureProxyConnectComplete(int result);
+  int DoSecureProxyHandleCertError(int result);
+  int DoSecureProxyHandleCertErrorComplete(int result);
   int DoSSLConnect();
   int DoSSLConnectComplete(int result);
+  int DoSSLHandleCertError(int result);
+  int DoSSLHandleCertErrorComplete(int result);
   int DoReadWrite(int result);
 
   GURL ProxyAuthOrigin() const;
@@ -311,6 +318,7 @@ class NET_EXPORT SocketStream
   void DoRestartWithAuth();
 
   int HandleCertificateError(int result);
+  int AllowCertErrorForReconnection(SSLConfig* ssl_config);
 
   SSLConfigService* ssl_config_service() const;
   ProxyService* proxy_service() const;