diff options
| -rw-r--r-- | chrome/browser/net/proxy_browsertest.cc | 9 | ||||
| -rw-r--r-- | net/data/websocket/echo-request-headers_wsh.py | 20 | ||||
| -rw-r--r-- | net/data/websocket/proxied_request_check.html | 32 | ||||
| -rw-r--r-- | net/http/http_network_transaction.cc | 21 | ||||
| -rw-r--r-- | net/http/http_network_transaction.h | 6 | ||||
| -rw-r--r-- | net/http/http_network_transaction_unittest.cc | 313 |
6 files changed, 386 insertions, 15 deletions
diff --git a/chrome/browser/net/proxy_browsertest.cc b/chrome/browser/net/proxy_browsertest.cc index 9f233b1..ca230ac 100644 --- a/chrome/browser/net/proxy_browsertest.cc +++ b/chrome/browser/net/proxy_browsertest.cc @@ -105,7 +105,8 @@ class ProxyBrowserTest : public InProcessBrowserTest { #define MAYBE_BasicAuthWSConnect BasicAuthWSConnect #endif // Test that the browser can establish a WebSocket connection via a proxy -// that requires basic authentication. +// that requires basic authentication. This test also checks the headers +// arrive at WebSocket server. IN_PROC_BROWSER_TEST_F(ProxyBrowserTest, MAYBE_BasicAuthWSConnect) { // Launch WebSocket server. net::SpawnedTestServer ws_server(net::SpawnedTestServer::TYPE_WS, @@ -131,9 +132,9 @@ IN_PROC_BROWSER_TEST_F(ProxyBrowserTest, MAYBE_BasicAuthWSConnect) { std::string scheme("http"); GURL::Replacements replacements; replacements.SetSchemeStr(scheme); - ui_test_utils::NavigateToURL( - browser(), - ws_server.GetURL("connect_check.html").ReplaceComponents(replacements)); + ui_test_utils::NavigateToURL(browser(), + ws_server.GetURL("proxied_request_check.html") + .ReplaceComponents(replacements)); const base::string16 result = watcher.WaitAndGetTitle(); EXPECT_TRUE(EqualsASCII(result, "PASS")); diff --git a/net/data/websocket/echo-request-headers_wsh.py b/net/data/websocket/echo-request-headers_wsh.py new file mode 100644 index 0000000..89aa28e --- /dev/null +++ b/net/data/websocket/echo-request-headers_wsh.py @@ -0,0 +1,20 @@ +# Copyright (c) 2014 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. +# +# This handler serializes the received headers into a JSON string and sends it +# back to the client. In |headers_in|, the keys are converted to lower-case, +# while the original case is retained for the values. + + +import json + + +def web_socket_do_extra_handshake(request): + pass + + +def web_socket_transfer_data(request): + request.ws_stream.send_message(json.dumps(dict(request.headers_in.items()))) + # Wait for closing handshake + request.ws_stream.receive_message() diff --git a/net/data/websocket/proxied_request_check.html b/net/data/websocket/proxied_request_check.html new file mode 100644 index 0000000..9d6d64e --- /dev/null +++ b/net/data/websocket/proxied_request_check.html @@ -0,0 +1,32 @@ +<!DOCTYPE html> +<head> +<title>test proxied ws connection</title> +</head> +<script type="text/javascript"> +// Do connection test and check the headers arrive at the WebSocket. + +var protocol = location.protocol.replace('http', 'ws'); +var url = protocol + '//' + location.host + '/echo-request-headers'; +var ws = new WebSocket(url); + +ws.onmessage = function(evt) +{ + var headers = JSON.parse(evt.data); + for (var name in headers) { + // The keys in the serialized data are lower cased. + if (name.startsWith('proxy-')) { + document.title = 'FAIL'; + return; + } + } + + // Set document title to 'PASS'. The test observer catches this title changes + // to know the result. + document.title = 'PASS'; +} + +ws.onclose = function() +{ + document.title = 'FAIL'; +} +</script> diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 97fc7db..8f33e20 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -570,6 +570,11 @@ bool HttpNetworkTransaction::is_https_request() const { return request_->url.SchemeIs("https"); } +bool HttpNetworkTransaction::UsingHttpProxyWithoutTunnel() const { + return (proxy_info_.is_http() || proxy_info_.is_https()) && + !(request_->url.SchemeIs("https") || request_->url.SchemeIsWSOrWSS()); +} + void HttpNetworkTransaction::DoCallback(int rv) { DCHECK_NE(rv, ERR_IO_PENDING); DCHECK(!callback_.is_null()); @@ -834,12 +839,13 @@ int HttpNetworkTransaction::DoGenerateServerAuthTokenComplete(int rv) { return rv; } -void HttpNetworkTransaction::BuildRequestHeaders(bool using_proxy) { +void HttpNetworkTransaction::BuildRequestHeaders( + bool using_http_proxy_without_tunnel) { request_headers_.SetHeader(HttpRequestHeaders::kHost, GetHostAndOptionalPort(request_->url)); // For compat with HTTP/1.0 servers and proxies: - if (using_proxy) { + if (using_http_proxy_without_tunnel) { request_headers_.SetHeader(HttpRequestHeaders::kProxyConnection, "keep-alive"); } else { @@ -882,7 +888,8 @@ void HttpNetworkTransaction::BuildRequestHeaders(bool using_proxy) { request_headers_.MergeFrom(request_->extra_headers); - if (using_proxy && !before_proxy_headers_sent_callback_.is_null()) + if (using_http_proxy_without_tunnel && + !before_proxy_headers_sent_callback_.is_null()) before_proxy_headers_sent_callback_.Run(proxy_info_, &request_headers_); response_.did_use_http_auth = @@ -911,9 +918,8 @@ int HttpNetworkTransaction::DoBuildRequest() { // This is constructed lazily (instead of within our Start method), so that // we have proxy info available. if (request_headers_.IsEmpty()) { - bool using_proxy = (proxy_info_.is_http() || proxy_info_.is_https()) && - !is_https_request(); - BuildRequestHeaders(using_proxy); + bool using_http_proxy_without_tunnel = UsingHttpProxyWithoutTunnel(); + BuildRequestHeaders(using_http_proxy_without_tunnel); } return OK; @@ -1417,8 +1423,7 @@ void HttpNetworkTransaction::ResetConnectionAndRequestForResend() { } bool HttpNetworkTransaction::ShouldApplyProxyAuth() const { - return !is_https_request() && - (proxy_info_.is_https() || proxy_info_.is_http()); + return UsingHttpProxyWithoutTunnel(); } bool HttpNetworkTransaction::ShouldApplyServerAuth() const { diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h index 229d397..2c4bd93 100644 --- a/net/http/http_network_transaction.h +++ b/net/http/http_network_transaction.h @@ -144,6 +144,10 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction bool is_https_request() const; + // Returns true if the request is using an HTTP(S) proxy without being + // tunneled via the CONNECT method. + bool UsingHttpProxyWithoutTunnel() const; + void DoCallback(int result); void OnIOComplete(int result); @@ -176,7 +180,7 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction int DoDrainBodyForAuthRestart(); int DoDrainBodyForAuthRestartComplete(int result); - void BuildRequestHeaders(bool using_proxy); + void BuildRequestHeaders(bool using_http_proxy_without_tunnel); // Writes a log message to help debugging in the field when we block a proxy // response to a CONNECT request. diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index 83fe640..71e775c 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -14,6 +14,7 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/json/json_writer.h" +#include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/run_loop.h" @@ -27,6 +28,7 @@ #include "net/base/elements_upload_data_stream.h" #include "net/base/load_timing_info.h" #include "net/base/load_timing_info_test_util.h" +#include "net/base/net_errors.h" #include "net/base/net_log.h" #include "net/base/net_log_unittest.h" #include "net/base/request_priority.h" @@ -41,12 +43,15 @@ #include "net/http/http_auth_handler_digest.h" #include "net/http/http_auth_handler_mock.h" #include "net/http/http_auth_handler_ntlm.h" +#include "net/http/http_basic_state.h" #include "net/http/http_basic_stream.h" #include "net/http/http_network_session.h" #include "net/http/http_network_session_peer.h" +#include "net/http/http_request_headers.h" #include "net/http/http_server_properties_impl.h" #include "net/http/http_stream.h" #include "net/http/http_stream_factory.h" +#include "net/http/http_stream_parser.h" #include "net/http/http_transaction_test_util.h" #include "net/proxy/proxy_config_service_fixed.h" #include "net/proxy/proxy_info.h" @@ -210,6 +215,14 @@ void TestLoadTimingNotReusedWithPac(const net::LoadTimingInfo& load_timing_info, EXPECT_TRUE(load_timing_info.receive_headers_end.is_null()); } +void AddWebSocketHeaders(net::HttpRequestHeaders* headers) { + headers->SetHeader("Connection", "Upgrade"); + headers->SetHeader("Upgrade", "websocket"); + headers->SetHeader("Origin", "http://www.google.com"); + headers->SetHeader("Sec-WebSocket-Version", "13"); + headers->SetHeader("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ=="); +} + } // namespace namespace net { @@ -12470,6 +12483,118 @@ class FakeStreamFactory : public HttpStreamFactory { DISALLOW_COPY_AND_ASSIGN(FakeStreamFactory); }; +// TODO(ricea): Maybe unify this with the one in +// url_request_http_job_unittest.cc ? +class FakeWebSocketBasicHandshakeStream : public WebSocketHandshakeStreamBase { + public: + FakeWebSocketBasicHandshakeStream(scoped_ptr<ClientSocketHandle> connection, + bool using_proxy) + : state_(connection.release(), using_proxy) {} + + // Fake implementation of HttpStreamBase methods. + // This ends up being quite "real" because this object has to really send data + // on the mock socket. It might be easier to use the real implementation, but + // the fact that the WebSocket code is not compiled on iOS makes that + // difficult. + int InitializeStream(const HttpRequestInfo* request_info, + RequestPriority priority, + const BoundNetLog& net_log, + const CompletionCallback& callback) override { + state_.Initialize(request_info, priority, net_log, callback); + return OK; + } + + int SendRequest(const HttpRequestHeaders& request_headers, + HttpResponseInfo* response, + const CompletionCallback& callback) override { + return parser()->SendRequest(state_.GenerateRequestLine(), request_headers, + response, callback); + } + + int ReadResponseHeaders(const CompletionCallback& callback) override { + return parser()->ReadResponseHeaders(callback); + } + + int ReadResponseBody(IOBuffer* buf, + int buf_len, + const CompletionCallback& callback) override { + NOTREACHED(); + return ERR_IO_PENDING; + } + + void Close(bool not_reusable) override { + if (parser()) + parser()->Close(true); + } + + bool IsResponseBodyComplete() const override { + NOTREACHED(); + return false; + } + + bool CanFindEndOfResponse() const override { + return parser()->CanFindEndOfResponse(); + } + + bool IsConnectionReused() const override { + NOTREACHED(); + return false; + } + void SetConnectionReused() override { NOTREACHED(); } + + bool IsConnectionReusable() const override { + NOTREACHED(); + return false; + } + + int64 GetTotalReceivedBytes() const override { + NOTREACHED(); + return 0; + } + + bool GetLoadTimingInfo(LoadTimingInfo* load_timing_info) const override { + NOTREACHED(); + return false; + } + + void GetSSLInfo(SSLInfo* ssl_info) override { NOTREACHED(); } + + void GetSSLCertRequestInfo(SSLCertRequestInfo* cert_request_info) override { + NOTREACHED(); + } + + bool IsSpdyHttpStream() const override { + NOTREACHED(); + return false; + } + + void Drain(HttpNetworkSession* session) override { NOTREACHED(); } + + void SetPriority(RequestPriority priority) override { NOTREACHED(); } + + UploadProgress GetUploadProgress() const override { + NOTREACHED(); + return UploadProgress(); + } + + HttpStream* RenewStreamForAuth() override { + NOTREACHED(); + return nullptr; + } + + // Fake implementation of WebSocketHandshakeStreamBase method(s) + scoped_ptr<WebSocketStream> Upgrade() override { + NOTREACHED(); + return scoped_ptr<WebSocketStream>(); + } + + private: + HttpStreamParser* parser() const { return state_.parser(); } + HttpBasicState state_; + + DISALLOW_COPY_AND_ASSIGN(FakeWebSocketBasicHandshakeStream); +}; + // TODO(yhirano): Split this class out into a net/websockets file, if it is // worth doing. class FakeWebSocketStreamCreateHelper : @@ -12478,8 +12603,8 @@ class FakeWebSocketStreamCreateHelper : WebSocketHandshakeStreamBase* CreateBasicStream( scoped_ptr<ClientSocketHandle> connection, bool using_proxy) override { - NOTREACHED(); - return NULL; + return new FakeWebSocketBasicHandshakeStream(connection.Pass(), + using_proxy); } WebSocketHandshakeStreamBase* CreateSpdyStream( @@ -13253,4 +13378,188 @@ TEST_P(HttpNetworkTransactionTest, PostIgnoresPartial400HeadersAfterReset) { EXPECT_TRUE(response == NULL); } +// Verify that proxy headers are not sent to the destination server when +// establishing a tunnel for a secure WebSocket connection. +TEST_P(HttpNetworkTransactionTest, ProxyHeadersNotSentOverWssTunnel) { + HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("wss://www.google.com/"); + AddWebSocketHeaders(&request.extra_headers); + + // Configure against proxy server "myproxy:70". + session_deps_.proxy_service.reset( + ProxyService::CreateFixedFromPacResult("PROXY myproxy:70")); + + scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_)); + + // Since a proxy is configured, try to establish a tunnel. + MockWrite data_writes[] = { + MockWrite( + "CONNECT www.google.com:443 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n\r\n"), + + // After calling trans->RestartWithAuth(), this is the request we should + // be issuing -- the final header line contains the credentials. + MockWrite( + "CONNECT www.google.com:443 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n" + "Proxy-Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + + MockWrite( + "GET / HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: Upgrade\r\n" + "Upgrade: websocket\r\n" + "Origin: http://www.google.com\r\n" + "Sec-WebSocket-Version: 13\r\n" + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n"), + }; + + // The proxy responds to the connect with a 407, using a persistent + // connection. + MockRead data_reads[] = { + // No credentials. + MockRead("HTTP/1.1 407 Proxy Authentication Required\r\n"), + MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"), + MockRead("Proxy-Connection: close\r\n\r\n"), + + MockRead("HTTP/1.1 200 Connection Established\r\n\r\n"), + + MockRead("HTTP/1.1 101 Switching Protocols\r\n"), + MockRead("Upgrade: websocket\r\n"), + MockRead("Connection: Upgrade\r\n"), + MockRead("Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n"), + }; + + StaticSocketDataProvider data(data_reads, arraysize(data_reads), data_writes, + arraysize(data_writes)); + session_deps_.socket_factory->AddSocketDataProvider(&data); + SSLSocketDataProvider ssl(ASYNC, OK); + session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl); + + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get())); + FakeWebSocketStreamCreateHelper websocket_stream_create_helper; + trans->SetWebSocketHandshakeStreamCreateHelper( + &websocket_stream_create_helper); + + { + TestCompletionCallback callback; + + int rv = trans->Start(&request, callback.callback(), BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + + rv = callback.WaitForResult(); + EXPECT_EQ(OK, rv); + } + + const HttpResponseInfo* response = trans->GetResponseInfo(); + ASSERT_TRUE(response); + ASSERT_TRUE(response->headers.get()); + EXPECT_EQ(407, response->headers->response_code()); + + { + TestCompletionCallback callback; + + int rv = trans->RestartWithAuth(AuthCredentials(kFoo, kBar), + callback.callback()); + EXPECT_EQ(ERR_IO_PENDING, rv); + + rv = callback.WaitForResult(); + EXPECT_EQ(OK, rv); + } + + response = trans->GetResponseInfo(); + ASSERT_TRUE(response); + ASSERT_TRUE(response->headers.get()); + + EXPECT_EQ(101, response->headers->response_code()); + + trans.reset(); + session->CloseAllConnections(); +} + +// Verify that proxy headers are not sent to the destination server when +// establishing a tunnel for an insecure WebSocket connection. +// This requires the authentication info to be injected into the auth cache +// due to crbug.com/395064 +// TODO(ricea): Change to use a 407 response once issue 395064 is fixed. +TEST_P(HttpNetworkTransactionTest, ProxyHeadersNotSentOverWsTunnel) { + HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("ws://www.google.com/"); + AddWebSocketHeaders(&request.extra_headers); + + // Configure against proxy server "myproxy:70". + session_deps_.proxy_service.reset( + ProxyService::CreateFixedFromPacResult("PROXY myproxy:70")); + + scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_)); + + MockWrite data_writes[] = { + // Try to establish a tunnel for the WebSocket connection, with + // credentials. Because WebSockets have a separate set of socket pools, + // they cannot and will not use the same TCP/IP connection as the + // preflight HTTP request. + MockWrite( + "CONNECT www.google.com:80 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n" + "Proxy-Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + + MockWrite( + "GET / HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: Upgrade\r\n" + "Upgrade: websocket\r\n" + "Origin: http://www.google.com\r\n" + "Sec-WebSocket-Version: 13\r\n" + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n"), + }; + + MockRead data_reads[] = { + // HTTP CONNECT with credentials. + MockRead("HTTP/1.1 200 Connection Established\r\n\r\n"), + + // WebSocket connection established inside tunnel. + MockRead("HTTP/1.1 101 Switching Protocols\r\n"), + MockRead("Upgrade: websocket\r\n"), + MockRead("Connection: Upgrade\r\n"), + MockRead("Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r\n\r\n"), + }; + + StaticSocketDataProvider data(data_reads, arraysize(data_reads), data_writes, + arraysize(data_writes)); + session_deps_.socket_factory->AddSocketDataProvider(&data); + + session->http_auth_cache()->Add( + GURL("http://myproxy:70/"), "MyRealm1", HttpAuth::AUTH_SCHEME_BASIC, + "Basic realm=MyRealm1", AuthCredentials(kFoo, kBar), "/"); + + scoped_ptr<HttpTransaction> trans( + new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get())); + FakeWebSocketStreamCreateHelper websocket_stream_create_helper; + trans->SetWebSocketHandshakeStreamCreateHelper( + &websocket_stream_create_helper); + + TestCompletionCallback callback; + + int rv = trans->Start(&request, callback.callback(), BoundNetLog()); + EXPECT_EQ(ERR_IO_PENDING, rv); + + rv = callback.WaitForResult(); + EXPECT_EQ(OK, rv); + + const HttpResponseInfo* response = trans->GetResponseInfo(); + ASSERT_TRUE(response); + ASSERT_TRUE(response->headers.get()); + + EXPECT_EQ(101, response->headers->response_code()); + + trans.reset(); + session->CloseAllConnections(); +} + } // namespace net |