diff options
| -rw-r--r-- | chrome/browser/net/websocket_browsertest.cc | 94 | ||||
| -rw-r--r-- | chrome/chrome_tests.gypi | 1 | ||||
| -rw-r--r-- | net/base/nss_memio.c | 8 | ||||
| -rw-r--r-- | net/base/nss_memio.h | 5 | ||||
| -rw-r--r-- | net/data/websocket/README | 13 | ||||
| -rw-r--r-- | net/data/websocket/close-with-split-packet_wsh.py | 30 | ||||
| -rw-r--r-- | net/data/websocket/split_packet_check.html | 37 | ||||
| -rw-r--r-- | net/socket/ssl_client_socket_nss.cc | 165 |
8 files changed, 324 insertions, 29 deletions
diff --git a/chrome/browser/net/websocket_browsertest.cc b/chrome/browser/net/websocket_browsertest.cc new file mode 100644 index 0000000..4537e4d --- /dev/null +++ b/chrome/browser/net/websocket_browsertest.cc @@ -0,0 +1,94 @@ +// Copyright (c) 2012 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 "base/string_util.h" +#include "base/utf_string_conversions.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_tabstrip.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "content/public/test/browser_test_utils.h" +#include "net/base/test_data_directory.h" +#include "net/test/test_server.h" + +namespace { + +class WebSocketBrowserTest : public InProcessBrowserTest { + public: + WebSocketBrowserTest() + : ws_server_(net::TestServer::TYPE_WS, + net::TestServer::kLocalhost, + net::GetWebSocketTestDataDirectory()), + wss_server_(net::TestServer::TYPE_WSS, + SSLOptions(SSLOptions::CERT_OK), + net::GetWebSocketTestDataDirectory()) { + } + + protected: + net::TestServer ws_server_; + net::TestServer wss_server_; + + private: + typedef net::TestServer::SSLOptions SSLOptions; + + DISALLOW_COPY_AND_ASSIGN(WebSocketBrowserTest); +}; + +// Test that the browser can handle a WebSocket frame split into multiple TCP +// segments. +IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, WebSocketSplitSegments) { + // Launch a WebSocket server. + ASSERT_TRUE(ws_server_.Start()); + + // Setup page title observer. + content::WebContents* tab = chrome::GetActiveWebContents(browser()); + content::TitleWatcher watcher(tab, ASCIIToUTF16("PASS")); + watcher.AlsoWaitForTitle(ASCIIToUTF16("FAIL")); + + // Visit a HTTP page for testing. + std::string scheme("http"); + GURL::Replacements replacements; + replacements.SetSchemeStr(scheme); + ui_test_utils::NavigateToURL( + browser(), + ws_server_.GetURL( + "split_packet_check.html").ReplaceComponents(replacements)); + + const string16 result = watcher.WaitAndGetTitle(); + EXPECT_TRUE(EqualsASCII(result, "PASS")); +} + +#if defined(USE_OPENSSL) +// TODO(toyoshim): Enable this test for OpenSSL once http://crbug.com/160033 +// is fixed against OpenSSL. +#define MAYBE_SecureWebSocketSplitRecords DISABLED_SecureWebSocketSplitRecords +#else +#define MAYBE_SecureWebSocketSplitRecords SecureWebSocketSplitRecords +#endif +// Test that the browser can handle a WebSocket frame split into multiple SSL +// records. +IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, + MAYBE_SecureWebSocketSplitRecords) { + // Launch a secure WebSocket server. + ASSERT_TRUE(wss_server_.Start()); + + // Setup page title observer. + content::WebContents* tab = chrome::GetActiveWebContents(browser()); + content::TitleWatcher watcher(tab, ASCIIToUTF16("PASS")); + watcher.AlsoWaitForTitle(ASCIIToUTF16("FAIL")); + + // Visit a HTTPS page for testing. + std::string scheme("https"); + GURL::Replacements replacements; + replacements.SetSchemeStr(scheme); + ui_test_utils::NavigateToURL( + browser(), + wss_server_.GetURL( + "split_packet_check.html").ReplaceComponents(replacements)); + + const string16 result = watcher.WaitAndGetTitle(); + EXPECT_TRUE(EqualsASCII(result, "PASS")); +} + +} // namespace diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index a50f187..69b3876 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1182,6 +1182,7 @@ 'browser/net/load_timing_observer_browsertest.cc', 'browser/net/predictor_browsertest.cc', 'browser/net/proxy_browsertest.cc', + 'browser/net/websocket_browsertest.cc', 'browser/page_cycler/page_cycler_browsertest.cc', 'browser/performance_monitor/performance_monitor_browsertest.cc', 'browser/policy/cloud_policy_browsertest.cc', diff --git a/net/base/nss_memio.c b/net/base/nss_memio.c index 4092c17..bd047106 100644 --- a/net/base/nss_memio.c +++ b/net/base/nss_memio.c @@ -402,6 +402,14 @@ int memio_GetReadParams(memio_Private *secret, char **buf) return memio_buffer_unused_contiguous(mb); } +int memio_GetReadableBufferSize(memio_Private *secret) +{ + struct memio_buffer* mb = &((PRFilePrivate *)secret)->readbuf; + PR_ASSERT(mb->bufsize); + + return memio_buffer_used_contiguous(mb); +} + void memio_PutReadResult(memio_Private *secret, int bytes_read) { struct memio_buffer* mb = &((PRFilePrivate *)secret)->readbuf; diff --git a/net/base/nss_memio.h b/net/base/nss_memio.h index a8bbc2c..8481d15 100644 --- a/net/base/nss_memio.h +++ b/net/base/nss_memio.h @@ -63,6 +63,11 @@ int memio_GetReadRequest(memio_Private *secret); */ int memio_GetReadParams(memio_Private *secret, char **buf); +/* Ask memio how many bytes are contained in the internal buffer. + * Returns bytes available to read, or 0 if none available. + */ +int memio_GetReadableBufferSize(memio_Private *secret); + /* Tell memio how many bytes were read from the network. * If bytes_read is 0, causes EOF to be reported to * NSS after it reads the last byte from the circular buffer. diff --git a/net/data/websocket/README b/net/data/websocket/README index 034a30d..9dab697 100644 --- a/net/data/websocket/README +++ b/net/data/websocket/README @@ -7,6 +7,13 @@ Multiple tests may share one resource, or URI handler. SSLUITest.TestWSSInvalidCertAndGoForward, SSLUITest.TestWSSClientCert, and SSLUITestIgnoreCertErrors.TestWSS. +- split_packet_check.html : A page for testing split packet handling. Here, + packets mean TCP segments for WebSocket, or SSL records for secure + WebSocket. This page changes the title to "PASS" to notify + content::TitleWatcher. + Used by WebSocketBrowserTest.WebSocketSplitSegments and + WebSocketBrowserTest.SecureWebSocketSplitRecords. + - websocket_shared_worker.html : A page provides simple WebSocket test in shared worker. This page changes page title to "PASS" to notify content::TitleWatcher. @@ -31,6 +38,12 @@ Multiple tests may share one resource, or URI handler. connection with arbitrary code and reason. Used by kinds of PPAPI tests for WebSocket. +- close-with-split-packet_wsh.py : A WebSocket URL handler for testing split + packet handling. Here, packets mean TCP segments for WebSocket, or SSL + records for secure WebSocket. + Used by WebSocketBrowserTest.WebSocketSplitSegments and + WebSocketBrowserTest.SecureWebSocketSplitRecords. + - protocol-test_wsh.py : A WebSocket URL handler for testing outgoing opening handshake protocol. Used by kinds of PPAPI tests for WebSocket. diff --git a/net/data/websocket/close-with-split-packet_wsh.py b/net/data/websocket/close-with-split-packet_wsh.py new file mode 100644 index 0000000..d5185c4 --- /dev/null +++ b/net/data/websocket/close-with-split-packet_wsh.py @@ -0,0 +1,30 @@ +# Copyright (c) 2012 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. + +import struct + +from mod_pywebsocket import handshake +from mod_pywebsocket import stream + + +def web_socket_do_extra_handshake(_request): + pass + + +def web_socket_transfer_data(request): + line = request.ws_stream.receive_message() + if line is None: + return + if isinstance(line, unicode): + request.ws_stream.send_message(line, binary=False) + else: + request.ws_stream.send_message(line, binary=True) + + +def web_socket_passive_closing_handshake(request): + code = struct.pack('!H', 1000) + packet = stream.create_close_frame(code + 'split test'.encode('utf-8')) + request.connection.write(packet[:1]) + request.connection.write(packet[1:]) + raise handshake.AbortedByUserException('Abort the connection') diff --git a/net/data/websocket/split_packet_check.html b/net/data/websocket/split_packet_check.html new file mode 100644 index 0000000..8e273ec --- /dev/null +++ b/net/data/websocket/split_packet_check.html @@ -0,0 +1,37 @@ +<!DOCTYPE html> +<html> +<head> +<title>test ws split packet</title> +<script type="text/javascript"> + +var href = window.location.href; +var hostBegin = href.indexOf('/') + 2; +var hostEnd = href.lastIndexOf(':'); +var host = href.slice(hostBegin, hostEnd); +var portBegin = hostEnd + 1; +var portEnd = href.lastIndexOf('/'); +var port = href.slice(portBegin, portEnd); +var scheme = href.indexOf('https') >= 0 ? 'wss' : 'ws'; +var url = scheme + '://' + host + ':' + port + '/close-with-split-packet'; + +// Do connection test. +var ws = new WebSocket(url); + +ws.onopen = function() +{ + // Close WebSocket connection once it is established. + ws.close(); +} + +ws.onclose = function(event) +{ + // Check wasClean, then set proper title. + if (event.wasClean) + document.title = 'PASS'; + else + document.title = 'FAIL'; +} + +</script> +</head> +</html> diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc index 40ed798..1b33032 100644 --- a/net/socket/ssl_client_socket_nss.cc +++ b/net/socket/ssl_client_socket_nss.cc @@ -642,6 +642,11 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> { int Read(IOBuffer* buf, int buf_len, const CompletionCallback& callback); int Write(IOBuffer* buf, int buf_len, const CompletionCallback& callback); + // Called on the network task runner. + bool IsConnected(); + bool HasPendingAsyncOperation(); + bool HasUnhandledReceivedData(); + private: friend class base::RefCountedThreadSafe<Core>; ~Core(); @@ -774,6 +779,9 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> { void OnGetDomainBoundCertComplete(int result); void OnHandshakeStateUpdated(const HandshakeState& state); + void OnNSSBufferUpdated(int amount_in_read_buffer); + void DidNSSRead(int result); + void DidNSSWrite(int result); //////////////////////////////////////////////////////////////////////////// // Methods that are called on both the network task runner and the NSS @@ -817,6 +825,12 @@ class SSLClientSocketNSS::Core : public base::RefCountedThreadSafe<Core> { ServerBoundCertService* server_bound_cert_service_; ServerBoundCertService::RequestHandle domain_bound_cert_request_handle_; + // The information about NSS task runner. + int unhandled_buffer_size_; + bool nss_waiting_read_; + bool nss_waiting_write_; + bool nss_is_closed_; + //////////////////////////////////////////////////////////////////////////// // Members that are ONLY accessed on the NSS task runner: //////////////////////////////////////////////////////////////////////////// @@ -896,6 +910,10 @@ SSLClientSocketNSS::Core::Core( transport_(transport), weak_net_log_factory_(net_log), server_bound_cert_service_(server_bound_cert_service), + unhandled_buffer_size_(0), + nss_waiting_read_(false), + nss_waiting_write_(false), + nss_is_closed_(false), host_and_port_(host_and_port), ssl_config_(ssl_config), nss_fd_(NULL), @@ -1089,11 +1107,17 @@ int SSLClientSocketNSS::Core::Read(IOBuffer* buf, int buf_len, DCHECK(OnNetworkTaskRunner()); DCHECK(!detached_); DCHECK(transport_); + DCHECK(!nss_waiting_read_); + nss_waiting_read_ = true; bool posted = nss_task_runner_->PostTask( FROM_HERE, base::Bind(IgnoreResult(&Core::Read), this, make_scoped_refptr(buf), buf_len, callback)); + if (!posted) { + nss_is_closed_ = true; + nss_waiting_read_ = false; + } return posted ? ERR_IO_PENDING : ERR_ABORTED; } @@ -1110,14 +1134,21 @@ int SSLClientSocketNSS::Core::Read(IOBuffer* buf, int buf_len, int rv = DoReadLoop(OK); if (rv == ERR_IO_PENDING) { + if (OnNetworkTaskRunner()) + nss_waiting_read_ = true; user_read_callback_ = callback; } else { user_read_buf_ = NULL; user_read_buf_len_ = 0; if (!OnNetworkTaskRunner()) { + PostOrRunCallback(FROM_HERE, base::Bind(&Core::DidNSSRead, this, rv)); PostOrRunCallback(FROM_HERE, base::Bind(callback, rv)); return ERR_IO_PENDING; + } else { + DCHECK(!nss_waiting_read_); + if (rv <= 0) + nss_is_closed_ = true; } } @@ -1130,13 +1161,18 @@ int SSLClientSocketNSS::Core::Write(IOBuffer* buf, int buf_len, DCHECK(OnNetworkTaskRunner()); DCHECK(!detached_); DCHECK(transport_); + DCHECK(!nss_waiting_write_); + nss_waiting_write_ = true; bool posted = nss_task_runner_->PostTask( FROM_HERE, base::Bind(IgnoreResult(&Core::Write), this, make_scoped_refptr(buf), buf_len, callback)); - int rv = posted ? ERR_IO_PENDING : ERR_ABORTED; - return rv; + if (!posted) { + nss_is_closed_ = true; + nss_waiting_write_ = false; + } + return posted ? ERR_IO_PENDING : ERR_ABORTED; } DCHECK(OnNSSTaskRunner()); @@ -1152,20 +1188,42 @@ int SSLClientSocketNSS::Core::Write(IOBuffer* buf, int buf_len, int rv = DoWriteLoop(OK); if (rv == ERR_IO_PENDING) { + if (OnNetworkTaskRunner()) + nss_waiting_write_ = true; user_write_callback_ = callback; } else { user_write_buf_ = NULL; user_write_buf_len_ = 0; if (!OnNetworkTaskRunner()) { + PostOrRunCallback(FROM_HERE, base::Bind(&Core::DidNSSWrite, this, rv)); PostOrRunCallback(FROM_HERE, base::Bind(callback, rv)); return ERR_IO_PENDING; + } else { + DCHECK(!nss_waiting_write_); + if (rv < 0) + nss_is_closed_ = true; } } return rv; } +bool SSLClientSocketNSS::Core::IsConnected() { + DCHECK(OnNetworkTaskRunner()); + return !nss_is_closed_; +} + +bool SSLClientSocketNSS::Core::HasPendingAsyncOperation() { + DCHECK(OnNetworkTaskRunner()); + return nss_waiting_read_ || nss_waiting_write_; +} + +bool SSLClientSocketNSS::Core::HasUnhandledReceivedData() { + DCHECK(OnNetworkTaskRunner()); + return unhandled_buffer_size_ != 0; +} + bool SSLClientSocketNSS::Core::OnNSSTaskRunner() const { return nss_task_runner_->RunsTasksOnCurrentThread(); } @@ -2044,6 +2102,13 @@ int SSLClientSocketNSS::Core::DoPayloadRead() { DCHECK_GT(user_read_buf_len_, 0); int rv = PR_Read(nss_fd_, user_read_buf_->data(), user_read_buf_len_); + // Update NSSTaskRunner status because PR_Read may consume |nss_bufs_|, then + // following |amount_in_read_buffer| may be changed here. + int amount_in_read_buffer = memio_GetReadableBufferSize(nss_bufs_); + PostOrRunCallback( + FROM_HERE, + base::Bind(&Core::OnNSSBufferUpdated, this, amount_in_read_buffer)); + if (client_auth_cert_needed_) { // We don't need to invalidate the non-client-authenticated SSL session // because the server will renegotiate anyway. @@ -2081,7 +2146,17 @@ int SSLClientSocketNSS::Core::DoPayloadWrite() { DCHECK(user_write_buf_); + int old_amount_in_read_buffer = memio_GetReadableBufferSize(nss_bufs_); int rv = PR_Write(nss_fd_, user_write_buf_->data(), user_write_buf_len_); + int new_amount_in_read_buffer = memio_GetReadableBufferSize(nss_bufs_); + // PR_Write could potentially consume the unhandled data in the memio read + // buffer if a renegotiation is in progress. If the buffer is consumed, + // notify the latest buffer size to NetworkRunner. + if (old_amount_in_read_buffer != new_amount_in_read_buffer) { + PostOrRunCallback( + FROM_HERE, + base::Bind(&Core::OnNSSBufferUpdated, this, new_amount_in_read_buffer)); + } if (rv >= 0) { PostOrRunCallback( FROM_HERE, @@ -2287,10 +2362,18 @@ void SSLClientSocketNSS::Core::DoReadCallback(int rv) { user_read_buf_ = NULL; user_read_buf_len_ = 0; - base::Closure c = base::Bind( - base::ResetAndReturn(&user_read_callback_), - rv); - PostOrRunCallback(FROM_HERE, c); + int amount_in_read_buffer = memio_GetReadableBufferSize(nss_bufs_); + // This is used to curry the |amount_int_read_buffer| and |user_cb| back to + // the network task runner. + PostOrRunCallback( + FROM_HERE, + base::Bind(&Core::OnNSSBufferUpdated, this, amount_in_read_buffer)); + PostOrRunCallback( + FROM_HERE, + base::Bind(&Core::DidNSSRead, this, rv)); + PostOrRunCallback( + FROM_HERE, + base::Bind(base::ResetAndReturn(&user_read_callback_), rv)); } void SSLClientSocketNSS::Core::DoWriteCallback(int rv) { @@ -2302,10 +2385,20 @@ void SSLClientSocketNSS::Core::DoWriteCallback(int rv) { // up front. user_write_buf_ = NULL; user_write_buf_len_ = 0; - base::Closure c = base::Bind( - base::ResetAndReturn(&user_write_callback_), - rv); - PostOrRunCallback(FROM_HERE, c); + // Update buffer status because DoWriteLoop called DoTransportIO which may + // perform read operations. + int amount_in_read_buffer = memio_GetReadableBufferSize(nss_bufs_); + // This is used to curry the |amount_int_read_buffer| and |user_cb| back to + // the network task runner. + PostOrRunCallback( + FROM_HERE, + base::Bind(&Core::OnNSSBufferUpdated, this, amount_in_read_buffer)); + PostOrRunCallback( + FROM_HERE, + base::Bind(&Core::DidNSSWrite, this, rv)); + PostOrRunCallback( + FROM_HERE, + base::Bind(base::ResetAndReturn(&user_write_callback_), rv)); } SECStatus SSLClientSocketNSS::Core::ClientChannelIDHandler( @@ -2565,9 +2658,9 @@ int SSLClientSocketNSS::Core::DoBufferSend(IOBuffer* send_buffer, int len) { return ERR_ABORTED; int rv = transport_->socket()->Write( - send_buffer, len, - base::Bind(&Core::BufferSendComplete, - base::Unretained(this))); + send_buffer, len, + base::Bind(&Core::BufferSendComplete, + base::Unretained(this))); if (!OnNSSTaskRunner() && rv != ERR_IO_PENDING) { nss_task_runner_->PostTask( @@ -2610,9 +2703,31 @@ int SSLClientSocketNSS::Core::DoGetDomainBoundCert( void SSLClientSocketNSS::Core::OnHandshakeStateUpdated( const HandshakeState& state) { + DCHECK(OnNetworkTaskRunner()); network_handshake_state_ = state; } +void SSLClientSocketNSS::Core::OnNSSBufferUpdated(int amount_in_read_buffer) { + DCHECK(OnNetworkTaskRunner()); + unhandled_buffer_size_ = amount_in_read_buffer; +} + +void SSLClientSocketNSS::Core::DidNSSRead(int result) { + DCHECK(OnNetworkTaskRunner()); + DCHECK(nss_waiting_read_); + nss_waiting_read_ = false; + if (result <= 0) + nss_is_closed_ = true; +} + +void SSLClientSocketNSS::Core::DidNSSWrite(int result) { + DCHECK(OnNetworkTaskRunner()); + DCHECK(nss_waiting_write_); + nss_waiting_write_ = false; + if (result < 0) + nss_is_closed_ = true; +} + void SSLClientSocketNSS::Core::BufferSendComplete(int result) { if (!OnNSSTaskRunner()) { if (detached_) @@ -2922,29 +3037,21 @@ void SSLClientSocketNSS::Disconnect() { } bool SSLClientSocketNSS::IsConnected() const { - // Ideally, we should also check if we have received the close_notify alert - // message from the server, and return false in that case. We're not doing - // that, so this function may return a false positive. Since the upper - // layer (HttpNetworkTransaction) needs to handle a persistent connection - // closed by the server when we send a request anyway, a false positive in - // exchange for simpler code is a good trade-off. EnterFunction(""); - bool ret = completed_handshake_ && transport_->socket()->IsConnected(); + bool ret = completed_handshake_ && + (core_->HasPendingAsyncOperation() || + (core_->IsConnected() && core_->HasUnhandledReceivedData()) || + transport_->socket()->IsConnected()); LeaveFunction(""); return ret; } bool SSLClientSocketNSS::IsConnectedAndIdle() const { - // Unlike IsConnected, this method doesn't return a false positive. - // - // Strictly speaking, we should check if we have received the close_notify - // alert message from the server, and return false in that case. Although - // the close_notify alert message means EOF in the SSL layer, it is just - // bytes to the transport layer below, so - // transport_->socket()->IsConnectedAndIdle() returns the desired false - // when we receive close_notify. EnterFunction(""); - bool ret = completed_handshake_ && transport_->socket()->IsConnectedAndIdle(); + bool ret = completed_handshake_ && + !core_->HasPendingAsyncOperation() && + !(core_->IsConnected() && core_->HasUnhandledReceivedData()) && + transport_->socket()->IsConnectedAndIdle(); LeaveFunction(""); return ret; } |