Rewrite session cache in OpenSSL ports.

net/socket/ssl_client_socket_unittest.cc

Index: net/socket/ssl_client_socket_unittest.cc
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index 004810a03145bdc7e62520be023049138eb90940..578456add59533a62a6da4152e8cfd3ea3d3ab55 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -348,6 +348,9 @@ class FakeBlockingStreamSocket : public WrappedStreamSocket {
             int buf_len,
             const CompletionCallback& callback) override;
 
+  int pending_read_result() const { return pending_read_result_; }
+  IOBuffer* pending_read_buf() const { return pending_read_buf_.get(); }
+
   // Blocks read results on the socket. Reads will not complete until
   // UnblockReadResult() has been called and a result is ready from the
   // underlying transport. Note: if BlockReadResult() is called while there is a
@@ -377,6 +380,9 @@ class FakeBlockingStreamSocket : public WrappedStreamSocket {
   // True if read callbacks are blocked.
   bool should_block_read_;
 
+  // The buffer for the pending read, or NULL if not consumed.
+  scoped_refptr<IOBuffer> pending_read_buf_;
+
   // The user callback for the pending read call.
   CompletionCallback pending_read_callback_;
 
@@ -414,6 +420,7 @@ FakeBlockingStreamSocket::FakeBlockingStreamSocket(
 int FakeBlockingStreamSocket::Read(IOBuffer* buf,
                                    int len,
                                    const CompletionCallback& callback) {
+  DCHECK(!pending_read_buf_);
   DCHECK(pending_read_callback_.is_null());
   DCHECK_EQ(ERR_IO_PENDING, pending_read_result_);
   DCHECK(!callback.is_null());
@@ -422,9 +429,11 @@ int FakeBlockingStreamSocket::Read(IOBuffer* buf,
       &FakeBlockingStreamSocket::OnReadCompleted, base::Unretained(this)));
   if (rv == ERR_IO_PENDING) {
     // Save the callback to be called later.
+    pending_read_buf_ = buf;
     pending_read_callback_ = callback;
   } else if (should_block_read_) {
     // Save the callback and read result to be called later.
+    pending_read_buf_ = buf;
     pending_read_callback_ = callback;
     OnReadCompleted(rv);
     rv = ERR_IO_PENDING;
@@ -471,6 +480,7 @@ void FakeBlockingStreamSocket::UnblockReadResult() {
   if (pending_read_result_ == ERR_IO_PENDING)
     return;
   int result = pending_read_result_;
+  pending_read_buf_ = nullptr;
   pending_read_result_ = ERR_IO_PENDING;
   base::ResetAndReturn(&pending_read_callback_).Run(result);
 }
@@ -538,7 +548,8 @@ void FakeBlockingStreamSocket::OnReadCompleted(int result) {
       read_loop_->Quit();
   } else {
     // Either the Read() was never blocked or UnblockReadResult() was called
-    // before the Read() completed. Either way, run the callback.
+    // before the Read() completed. Either way, return the result to the caller.
+    pending_read_buf_ = nullptr;
     base::ResetAndReturn(&pending_read_callback_).Run(result);
   }
 }
@@ -2812,6 +2823,85 @@ TEST_F(SSLClientSocketTest, ReuseStates) {
   // attempt to read one byte extra.
 }
 
+// Tests that basic session resumption works.
+TEST_F(SSLClientSocketTest, SessionResumption) {
+  SpawnedTestServer::SSLOptions ssl_options;
+  ASSERT_TRUE(StartTestServer(ssl_options));
+
+  // First, perform a full handshake.
+  SSLConfig ssl_config;
+  TestCompletionCallback callback;
+  scoped_ptr<StreamSocket> transport(
+      new TCPClientSocket(addr(), &log_, NetLog::Source()));
+  ASSERT_EQ(OK, callback.GetResult(transport->Connect(callback.callback())));
+  scoped_ptr<SSLClientSocket> sock(CreateSSLClientSocket(
+      transport.Pass(), test_server()->host_port_pair(), ssl_config));
+  ASSERT_EQ(OK, callback.GetResult(sock->Connect(callback.callback())));
+  SSLInfo ssl_info;
+  ASSERT_TRUE(sock->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+
+  // The next connection should resume.
+  transport.reset(new TCPClientSocket(addr(), &log_, NetLog::Source()));
+  ASSERT_EQ(OK, callback.GetResult(transport->Connect(callback.callback())));
+  sock = CreateSSLClientSocket(transport.Pass(),
+                               test_server()->host_port_pair(), ssl_config);
+  ASSERT_EQ(OK, callback.GetResult(sock->Connect(callback.callback())));
+  ASSERT_TRUE(sock->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_RESUME, ssl_info.handshake_type);
+
+  // Using a different HostPortPair uses a different session cache key.
+  transport.reset(new TCPClientSocket(addr(), &log_, NetLog::Source()));
+  ASSERT_EQ(OK, callback.GetResult(transport->Connect(callback.callback())));
+  sock = CreateSSLClientSocket(transport.Pass(),
+                               HostPortPair("example.com", 443), ssl_config);
+  ASSERT_EQ(OK, callback.GetResult(sock->Connect(callback.callback())));
+  ASSERT_TRUE(sock->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+
+  SSLClientSocket::ClearSessionCache();
+
+  // After clearing the session cache, the next handshake doesn't resume.
+  transport.reset(new TCPClientSocket(addr(), &log_, NetLog::Source()));
+  ASSERT_EQ(OK, callback.GetResult(transport->Connect(callback.callback())));
+  sock = CreateSSLClientSocket(transport.Pass(),
+                               test_server()->host_port_pair(), ssl_config);
+  ASSERT_EQ(OK, callback.GetResult(sock->Connect(callback.callback())));
+  ASSERT_TRUE(sock->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+}
+
+// Tests that connections with certificate errors do not add entries to the
+// session cache.
+TEST_F(SSLClientSocketTest, CertificateErrorNoResume) {
+  SpawnedTestServer::SSLOptions ssl_options;
+  ASSERT_TRUE(StartTestServer(ssl_options));
+
+  cert_verifier_->set_default_result(ERR_CERT_COMMON_NAME_INVALID);
+
+  SSLConfig ssl_config;
+  TestCompletionCallback callback;
+  scoped_ptr<StreamSocket> transport(
+      new TCPClientSocket(addr(), &log_, NetLog::Source()));
+  ASSERT_EQ(OK, callback.GetResult(transport->Connect(callback.callback())));
+  scoped_ptr<SSLClientSocket> sock(CreateSSLClientSocket(
+      transport.Pass(), test_server()->host_port_pair(), ssl_config));
+  EXPECT_EQ(ERR_CERT_COMMON_NAME_INVALID,
+            callback.GetResult(sock->Connect(callback.callback())));
+
+  cert_verifier_->set_default_result(OK);
+
+  // The next connection should perform a full handshake.
+  transport.reset(new TCPClientSocket(addr(), &log_, NetLog::Source()));
+  ASSERT_EQ(OK, callback.GetResult(transport->Connect(callback.callback())));
+  sock = CreateSSLClientSocket(transport.Pass(),
+                               test_server()->host_port_pair(), ssl_config);
+  ASSERT_EQ(OK, callback.GetResult(sock->Connect(callback.callback())));
+  SSLInfo ssl_info;
+  ASSERT_TRUE(sock->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+}
+
 // Tests that session caches are sharded by max_version.
 TEST_F(SSLClientSocketTest, FallbackShardSessionCache) {
   SpawnedTestServer::SSLOptions ssl_options;
@@ -3105,9 +3195,9 @@ TEST_F(SSLClientSocketFalseStartTest, SessionResumption) {
   EXPECT_EQ(SSLInfo::HANDSHAKE_RESUME, ssl_info.handshake_type);
 }
 
-// Test that sessions are not resumable before receiving the server Finished
-// message.
-TEST_F(SSLClientSocketFalseStartTest, NoSessionResumptionBeforeFinish) {
+// Test that False Started sessions are not resumable before receiving the
+// server Finished message.
+TEST_F(SSLClientSocketFalseStartTest, NoSessionResumptionBeforeFinished) {
   if (!SupportsAESGCM()) {
     LOG(WARNING) << "Skipping test because AES-GCM is not supported.";
     return;
@@ -3142,8 +3232,8 @@ TEST_F(SSLClientSocketFalseStartTest, NoSessionResumptionBeforeFinish) {
   //
   // The actual read on |sock1| will not complete until the Finished message is
   // processed; however, pump the underlying transport so that it is read from
-  // the socket. This still has the potential to race, but is generally unlikely
-  // due to socket buffer sizes.
+  // the socket. NOTE: This may flakily pass if the server's final flight
+  // doesn't come in one Read.
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
   int rv = sock1->Read(buf.get(), 4096, callback.callback());
   EXPECT_EQ(ERR_IO_PENDING, rv);
@@ -3168,6 +3258,78 @@ TEST_F(SSLClientSocketFalseStartTest, NoSessionResumptionBeforeFinish) {
   EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
 }
 
+// Test that False Started sessions are not resumable if the server Finished
+// message was bad.
+TEST_F(SSLClientSocketFalseStartTest, NoSessionResumptionBadFinished) {
+  if (!SupportsAESGCM()) {
+    LOG(WARNING) << "Skipping test because AES-GCM is not supported.";
+    return;
+  }
+
+  // Start a server.
+  SpawnedTestServer::SSLOptions server_options;
+  server_options.key_exchanges =
+      SpawnedTestServer::SSLOptions::KEY_EXCHANGE_ECDHE_RSA;
+  server_options.bulk_ciphers =
+      SpawnedTestServer::SSLOptions::BULK_CIPHER_AES128GCM;
+  server_options.enable_npn = true;
+  ASSERT_TRUE(StartTestServer(server_options));
+
+  SSLConfig client_config;
+  client_config.next_protos.push_back(kProtoHTTP11);
+
+  // Start a handshake up to the server Finished message.
+  TestCompletionCallback callback;
+  FakeBlockingStreamSocket* raw_transport1 = NULL;
+  scoped_ptr<SSLClientSocket> sock1;
+  ASSERT_NO_FATAL_FAILURE(CreateAndConnectUntilServerFinishedReceived(
+      client_config, &callback, &raw_transport1, &sock1));
+  // Although raw_transport1 has the server Finished blocked, the handshake
+  // still completes.
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  // Continue to block the client (|sock1|) from processing the Finished
+  // message, but allow it to arrive on the socket. This ensures that, from the
+  // server's point of view, it has completed the handshake and added the
+  // session to its session cache.
+  //
+  // The actual read on |sock1| will not complete until the Finished message is
+  // processed; however, pump the underlying transport so that it is read from
+  // the socket.
+  scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
+  int rv = sock1->Read(buf.get(), 4096, callback.callback());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  raw_transport1->WaitForReadResult();
+
+  // The server's second leg, or part of it, is now received but not yet sent to
+  // |sock1|. Before doing so, break the server's second leg.
+  int bytes_read = raw_transport1->pending_read_result();
+  ASSERT_LT(0, bytes_read);
+  raw_transport1->pending_read_buf()->data()[bytes_read - 1]++;
+
+  // Unblock the Finished message. |sock1->Read| should now fail.
+  raw_transport1->UnblockReadResult();
+  EXPECT_EQ(ERR_SSL_PROTOCOL_ERROR, callback.GetResult(rv));
+
+  // Drop the old socket. This is needed because the Python test server can't
+  // service two sockets in parallel.
+  sock1.reset();
+
+  // Start a second connection.
+  scoped_ptr<StreamSocket> transport2(
+      new TCPClientSocket(addr(), &log_, NetLog::Source()));
+  EXPECT_EQ(OK, callback.GetResult(transport2->Connect(callback.callback())));
+  scoped_ptr<SSLClientSocket> sock2 = CreateSSLClientSocket(
+      transport2.Pass(), test_server()->host_port_pair(), client_config);
+  EXPECT_EQ(OK, callback.GetResult(sock2->Connect(callback.callback())));
+
+  // No session resumption because the first connection never received a server
+  // Finished message.
+  SSLInfo ssl_info;
+  EXPECT_TRUE(sock2->GetSSLInfo(&ssl_info));
+  EXPECT_EQ(SSLInfo::HANDSHAKE_FULL, ssl_info.handshake_type);
+}
+
 // Connect to a server using channel id. It should allow the connection.
 TEST_F(SSLClientSocketChannelIDTest, SendChannelID) {
   SpawnedTestServer::SSLOptions ssl_options;