Disable session caching with the default (empty) shard.

This is a reland of https://chromium-review.googlesource.com/c/595155
with a fix to a field that wasn't initialized.

Until we do https://crbug.com/458365 and remove the global session
cache, consumers making SSLClientSockets with the default empty shard
should not enable session caching, as they'll interact with other
sockets which do that.

Bug: 458365
Change-Id: Ib8d0ecf72584b687a768e080dc31439c625ac412
Reviewed-on: https://chromium-review.googlesource.com/598709
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491750}

net/socket/ssl_client_socket.h

@@ -62,7 +62,7 @@ struct SSLClientSocketContext {
   // ssl_session_cache_shard is an opaque string that identifies a shard of the
   // SSL session cache. SSL sockets with the same ssl_session_cache_shard may
   // resume each other's SSL sessions but we'll never sessions between shards.
-  const std::string ssl_session_cache_shard;
+  std::string ssl_session_cache_shard;
 };
 
 // Details on a failed operation. This enum is used to diagnose causes of TLS

net/socket/ssl_client_socket_impl.cc

@@ -485,6 +485,7 @@ SSLClientSocketImpl::SSLClientSocketImpl(
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
       ssl_session_cache_shard_(context.ssl_session_cache_shard),
+      ssl_session_cache_lookup_count_(0),
       next_handshake_state_(STATE_NONE),
       disconnected_(false),
       negotiated_protocol_(kProtoUnknown),
@@ -901,10 +902,12 @@ int SSLClientSocketImpl::Init() {
     return ERR_UNEXPECTED;
   }
 
-  bssl::UniquePtr<SSL_SESSION> session = context->session_cache()->Lookup(
-      GetSessionCacheKey(), &ssl_session_cache_lookup_count_);
-  if (session)
-    SSL_set_session(ssl_.get(), session.get());
+  if (!ssl_session_cache_shard_.empty()) {
+    bssl::UniquePtr<SSL_SESSION> session = context->session_cache()->Lookup(
+        GetSessionCacheKey(), &ssl_session_cache_lookup_count_);
+    if (session)
+      SSL_set_session(ssl_.get(), session.get());
+  }
 
   transport_adapter_.reset(new SocketBIOAdapter(
       transport_->socket(), GetBufferSize("SSLBufferSizeRecv"),
@@ -1143,8 +1146,11 @@ int SSLClientSocketImpl::DoHandshakeComplete(int result) {
     return ERR_SSL_VERSION_INTERFERENCE;
   }
 
-  SSLContext::GetInstance()->session_cache()->ResetLookupCount(
-      GetSessionCacheKey());
+  if (!ssl_session_cache_shard_.empty()) {
+    SSLContext::GetInstance()->session_cache()->ResetLookupCount(
+        GetSessionCacheKey());
+  }
+
   // Check that if token binding was negotiated, then extended master secret
   // and renegotiation indication must also be negotiated.
   if (tb_was_negotiated_ &&
@@ -1736,8 +1742,10 @@ void SSLClientSocketImpl::MaybeCacheSession() {
   // Only cache the session once both a new session has been established and the
   // certificate has been verified. Due to False Start, these events may happen
   // in either order.
-  if (!pending_session_ || !certificate_verified_)
+  if (!pending_session_ || !certificate_verified_ ||
+      ssl_session_cache_shard_.empty()) {
     return;
+  }
 
   SSLContext::GetInstance()->session_cache()->Insert(GetSessionCacheKey(),
                                                      pending_session_.get());
@@ -1745,6 +1753,9 @@ void SSLClientSocketImpl::MaybeCacheSession() {
 }
 
 int SSLClientSocketImpl::NewSessionCallback(SSL_SESSION* session) {
+  if (ssl_session_cache_shard_.empty())
+    return 0;
+
   // OpenSSL passes a reference to |session|.
   pending_session_.reset(session);
   MaybeCacheSession();
@@ -1756,6 +1767,11 @@ void SSLClientSocketImpl::AddCTInfoToSSLInfo(SSLInfo* ssl_info) const {
 }
 
 std::string SSLClientSocketImpl::GetSessionCacheKey() const {
+  // If there is no session cache shard configured, disable session
+  // caching. GetSessionCacheKey may not be called. When
+  // https://crbug.com/458365 is fixed, this check will not be needed.
+  DCHECK(!ssl_session_cache_shard_.empty());
+
   std::string result = host_and_port_.ToString();
   result.push_back('/');
   result.append(ssl_session_cache_shard_);

net/socket/ssl_client_socket_impl.h

@@ -192,8 +192,8 @@ class SSLClientSocketImpl : public SSLClientSocket,
   // the |ssl_info|.signed_certificate_timestamps list.
   void AddCTInfoToSSLInfo(SSLInfo* ssl_info) const;
 
-  // Returns a unique key string for the SSL session cache for
-  // this socket.
+  // Returns a unique key string for the SSL session cache for this socket. This
+  // must not be called if |ssl_session_cache_shard_| is empty.
   std::string GetSessionCacheKey() const;
 
   // Returns true if renegotiations are allowed.

net/socket/ssl_client_socket_unittest.cc

@@ -891,6 +891,8 @@ class SSLClientSocketTest : public PlatformTest {
     context_.transport_security_state = transport_security_state_.get();
     context_.cert_transparency_verifier = ct_verifier_.get();
     context_.ct_policy_enforcer = ct_policy_enforcer_.get();
+    // Set a dummy session cache shard to enable session caching.
+    context_.ssl_session_cache_shard = "shard";
 
     EXPECT_CALL(*ct_policy_enforcer_, DoesConformToCertPolicy(_, _, _))
         .WillRepeatedly(
@@ -3994,4 +3996,50 @@ TEST_P(SSLClientSocketReadTest, IdleAfterRead) {
   EXPECT_EQ(stats.cert_size, stats.total_size);
 }
 
+// Test that session caches are properly sharded.
+TEST_F(SSLClientSocketTest, SessionCacheShard) {
+  ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
+
+  // Perform a full handshake.
+  context_.ssl_session_cache_shard = "A";
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
+  ASSERT_THAT(rv, IsOk());
+  SSLInfo ssl_info;
+  ASSERT_TRUE(sock_->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+
+  // The next connection resumes the session.
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
+  ASSERT_THAT(rv, IsOk());
+  ASSERT_TRUE(sock_->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_RESUME, ssl_info.handshake_type);
+
+  // A different shard does not resume.
+  context_.ssl_session_cache_shard = "B";
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
+  ASSERT_THAT(rv, IsOk());
+  ASSERT_TRUE(sock_->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+
+  // The original shard still resumes.
+  context_.ssl_session_cache_shard = "A";
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
+  ASSERT_THAT(rv, IsOk());
+  ASSERT_TRUE(sock_->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_RESUME, ssl_info.handshake_type);
+
+  // The empty shard never resumes.
+  context_.ssl_session_cache_shard = "";
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
+  ASSERT_THAT(rv, IsOk());
+  ASSERT_TRUE(sock_->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
+  ASSERT_THAT(rv, IsOk());
+  ASSERT_TRUE(sock_->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+}
+
 }  // namespace net

net/socket/ssl_server_socket_unittest.cc

@@ -402,6 +402,8 @@ class SSLServerSocketTest : public PlatformTest {
     context.transport_security_state = transport_security_state_.get();
     context.cert_transparency_verifier = ct_verifier_.get();
     context.ct_policy_enforcer = ct_policy_enforcer_.get();
+    // Set a dummy session cache shard to enable session caching.
+    context.ssl_session_cache_shard = "shard";
 
     client_socket_ = socket_factory_->CreateSSLClientSocket(
         std::move(client_connection), host_and_pair, client_ssl_config_,