Revert "Disable session caching with the default (empty) shard."

This reverts commit cb3af0b9.

Reason for revert: This cl introduced MSan bot failure.

https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20MSan%20Tests/builds/2975

[ RUN      ] SSLClientSocketTest.SessionCacheShard
HTTPS server started on https://127.0.0.1:39100...
sending server_data: {"host": "127.0.0.1", "port": 39100} (36 bytes)
==19370:19370==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0xb1f3705 in net::SSLClientSocketImpl::DoHandshakeComplete(int) net/socket/ssl_client_socket_impl.cc:1175:39
    #1 0xb1eb33f in net::SSLClientSocketImpl::DoHandshakeLoop(int) net/socket/ssl_client_socket_impl.cc:1388:14
    #2 0xb1f44fe in net::SSLClientSocketImpl::OnHandshakeIOComplete(int) net/socket/ssl_client_socket_impl.cc:1365:12


[ RUN      ] SslCastSocketTest.TestMessageEndToEndWithRealSSL
==27991:27991==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x12dacae5 in net::SSLClientSocketImpl::DoHandshakeComplete(int) net/socket/ssl_client_socket_impl.cc:1175:39
    #1 0x12da471f in net::SSLClientSocketImpl::DoHandshakeLoop(int) net/socket/ssl_client_socket_impl.cc:1388:14
    #2 0x12dad8de in net::SSLClientSocketImpl::OnHandshakeIOComplete(int) net/socket/ssl_client_socket_impl.cc:1365:12




Original change's description:
> Disable session caching with the default (empty) shard.
> 
> 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: Ib8bb5cc6fb51240c4e9a89184ab51775e9653168
> Reviewed-on: https://chromium-review.googlesource.com/595155
> Commit-Queue: Steven Valdez <svaldez@chromium.org>
> Reviewed-by: Steven Valdez <svaldez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#491118}

TBR=davidben@chromium.org,svaldez@chromium.org

Change-Id: Id7dc73921a36d80c9764bbb49ae36c19ef2ccaca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 458365
Reviewed-on: https://chromium-review.googlesource.com/597328Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491232}

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.
-  std::string ssl_session_cache_shard;
+  const 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

@@ -901,12 +901,10 @@ int SSLClientSocketImpl::Init() {
     return ERR_UNEXPECTED;
   }
 
-  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());
-  }
+  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"),
@@ -1145,11 +1143,8 @@ int SSLClientSocketImpl::DoHandshakeComplete(int result) {
     return ERR_SSL_VERSION_INTERFERENCE;
   }
 
-  if (!ssl_session_cache_shard_.empty()) {
-    SSLContext::GetInstance()->session_cache()->ResetLookupCount(
-        GetSessionCacheKey());
-  }
-
+  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_ &&
@@ -1741,10 +1736,8 @@ 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_ ||
-      ssl_session_cache_shard_.empty()) {
+  if (!pending_session_ || !certificate_verified_)
     return;
-  }
 
   SSLContext::GetInstance()->session_cache()->Insert(GetSessionCacheKey(),
                                                      pending_session_.get());
@@ -1752,9 +1745,6 @@ 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();
@@ -1766,11 +1756,6 @@ 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. This
-  // must not be called if |ssl_session_cache_shard_| is empty.
+  // Returns a unique key string for the SSL session cache for
+  // this socket.
   std::string GetSessionCacheKey() const;
 
   // Returns true if renegotiations are allowed.

net/socket/ssl_client_socket_unittest.cc

@@ -891,8 +891,6 @@ 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(
@@ -3996,50 +3994,4 @@ 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,8 +402,6 @@ 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_,