This CL corrects a bug in which the OnHandshakeComplete callback for an ssl session was never
called if that session had an empty session id (i.e. the session wasn't added to the cache).
This CL sets the OpenSSL info_callback such that CheckIfSessionAdded will be run whenever
an OpenSSL session has finished its handshake. Previously, CheckIfSessionAdded was called in
NewSessionCallbackStatic. NewSessionCallbackStatic is only called when a session is
added to the cache. Thus, leading connections with sessions that were not added to the cache would
force their pending jobs to wait indefinitely instead of letting the jobs proceed at the completion
of the leading job's connection.
[email protected],[email protected],[email protected], [email protected]
BUG=398967
Review URL: https://codereview.chromium.org/416683002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288024 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index a6f5caf..4d108fc9 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -363,6 +363,8 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
// 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 @@
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,
