Add tests for session cache and false start behavior.

net/socket/ssl_client_socket_unittest.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.
 
 #include "net/socket/ssl_client_socket.h"
 
 #include "base/callback_helpers.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "net/base/address_list.h"
@@ skipping 581 matching lines @@
   SSLClientSocketTest()
       : socket_factory_(ClientSocketFactory::GetDefaultFactory()),
         cert_verifier_(new MockCertVerifier),
         transport_security_state_(new TransportSecurityState) {
     cert_verifier_->set_default_result(OK);
     context_.cert_verifier = cert_verifier_.get();
     context_.transport_security_state = transport_security_state_.get();
   }
 
  protected:
-  // Sets up a TCP connection to a HTTPS server. To actually do the SSL
-  // handshake, follow up with call to CreateAndConnectSSLClientSocket() below.
-  bool ConnectToTestServer(SpawnedTestServer::SSLOptions& ssl_options) {
+  bool StartTestServer(const SpawnedTestServer::SSLOptions& ssl_options) {
     test_server_.reset(new SpawnedTestServer(
         SpawnedTestServer::TYPE_HTTPS, ssl_options, base::FilePath()));
     if (!test_server_->Start()) {
       LOG(ERROR) << "Could not start SpawnedTestServer";
       return false;
     }
 
     if (!test_server_->GetAddressList(&addr_)) {
       LOG(ERROR) << "Could not get SpawnedTestServer address list";
       return false;
     }
+    return true;
+  }
+
+  // Sets up a TCP connection to a HTTPS server. To actually do the SSL
+  // handshake, follow up with call to CreateAndConnectSSLClientSocket() below.
+  bool ConnectToTestServer(const SpawnedTestServer::SSLOptions& ssl_options) {
+    if (!StartTestServer(ssl_options))
+      return false;
 
     transport_.reset(new TCPClientSocket(addr_, &log_, NetLog::Source()));
     int rv = callback_.GetResult(transport_->Connect(callback_.callback()));
     if (rv != OK) {
       LOG(ERROR) << "Could not connect to SpawnedTestServer";
       return false;
     }
     return true;
   }
 
@@ skipping 27 matching lines @@
     *result = callback_.GetResult(sock_->Connect(callback_.callback()));
     return true;
   }
 
   ClientSocketFactory* socket_factory_;
   scoped_ptr<MockCertVerifier> cert_verifier_;
   scoped_ptr<TransportSecurityState> transport_security_state_;
   SSLClientSocketContext context_;
   scoped_ptr<SSLClientSocket> sock_;
   CapturingNetLog log_;
+  AddressList addr_;
+  scoped_ptr<SpawnedTestServer> test_server_;
 
  private:
   scoped_ptr<StreamSocket> transport_;
-  scoped_ptr<SpawnedTestServer> test_server_;
   TestCompletionCallback callback_;
-  AddressList addr_;
 };
 
 // Verifies the correctness of GetSSLCertRequestInfo.
 class SSLClientSocketCertRequestInfoTest : public SSLClientSocketTest {
  protected:
   // Creates a test server with the given SSLOptions, connects to it and returns
   // the SSLCertRequestInfo reported by the socket.
   scoped_refptr<SSLCertRequestInfo> GetCertRequest(
       SpawnedTestServer::SSLOptions ssl_options) {
     SpawnedTestServer test_server(
@@ skipping 27 matching lines @@
     EXPECT_FALSE(sock->IsConnected());
     EXPECT_TRUE(
         test_server.host_port_pair().Equals(request_info->host_and_port));
 
     return request_info;
   }
 };
 
 class SSLClientSocketFalseStartTest : public SSLClientSocketTest {
  protected:
-  void TestFalseStart(const SpawnedTestServer::SSLOptions& server_options,
-                      const SSLConfig& client_config,
-                      bool expect_false_start) {
-    SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
-                                  server_options,
-                                  base::FilePath());
-    ASSERT_TRUE(test_server.Start());
-
-    AddressList addr;
-    ASSERT_TRUE(test_server.GetAddressList(&addr));
-
-    TestCompletionCallback callback;
+  void CreateAndConnectUpToServerFinished(

[Ryan Sleevi, 2014/05/31 00:46:11]

s/UpToServerFinished/UntilServerFinishedReceived?

The "UpTo" bit read weird at first - I thought you were saying "ConnectUp" (as a
colloquial phrase). Hence the suggestion :)

[davidben, 2014/06/02 18:45:38]

Done.

+      const SSLConfig& client_config,
+      TestCompletionCallback* callback,
+      FakeBlockingStreamSocket** out_raw_transport,

[Ryan Sleevi, 2014/05/31 00:46:11]

More documentation is needed here. My red alert alarms are going off on having a
FakeBlockingStreamSocket**, because it's not clear if ownership is transferred
(and this should be a scoped_ptr<FakeBlockingStreamSocket>* out_raw_socket) or
not.

It's also not clear whether or not it's safe to call this before the test server
has been started - presumably not, given that you're using addr_ (line 728)

[davidben, 2014/06/02 18:45:38]

Documented. (The FakeBlockingStreamSocket annoyingly is owned by the
SSLClientSocket, so it has to be a raw pointer.)
Documented.

+      scoped_ptr<SSLClientSocket>* out_sock) {
     scoped_ptr<StreamSocket> real_transport(
-        new TCPClientSocket(addr, NULL, NetLog::Source()));
+        new TCPClientSocket(addr_, NULL, NetLog::Source()));
     scoped_ptr<FakeBlockingStreamSocket> transport(
         new FakeBlockingStreamSocket(real_transport.Pass()));
-    int rv = callback.GetResult(transport->Connect(callback.callback()));
+    int rv = callback->GetResult(transport->Connect(callback->callback()));
     EXPECT_EQ(OK, rv);
 
     FakeBlockingStreamSocket* raw_transport = transport.get();
-    scoped_ptr<SSLClientSocket> sock(
+    scoped_ptr<SSLClientSocket> sock =
         CreateSSLClientSocket(transport.PassAs<StreamSocket>(),
-                              test_server.host_port_pair(),
-                              client_config));
+                              test_server_->host_port_pair(),
+                              client_config);
 
     // Connect. Stop before the client processes the first server leg
     // (ServerHello, etc.)
     raw_transport->BlockReadResult();
-    rv = sock->Connect(callback.callback());
+    rv = sock->Connect(callback->callback());
     EXPECT_EQ(ERR_IO_PENDING, rv);
     raw_transport->WaitForReadResult();
 
     // Release the ServerHello and wait for the client to write
     // ClientKeyExchange, etc. (A proxy for waiting for the entirety of the
     // server's leg to complete, since it may span multiple reads.)
-    EXPECT_FALSE(callback.have_result());
+    EXPECT_FALSE(callback->have_result());
     raw_transport->BlockWrite();
     raw_transport->UnblockReadResult();
     raw_transport->WaitForWrite();
 
     // And, finally, release that and block the next server leg
-    // (ChangeCipherSpec, Finished). Note: callback.have_result() may or may not
-    // be true at this point depending on whether the SSL implementation waits
-    // for the client second leg to clear the internal write buffer and hit the
-    // network.
+    // (ChangeCipherSpec, Finished). Note: callback->have_result() may or may
+    // not be true at this point depending on whether the SSL implementation
+    // waits for the client second leg to clear the internal write buffer and
+    // hit the network.
     raw_transport->BlockReadResult();
     raw_transport->UnblockWrite();
 
+    *out_raw_transport = raw_transport;
+    *out_sock = sock.Pass();
+  }
+
+  void TestFalseStart(const SpawnedTestServer::SSLOptions& server_options,
+                      const SSLConfig& client_config,
+                      bool expect_false_start) {
+    ASSERT_TRUE(StartTestServer(server_options));
+
+    TestCompletionCallback callback;
+    FakeBlockingStreamSocket* raw_transport;
+    scoped_ptr<SSLClientSocket> sock;
+    CreateAndConnectUpToServerFinished(
+        client_config, &callback, &raw_transport, &sock);

[Ryan Sleevi, 2014/05/31 00:46:11]

DANGER: If you ever added an ASSERT_() to CreateAndConnectUpToServerFinished(),
your test will still continue, because it's wrapped within a function.

As such, it would be good to wrap this call (and calls to this TestFalseStart
function) in ASSERT_NO_FATAL_FAILURE() before continuing.

eg: ASSERT_NO_FATAL_FAILURE(CreateAndConnectUpToServerFinished(...));

[davidben, 2014/06/02 18:45:38]

Oh, so that's how you do it... done.

+
     if (expect_false_start) {
       // When False Starting, the handshake should complete before receiving the
       // Change Cipher Spec and Finished messages.
-      rv = callback.GetResult(rv);
+      int rv = callback.GetResult(ERR_IO_PENDING);

[Ryan Sleevi, 2014/05/31 00:46:11]

Just use

int rv = callback.WaitForResult();

no need to fake it like this.

[davidben, 2014/06/02 18:45:38]

Done.

       EXPECT_EQ(OK, rv);
       EXPECT_TRUE(sock->IsConnected());
 
       const char request_text[] = "GET / HTTP/1.0\r\n\r\n";
       static const int kRequestTextSize =
           static_cast<int>(arraysize(request_text) - 1);
       scoped_refptr<IOBuffer> request_buffer(new IOBuffer(kRequestTextSize));
       memcpy(request_buffer->data(), request_text, kRequestTextSize);
 
       // Write the request.
@@ skipping 1697 matching lines @@
 TEST_F(SSLClientSocketFalseStartTest, NoForwardSecrecy) {
   SpawnedTestServer::SSLOptions server_options;
   server_options.key_exchanges =
       SpawnedTestServer::SSLOptions::KEY_EXCHANGE_RSA;
   server_options.enable_npn = true;
   SSLConfig client_config;
   client_config.next_protos.push_back("http/1.1");
   TestFalseStart(server_options, client_config, false);
 }
 
+// Test that sessions are resumable after receiving the server Finished message.
+TEST_F(SSLClientSocketFalseStartTest, SessionResumption) {
+  // Start a server.
+  SpawnedTestServer::SSLOptions server_options;
+  server_options.key_exchanges =
+      SpawnedTestServer::SSLOptions::KEY_EXCHANGE_DHE_RSA;
+  server_options.enable_npn = true;
+  SSLConfig client_config;
+  client_config.next_protos.push_back("http/1.1");
+
+  // Let a full handshake complete with False Start.
+  TestFalseStart(server_options, client_config, true);
+
+  // Make a second connection.
+  TestCompletionCallback callback;
+  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())));
+
+  // It should resume the session.
+  SSLInfo ssl_info;
+  EXPECT_TRUE(sock2->GetSSLInfo(&ssl_info));
+  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) {
+  // Start a server.
+  SpawnedTestServer::SSLOptions server_options;
+  server_options.key_exchanges =
+      SpawnedTestServer::SSLOptions::KEY_EXCHANGE_DHE_RSA;
+  server_options.enable_npn = true;
+  ASSERT_TRUE(StartTestServer(server_options));
+
+  SSLConfig client_config;
+  client_config.next_protos.push_back("http/1.1");
+
+  // Start a handshake up to the server Finished message.
+  TestCompletionCallback callback;
+  FakeBlockingStreamSocket* raw_transport1;
+  scoped_ptr<SSLClientSocket> sock1;
+  CreateAndConnectUpToServerFinished(
+      client_config, &callback, &raw_transport1, &sock1);
+  // The handshake should False Start.
+  EXPECT_EQ(OK, callback.GetResult(ERR_IO_PENDING));

[Ryan Sleevi, 2014/05/31 00:46:11]

ditto WaitForResult()

However, it's unclear to me how this guarantees false start here. This is why
documentation on CreateAndConnect* is important.

[davidben, 2014/06/02 18:45:38]

Done. Also added a comment.

+
+  // Drop the old socket. This is needed because the Python test server can't
+  // service two sockets in parallel.
+  sock1.reset();
+
+  // Before the first connection receives ServerFinished, 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.
+  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;
 
   ASSERT_TRUE(ConnectToTestServer(ssl_options));
 
   EnableChannelID();
   SSLConfig ssl_config = kDefaultSSLConfig;
   ssl_config.channel_id_enabled = true;
 
@@ skipping 24 matching lines @@
 
   // TODO(haavardm@opera.com): Due to differences in threading, Linux returns
   // ERR_UNEXPECTED while Mac and Windows return ERR_PROTOCOL_ERROR. Accept all
   // error codes for now.
   // http://crbug.com/373670
   EXPECT_NE(OK, rv);
   EXPECT_FALSE(sock_->IsConnected());
 }
 
 }  // namespace net