Rewrite session cache in OpenSSL ports.

net/socket/ssl_client_socket_openssl.cc

 // 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.
 
 // OpenSSL binding for SSLClientSocket. The class layout and general principle
 // of operation is derived from SSLClientSocketNSS.
 
 #include "net/socket/ssl_client_socket_openssl.h"
 
 #include <errno.h>
@@ skipping 16 matching lines @@
 #include "crypto/scoped_openssl_types.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_policy_enforcer.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/ct_ev_whitelist.h"
 #include "net/cert/ct_verifier.h"
 #include "net/cert/single_request_cert_verifier.h"
 #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"
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
 #endif
 
 #if defined(USE_OPENSSL_CERTS)
 #include "net/ssl/openssl_client_key_store.h"
 #else
@@ skipping 107 matching lines @@
   return false;
 #endif
 }
 
 }  // namespace
 
 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);
     SSLClientSocketOpenSSL* socket = static_cast<SSLClientSocketOpenSSL*>(
         SSL_get_ex_data(ssl, ssl_socket_data_index_));
     DCHECK(socket);
     return socket;
   }
 
   bool SetClientSocketForSSL(SSL* ssl, SSLClientSocketOpenSSL* socket) {
     return SSL_set_ex_data(ssl, ssl_socket_data_index_, socket) != 0;
   }
 
  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);
     // This stops |SSL_shutdown| from generating the close_notify message, which
     // is currently not sent on the network.
     // TODO(haavardm): Remove setting quiet shutdown once 118366 is fixed.
     SSL_CTX_set_quiet_shutdown(ssl_ctx_.get(), 1);
     // TODO(kristianm): Only select this if ssl_config_.next_proto is not empty.
     // It would be better if the callback were not a global setting,
     // but that is an OpenSSL issue.
     SSL_CTX_set_next_proto_select_cb(ssl_ctx_.get(), SelectNextProtoCallback,
                                      NULL);
     ssl_ctx_->tlsext_channel_id_enabled_new = 1;
+    SSL_CTX_set_info_callback(ssl_ctx_.get(), InfoCallback);
+
+    // 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.

[Ryan Sleevi, 2015/03/24 23:47:21]

Is there any automated pruning of expired sessions now? Or if I leave Chrome
running for a day, don't touch it, then come back, *all* the sessions will be
expired-but-cached?

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

That comment and call is the same as the file from before. Looking at it again,
the comment is confusing and the call is almost completely useless:

- NO_AUTO_CLEAR is a no-op if you have NO_INTERNAL_STORE. It will try to do
something (though I have a CL not yet rolled into DEPS to make it actually not
do anything), but it's moot because OpenSSL has nothing to iterate over.

- Since we only use this on the client, both NO_AUTO_CLEAR and NO_INTERNAL_STORE
are no-ops unless SSL_SESS_CACHE_CLIENT is set.

- If SSL_SESS_CACHE_CLIENT is not set, neither the internal cache nor the
SSL_CTX-level cache hooks are used. But it doesn't matter because we're not
using them.

- The default is SSL_SESS_CACHE_SERVER without SSL_SESS_CACHE_CLIENT.

- Now that I look at the code, there seems to be a bug on the server end where
SSL_SESS_CACHE_SERVER is only consulted on lookup and not install. Ooops.

- There's also NO_INTERNAL_LOOKUP which is only meaningful on the server's end.
Even when using the internal session cache, OpenSSL expects the caller to
perform the lookup because OpenSSL knows nothing about session cache keys.

- Basically this API is confusing, the previous SSLSessionCacheOpenSSL was
somewhat botched, and, as always with OpenSSL, the only way to have any clue
what's going on is to read the source. :-)

The upshot is we could remove that line altogether and everything would still
work.

I've replaced it with SSL_SESS_CACHE_CLIENT|SSL_SESS_CACHE_NO_INTERNAL. I think
that's a good mix of not being overly confusing or reliant on whatever the
default session cache mode happens to be. SSL_SESS_CACHE_OFF (0) would work just
as well, but it seems slightly confusing there; I could imagine the API being
defined to, say, disable requesting session tickets altogether. (Although it
does not do such a thing.) NO_INTERNAL is NO_INTERNAL_STORE +
NO_INTERNAL_LOOKUP.


The pruning happens every 256 lookups into the cache. This is the same as the
current code. It is also roughly the same as the even older code that used the
internal session cache. It may be worth revising this[*], but I've left it alone
for now. My main goal was to fix the explosions if you cache two sessions with
the same id. That bug's been sitting around for a while and is probably a remote
DoS of some sort, though I haven't analyzed it carefully.

I gone ahead and made a slight improvement over the old code which is that it
will also check for expiration before returning anything in Lookup. So at least
it behaves as if it constantly checked expiration, but for size() being off.

[*] I could imagine keeping a list of the sessions in order of expiration time
to avoid having the magic 256 parameter.

+    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) &&
         !ssl_keylog_file.empty()) {
       crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
       BIO* bio = BIO_new_file(ssl_keylog_file.c_str(), "a");
       if (!bio) {
         LOG(ERROR) << "Failed to open " << ssl_keylog_file;
         ERR_print_errors_cb(&LogErrorCallback, NULL);
       } else {
         SSL_CTX_set_keylog_bio(ssl_ctx_.get(), bio);
       }
     }
   }
 
-  static std::string GetSessionCacheKey(const SSL* ssl) {
-    SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
-    CHECK(socket);
-    return socket->GetSessionCacheKey();
-  }
-
-  static SSLSessionCacheOpenSSL::Config kDefaultSessionCacheConfig;
-
   static int ClientCertRequestCallback(SSL* ssl, void* arg) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     DCHECK(socket);
     return socket->ClientCertRequestCallback(ssl);
   }
 
   static int CertVerifyCallback(X509_STORE_CTX *store_ctx, void *arg) {
     SSL* ssl = reinterpret_cast<SSL*>(X509_STORE_CTX_get_ex_data(
         store_ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     CHECK(socket);
 
     return socket->CertVerifyCallback(store_ctx);
   }
 
   static int SelectNextProtoCallback(SSL* ssl,
                                      unsigned char** out, unsigned char* outlen,
                                      const unsigned char* in,
                                      unsigned int inlen, void* arg) {
     SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
     return socket->SelectNextProtoCallback(out, outlen, in, inlen);
   }
 
+  static void InfoCallback(const SSL* ssl, int type, int val) {
+    SSLClientSocketOpenSSL* socket = GetInstance()->GetClientSocketFromSSL(ssl);
+    socket->InfoCallback(type, val);
+  }
+
   // This is the index used with SSL_get_ex_data to retrieve the owner
   // SSLClientSocketOpenSSL object from an SSL instance.
   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
 // chain, as given by the server, from an OpenSSL socket and performs the needed
 // resource management. The first element of the chain is the leaf certificate
 // and the other elements are in the order given by the server.
 class SSLClientSocketOpenSSL::PeerCertificateChain {
  public:
   explicit PeerCertificateChain(STACK_OF(X509)* chain) { Reset(chain); }
   PeerCertificateChain(const PeerCertificateChain& other) { *this = other; }
@@ skipping 64 matching lines @@
     if (!x509_util::GetDER(x, &der))
       return NULL;
     der_chain.push_back(der);
   }
 
   return make_scoped_refptr(X509Certificate::CreateFromDERCertChain(der_chain));
 #endif
 }
 
 // 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();
   context->session_cache()->Flush();
 }
 
 // static
 uint16 SSLClientSocket::GetMaxSupportedSSLVersion() {
   return SSL_PROTOCOL_VERSION_TLS1_2;
 }
@@ skipping 15 matching lines @@
       client_auth_cert_needed_(false),
       cert_verifier_(context.cert_verifier),
       cert_transparency_verifier_(context.cert_transparency_verifier),
       channel_id_service_(context.channel_id_service),
       ssl_(NULL),
       transport_bio_(NULL),
       transport_(transport_socket.Pass()),
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
       ssl_session_cache_shard_(context.ssl_session_cache_shard),
-      trying_cached_session_(false),
       next_handshake_state_(STATE_NONE),
       npn_status_(kNextProtoUnsupported),
       channel_id_xtn_negotiated_(false),
+      handshake_completed_(false),
+      certificate_verified_(false),
       transport_security_state_(context.transport_security_state),
       policy_enforcer_(context.cert_policy_enforcer),
       net_log_(transport_->socket()->NetLog()),
       weak_factory_(this) {
 }
 
 SSLClientSocketOpenSSL::~SSLClientSocketOpenSSL() {
   Disconnect();
 }
 
@@ skipping 293 matching lines @@
   SSLContext* context = SSLContext::GetInstance();
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
 
   ssl_ = SSL_new(context->ssl_ctx());
   if (!ssl_ || !context->SetClientSocketForSSL(ssl_, this))
     return ERR_UNEXPECTED;
 
   if (!SSL_set_tlsext_host_name(ssl_, host_and_port_.host().c_str()))
     return ERR_UNEXPECTED;
 
-  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);
 
   send_buffer_ = new GrowableIOBuffer();
   send_buffer_->SetCapacity(KDefaultOpenSSLBufferSize);
   recv_buffer_ = new GrowableIOBuffer();
   recv_buffer_->SetCapacity(KDefaultOpenSSLBufferSize);
 
   BIO* ssl_bio = NULL;
 
   // SSLClientSocketOpenSSL retains ownership of the BIO buffers.
   if (!BIO_new_bio_pair_external_buf(
@@ skipping 231 matching lines @@
         int rv = SSL_CTX_remove_session(SSL_get_SSL_CTX(ssl_), session);
         LOG_IF(WARNING, !rv) << "Couldn't invalidate SSL session: " << session;
       }
     }
   } else if (rv == 1) {
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed.
     tracked_objects::ScopedTracker tracking_profile3(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "424386 SSLClientSocketOpenSSL::DoHandshake3"));
 
-    if (trying_cached_session_ && logging::DEBUG_MODE) {
-      DVLOG(2) << "Result of session reuse for " << host_and_port_.ToString()
-               << " is: " << (SSL_session_reused(ssl_) ? "Success" : "Fail");
-    }
-
     if (ssl_config_.version_fallback &&
         ssl_config_.version_max < ssl_config_.version_fallback_min) {
       return ERR_SSL_FALLBACK_BEYOND_MINIMUM_VERSION;
     }
 
     // SSL handshake is completed. If NPN wasn't negotiated, see if ALPN was.
     if (npn_status_ == kNextProtoUnsupported) {
       const uint8_t* alpn_proto = NULL;
       unsigned alpn_len = 0;
       SSL_get0_alpn_selected(ssl_, &alpn_proto, &alpn_len);
@@ skipping 204 matching lines @@
           server_cert_verify_result_.public_key_hashes,
           &pinning_failure_log_)) {
     result = ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN;
   }
 
   if (result == OK) {
     // Only check Certificate Transparency if there were no other errors with
     // 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_);
+    DCHECK(!certificate_verified_);
+    certificate_verified_ = true;
+    MaybeCacheSession();
   } else {
     DVLOG(1) << "DoVerifyCertComplete error " << ErrorToString(result)
              << " (" << result << ")";
   }
 
   completed_connect_ = true;
   // Exit DoHandshakeLoop and return the result to the caller to Connect.
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
   return result;
 }
@@ skipping 718 matching lines @@
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "424386 SSLClientSocketOpenSSL::BIOCallback"));
 
   SSLClientSocketOpenSSL* socket = reinterpret_cast<SSLClientSocketOpenSSL*>(
       BIO_get_callback_arg(bio));
   CHECK(socket);
   return socket->MaybeReplayTransportError(
       bio, cmd, argp, argi, argl, retvalue);
 }
 
+void SSLClientSocketOpenSSL::MaybeCacheSession() {
+  // Only cache the session once both the handshake has completed and the
+  // certificate has been verified.
+  if (!handshake_completed_ || !certificate_verified_ ||
+      SSL_session_reused(ssl_)) {
+    return;
+  }
+
+  SSLContext::GetInstance()->session_cache()->Insert(GetSessionCacheKey(),
+                                                     SSL_get_session(ssl_));
+}
+
+void SSLClientSocketOpenSSL::InfoCallback(int type, int val) {
+  // TODO(vadimt): Remove ScopedTracker below once crbug.com/424386 is fixed.
+  tracked_objects::ScopedTracker tracking_profile(
+      FROM_HERE_WITH_EXPLICIT_FUNCTION(
+          "424386 SSLClientSocketOpenSSL::InfoCallback"));

[Ryan Sleevi, 2015/03/24 23:47:22]

Nuke this. Vadim orphaned the bug :(

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

Done.

+
+  if (type == SSL_CB_HANDSHAKE_DONE) {
+    // SSL_CB_HANDSHAKE_DONE may be signaled multiple times if the socket
+    // renegotiates.
+    if (!handshake_completed_) {
+      handshake_completed_ = true;
+      MaybeCacheSession();
+    }
+  }
+}
+
 void SSLClientSocketOpenSSL::AddSCTInfoToSSLInfo(SSLInfo* ssl_info) const {
   for (ct::SCTList::const_iterator iter =
        ct_verify_result_.verified_scts.begin();
        iter != ct_verify_result_.verified_scts.end(); ++iter) {
     ssl_info->signed_certificate_timestamps.push_back(
         SignedCertificateTimestampAndStatus(*iter, ct::SCT_STATUS_OK));
   }
   for (ct::SCTList::const_iterator iter =
        ct_verify_result_.invalid_scts.begin();
        iter != ct_verify_result_.invalid_scts.end(); ++iter) {
@@ skipping 36 matching lines @@
 
   return result;
 }
 
 scoped_refptr<X509Certificate>
 SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
   return server_cert_;
 }
 
 }  // namespace net