diff options
| -rw-r--r-- | net/socket/ssl_client_socket_nss.cc | 1 | ||||
| -rw-r--r-- | net/socket/ssl_client_socket_openssl.cc | 42 | ||||
| -rw-r--r-- | net/socket/ssl_client_socket_openssl.h | 11 | ||||
| -rw-r--r-- | net/socket/ssl_client_socket_unittest.cc | 39 | ||||
| -rw-r--r-- | net/socket/ssl_session_cache_openssl.cc | 56 | ||||
| -rw-r--r-- | net/socket/ssl_session_cache_openssl.h | 7 | ||||
| -rw-r--r-- | net/test/spawned_test_server/base_test_server.cc | 10 | ||||
| -rw-r--r-- | net/test/spawned_test_server/base_test_server.h | 4 | ||||
| -rwxr-xr-x | net/tools/testserver/testserver.py | 15 |
9 files changed, 104 insertions, 81 deletions
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc index 0237eb5..0d3c53d 100644 --- a/net/socket/ssl_client_socket_nss.cc +++ b/net/socket/ssl_client_socket_nss.cc @@ -2843,6 +2843,7 @@ void SSLClientSocketNSS::SetHandshakeCompletionCallback( const base::Closure& callback) { NOTIMPLEMENTED(); } + void SSLClientSocketNSS::GetSSLCertRequestInfo( SSLCertRequestInfo* cert_request_info) { EnterFunction(""); diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc index a6f5caf..4d108fc 100644 --- a/net/socket/ssl_client_socket_openssl.cc +++ b/net/socket/ssl_client_socket_openssl.cc @@ -363,6 +363,8 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL( next_handshake_state_(STATE_NONE), npn_status_(kNextProtoUnsupported), channel_id_xtn_negotiated_(false), + ran_handshake_finished_callback_(false), + marked_session_as_good_(false), net_log_(transport_->socket()->NetLog()) { } @@ -435,14 +437,6 @@ int SSLClientSocketOpenSSL::Connect(const CompletionCallback& callback) { return rv; } - if (!handshake_completion_callback_.is_null()) { - SSLContext* context = SSLContext::GetInstance(); - context->session_cache()->SetSessionAddedCallback( - ssl_, - base::Bind(&SSLClientSocketOpenSSL::OnHandshakeCompletion, - base::Unretained(this))); - } - // Set SSL to client mode. Handshake happens in the loop below. SSL_set_connect_state(ssl_); @@ -465,8 +459,6 @@ void SSLClientSocketOpenSSL::Disconnect() { // completed, this is a no-op. OnHandshakeCompletion(); if (ssl_) { - SSLContext* context = SSLContext::GetInstance(); - context->session_cache()->RemoveSessionAddedCallback(ssl_); // Calling SSL_shutdown prevents the session from being marked as // unresumable. SSL_shutdown(ssl_); @@ -687,6 +679,18 @@ int SSLClientSocketOpenSSL::SetSendBufferSize(int32 size) { return transport_->socket()->SetSendBufferSize(size); } +// static +void SSLClientSocketOpenSSL::InfoCallback(const SSL* ssl, + int result, + int /*unused*/) { + SSLClientSocketOpenSSL* ssl_socket = + SSLContext::GetInstance()->GetClientSocketFromSSL(ssl); + if (result == SSL_CB_HANDSHAKE_DONE) { + ssl_socket->ran_handshake_finished_callback_ = true; + ssl_socket->CheckIfHandshakeFinished(); + } +} + int SSLClientSocketOpenSSL::Init() { DCHECK(!ssl_); DCHECK(!transport_bio_); @@ -701,6 +705,9 @@ int SSLClientSocketOpenSSL::Init() { if (!SSL_set_tlsext_host_name(ssl_, host_and_port_.host().c_str())) return ERR_UNEXPECTED; + // Set an OpenSSL callback to monitor this SSL*'s connection. + SSL_set_info_callback(ssl_, &InfoCallback); + trying_cached_session_ = context->session_cache()->SetSSLSessionWithKey( ssl_, GetSessionCacheKey()); @@ -1032,6 +1039,8 @@ int SSLClientSocketOpenSSL::DoVerifyCertComplete(int result) { // TODO(joth): Work out if we need to remember the intermediate CA certs // when the server sends them to us, and do so here. SSLContext::GetInstance()->session_cache()->MarkSSLSessionAsGood(ssl_); + marked_session_as_good_ = true; + CheckIfHandshakeFinished(); } else { DVLOG(1) << "DoVerifyCertComplete error " << ErrorToString(result) << " (" << result << ")"; @@ -1577,6 +1586,19 @@ long SSLClientSocketOpenSSL::MaybeReplayTransportError( return retvalue; } +// Determines if the session for |ssl_| is in the cache, and calls the +// handshake completion callback if that is the case. +// +// CheckIfHandshakeFinished is called twice per connection: once after +// MarkSSLSessionAsGood, when the certificate has been verified, and +// once via an OpenSSL callback when the handshake has completed. On the +// second call, when the certificate has been verified and the handshake +// has completed, the connection's handshake completion callback is run. +void SSLClientSocketOpenSSL::CheckIfHandshakeFinished() { + if (ran_handshake_finished_callback_ && marked_session_as_good_) + OnHandshakeCompletion(); +} + // static long SSLClientSocketOpenSSL::BIOCallback( BIO *bio, diff --git a/net/socket/ssl_client_socket_openssl.h b/net/socket/ssl_client_socket_openssl.h index ac65710..3d2194b 100644 --- a/net/socket/ssl_client_socket_openssl.h +++ b/net/socket/ssl_client_socket_openssl.h @@ -106,6 +106,10 @@ class SSLClientSocketOpenSSL : public SSLClientSocket { friend class SSLClientSocket; friend class SSLContext; + // Callback that is run by OpenSSL to obtain information about the + // state of the SSL handshake. + static void InfoCallback(const SSL* ssl, int result, int unused); + int Init(); void DoReadCallback(int result); void DoWriteCallback(int result); @@ -168,6 +172,8 @@ class SSLClientSocketOpenSSL : public SSLClientSocket { const char *argp, int argi, long argl, long retvalue); + void CheckIfHandshakeFinished(); + bool transport_send_busy_; bool transport_recv_busy_; @@ -268,6 +274,11 @@ class SSLClientSocketOpenSSL : public SSLClientSocket { std::string channel_id_cert_; // True if channel ID extension was negotiated. bool channel_id_xtn_negotiated_; + // True if InfoCallback has been run with result = SSL_CB_HANDSHAKE_DONE. + bool ran_handshake_finished_callback_; + // True if MarkSSLSessionAsGood has been called for this socket's + // connection's SSL session. + bool marked_session_as_good_; // The request handle for |channel_id_service_|. ChannelIDService::RequestHandle channel_id_request_handle_; BoundNetLog net_log_; diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc index cdb2685..6789d67 100644 --- a/net/socket/ssl_client_socket_unittest.cc +++ b/net/socket/ssl_client_socket_unittest.cc @@ -2721,9 +2721,6 @@ TEST_F(SSLClientSocketTest, HandshakeCallbackIsRun_WithSuccess) { sock->SetHandshakeCompletionCallback(base::Bind( &SSLClientSocketTest::RecordCompletedHandshake, base::Unretained(this))); - if (sock->IsConnected()) - LOG(ERROR) << "SSL Socket prematurely connected"; - rv = callback.GetResult(sock->Connect(callback.callback())); EXPECT_EQ(OK, rv); @@ -2731,6 +2728,42 @@ TEST_F(SSLClientSocketTest, HandshakeCallbackIsRun_WithSuccess) { EXPECT_TRUE(ran_handshake_completion_callback_); } +// Tests that the completion callback is run with connections +// that do not cache their session. +TEST_F(SSLClientSocketTest, HandshakeCallbackIsRun_WithDisabledSessionCache) { + SpawnedTestServer::SSLOptions ssl_options; + ssl_options.disable_session_cache = true; + SpawnedTestServer test_server( + SpawnedTestServer::TYPE_HTTPS, ssl_options, base::FilePath()); + ASSERT_TRUE(test_server.Start()); + + AddressList addr; + ASSERT_TRUE(test_server.GetAddressList(&addr)); + + scoped_ptr<StreamSocket> transport( + new TCPClientSocket(addr, NULL, NetLog::Source())); + + TestCompletionCallback callback; + int rv = transport->Connect(callback.callback()); + if (rv == ERR_IO_PENDING) + rv = callback.WaitForResult(); + EXPECT_EQ(OK, rv); + + SSLConfig ssl_config = kDefaultSSLConfig; + ssl_config.false_start_enabled = false; + + scoped_ptr<SSLClientSocket> sock(CreateSSLClientSocket( + transport.Pass(), test_server.host_port_pair(), ssl_config)); + + sock->SetHandshakeCompletionCallback(base::Bind( + &SSLClientSocketTest::RecordCompletedHandshake, base::Unretained(this))); + + rv = callback.GetResult(sock->Connect(callback.callback())); + + EXPECT_EQ(OK, rv); + EXPECT_TRUE(sock->IsConnected()); + EXPECT_TRUE(ran_handshake_completion_callback_); +} #endif // defined(USE_OPENSSL) TEST_F(SSLClientSocketFalseStartTest, FalseStartEnabled) { diff --git a/net/socket/ssl_session_cache_openssl.cc b/net/socket/ssl_session_cache_openssl.cc index 5bb0d16..2c19c60 100644 --- a/net/socket/ssl_session_cache_openssl.cc +++ b/net/socket/ssl_session_cache_openssl.cc @@ -253,31 +253,6 @@ class SSLSessionCacheOpenSSLImpl { return session_is_good; } - void SetSessionAddedCallback(SSL* ssl, const base::Closure& callback) { - // Add |ssl| to the SSLToCallbackMap. - ssl_to_callback_map_.insert(SSLToCallbackMap::value_type( - ssl, CallbackAndCompletionCount(callback, 0))); - } - - // Determines if the session for |ssl| is in the cache, and calls the - // appropriate callback if that is the case. - // - // The session must be both MarkedAsGood and Added in order for the - // callback to be run. These two events can occur in either order. - void CheckIfSessionFinished(SSL* ssl) { - SSLToCallbackMap::iterator it = ssl_to_callback_map_.find(ssl); - if (it == ssl_to_callback_map_.end()) - return; - // Increment the session's completion count. - if (++it->second.count == 2) { - base::Closure callback = it->second.callback; - ssl_to_callback_map_.erase(it); - callback.Run(); - } - } - - void RemoveSessionAddedCallback(SSL* ssl) { ssl_to_callback_map_.erase(ssl); } - void MarkSSLSessionAsGood(SSL* ssl) { SSL_SESSION* session = SSL_get_session(ssl); CHECK(session); @@ -285,8 +260,6 @@ class SSLSessionCacheOpenSSLImpl { // Mark the session as good, allowing it to be used for future connections. SSL_SESSION_set_ex_data( session, GetSSLSessionExIndex(), reinterpret_cast<void*>(1)); - - CheckIfSessionFinished(ssl); } // Flush all entries from the cache. @@ -302,30 +275,12 @@ class SSLSessionCacheOpenSSLImpl { } private: - // CallbackAndCompletionCounts are used to group a callback that should be - // run when a certain sesssion is added to the session cache with an integer - // indicating the status of that session. - struct CallbackAndCompletionCount { - CallbackAndCompletionCount(const base::Closure& completion_callback, - int completion_count) - : callback(completion_callback), count(completion_count) {} - - const base::Closure callback; - // |count| < 2 means that the ssl session associated with this object - // has not been added to the session cache or has not been marked as good. - // |count| is incremented when a session is added to the cache or marked as - // good, thus |count| == 2 means that the session is ready for use. - int count; - }; - // Type for list of SSL_SESSION handles, ordered in MRU order. typedef std::list<SSL_SESSION*> MRUSessionList; // Type for a dictionary from unique cache keys to session list nodes. typedef base::hash_map<std::string, MRUSessionList::iterator> KeyIndex; // Type for a dictionary from SessionId values to key index nodes. typedef base::hash_map<SessionId, KeyIndex::iterator> SessionIdIndex; - // Type for a map from SSL* to associated callbacks - typedef std::map<SSL*, CallbackAndCompletionCount> SSLToCallbackMap; // Return the key associated with a given session, or the empty string if // none exist. This shall only be used for debugging. @@ -405,7 +360,6 @@ class SSLSessionCacheOpenSSLImpl { static int NewSessionCallbackStatic(SSL* ssl, SSL_SESSION* session) { SSLSessionCacheOpenSSLImpl* cache = GetCache(ssl->ctx); cache->OnSessionAdded(ssl, session); - cache->CheckIfSessionFinished(ssl); return 1; } @@ -529,7 +483,6 @@ class SSLSessionCacheOpenSSLImpl { SSL_CTX* ctx_; SSLSessionCacheOpenSSL::Config config_; - SSLToCallbackMap ssl_to_callback_map_; // method to get the index which can later be used with SSL_CTX_get_ex_data() // or SSL_CTX_set_ex_data(). @@ -568,15 +521,6 @@ bool SSLSessionCacheOpenSSL::SSLSessionIsInCache( return impl_->SSLSessionIsInCache(cache_key); } -void SSLSessionCacheOpenSSL::RemoveSessionAddedCallback(SSL* ssl) { - impl_->RemoveSessionAddedCallback(ssl); -} - -void SSLSessionCacheOpenSSL::SetSessionAddedCallback(SSL* ssl, - const base::Closure& cb) { - impl_->SetSessionAddedCallback(ssl, cb); -} - void SSLSessionCacheOpenSSL::MarkSSLSessionAsGood(SSL* ssl) { return impl_->MarkSSLSessionAsGood(ssl); } diff --git a/net/socket/ssl_session_cache_openssl.h b/net/socket/ssl_session_cache_openssl.h index 00f93e1..8c2ed4f 100644 --- a/net/socket/ssl_session_cache_openssl.h +++ b/net/socket/ssl_session_cache_openssl.h @@ -117,13 +117,6 @@ class NET_EXPORT SSLSessionCacheOpenSSL { // Return true iff a cached session was associated with the given |cache_key|. bool SSLSessionIsInCache(const std::string& cache_key) const; - // Informs the cache that it should run a callback when |ssl|'s session is - // added to the cache. - void SetSessionAddedCallback(SSL* ssl, const base::Closure& callback); - - // Removes the entry for |ssl| from cache's callback map. - void RemoveSessionAddedCallback(SSL* ssl); - // Indicates that the SSL session associated with |ssl| is "good" - that is, // that all associated cryptographic parameters that were negotiated, // including the peer's certificate, were successfully validated. Because diff --git a/net/test/spawned_test_server/base_test_server.cc b/net/test/spawned_test_server/base_test_server.cc index 26608c6..5cc6eb6 100644 --- a/net/test/spawned_test_server/base_test_server.cc +++ b/net/test/spawned_test_server/base_test_server.cc @@ -101,7 +101,9 @@ BaseTestServer::SSLOptions::SSLOptions() tls_intolerance_type(TLS_INTOLERANCE_ALERT), fallback_scsv_enabled(false), staple_ocsp_response(false), - enable_npn(false) {} + enable_npn(false), + disable_session_cache(false) { +} BaseTestServer::SSLOptions::SSLOptions( BaseTestServer::SSLOptions::ServerCertificate cert) @@ -116,7 +118,9 @@ BaseTestServer::SSLOptions::SSLOptions( tls_intolerance_type(TLS_INTOLERANCE_ALERT), fallback_scsv_enabled(false), staple_ocsp_response(false), - enable_npn(false) {} + enable_npn(false), + disable_session_cache(false) { +} BaseTestServer::SSLOptions::~SSLOptions() {} @@ -474,6 +478,8 @@ bool BaseTestServer::GenerateArguments(base::DictionaryValue* arguments) const { arguments->Set("staple-ocsp-response", base::Value::CreateNullValue()); if (ssl_options_.enable_npn) arguments->Set("enable-npn", base::Value::CreateNullValue()); + if (ssl_options_.disable_session_cache) + arguments->Set("disable-session-cache", base::Value::CreateNullValue()); } return GenerateAdditionalArguments(arguments); diff --git a/net/test/spawned_test_server/base_test_server.h b/net/test/spawned_test_server/base_test_server.h index 6dd67db..fb71222 100644 --- a/net/test/spawned_test_server/base_test_server.h +++ b/net/test/spawned_test_server/base_test_server.h @@ -203,6 +203,10 @@ class BaseTestServer { // Whether to enable NPN support. bool enable_npn; + + // Whether to disable TLS session caching. When session caching is + // disabled, the server will generate a new Session ID for every connection. + bool disable_session_cache; }; // Pass as the 'host' parameter during construction to server on 127.0.0.1 diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py index 60b7b28..9babbb2 100755 --- a/net/tools/testserver/testserver.py +++ b/net/tools/testserver/testserver.py @@ -157,7 +157,7 @@ class HTTPSServer(tlslite.api.TLSSocketServerMixIn, ssl_bulk_ciphers, ssl_key_exchanges, enable_npn, record_resume_info, tls_intolerant, tls_intolerance_type, signed_cert_timestamps, - fallback_scsv_enabled, ocsp_response): + fallback_scsv_enabled, ocsp_response, disable_session_cache): self.cert_chain = tlslite.api.X509CertChain() self.cert_chain.parsePemList(pem_cert_and_key) # Force using only python implementation - otherwise behavior is different @@ -201,7 +201,10 @@ class HTTPSServer(tlslite.api.TLSSocketServerMixIn, self.ssl_handshake_settings.tlsIntolerant = (3, tls_intolerant) self.ssl_handshake_settings.tlsIntoleranceType = tls_intolerance_type - if record_resume_info: + + if disable_session_cache: + self.session_cache = None + elif record_resume_info: # If record_resume_info is true then we'll replace the session cache with # an object that records the lookups and inserts that it sees. self.session_cache = RecordingSSLSessionCache() @@ -1986,7 +1989,8 @@ class ServerRunner(testserver_base.TestServerRunner): self.options.signed_cert_timestamps_tls_ext.decode( "base64"), self.options.fallback_scsv, - stapled_ocsp_response) + stapled_ocsp_response, + self.options.disable_session_cache) print 'HTTPS server started on https://%s:%d...' % \ (host, server.server_port) else: @@ -2086,6 +2090,11 @@ class ServerRunner(testserver_base.TestServerRunner): def add_options(self): testserver_base.TestServerRunner.add_options(self) + self.option_parser.add_option('--disable-session-cache', + action='store_true', + dest='disable_session_cache', + help='tells the server to disable the' + 'TLS session cache.') self.option_parser.add_option('-f', '--ftp', action='store_const', const=SERVER_FTP, default=SERVER_HTTP, dest='server_type', |