Allow chrome to handle 407 auth challenges to CONNECT requests

through HTTPS Proxies.  This also changes the mechanism used 
to restart HttpProxyClientSocket requests with auth.  Previously
the transport socket would be Disconnected, and then re-Connected
(which was not implemented for SSLClientSockets).  However, the 
approach was problematic in the face of, for example, ipv6.  The
new approach is to close the HttpProxyClientSocket, and request
a new socket from the pool.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110529

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110879

Review URL: http://codereview.chromium.org/8502024

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

net/http/http_proxy_client_socket.cc

@@ -68,7 +68,7 @@ int HttpProxyClientSocket::RestartWithAuth(OldCompletionCallback* callback) {
   DCHECK(!user_callback_);
 
   int rv = PrepareForAuthRestart();
-  if (rv != OK)
+  if (rv != OK || next_state_ == STATE_NONE)
     return rv;
 
   rv = DoLoop(OK);
@@ -77,6 +77,11 @@ int HttpProxyClientSocket::RestartWithAuth(OldCompletionCallback* callback) {
   return rv;
 }
 
+const
+scoped_refptr<HttpAuthController>& HttpProxyClientSocket::auth_controller() {
+  return auth_;
+}
+
 const HttpResponseInfo* HttpProxyClientSocket::GetConnectResponseInfo() const {
   return response_.headers ? &response_ : NULL;
 }
@@ -251,10 +256,7 @@ int HttpProxyClientSocket::DidDrainBodyForAuthRestart(bool keep_alive) {
     next_state_ = STATE_GENERATE_AUTH_TOKEN;
     transport_->set_is_reused(true);
   } else {
-    // This assumes that the underlying transport socket is a TCP socket,
-    // since only TCP sockets are restartable.
-    next_state_ = STATE_TCP_RESTART;
-    transport_->socket()->Disconnect();
+    next_state_ = STATE_NONE;
   }
 
   // Reset the other member variables.
@@ -267,17 +269,6 @@ int HttpProxyClientSocket::DidDrainBodyForAuthRestart(bool keep_alive) {
   return OK;
 }
 
-int HttpProxyClientSocket::HandleAuthChallenge() {
-  DCHECK(response_.headers);
-
-  int rv = auth_->HandleAuthChallenge(response_.headers, false, true, net_log_);
-  response_.auth_challenge = auth_->auth_info();
-  if (rv == OK)
-    return ERR_PROXY_AUTH_REQUESTED;
-
-  return rv;
-}
-
 void HttpProxyClientSocket::LogBlockedTunnelResponse(int response_code) const {
   LOG(WARNING) << "Blocked proxy response with status " << response_code
                << " to CONNECT request for "
@@ -347,13 +338,6 @@ int HttpProxyClientSocket::DoLoop(int last_io_result) {
       case STATE_DRAIN_BODY_COMPLETE:
         rv = DoDrainBodyComplete(rv);
         break;
-      case STATE_TCP_RESTART:
-        DCHECK_EQ(OK, rv);
-        rv = DoTCPRestart();
-        break;
-      case STATE_TCP_RESTART_COMPLETE:
-        rv = DoTCPRestartComplete(rv);
-        break;
       case STATE_DONE:
         break;
       default:
@@ -452,7 +436,7 @@ int HttpProxyClientSocket::DoReadHeadersComplete(int result) {
       // authentication code is smart enough to avoid being tricked by an
       // active network attacker.
       // The next state is intentionally not set as it should be STATE_NONE;
-      return HandleAuthChallenge();
+      return HandleAuthChallenge(auth_, &response_, net_log_);
 
     default:
       if (is_https_proxy_)
@@ -488,17 +472,4 @@ int HttpProxyClientSocket::DoDrainBodyComplete(int result) {
   return OK;
 }
 
-int HttpProxyClientSocket::DoTCPRestart() {
-  next_state_ = STATE_TCP_RESTART_COMPLETE;
-  return transport_->socket()->Connect(&io_callback_);
-}
-
-int HttpProxyClientSocket::DoTCPRestartComplete(int result) {
-  if (result != OK)
-    return result;
-
-  next_state_ = STATE_GENERATE_AUTH_TOKEN;
-  return result;
-}
-
 }  // namespace net

net/http/http_proxy_client_socket.h

@@ -50,15 +50,6 @@ class HttpProxyClientSocket : public ProxyClientSocket {
   // On destruction Disconnect() is called.
   virtual ~HttpProxyClientSocket();
 
-  // If Connect (or its callback) returns PROXY_AUTH_REQUESTED, then
-  // credentials should be added to the HttpAuthController before calling
-  // RestartWithAuth.
-  int RestartWithAuth(OldCompletionCallback* callback);
-
-  const scoped_refptr<HttpAuthController>& auth_controller() {
-    return auth_;
-  }
-
   bool using_spdy() {
     return using_spdy_;
   }
@@ -66,6 +57,8 @@ class HttpProxyClientSocket : public ProxyClientSocket {
   // ProxyClientSocket methods:
   virtual const HttpResponseInfo* GetConnectResponseInfo() const OVERRIDE;
   virtual HttpStream* CreateConnectResponseStream() OVERRIDE;
+  virtual int RestartWithAuth(OldCompletionCallback* callback) OVERRIDE;
+  virtual const scoped_refptr<HttpAuthController>& auth_controller() OVERRIDE;
 
   // StreamSocket methods:
   virtual int Connect(OldCompletionCallback* callback) OVERRIDE;
@@ -103,8 +96,6 @@ class HttpProxyClientSocket : public ProxyClientSocket {
     STATE_READ_HEADERS_COMPLETE,
     STATE_DRAIN_BODY,
     STATE_DRAIN_BODY_COMPLETE,
-    STATE_TCP_RESTART,
-    STATE_TCP_RESTART_COMPLETE,
     STATE_DONE,
   };
 
@@ -116,8 +107,6 @@ class HttpProxyClientSocket : public ProxyClientSocket {
   int PrepareForAuthRestart();
   int DidDrainBodyForAuthRestart(bool keep_alive);
 
-  int HandleAuthChallenge();
-
   void LogBlockedTunnelResponse(int response_code) const;
 
   void DoCallback(int result);
@@ -132,8 +121,6 @@ class HttpProxyClientSocket : public ProxyClientSocket {
   int DoReadHeadersComplete(int result);
   int DoDrainBody();
   int DoDrainBodyComplete(int result);
-  int DoTCPRestart();
-  int DoTCPRestartComplete(int result);
 
   OldCompletionCallbackImpl<HttpProxyClientSocket> io_callback_;
   State next_state_;

net/http/http_proxy_client_socket_pool_unittest.cc

@@ -257,9 +257,22 @@ TEST_P(HttpProxyClientSocketPoolTest, NeedAuth) {
     CreateMockWrite(*req, 0, true),
     CreateMockWrite(*rst, 2, true),
   };
+  static const char* const kAuthChallenge[] = {
+    "status", "407 Proxy Authentication Required",
+    "version", "HTTP/1.1",
+    "proxy-authenticate", "Basic realm=\"MyRealm1\"",
+  };
+
   scoped_ptr<spdy::SpdyFrame> resp(
-      ConstructSpdySynReplyError(
-          "407 Proxy Authentication Required", NULL, 0, 1));
+      ConstructSpdyControlFrame(NULL,
+                                0,
+                                false,
+                                1,
+                                LOWEST,
+                                spdy::SYN_REPLY,
+                                spdy::CONTROL_FLAG_NONE,
+                                kAuthChallenge,
+                                arraysize(kAuthChallenge)));
   MockRead spdy_reads[] = {
     CreateMockWrite(*resp, 1, true),
     MockRead(true, 0, 3)
@@ -276,21 +289,16 @@ TEST_P(HttpProxyClientSocketPoolTest, NeedAuth) {
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
-  data_->RunFor(4);
+  data_->RunFor(GetParam() == SPDY ? 2 : 4);
   rv = callback_.WaitForResult();
+  EXPECT_EQ(ERR_PROXY_AUTH_REQUESTED, rv);
+  EXPECT_TRUE(handle_.is_initialized());
+  ASSERT_TRUE(handle_.socket());
   if (GetParam() != SPDY) {
-    EXPECT_EQ(ERR_PROXY_AUTH_REQUESTED, rv);
-    EXPECT_TRUE(handle_.is_initialized());
-    ASSERT_TRUE(handle_.socket());
     HttpProxyClientSocket* tunnel_socket =
             static_cast<HttpProxyClientSocket*>(handle_.socket());
     EXPECT_FALSE(tunnel_socket->IsConnected());
     EXPECT_FALSE(tunnel_socket->using_spdy());
-  } else {
-    // Proxy auth is not really implemented for SPDY yet
-    EXPECT_EQ(ERR_TUNNEL_CONNECTION_FAILED, rv);
-    EXPECT_FALSE(handle_.is_initialized());
-    EXPECT_FALSE(handle_.socket());
   }
 }
 

net/http/http_proxy_utils.cc

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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/http/http_proxy_utils.h"
 
+#include "base/logging.h"
 #include "base/stringprintf.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/host_port_pair.h"
+#include "net/base/net_errors.h"
 #include "net/base/net_util.h"
+#include "net/http/http_auth_controller.h"
 #include "net/http/http_request_info.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_response_info.h"
 
 namespace net {
 
@@ -36,4 +41,17 @@ void BuildTunnelRequest(
   request_headers->MergeFrom(auth_headers);
 }
 
+int HandleAuthChallenge(HttpAuthController* auth,
+                        HttpResponseInfo* response,
+                        const BoundNetLog& net_log) {
+  DCHECK(response->headers);
+
+  int rv = auth->HandleAuthChallenge(response->headers, false, true, net_log);
+  response->auth_challenge = auth->auth_info();
+  if (rv == OK)
+    return ERR_PROXY_AUTH_REQUESTED;
+
+  return rv;
+}
+
 }  // namespace net

net/http/http_proxy_utils.h

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -10,6 +10,9 @@
 
 namespace net {
 
+class BoundNetLog;
+class HttpAuthController;
+class HttpResponseInfo;
 struct HttpRequestInfo;
 class HttpRequestHeaders;
 class HostPortPair;
@@ -23,6 +26,12 @@ void BuildTunnelRequest(const HttpRequestInfo& request_info,
                         std::string* request_line,
                         HttpRequestHeaders* request_headers);
 
+// When an auth challenge (407 response) is received during tunnel construction
+// this method should be called.
+int HandleAuthChallenge(HttpAuthController* auth,
+                        HttpResponseInfo* response,
+                        const BoundNetLog& net_log);
+
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_PROXY_UTILS_H_

net/http/http_stream_factory_impl_job.cc

@@ -369,10 +369,10 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
         DCHECK(connection_->socket());
         DCHECK(establishing_tunnel_);
 
-        HttpProxyClientSocket* http_proxy_socket =
-            static_cast<HttpProxyClientSocket*>(connection_->socket());
+        ProxyClientSocket* proxy_socket =
+            static_cast<ProxyClientSocket*>(connection_->socket());
         const HttpResponseInfo* tunnel_auth_response =
-            http_proxy_socket->GetConnectResponseInfo();
+            proxy_socket->GetConnectResponseInfo();
 
         next_state_ = STATE_WAITING_USER_ACTION;
         MessageLoop::current()->PostTask(
@@ -381,7 +381,7 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
                 &HttpStreamFactoryImpl::Job::OnNeedsProxyAuthCallback,
                 ptr_factory_.GetWeakPtr(),
                 *tunnel_auth_response,
-                http_proxy_socket->auth_controller()));
+                proxy_socket->auth_controller()));
       }
       return ERR_IO_PENDING;
 
@@ -898,9 +898,9 @@ int HttpStreamFactoryImpl::Job::DoCreateStreamComplete(int result) {
 
 int HttpStreamFactoryImpl::Job::DoRestartTunnelAuth() {
   next_state_ = STATE_RESTART_TUNNEL_AUTH_COMPLETE;
-  HttpProxyClientSocket* http_proxy_socket =
-      static_cast<HttpProxyClientSocket*>(connection_->socket());
-  return http_proxy_socket->RestartWithAuth(&io_callback_);
+  ProxyClientSocket* proxy_socket =
+      static_cast<ProxyClientSocket*>(connection_->socket());
+  return proxy_socket->RestartWithAuth(&io_callback_);
 }
 
 int HttpStreamFactoryImpl::Job::DoRestartTunnelAuthComplete(int result) {
@@ -908,14 +908,14 @@ int HttpStreamFactoryImpl::Job::DoRestartTunnelAuthComplete(int result) {
     return result;
 
   if (result == OK) {
-    // Now that we've got the HttpProxyClientSocket connected.  We have
+    // Now that we've got the ProxyClientSocket prepared to restart.  We have
     // to release it as an idle socket into the pool and start the connection
     // process from the beginning.  Trying to pass it in with the
     // SSLSocketParams might cause a deadlock since params are dispatched
     // interchangeably.  This request won't necessarily get this http proxy
     // socket, but there will be forward progress.
     establishing_tunnel_ = false;
-    ReturnToStateInitConnection(false /* do not close connection */);
+    ReturnToStateInitConnection(!connection_->socket()->IsConnectedAndIdle());
     return OK;
   }
 

net/http/proxy_client_socket.h

@@ -10,6 +10,7 @@
 
 namespace net {
 
+class HttpAuthController;
 class HttpStream;
 class HttpResponseInfo;
 
@@ -26,6 +27,15 @@ class NET_EXPORT_PRIVATE ProxyClientSocket : public StreamSocket {
   // which can be used to read the response body.
   virtual HttpStream* CreateConnectResponseStream() = 0;
 
+  // Returns the HttpAuthController which can be used
+  // to interact with an HTTP Proxy Authorization Required (407) request.
+  virtual const scoped_refptr<HttpAuthController>& auth_controller() = 0;
+
+  // If Connect (or its callback) returns PROXY_AUTH_REQUESTED, then
+  // credentials should be added to the HttpAuthController before calling
+  // RestartWithAuth.
+  virtual int RestartWithAuth(OldCompletionCallback* callback) = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ProxyClientSocket);
 };

net/spdy/spdy_proxy_client_socket.cc

@@ -63,6 +63,19 @@ const HttpResponseInfo* SpdyProxyClientSocket::GetConnectResponseInfo() const {
   return response_.headers ? &response_ : NULL;
 }
 
+int SpdyProxyClientSocket::RestartWithAuth(OldCompletionCallback* callback) {
+  // A SPDY Stream can only handle a single request, so the underlying
+  // stream may not be reused and a new SpdyProxyClientSocket must be
+  // created (possibly on top of the same SPDY Session).
+  next_state_ = STATE_DISCONNECTED;
+  return OK;
+}
+
+const
+scoped_refptr<HttpAuthController>& SpdyProxyClientSocket::auth_controller() {
+  return auth_;
+}
+
 HttpStream* SpdyProxyClientSocket::CreateConnectResponseStream() {
   DCHECK(response_stream_.get());
   return response_stream_.release();
@@ -384,6 +397,16 @@ int SpdyProxyClientSocket::DoReadReplyComplete(int result) {
   if (response_.headers->response_code() == 200) {
     return OK;
   } else if (response_.headers->response_code() == 407) {
+    int rv = HandleAuthChallenge(auth_, &response_, net_log_);
+    if (rv != ERR_PROXY_AUTH_REQUESTED) {
+      return rv;
+    }
+    // SPDY only supports basic and digest auth
+    if (auth_->auth_info() &&
+        (auth_->auth_info()->scheme == "basic" ||
+         auth_->auth_info()->scheme == "digest")) {
+      return ERR_PROXY_AUTH_REQUESTED;
+    }
     return ERR_TUNNEL_CONNECTION_FAILED;
   } else {
     // Immediately hand off our SpdyStream to a newly created SpdyHttpStream

net/spdy/spdy_proxy_client_socket.h

@@ -53,17 +53,11 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket,
   // On destruction Disconnect() is called.
   virtual ~SpdyProxyClientSocket();
 
-  const scoped_refptr<HttpAuthController>& auth_controller() {
-    return auth_;
-  }
-
   // ProxyClientSocket methods:
   virtual const HttpResponseInfo* GetConnectResponseInfo() const OVERRIDE;
-
-  // In the event of a non-200 response to the CONNECT request, this
-  // method may be called to return an HttpStream in order to read
-  // the response body.
   virtual HttpStream* CreateConnectResponseStream() OVERRIDE;
+  virtual int RestartWithAuth(OldCompletionCallback* callback) OVERRIDE;
+  virtual const scoped_refptr<HttpAuthController>& auth_controller() OVERRIDE;
 
   // StreamSocket methods:
   virtual int Connect(OldCompletionCallback* callback) OVERRIDE;

net/spdy/spdy_proxy_client_socket_unittest.cc

@@ -66,6 +66,7 @@ class SpdyProxyClientSocketTest : public PlatformTest {
   spdy::SpdyFrame* ConstructConnectAuthRequestFrame();
   spdy::SpdyFrame* ConstructConnectReplyFrame();
   spdy::SpdyFrame* ConstructConnectAuthReplyFrame();
+  spdy::SpdyFrame* ConstructNtlmAuthReplyFrame();
   spdy::SpdyFrame* ConstructConnectErrorReplyFrame();
   spdy::SpdyFrame* ConstructBodyFrame(const char* data, int length);
   scoped_refptr<IOBufferWithSize> CreateBuffer(const char* data, int size);
@@ -387,6 +388,26 @@ spdy::SpdyFrame* SpdyProxyClientSocketTest::ConstructConnectAuthReplyFrame() {
                                    arraysize(kStandardReplyHeaders));
 }
 
+// Constructs a SPDY SYN_REPLY frame to match the SPDY CONNECT which
+// requires Proxy Authentication using NTLM.
+spdy::SpdyFrame* SpdyProxyClientSocketTest::ConstructNtlmAuthReplyFrame() {
+  const char* const kStandardReplyHeaders[] = {
+      "status", "407 Proxy Authentication Required",
+      "version", "HTTP/1.1",
+      "proxy-authenticate", "NTLM",
+  };
+
+  return ConstructSpdyControlFrame(NULL,
+                                   0,
+                                   false,
+                                   kStreamId,
+                                   LOWEST,
+                                   spdy::SYN_REPLY,
+                                   spdy::CONTROL_FLAG_NONE,
+                                   kStandardReplyHeaders,
+                                   arraysize(kStandardReplyHeaders));
+}
+
 // Constructs a SPDY SYN_REPLY frame with an HTTP 500 error.
 spdy::SpdyFrame* SpdyProxyClientSocketTest::ConstructConnectErrorReplyFrame() {
   const char* const kStandardReplyHeaders[] = {
@@ -433,6 +454,23 @@ TEST_F(SpdyProxyClientSocketTest, ConnectSendsCorrectRequest) {
   AssertConnectionEstablished();
 }
 
+TEST_F(SpdyProxyClientSocketTest, ConnectWithUnsupportedAuth) {
+  scoped_ptr<spdy::SpdyFrame> conn(ConstructConnectRequestFrame());
+  MockWrite writes[] = {
+    CreateMockWrite(*conn, 0, false),
+  };
+
+  scoped_ptr<spdy::SpdyFrame> resp(ConstructNtlmAuthReplyFrame());
+  MockRead reads[] = {
+    CreateMockRead(*resp, 1, true),
+    MockRead(true, 0, 3),  // EOF
+  };
+
+  Initialize(reads, arraysize(reads), writes, arraysize(writes));
+
+  AssertConnectFails(ERR_TUNNEL_CONNECTION_FAILED);
+}
+
 TEST_F(SpdyProxyClientSocketTest, ConnectWithAuthRequested) {
   scoped_ptr<spdy::SpdyFrame> conn(ConstructConnectRequestFrame());
   MockWrite writes[] = {
@@ -447,7 +485,7 @@ TEST_F(SpdyProxyClientSocketTest, ConnectWithAuthRequested) {
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes));
 
-  AssertConnectFails(ERR_TUNNEL_CONNECTION_FAILED);
+  AssertConnectFails(ERR_PROXY_AUTH_REQUESTED);
 
   const HttpResponseInfo* response = sock_->GetConnectResponseInfo();
   ASSERT_TRUE(response != NULL);
@@ -476,6 +514,38 @@ TEST_F(SpdyProxyClientSocketTest, ConnectWithAuthCredentials) {
   AssertConnectionEstablished();
 }
 
+TEST_F(SpdyProxyClientSocketTest, ConnectWithAuthRestart) {
+  scoped_ptr<spdy::SpdyFrame> conn(ConstructConnectRequestFrame());
+  scoped_ptr<spdy::SpdyFrame> auth(ConstructConnectAuthRequestFrame());
+  MockWrite writes[] = {
+    CreateMockWrite(*conn, 0, false),
+  };
+
+  scoped_ptr<spdy::SpdyFrame> resp(ConstructConnectAuthReplyFrame());
+  scoped_ptr<spdy::SpdyFrame> auth_resp(ConstructConnectReplyFrame());
+  MockRead reads[] = {
+    CreateMockRead(*resp, 1, true),
+    MockRead(true, 0, 3),  // EOF
+  };
+
+  Initialize(reads, arraysize(reads), writes, arraysize(writes));
+
+  AssertConnectFails(ERR_PROXY_AUTH_REQUESTED);
+
+  const HttpResponseInfo* response = sock_->GetConnectResponseInfo();
+  ASSERT_TRUE(response != NULL);
+  ASSERT_EQ(407, response->headers->response_code());
+  ASSERT_EQ("Proxy Authentication Required",
+            response->headers->GetStatusText());
+
+  AddAuthToCache();
+
+  ASSERT_EQ(OK, sock_->RestartWithAuth(&read_callback_));
+  // A SpdyProxyClientSocket sits on a single SPDY stream which can
+  // only be used for a single request/response.
+  ASSERT_FALSE(sock_->IsConnectedAndIdle());
+}
+
 TEST_F(SpdyProxyClientSocketTest, ConnectFails) {
   scoped_ptr<spdy::SpdyFrame> conn(ConstructConnectRequestFrame());
   MockWrite writes[] = {
@@ -821,7 +891,7 @@ TEST_F(SpdyProxyClientSocketTest, ReadAuthResponseBody) {
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes));
 
-  AssertConnectFails(ERR_TUNNEL_CONNECTION_FAILED);
+  AssertConnectFails(ERR_PROXY_AUTH_REQUESTED);
 
   Run(2);  // SpdySession consumes the next two reads and sends then to
            // sock_ to be buffered.