Support for client certs in ssl_server_socket.

net/socket/ssl_server_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.
 
 // This test suite uses SSLClientSocket to test the implementation of
 // SSLServerSocket. In order to establish connections between the sockets
 // we need two additional classes:
 // 1. FakeSocket
 //    Connects SSL socket to FakeDataChannel. This class is just a stub.
 //
 // 2. FakeDataChannel
 //    Implements the actual exchange of data between two FakeSockets.
 //
 // Implementations of these two classes are included in this file.
 
 #include "net/socket/ssl_server_socket.h"
 
+#if defined(USE_OPENSSL)
+#include <openssl/evp.h>
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#endif

[davidben, 2016/02/04 00:40:11]

Move this after line 67. The problem is ssl_server_socket.h might not have
included build/build_config.h and so badness ensues. (The style guide says has
an exception to include order about conditional includes for this reason.)

[ryanchung, 2016/02/05 01:56:13]

Done.

+
 #include <stdint.h>
 #include <stdlib.h>
 #include <queue>
 #include <utility>
 
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
+#include "build/build_config.h"
 #include "crypto/nss_util.h"
 #include "crypto/rsa_private_key.h"
+#include "crypto/signature_creator.h"
 #include "net/base/address_list.h"
 #include "net/base/completion_callback.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_data_directory.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/mock_cert_verifier.h"
+#include "net/cert/mock_client_cert_verifier.h"
 #include "net/cert/x509_certificate.h"
 #include "net/http/transport_security_state.h"
 #include "net/log/net_log.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/socket_test_util.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/stream_socket.h"
+#include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_info.h"
+#include "net/ssl/ssl_private_key.h"
 #include "net/ssl/ssl_server_config.h"
 #include "net/test/cert_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 namespace net {
 
 namespace {
 
+const char kClientCertFileName[] = "client_1.pem";
+const char kClientPrivateKeyFileName[] = "client_1.pk8";
+const char kWrongClientCertFileName[] = "client_2.pem";
+const char kWrongClientPrivateKeyFileName[] = "client_2.pk8";
+const char kClientCertCAFileName[] = "client_1_ca.pem";
+
 class FakeDataChannel {
  public:
   FakeDataChannel()
       : read_buf_len_(0),
         closed_(false),
         write_called_after_close_(false),
         weak_factory_(this) {
   }
 
   int Read(IOBuffer* buf, int buf_len, const CompletionCallback& callback) {
@@ skipping 31 matching lines @@
                               weak_factory_.GetWeakPtr()));
     return buf_len;
   }
 
   // Closes the FakeDataChannel. After Close() is called, Read() returns 0,
   // indicating EOF, and Write() fails with ERR_CONNECTION_RESET. Note that
   // after the FakeDataChannel is closed, the first Write() call completes
   // asynchronously, which is necessary to reproduce bug 127822.
   void Close() {
     closed_ = true;
+    data_.push(
+        new DrainableIOBuffer(new StringIOBuffer(std::string("0", 1)), 1));

[davidben, 2016/02/04 00:40:11]

We chatted over IM, but rather than push fake data_ here, you want the pending
read callback to get an ERR_CONNECTION_CLOSED or some such. I *think* it'd be
sufficient to remove this line and make DoReadCallback say something like:

if (read_callback_.is_null())
  return;

if (closed_) {
  base::ResetAndReturn(&read_callback_).Run(ERR_CONNECTION_CLOSED);
  return;
}

if (data_.empty())
  return;
// Existing logic.

But you'll want to check me on that.

[ryanchung, 2016/02/05 01:56:13]

Done. That works.

+    if (!read_callback_.is_null()) {
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::Bind(&FakeDataChannel::DoReadCallback,
+                                weak_factory_.GetWeakPtr()));
+    }
   }
 
  private:
   void DoReadCallback() {
     if (read_callback_.is_null() || data_.empty())
       return;
-

[davidben, 2016/02/04 00:40:11]

Stray backspace?

(Unless you intentionally removed that line. I don't personally care much one
way or another.)

[ryanchung, 2016/02/05 01:56:13]

Done. Unintentional.

     int copied = PropagateData(read_buf_, read_buf_len_);
     CompletionCallback callback = read_callback_;
     read_callback_.Reset();
     read_buf_ = NULL;
     read_buf_len_ = 0;
     callback.Run(copied);
   }
 
   void DoWriteCallback() {
     if (write_callback_.is_null())
@@ skipping 118 matching lines @@
   }
 
  private:
   BoundNetLog net_log_;
   FakeDataChannel* incoming_;
   FakeDataChannel* outgoing_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeSocket);
 };
 
+#if defined(USE_OPENSSL)
+class TestSSLPrivateKey : public SSLPrivateKey {

[davidben, 2016/02/04 00:40:11]

If you rebase, you can replace this class with a call to this function:
https://chromium.googlesource.com/chromium/src/+/master/net/ssl/test_ssl_priv...

(The key() method on RSAPrivateKey returns an EVP_PKEY. You can use
ScopedEVP_PKEY(EVP_PKEY_up_ref(key->key())) to take a reference possibly we
should have made that take a bare pointer, I dunno...)

[ryanchung, 2016/02/05 01:56:13]

Done.

+ public:
+  TestSSLPrivateKey(scoped_ptr<crypto::RSAPrivateKey> rsa_private_key)
+      : rsa_private_key_(std::move(rsa_private_key)) {}
+
+  Type GetType() override { return SSLPrivateKey::Type::RSA; }
+
+  std::vector<SSLPrivateKey::Hash> GetDigestPreferences() override {
+    static const SSLPrivateKey::Hash kHashes[] = {SSLPrivateKey::Hash::SHA256,
+                                                  SSLPrivateKey::Hash::SHA1};
+    return std::vector<SSLPrivateKey::Hash>(std::begin(kHashes),
+                                            std::end(kHashes));
+  }
+
+  size_t GetMaxSignatureLengthInBytes() override {
+    return EVP_PKEY_size(rsa_private_key_->key());
+  }
+
+  void SignDigest(Hash hash,
+                  const base::StringPiece& input,
+                  const SignCallback& callback) override {
+    std::vector<uint8_t> signature;
+    crypto::SignatureCreator::HashAlgorithm hash_alg;
+    switch (hash) {
+      case Hash::SHA1:
+        hash_alg = crypto::SignatureCreator::SHA1;
+        break;
+
+      case Hash::SHA256:
+        hash_alg = crypto::SignatureCreator::SHA256;
+        break;
+
+      default:
+        FAIL() << "Unsupported hash function";
+    }
+    crypto::SignatureCreator::Sign(
+        rsa_private_key_.get(), hash_alg,
+        reinterpret_cast<const uint8_t*>(input.data()), input.size(),
+        &signature);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(callback, OK, signature));
+  }
+
+ private:
+  ~TestSSLPrivateKey() override {}
+
+  scoped_ptr<crypto::RSAPrivateKey> rsa_private_key_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestSSLPrivateKey);
+};
+#endif
+
 }  // namespace
 
 // Verify the correctness of the test helper classes first.
 TEST(FakeSocketTest, DataTransfer) {
   // Establish channels between two sockets.
   FakeDataChannel channel_1;
   FakeDataChannel channel_2;
   FakeSocket client(&channel_1, &channel_2);
   FakeSocket server(&channel_2, &channel_1);
 
@@ skipping 27 matching lines @@
   EXPECT_GT(read, 0);
   EXPECT_LE(read, written);
   EXPECT_EQ(0, memcmp(kTestData, read_buf->data(), read));
 }
 
 class SSLServerSocketTest : public PlatformTest {
  public:
   SSLServerSocketTest()
       : socket_factory_(ClientSocketFactory::GetDefaultFactory()),
         cert_verifier_(new MockCertVerifier()),
+        client_cert_verifier_(new MockClientCertVerifier()),
         transport_security_state_(new TransportSecurityState) {
-    cert_verifier_->set_default_result(CERT_STATUS_AUTHORITY_INVALID);
+    cert_verifier_->set_default_result(ERR_CERT_AUTHORITY_INVALID);
+    client_cert_verifier_->set_default_result(ERR_CERT_AUTHORITY_INVALID);
   }
 
  protected:
   void Initialize() {
     scoped_ptr<ClientSocketHandle> client_connection(new ClientSocketHandle);
     client_connection->SetSocket(
         scoped_ptr<StreamSocket>(new FakeSocket(&channel_1_, &channel_2_)));
     scoped_ptr<StreamSocket> server_socket(
         new FakeSocket(&channel_2_, &channel_1_));
 
-    base::FilePath certs_dir(GetTestCertsDirectory());
-
-    base::FilePath cert_path = certs_dir.AppendASCII("unittest.selfsigned.der");
-    std::string cert_der;
-    ASSERT_TRUE(base::ReadFileToString(cert_path, &cert_der));
-
-    scoped_refptr<X509Certificate> cert =
-        X509Certificate::CreateFromBytes(cert_der.data(), cert_der.size());
-
-    base::FilePath key_path = certs_dir.AppendASCII("unittest.key.bin");
-    std::string key_string;
-    ASSERT_TRUE(base::ReadFileToString(key_path, &key_string));
-    std::vector<uint8_t> key_vector(
-        reinterpret_cast<const uint8_t*>(key_string.data()),
-        reinterpret_cast<const uint8_t*>(key_string.data() +
-                                         key_string.length()));
-
-    scoped_ptr<crypto::RSAPrivateKey> private_key(
-        crypto::RSAPrivateKey::CreateFromPrivateKeyInfo(key_vector));
+    scoped_refptr<X509Certificate> server_cert(
+        ImportCertFromFile(GetTestCertsDirectory(), "unittest.selfsigned.der"));
+    scoped_ptr<crypto::RSAPrivateKey> server_private_key(
+        ReadTestKey("unittest.key.bin"));
 
     client_ssl_config_.false_start_enabled = false;
     client_ssl_config_.channel_id_enabled = false;
 
     // Certificate provided by the host doesn't need authority.
     SSLConfig::CertAndStatus cert_and_status;
     cert_and_status.cert_status = CERT_STATUS_AUTHORITY_INVALID;
-    cert_and_status.der_cert = cert_der;
+    std::string server_cert_der;
+    CHECK(X509Certificate::GetDEREncoded(server_cert->os_cert_handle(),
+                                         &server_cert_der));
+    cert_and_status.der_cert = server_cert_der;
     client_ssl_config_.allowed_bad_certs.push_back(cert_and_status);
 
     HostPortPair host_and_pair("unittest", 0);
     SSLClientSocketContext context;
     context.cert_verifier = cert_verifier_.get();
     context.transport_security_state = transport_security_state_.get();
+    socket_factory_->ClearSSLSessionCache();
     client_socket_ = socket_factory_->CreateSSLClientSocket(
         std::move(client_connection), host_and_pair, client_ssl_config_,
         context);
-    server_socket_ = CreateSSLServerSocket(std::move(server_socket), cert.get(),
-                                           *private_key, server_ssl_config_);
+    server_socket_ =
+        CreateSSLServerSocket(std::move(server_socket), server_cert.get(),
+                              *server_private_key, server_ssl_config_);
   }
 
+#if defined(USE_OPENSSL)
+  void ConfigureClientCertsForClient(const char* cert_file_name,
+                                     const char* private_key_file_name) {
+    scoped_refptr<X509Certificate> cert;
+    scoped_refptr<net::SSLPrivateKey> key;
+    if (cert_file_name && private_key_file_name) {
+      cert = ImportCertFromFile(GetTestCertsDirectory(), cert_file_name);
+      key = new TestSSLPrivateKey(ReadTestKey(private_key_file_name));
+    }
+    client_ssl_config_.send_client_cert = true;
+    client_ssl_config_.client_cert = cert;
+    client_ssl_config_.client_private_key = key;
+  }
+
+  void ConfigureClientCertsForServer() {
+    server_ssl_config_.require_client_cert = true;
+
+    STACK_OF(X509_NAME)* cert_names;
+    cert_names = SSL_load_client_CA_file(GetTestCertsDirectory()

[davidben, 2016/02/04 00:40:11]

ScopedX509NameStack cert_name(SSL_load_.....);

[ryanchung, 2016/02/05 01:56:13]

Done.

+                                             .AppendASCII(kClientCertCAFileName)
+                                             .MaybeAsASCII()
+                                             .c_str());
+    ASSERT_TRUE(cert_names != nullptr);

[davidben, 2016/02/04 00:40:11]

Nit: This can just be ASSERT_TRUE(cert_names);

[ryanchung, 2016/02/05 01:56:13]

Done.

+    for (size_t i = 0; i < sk_X509_NAME_num(cert_names); ++i) {
+      uint8_t* str = nullptr;
+      int length = i2d_X509_NAME(sk_X509_NAME_value(cert_names, i), &str);
+      server_ssl_config_.cert_authorities_.push_back(std::string(
+          reinterpret_cast<const char*>(str), static_cast<size_t>(length)));
+      OPENSSL_free(str);
+    }
+    sk_X509_NAME_pop_free(cert_names, X509_NAME_free);
+
+    scoped_refptr<X509Certificate> expected_client_cert(
+        ImportCertFromFile(GetTestCertsDirectory(), kClientCertFileName));
+    client_cert_verifier_->AddResultForCert(expected_client_cert.get(), OK);
+
+    server_ssl_config_.client_cert_verifier = client_cert_verifier_.get();
+  }
+
+  scoped_ptr<crypto::RSAPrivateKey> ReadTestKey(const base::StringPiece& name) {
+    base::FilePath certs_dir(GetTestCertsDirectory());
+    base::FilePath key_path = certs_dir.AppendASCII(name);
+    std::string key_string;
+    if (!base::ReadFileToString(key_path, &key_string))
+      return nullptr;
+    std::vector<uint8_t> key_vector(
+        reinterpret_cast<const uint8_t*>(key_string.data()),
+        reinterpret_cast<const uint8_t*>(key_string.data() +
+                                         key_string.length()));
+    scoped_ptr<crypto::RSAPrivateKey> key(
+        crypto::RSAPrivateKey::CreateFromPrivateKeyInfo(key_vector));
+    return key;
+  }
+#endif
+
   FakeDataChannel channel_1_;
   FakeDataChannel channel_2_;
   SSLConfig client_ssl_config_;
   SSLServerConfig server_ssl_config_;
   scoped_ptr<SSLClientSocket> client_socket_;
   scoped_ptr<SSLServerSocket> server_socket_;
   ClientSocketFactory* socket_factory_;
   scoped_ptr<MockCertVerifier> cert_verifier_;
+  scoped_ptr<MockClientCertVerifier> client_cert_verifier_;
   scoped_ptr<TransportSecurityState> transport_security_state_;
 };
 
 // This test only executes creation of client and server sockets. This is to
 // test that creation of sockets doesn't crash and have minimal code to run
 // under valgrind in order to help debugging memory problems.
 TEST_F(SSLServerSocketTest, Initialize) {
   Initialize();
 }
 
 // This test executes Connect() on SSLClientSocket and Handshake() on
 // SSLServerSocket to make sure handshaking between the two sockets is
 // completed successfully.
 TEST_F(SSLServerSocketTest, Handshake) {
   Initialize();
 
+  TestCompletionCallback handshake_callback;
+  int server_ret = server_socket_->Handshake(handshake_callback.callback());
+
   TestCompletionCallback connect_callback;
-  TestCompletionCallback handshake_callback;
+  int client_ret = client_socket_->Connect(connect_callback.callback());
 
-  int server_ret = server_socket_->Handshake(handshake_callback.callback());
-  EXPECT_TRUE(server_ret == OK || server_ret == ERR_IO_PENDING);
+  client_ret = connect_callback.GetResult(client_ret);
+  server_ret = handshake_callback.GetResult(server_ret);
 
-  int client_ret = client_socket_->Connect(connect_callback.callback());
-  EXPECT_TRUE(client_ret == OK || client_ret == ERR_IO_PENDING);
-
-  if (client_ret == ERR_IO_PENDING) {
-    EXPECT_EQ(OK, connect_callback.WaitForResult());
-  }
-  if (server_ret == ERR_IO_PENDING) {
-    EXPECT_EQ(OK, handshake_callback.WaitForResult());
-  }
+  ASSERT_EQ(OK, client_ret);
+  ASSERT_EQ(OK, server_ret);
 
   // Make sure the cert status is expected.
   SSLInfo ssl_info;
   ASSERT_TRUE(client_socket_->GetSSLInfo(&ssl_info));
   EXPECT_EQ(CERT_STATUS_AUTHORITY_INVALID, ssl_info.cert_status);
 
   // The default cipher suite should be ECDHE and, unless on NSS and the
   // platform doesn't support it, an AEAD.
   uint16_t cipher_suite =
       SSLConnectionStatusToCipherSuite(ssl_info.connection_status);
   const char* key_exchange;
   const char* cipher;
   const char* mac;
   bool is_aead;
   SSLCipherSuiteToStrings(&key_exchange, &cipher, &mac, &is_aead, cipher_suite);
   EXPECT_STREQ("ECDHE_RSA", key_exchange);
   EXPECT_TRUE(is_aead);
 }
 
+// NSS ports don't support client certificates.
+#if defined(USE_OPENSSL)
+
+// This test executes Connect() on SSLClientSocket and Handshake() on
+// SSLServerSocket to make sure handshaking between the two sockets is
+// completed successfully, using client certificate.
+TEST_F(SSLServerSocketTest, HandshakeWithClientCert) {
+  scoped_refptr<X509Certificate> client_cert =
+      ImportCertFromFile(GetTestCertsDirectory(), kClientCertFileName);
+  ConfigureClientCertsForClient(kClientCertFileName, kClientPrivateKeyFileName);
+  ConfigureClientCertsForServer();
+  Initialize();
+
+  TestCompletionCallback handshake_callback;
+  int server_ret = server_socket_->Handshake(handshake_callback.callback());
+
+  TestCompletionCallback connect_callback;
+  int client_ret = client_socket_->Connect(connect_callback.callback());
+
+  client_ret = connect_callback.GetResult(client_ret);
+  server_ret = handshake_callback.GetResult(server_ret);
+
+  ASSERT_EQ(OK, client_ret);
+  ASSERT_EQ(OK, server_ret);
+
+  // Make sure the cert status is expected.
+  SSLInfo ssl_info;
+  client_socket_->GetSSLInfo(&ssl_info);
+  EXPECT_EQ(CERT_STATUS_AUTHORITY_INVALID, ssl_info.cert_status);
+  server_socket_->GetSSLInfo(&ssl_info);
+  EXPECT_TRUE(ssl_info.cert.get());
+  EXPECT_TRUE(client_cert->Equals(ssl_info.cert.get()));
+}
+
+TEST_F(SSLServerSocketTest, HandshakeWithClientCertRequiredNotSupplied) {
+  ConfigureClientCertsForServer();
+  Initialize();
+  // Use the default setting for the client socket, which is to not send
+  // a client certificate. This will cause the client to receive an
+  // ERR_SSL_CLIENT_AUTH_CERT_NEEDED error, and allow for inspecting the
+  // requested cert_authorities from the CertificateRequest sent by the
+  // server.
+
+  TestCompletionCallback handshake_callback;
+  int server_ret = server_socket_->Handshake(handshake_callback.callback());
+
+  TestCompletionCallback connect_callback;
+  EXPECT_EQ(ERR_SSL_CLIENT_AUTH_CERT_NEEDED,
+            connect_callback.GetResult(
+                client_socket_->Connect(connect_callback.callback())));
+
+  scoped_refptr<SSLCertRequestInfo> request_info = new SSLCertRequestInfo();
+  client_socket_->GetSSLCertRequestInfo(request_info.get());
+
+  // Check that the authority name that arrived in the CertificateRequest
+  // handshake message is as expected.
+  scoped_refptr<X509Certificate> client_cert =
+      ImportCertFromFile(GetTestCertsDirectory(), kClientCertFileName);
+  EXPECT_TRUE(client_cert->IsIssuedByEncoded(request_info->cert_authorities));
+
+  client_socket_->Disconnect();
+
+  EXPECT_EQ(ERR_FAILED, handshake_callback.GetResult(server_ret));
+}
+
+TEST_F(SSLServerSocketTest, HandshakeWithWrongClientCertSupplied) {
+  scoped_refptr<X509Certificate> client_cert =
+      ImportCertFromFile(GetTestCertsDirectory(), kClientCertFileName);
+  ConfigureClientCertsForClient(kWrongClientCertFileName,
+                                kWrongClientPrivateKeyFileName);
+  ConfigureClientCertsForServer();
+  Initialize();
+
+  TestCompletionCallback handshake_callback;
+  int server_ret = server_socket_->Handshake(handshake_callback.callback());
+
+  TestCompletionCallback connect_callback;
+  int client_ret = client_socket_->Connect(connect_callback.callback());
+
+  EXPECT_EQ(ERR_BAD_SSL_CLIENT_AUTH_CERT,
+            connect_callback.GetResult(client_ret));
+  EXPECT_EQ(ERR_BAD_SSL_CLIENT_AUTH_CERT,
+            handshake_callback.GetResult(server_ret));
+}
+#endif  // defined(USE_OPENSSL)
+
 TEST_F(SSLServerSocketTest, DataTransfer) {
   Initialize();
 
+  // Establish connection.
   TestCompletionCallback connect_callback;
-  TestCompletionCallback handshake_callback;
-
-  // Establish connection.
   int client_ret = client_socket_->Connect(connect_callback.callback());
   ASSERT_TRUE(client_ret == OK || client_ret == ERR_IO_PENDING);
 
+  TestCompletionCallback handshake_callback;
   int server_ret = server_socket_->Handshake(handshake_callback.callback());
   ASSERT_TRUE(server_ret == OK || server_ret == ERR_IO_PENDING);
 
   client_ret = connect_callback.GetResult(client_ret);
   ASSERT_EQ(OK, client_ret);
   server_ret = handshake_callback.GetResult(server_ret);
   ASSERT_EQ(OK, server_ret);
 
   const int kReadBufSize = 1024;
   scoped_refptr<StringIOBuffer> write_buf =
@@ skipping 57 matching lines @@
   EXPECT_EQ(0, memcmp(write_buf->data(), read_buf->data(), write_buf->size()));
 }
 
 // A regression test for bug 127822 (http://crbug.com/127822).
 // If the server closes the connection after the handshake is finished,
 // the client's Write() call should not cause an infinite loop.
 // NOTE: this is a test for SSLClientSocket rather than SSLServerSocket.
 TEST_F(SSLServerSocketTest, ClientWriteAfterServerClose) {
   Initialize();
 
-  TestCompletionCallback connect_callback;
-  TestCompletionCallback handshake_callback;
 
   // Establish connection.
+  TestCompletionCallback connect_callback;
   int client_ret = client_socket_->Connect(connect_callback.callback());
   ASSERT_TRUE(client_ret == OK || client_ret == ERR_IO_PENDING);
 
+  TestCompletionCallback handshake_callback;
   int server_ret = server_socket_->Handshake(handshake_callback.callback());
   ASSERT_TRUE(server_ret == OK || server_ret == ERR_IO_PENDING);
 
   client_ret = connect_callback.GetResult(client_ret);
   ASSERT_EQ(OK, client_ret);
   server_ret = handshake_callback.GetResult(server_ret);
   ASSERT_EQ(OK, server_ret);
 
   scoped_refptr<StringIOBuffer> write_buf = new StringIOBuffer("testing123");
 
   // The server closes the connection. The server needs to write some
   // data first so that the client's Read() calls from the transport
   // socket won't return ERR_IO_PENDING.  This ensures that the client
   // will call Read() on the transport socket again.
   TestCompletionCallback write_callback;
-
   server_ret = server_socket_->Write(
       write_buf.get(), write_buf->size(), write_callback.callback());
   EXPECT_TRUE(server_ret > 0 || server_ret == ERR_IO_PENDING);
 
   server_ret = write_callback.GetResult(server_ret);
   EXPECT_GT(server_ret, 0);
 
   server_socket_->Disconnect();
 
   // The client writes some data. This should not cause an infinite loop.
@@ skipping 10 matching lines @@
   base::MessageLoop::current()->Run();
 }
 
 // This test executes ExportKeyingMaterial() on the client and server sockets,
 // after connecting them, and verifies that the results match.
 // This test will fail if False Start is enabled (see crbug.com/90208).
 TEST_F(SSLServerSocketTest, ExportKeyingMaterial) {
   Initialize();
 
   TestCompletionCallback connect_callback;
-  TestCompletionCallback handshake_callback;
-
   int client_ret = client_socket_->Connect(connect_callback.callback());
   ASSERT_TRUE(client_ret == OK || client_ret == ERR_IO_PENDING);
 
+  TestCompletionCallback handshake_callback;
   int server_ret = server_socket_->Handshake(handshake_callback.callback());
   ASSERT_TRUE(server_ret == OK || server_ret == ERR_IO_PENDING);
 
   if (client_ret == ERR_IO_PENDING) {
     ASSERT_EQ(OK, connect_callback.WaitForResult());
   }
   if (server_ret == ERR_IO_PENDING) {
     ASSERT_EQ(OK, handshake_callback.WaitForResult());
   }
 
@@ skipping 39 matching lines @@
   };
   client_ssl_config_.disabled_cipher_suites.assign(
       kEcdheCiphers, kEcdheCiphers + arraysize(kEcdheCiphers));
 
   // Require ECDHE on the server.
   server_ssl_config_.require_ecdhe = true;
 
   Initialize();
 
   TestCompletionCallback connect_callback;
+  int client_ret = client_socket_->Connect(connect_callback.callback());
+
   TestCompletionCallback handshake_callback;
-
-  int client_ret = client_socket_->Connect(connect_callback.callback());
   int server_ret = server_socket_->Handshake(handshake_callback.callback());
 
   client_ret = connect_callback.GetResult(client_ret);
   server_ret = handshake_callback.GetResult(server_ret);
 
   ASSERT_EQ(ERR_SSL_VERSION_OR_CIPHER_MISMATCH, client_ret);
   ASSERT_EQ(ERR_SSL_VERSION_OR_CIPHER_MISMATCH, server_ret);
 }
 
 }  // namespace net