Rewrite session cache in OpenSSL ports.

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index d8713f28f9e37e3fc48e0d94d38dba708b895380..718553c67978abcd6665896e349e0339497c4b4e 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -34,9 +34,9 @@
 #include "net/cert/x509_certificate_net_log_param.h"
 #include "net/cert/x509_util_openssl.h"
 #include "net/http/transport_security_state.h"
-#include "net/socket/ssl_session_cache_openssl.h"
 #include "net/ssl/scoped_openssl_types.h"
 #include "net/ssl/ssl_cert_request_info.h"
+#include "net/ssl/ssl_client_session_cache_openssl.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_info.h"
 
@@ -164,7 +164,7 @@ class SSLClientSocketOpenSSL::SSLContext {
  public:
   static SSLContext* GetInstance() { return Singleton<SSLContext>::get(); }
   SSL_CTX* ssl_ctx() { return ssl_ctx_.get(); }
-  SSLSessionCacheOpenSSL* session_cache() { return &session_cache_; }
+  SSLClientSessionCacheOpenSSL* session_cache() { return &session_cache_; }
 
   SSLClientSocketOpenSSL* GetClientSocketFromSSL(const SSL* ssl) {
     DCHECK(ssl);
@@ -181,12 +181,11 @@ class SSLClientSocketOpenSSL::SSLContext {
  private:
   friend struct DefaultSingletonTraits<SSLContext>;
 
-  SSLContext() {
+  SSLContext() : session_cache_(SSLClientSessionCacheOpenSSL::Config()) {
     crypto::EnsureOpenSSLInit();
     ssl_socket_data_index_ = SSL_get_ex_new_index(0, 0, 0, 0, 0);
     DCHECK_NE(ssl_socket_data_index_, -1);
     ssl_ctx_.reset(SSL_CTX_new(SSLv23_client_method()));
-    session_cache_.Reset(ssl_ctx_.get(), kDefaultSessionCacheConfig);
     SSL_CTX_set_cert_verify_callback(ssl_ctx_.get(), CertVerifyCallback, NULL);
     SSL_CTX_set_cert_cb(ssl_ctx_.get(), ClientCertRequestCallback, NULL);
     SSL_CTX_set_verify(ssl_ctx_.get(), SSL_VERIFY_PEER, NULL);
@@ -201,6 +200,15 @@ class SSLClientSocketOpenSSL::SSLContext {
                                      NULL);
     ssl_ctx_->tlsext_channel_id_enabled_new = 1;
 
+    // Set up the session cache. NO_INTERNAL_STORE disables OpenSSL's builtin
+    // cache, and NO_AUTO_CLEAR disables the call to SSL_CTX_flush_sessions
+    // every 256 connections. (This number is hard-coded in the library and
+    // can't be changed.)
+    SSL_CTX_set_session_cache_mode(ssl_ctx_.get(),
+                                   SSL_SESS_CACHE_CLIENT |
+                                       SSL_SESS_CACHE_NO_INTERNAL_STORE |
+                                       SSL_SESS_CACHE_NO_AUTO_CLEAR);
+
     scoped_ptr<base::Environment> env(base::Environment::Create());
     std::string ssl_keylog_file;
     if (env->GetVar("SSLKEYLOGFILE", &ssl_keylog_file) &&
@@ -216,14 +224,6 @@ class SSLClientSocketOpenSSL::SSLContext {
     }
   }
 
-  static std::string GetSessionCacheKey(const SSL* ssl) {
-    SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
-    DCHECK(socket);
-    return socket->GetSessionCacheKey();
-  }
-
-  static SSLSessionCacheOpenSSL::Config kDefaultSessionCacheConfig;
-
   static int ClientCertRequestCallback(SSL* ssl, void* arg) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     DCHECK(socket);
@@ -252,8 +252,13 @@ class SSLClientSocketOpenSSL::SSLContext {
   int ssl_socket_data_index_;
 
   ScopedSSL_CTX ssl_ctx_;
-  // |session_cache_| must be destroyed before |ssl_ctx_|.
-  SSLSessionCacheOpenSSL session_cache_;
+
+  // TODO(davidben): Use a separate cache per URLRequestContext.
+  // https://crbug.com/458365
+  //
+  // TODO(davidben): Sessions should be invalidated on fatal
+  // alerts. https://crbug.com/466352
+  SSLClientSessionCacheOpenSSL session_cache_;
 };
 
 // PeerCertificateChain is a helper object which extracts the certificate
@@ -338,15 +343,6 @@ SSLClientSocketOpenSSL::PeerCertificateChain::AsOSChain() const {
 }
 
 // static
-SSLSessionCacheOpenSSL::Config
-    SSLClientSocketOpenSSL::SSLContext::kDefaultSessionCacheConfig = {
-        &GetSessionCacheKey,  // key_func
-        1024,                 // max_entries
-        256,                  // expiration_check_count
-        60 * 60,              // timeout_seconds
-};
-
-// static
 void SSLClientSocket::ClearSessionCache() {
   SSLClientSocketOpenSSL::SSLContext* context =
       SSLClientSocketOpenSSL::SSLContext::GetInstance();
@@ -387,7 +383,7 @@ SSLClientSocketOpenSSL::SSLClientSocketOpenSSL(
       npn_status_(kNextProtoUnsupported),
       channel_id_xtn_negotiated_(false),
       handshake_succeeded_(false),
-      marked_session_as_good_(false),
+      certificate_verified_(false),
       transport_security_state_(context.transport_security_state),
       policy_enforcer_(context.cert_policy_enforcer),
       net_log_(transport_->socket()->NetLog()),
@@ -429,7 +425,7 @@ std::string SSLClientSocketOpenSSL::GetSessionCacheKey() const {
 bool SSLClientSocketOpenSSL::InSessionCache() const {
   SSLContext* context = SSLContext::GetInstance();
   std::string cache_key = GetSessionCacheKey();
-  return context->session_cache()->SSLSessionIsInCache(cache_key);
+  return context->session_cache()->Lookup(cache_key) != nullptr;
 }
 
 void SSLClientSocketOpenSSL::SetHandshakeCompletionCallback(
@@ -759,8 +755,11 @@ int SSLClientSocketOpenSSL::Init() {
   // 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());
+  SSL_SESSION* session = context->session_cache()->Lookup(GetSessionCacheKey());
+  if (session != nullptr) {
+    SSL_set_session(ssl_, session);
+    trying_cached_session_ = true;
+  }
 
   send_buffer_ = new GrowableIOBuffer();
   send_buffer_->SetCapacity(KDefaultOpenSSLBufferSize);
@@ -1256,10 +1255,8 @@ int SSLClientSocketOpenSSL::DoVerifyCertComplete(int result) {
     // the connection.
     VerifyCT();
 
-    // 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;
+    DCHECK(!certificate_verified_);
+    certificate_verified_ = true;
     CheckIfHandshakeFinished();
   } else {
     DVLOG(1) << "DoVerifyCertComplete error " << ErrorToString(result)
@@ -2003,8 +2000,10 @@ void SSLClientSocketOpenSSL::InfoCallback(const SSL* ssl,
   if (type == SSL_CB_HANDSHAKE_DONE) {
     SSLClientSocketOpenSSL* ssl_socket =
         SSLContext::GetInstance()->GetClientSocketFromSSL(ssl);
-    ssl_socket->handshake_succeeded_ = true;
-    ssl_socket->CheckIfHandshakeFinished();
+    if (!ssl_socket->handshake_succeeded_) {
+      ssl_socket->handshake_succeeded_ = true;
+      ssl_socket->CheckIfHandshakeFinished();
+    }
   }
 }
 
@@ -2018,8 +2017,15 @@ void SSLClientSocketOpenSSL::InfoCallback(const SSL* ssl,
 // 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 (handshake_succeeded_ && marked_session_as_good_)
+  if (handshake_succeeded_ && certificate_verified_) {
+    if (!SSL_session_reused(ssl_)) {

[Ryan Sleevi, 2015/03/17 00:50:33]

Why is this check needed? Isn't it a nop if it exists? Doesn't this prevent
refreshing the MRU of the session?

[davidben, 2015/03/20 22:41:26]

I guess it's not strictly needed? MRU is updated on lookup, and this is
consistent with the old code. (ssl_update_cache only calls new_session_cb if
!s->hit.)

+      // Only insert the session into the session cache once both the handshake
+      // has completed and the certificate has been verified.
+      SSLContext::GetInstance()->session_cache()->Insert(GetSessionCacheKey(),
+                                                         SSL_get_session(ssl_));
+    }
     OnHandshakeCompletion();
+  }
 }
 
 void SSLClientSocketOpenSSL::AddSCTInfoToSSLInfo(SSLInfo* ssl_info) const {