Use SSLPrivateKey for testing client authentication

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 6a9d4654bdf5c0cf3923e6b41898f74cd4dbb3a1..cc4bfe674fc3912038944cce747ae91cabb1dda9 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/callback_helpers.h"
+#include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -49,6 +50,21 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
+#if defined(USE_OPENSSL)
+#include <errno.h>
+#include <openssl/bio.h>
+#include <openssl/bn.h>

[davidben, 2016/01/28 22:24:10]

Unused?

[svaldez, 2016/01/29 17:19:29]

Done.

+#include <openssl/evp.h>
+#include <openssl/pem.h>
+#include <openssl/rsa.h>

[davidben, 2016/01/28 22:24:09]

Unused?

[svaldez, 2016/01/29 17:19:29]

Done.

+#include <string.h>
+#include <utility>

[davidben, 2016/01/28 22:24:10]

Already included up top.

[svaldez, 2016/01/29 17:19:30]

Done.

+
+#include "crypto/openssl_util.h"

[davidben, 2016/01/28 22:24:10]

Unused?

[svaldez, 2016/01/29 17:19:29]

Done.

+#include "crypto/scoped_openssl_types.h"
+#include "net/ssl/ssl_platform_key_test.h"
+#endif
+
 using testing::_;
 using testing::Return;
 using testing::Truly;
@@ -3240,4 +3256,206 @@ TEST_F(SSLClientSocketTest, NPNServerDisabled) {
             sock_->GetNextProto(&proto));
 }
 
+// These client auth tests are currently dependent on OpenSSL's struct X509.

[davidben, 2016/01/28 22:24:09]

// Client auth is not supported in NSS ports.

or something like that.

[svaldez, 2016/01/29 17:19:30]

Done.

+#if defined(USE_OPENSSL)
+
+namespace {
+
+// Loads a PEM-encoded private key file into a SSLPrivateKey object.
+// |filepath| is the private key file path.
+// |*pkey| is reset to the new SSLPrivateKey on success, untouched otherwise.
+// Returns true on success, false on failure.
+bool LoadPrivateKeyOpenSSL(const base::FilePath& filepath,
+                           scoped_refptr<SSLPrivateKey>* pkey) {

[davidben, 2016/01/28 22:24:09]

Why not just return a scoped_refptr<Thingy>?

[svaldez, 2016/01/29 17:19:30]

Done.

+  std::string data;
+  if (!base::ReadFileToString(filepath, &data)) {
+    LOG(ERROR) << "Could not read private key file: "
+               << filepath.value() << ": " << strerror(errno);

[davidben, 2016/01/28 22:24:10]

I'd drop the errno bit. I'm not sure that works on Windows anyway.

[svaldez, 2016/01/29 17:19:29]

Done.

+    return false;
+  }
+  crypto::ScopedBIO bio(BIO_new_mem_buf(
+      const_cast<char*>(reinterpret_cast<const char*>(data.data())),

[davidben, 2016/01/28 22:24:10]

I think you only need the const_cast.

[svaldez, 2016/01/29 17:19:29]

Done.

+      static_cast<int>(data.size())));
+  if (!bio.get()) {

[davidben, 2016/01/28 22:24:10]

!bio also works.

[svaldez, 2016/01/29 17:19:30]

Done.

+    LOG(ERROR) << "Could not allocate BIO for buffer?";
+    return false;
+  }
+  EVP_PKEY* result = PEM_read_bio_PrivateKey(bio.get(), NULL, NULL, NULL);

[davidben, 2016/01/28 22:24:09]

crypto::ScopedEVP_PKEY result(PEM_read_bio_PrivateKey(bio.get(), nullptr,
nullptr, nullptr));
if (!result) {
  ...
}
*pkey = WrapOpenSSLPrivateKey(std::move(result));

[svaldez, 2016/01/29 17:19:30]

Done.

+  if (result == NULL) {
+    LOG(ERROR) << "Could not decode private key file: "
+               << filepath.value();
+    return false;
+  }
+  *pkey = WrapOpenSSLPrivateKey(crypto::ScopedEVP_PKEY(result));
+  return true;
+}
+
+class SSLClientSocketOpenSSLClientAuthTest : public PlatformTest {

[davidben, 2016/01/28 22:24:09]

I think this class is more-or-less a subset of SSLClientSocketTest.
CheckSSLClientSocketSentCert isn't in there, but you can just inline that, TBH.

[svaldez, 2016/01/29 17:19:29]

Done.

+ public:
+  SSLClientSocketOpenSSLClientAuthTest()
+      : 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();
+  }
+
+  ~SSLClientSocketOpenSSLClientAuthTest() override {}
+
+ protected:
+  scoped_ptr<SSLClientSocket> CreateSSLClientSocket(
+      scoped_ptr<StreamSocket> transport_socket,
+      const HostPortPair& host_and_port,
+      const SSLConfig& ssl_config) {
+    scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
+    connection->SetSocket(std::move(transport_socket));
+    return socket_factory_->CreateSSLClientSocket(
+        std::move(connection), host_and_port, ssl_config, context_);
+  }
+
+  // Connect to a HTTPS test server.
+  bool ConnectToTestServer(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;
+    }
+
+    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;
+  }
+
+  // Create an SSLClientSocket object and use it to connect to a test
+  // server, then wait for connection results. This must be called after
+  // a succesful ConnectToTestServer() call.
+  // |ssl_config| the SSL configuration to use.
+  // |result| will retrieve the ::Connect() result value.
+  // Returns true on succes, false otherwise. Success means that the socket
+  // could be created and its Connect() was called, not that the connection
+  // itself was a success.
+  bool CreateAndConnectSSLClientSocket(const SSLConfig& ssl_config,
+                                       int* result) {
+    sock_ = CreateSSLClientSocket(std::move(transport_),
+                                  test_server_->host_port_pair(), ssl_config);
+
+    if (sock_->IsConnected()) {
+      LOG(ERROR) << "SSL Socket prematurely connected";
+      return false;
+    }
+
+    *result = callback_.GetResult(sock_->Connect(callback_.callback()));
+    return true;
+  }
+
+
+  // Check that the client certificate was sent.
+  // Returns true on success.
+  bool CheckSSLClientSocketSentCert() {
+    SSLInfo ssl_info;
+    sock_->GetSSLInfo(&ssl_info);
+    return ssl_info.client_cert_sent;
+  }
+
+  ClientSocketFactory* socket_factory_;
+  scoped_ptr<MockCertVerifier> cert_verifier_;
+  scoped_ptr<TransportSecurityState> transport_security_state_;
+  SSLClientSocketContext context_;
+  scoped_ptr<SpawnedTestServer> test_server_;
+  AddressList addr_;
+  TestCompletionCallback callback_;
+  NetLog log_;
+  scoped_ptr<StreamSocket> transport_;
+  scoped_ptr<SSLClientSocket> sock_;
+};
+
+// Connect to a server requesting client authentication, do not send
+// any client certificates. It should refuse the connection.
+TEST_F(SSLClientSocketOpenSSLClientAuthTest, NoCert) {

[davidben, 2016/01/28 22:24:10]

Why is this test in the anonymous namespace and not the rest?

[svaldez, 2016/01/29 17:19:30]

Done.

+  SpawnedTestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = GetTestCertsDirectory();
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
+
+  EXPECT_EQ(ERR_SSL_CLIENT_AUTH_CERT_NEEDED, rv);
+  EXPECT_FALSE(sock_->IsConnected());
+}
+
+}  // namespace
+
+// Connect to a server requesting client authentication, and send it
+// an empty certificate. It should refuse the connection.

[davidben, 2016/01/28 22:24:09]

It doesn't seem to actually refuse the connection...

[davidben, 2016/01/28 22:24:10]

an empty -> no

[svaldez, 2016/01/29 17:19:30]

This sends an empty cert (send_client_cert is true).

+TEST_F(SSLClientSocketOpenSSLClientAuthTest, SendEmptyCert) {
+  SpawnedTestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+  ssl_options.client_authorities.push_back(
+      GetTestClientCertsDirectory().AppendASCII("client_1_ca.pem"));
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = GetTestCertsDirectory();

[davidben, 2016/01/28 22:24:09]

This looks unused.

[svaldez, 2016/01/29 17:19:29]

Done.

+  SSLConfig ssl_config;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = NULL;

[davidben, 2016/01/28 22:24:09]

nullptr

[svaldez, 2016/01/29 17:19:29]

Done.

+  ssl_config.client_private_key = NULL;
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(sock_->IsConnected());
+}
+
+// Connect to a server requesting client authentication. Send it a
+// matching certificate. It should allow the connection.
+TEST_F(SSLClientSocketOpenSSLClientAuthTest, SendGoodCert) {
+  SpawnedTestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+  ssl_options.client_authorities.push_back(
+      GetTestClientCertsDirectory().AppendASCII("client_1_ca.pem"));
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = GetTestCertsDirectory();
+  SSLConfig ssl_config;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = ImportCertFromFile(certs_dir, "client_1.pem");
+
+  // This is required to ensure that signing works with the client
+  // certificate's private key.
+  ASSERT_TRUE(LoadPrivateKeyOpenSSL(certs_dir.AppendASCII("client_1.key"),
+                                    &(ssl_config.client_private_key)));
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+
+  EXPECT_EQ(OK, rv);
+  EXPECT_TRUE(sock_->IsConnected());
+
+  EXPECT_TRUE(CheckSSLClientSocketSentCert());
+
+  sock_->Disconnect();
+  EXPECT_FALSE(sock_->IsConnected());
+}
+#endif  // defined(USE_OPENSSL)
+
 }  // namespace net