Wire up SSL client authentication for OpenSSL/Android through the net/ stack

net/socket/ssl_client_socket_openssl_unittest.cc

Index: net/socket/ssl_client_socket_openssl_unittest.cc
diff --git a/net/socket/ssl_client_socket_openssl_unittest.cc b/net/socket/ssl_client_socket_openssl_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5c383d40697ae202b54965dda74a177a79924c72
--- /dev/null
+++ b/net/socket/ssl_client_socket_openssl_unittest.cc
@@ -0,0 +1,424 @@
+// 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 <errno.h>
+#include <string.h>
+
+#include <openssl/bn.h>
+#include <openssl/evp.h>
+#include <openssl/pem.h>
+#include <openssl/rsa.h>
+
+#include "base/file_util.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_handle.h"
+#include "base/values.h"
+#include "crypto/openssl_util.h"
+#include "net/base/address_list.h"
+#include "net/base/cert_test_util.h"
+#include "net/base/host_resolver.h"
+#include "net/base/io_buffer.h"
+#include "net/base/mock_cert_verifier.h"
+#include "net/base/net_errors.h"
+#include "net/base/net_log.h"
+#include "net/base/net_log_unittest.h"
+#include "net/base/openssl_private_key_store.h"
+#include "net/base/ssl_cert_request_info.h"
+#include "net/base/ssl_config_service.h"
+#include "net/base/test_completion_callback.h"
+#include "net/base/test_data_directory.h"
+#include "net/base/test_root_certs.h"
+#include "net/socket/client_socket_factory.h"
+#include "net/socket/client_socket_handle.h"
+#include "net/socket/socket_test_util.h"
+#include "net/socket/tcp_client_socket.h"
+#include "net/test/test_server.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+//-----------------------------------------------------------------------------
+
+namespace {
+
+typedef net::OpenSSLPrivateKeyStore::ScopedEVP_PKEY ScopedEVP_PKEY;
+
+typedef crypto::ScopedOpenSSL<RSA, RSA_free> ScopedRSA;
+typedef crypto::ScopedOpenSSL<BIGNUM, BN_free> ScopedBIGNUM;

[Ryan Sleevi, 2013/02/13 23:25:55]

scoped_ptr<> ?

[digit1, 2013/02/14 06:23:50]

I don't really see the point given that it requires so much more typing, and
this is unit test code, not something exported by the net/ stack.

Slightly off-topic, it would be nice to have a header defining common
declarations for scoped OpenSSL types that can be reused by several source files
(currently, each one has its own set of typedefs like the one above).  Do you
find this idea wortwhile, and if so, do you have a suggestion for this header
(net/base/x509_util.h maybe?). I don't intend to address this point in this
patch, nor decide whether the declarations should use ScopedOpenSSL or
scoped_ptr, but we're discussing a related issue here, so your input is
welcomed.

[Ryan Sleevi, 2013/02/14 07:15:00]

crypto/openssl_util -> crypto/scoped_openssl_types.h ?

If the types are not SSL-specific (eg: SSL/SSL_CTX), it may make more sense
there.

Alternatively, the benefit of explicitly avoiding "_util" headers is that to get
one type you're not forced to pull in 6 other unrelated headers.

+
+// The following is needed to construct paths to certificates passed as
+// |client_authorities| in server SSLOptions. Current implementation of
+// RemoteTestServer (used on Android) expects relative paths, as opposed to
+// LocalTestServer, which expects absolute paths (what to fix?).
+base::FilePath CertDirectory() {
+#ifdef OS_ANDROID
+  return net::GetTestCertsDirectoryRelative();
+#else
+  return net::GetTestCertsDirectory();
+#endif
+}

[Ryan Sleevi, 2013/02/13 23:25:55]

This shouldn't be in a unittest file like this.

[digit1, 2013/02/14 06:23:50]

Can you clarify? This comes straight from
net/socket/ssl_client_socket_unittest.cc btw.

[Ryan Sleevi, 2013/02/14 07:15:00]

Then we should refactor the logic in test_data_directory to hide the OS_ANDROID.
Repeating subtle logic in multiple files like this is a recipe for breakages, so
it'd be better to centralize that logic. Given that GetTestCertsDirectory and
GetTestCertsDirectoryRelative both live in test_data_directory, and given that
GetTestCertsDirectoryRelative is *only* called by ssl_client_socket*, perhaps it
should be GetTestClientCertsDirectory() and have the implementation in
test_data_directory do the #ifdef switch as appropriate.

[digit1, 2013/02/14 08:24:39]

I see, I'll do that then.

+
+// Loads a PEM-encoded private key file into a scoped EVP_PKEY object.
+// |filepath| is the private key file path.
+// |*pkey| is reset to the new EVP_PKEY on success, untouched otherwise.
+// Returns true on success, false on failure.
+bool LoadPrivateKeyOpenSSL(
+    const base::FilePath& filepath,
+    net::OpenSSLPrivateKeyStore::ScopedEVP_PKEY* pkey) {
+  ScopedStdioHandle file(file_util::OpenFile(filepath, "rb"));

[Ryan Sleevi, 2013/02/13 23:25:55]

Use FileUtil::ReadFileToString, rather than a ScopedStdioHandle. Avoid the extra
dependency on FILE* glue.

[digit1, 2013/02/14 06:23:50]

I'm surprised by this request, given that last week you asked for _exactly_ the
opposite (i.e. I was using file_util::ReadFileToString, but you asked me to use
PEM_read_PrivateKey(), which requires FILE* handles). Are you really sure you
want this changed in the opposite direction?

[Ryan Sleevi, 2013/02/14 07:15:00]

I seem to recall that you were using the _BIO variant to open the file (that is,
not using ReadFileToString, but BIO_new_file/BIO_s_file)

For OpenSSL, we should be handling all File I/O using our API routines, reading
into a buffer, and supplying that directly (to the OpenSSL functions that accept
raw buffers - ie: most). For the API functions that *require* a BIO or FILE (the
PEM variants include this), use BIO_new_mem_buf(some_string.data(),
some_string.size()) to create a non-mutable, read-only BIO that wraps the
string.

[digit1, 2013/02/14 08:24:39]

No, I was using a memory BIO to pass the data read from ReadFileToString(), just
like you recommend below. Found it at
https://chromiumcodereview.appspot.com/11571059/diff/51001/net/android/keysto...
 
Sigh. Will do :(

[Ryan Sleevi, 2013/02/14 08:51:15]

Bah! I think at the time I wrote that comment I was thinking PEM_read_PrivateKey
was the char* based version 9that is, wrapping it in BIO_new_mem_buf was
overkill for a pure memory based).

Your original change was correct. Your reviewer was asleep at the wheel.

+  if (!file.get()) {
+    LOG(ERROR) << "Could not open private key file: "
+               << filepath.value() << ": " << strerror(errno);
+    return false;
+  }
+  EVP_PKEY* result = PEM_read_PrivateKey(file.get(), NULL, NULL, NULL);
+  if (result == NULL) {
+    LOG(ERROR) << "Could not read private key file: "
+               << filepath.value();
+    return false;
+  }
+  pkey->reset(result);
+  return true;
+}
+
+// By default, OpenSSL checks that the private key and the certificate
+// match. If not, it will reject them immediately, resulting in a
+// net::ERR_SSL_PROTOCOL_ERROR, and no client certificate being sent
+// to the server.
+//
+// This function create a new random RSA-based private key that can
+// bypass this check, by setting the RSA_METHOD_FLAG_NO_CHECK in its
+// RSA_METHOD.
+//
+// |num_bits| is the key length in bits.
+// |*pkey| is reset to the new key value on success, untouched otherwise.
+// Returns true on success.
+bool CreateRandomPrivateKey(

[Ryan Sleevi, 2013/02/13 23:25:55]

This is not something we want to do in tests, since it makes it significantly
worse under ASAN/TSAN/Valgrind/any form of runtime inspection.

There's no need to generate a new key every run.

[digit1, 2013/02/14 06:23:50]

I'm not sure to understand what you mean here. Can you clarify?
This is the simplest way I found to generate an EVP_PKEY which had the
interesting property of not being rejected by OpenSSL when used with a
non-matching certificate. The tricky part is that this requires a custom
RSA_METHOD, and functions like PEM_read_PrivateKey() cannot be used to load an
EVP_PKEY with said custom method. The alternative is to implement a custom
ENGINE to address this single point, which appears to be a _lot_ more work.

But the point is moot, because the SendBadSignature test is going to be removed,
so let's not lose time over this anyway.

[Ryan Sleevi, 2013/02/14 07:15:00]

We don't want to generate random keys in tests. Every test where key generation
is performed is disabled under every one of our memory sanitizers, because
there's no way to efficiently do things like primality tests under the
additional checks.
If that's the case (and I haven't looked closely at the OpenSSL code to see if
there's any workarounds), then given the constraint, wouldn't it be easier to
just make the server 'lie' about the key being invalid? Are you testing client
behaviour (that signatures are pairwise consistency checked?) or are you
checking server behaviour (that the server will reject the signature?) I didn't
think OpenSSL did any PCTs in non-FIPS mode for signatures, due to performance,
so I'd be very surprised if you were testing client behaviour here, so much as
checking that the client handles when the server *says* it was invalid.

[digit1, 2013/02/14 08:24:39]

I see, I didn't realize that. Thanks for the clarification.
 
I just wanted to have a test where the server would refuse the connection even
though a certificate is sent by the client socket. Normally SendBadCert would be
enough, but it is disabled because our version of tlslite doesn't verify client
certificate chains (it would accept _any_ client certificate, I looked at the
source code the check this).

SendBadSignature was just a hacky work-around for this issue. I don't think
there is a way to configure the server to lie about the key being invalid, and I
certainly don't want to have to send a patch upstream to add this "feature", or
fix the verification problem.

However, I don't see this issue critical to test SSLClientSocketOpenSSL, so I
prefer to drop this single test entirely.

[Ryan Sleevi, 2013/02/14 08:51:15]

We're effectively a forked version of TLSlite. I've been talking with Trevor
Perrin, the upstream maintainer, on getting us de-forked (the project wasn't
active for 6 years, so we 'were' effectively upstream for a while). So you can
add it to the Python code and be done.

That said, it's fine since you dropped it.

+    int num_bits,
+    net::OpenSSLPrivateKeyStore::ScopedEVP_PKEY* pkey) {
+  // Ensure the initialization of a custom RSA_METHOD that has the
+  // RSA_METHOD_FLAG_NO_CHECK flag set.
+  static int s_rsa_method_init;
+  static RSA_METHOD s_rsa_method;
+
+  if (!s_rsa_method_init) {
+    s_rsa_method = *RSA_get_default_method();
+    s_rsa_method.flags |= RSA_METHOD_FLAG_NO_CHECK;
+    s_rsa_method_init = 1;
+  }
+
+  // Create a new RSA key with the new method.
+  ScopedRSA rsa(RSA_new());
+  RSA_set_method(rsa.get(), &s_rsa_method);
+
+  // Create exponent as a BIGNUM, this copies the code in
+  // RSA_generate_key(), which can't be used here because it creates
+  // a new RSA key.
+  const unsigned long exponent = 65537;
+  ScopedBIGNUM bn_exponent(BN_new());
+  for (int i = 0; i < static_cast<int>(8*sizeof(unsigned long)); ++i) {
+    if (exponent & (1UL << i))
+      BN_set_bit(bn_exponent.get(), i);
+  }
+
+  if (!RSA_generate_key_ex(rsa.get(), num_bits, bn_exponent.get(), NULL)) {
+    LOG(ERROR) << "Can't generate RSA private key";
+    return false;
+  }
+
+  // Generate EVP_PKEY wrapper
+  ScopedEVP_PKEY result(EVP_PKEY_new());
+  if (!EVP_PKEY_set1_RSA(result.get(), rsa.get())) {
+    LOG(ERROR) << "Could not create EVP_PKEY wrapper for RSA private key";
+    return false;
+  }
+  rsa.release();
+
+  pkey->swap(result);
+  return true;
+}
+
+// LogContainsSSLConnectEndEvent returns true if the given index in the given
+// log is an SSL connect end event. The NSS sockets will cork in an attempt to
+// merge the first application data record with the Finished message when false
+// starting. However, in order to avoid the server timing out the handshake,
+// they'll give up waiting for application data and send the Finished after a
+// timeout. This means that an SSL connect end event may appear as a socket
+// write.
+bool LogContainsSSLConnectEndEvent(
+    const net::CapturingNetLog::CapturedEntryList& log, int i) {
+  return net::LogContainsEndEvent(log, i, net::NetLog::TYPE_SSL_CONNECT) ||
+         net::LogContainsEvent(log, i, net::NetLog::TYPE_SOCKET_BYTES_SENT,
+                                net::NetLog::PHASE_NONE);
+};
+
+}  // namespace

[Ryan Sleevi, 2013/02/13 23:25:55]

Put the entire file into namespace net { namespace { } }.

[digit1, 2013/02/14 06:23:50]

This would result in a link error, because there are similarly named functions
in net/socket/ssl_client_socket_unittest.cc (where they come from), which are
already in the net:: namespace.

I didn't want to move them to yet another common header/source file, so putting
them in an anonymous namespace seemed the simpler thing. The alternative is to
rename the functions though. Are you really sure about this point?

[Ryan Sleevi, 2013/02/14 07:15:00]

Not if you put them in the unnamed namespace, like I was suggesting. The point
was to eliminate the need to do net:: everywhere, while still preserving the
unnamed namespace.
Yes. Because I'm cleaning up ssl_client_socket_unittest tomorrow to move them
into net::, as this was something that I realized when I was adding a unittest
myself (see https://codereview.chromium.org/12025040/#msg1 ). I want to avoid
the mistakes in the new file :)

[digit1, 2013/02/14 08:24:39]

turns out that using 'static' is enough to put the function safely in the net
namespace here.

[digit1, 2013/02/14 08:24:39]

Oh, I realize I misread your initial comment. will do.
  

+
+//-----------------------------------------------------------------------------

[Ryan Sleevi, 2013/02/13 23:25:55]

Remove this

[digit1, 2013/02/14 06:23:50]

Again, this came from net/socket/ssl_client_socket_unittest.cc. I'll remove them
though.

[Ryan Sleevi, 2013/02/14 07:15:00]

Same cleanup I'm working on there, trying to avoid having to have that persist
in the new file.

+
+const net::SSLConfig kDefaultSSLConfig;
+
+class SSLClientSocketOpenSSLClientAuthTest : public PlatformTest {
+ public:
+  SSLClientSocketOpenSSLClientAuthTest()
+      : socket_factory_(net::ClientSocketFactory::GetDefaultFactory()),
+        cert_verifier_(new net::MockCertVerifier) {
+    cert_verifier_->set_default_result(net::OK);
+    context_.cert_verifier = cert_verifier_.get();
+    key_store_ = net::OpenSSLPrivateKeyStore::GetInstance();
+  }
+
+ protected:
+  virtual ~SSLClientSocketOpenSSLClientAuthTest() {
+    key_store_->Flush();
+  }
+
+  virtual void TearDown() {
+//     if (sock_.get() && sock_->IsConnected())
+//       sock_->Disconnect();
+  }
+
+  net::SSLClientSocket* CreateSSLClientSocket(
+      net::StreamSocket* transport_socket,
+      const net::HostPortPair& host_and_port,
+      const net::SSLConfig& ssl_config) {
+    return socket_factory_->CreateSSLClientSocket(transport_socket,
+                                                  host_and_port,
+                                                  ssl_config,
+                                                  context_);
+  }
+
+  // Connect to a HTTPS test server.
+  bool ConnectToTestServer(net::TestServer::SSLOptions& ssl_options) {
+    test_server_.reset(new net::TestServer(net::TestServer::TYPE_HTTPS,
+                                           ssl_options,
+                                           base::FilePath()));
+    if (!test_server_.get()) {
+      LOG(ERROR) << "Could not create new TestServer";
+      return false;
+    }
+    if (!test_server_->Start()) {
+      LOG(ERROR) << "Could not start TestServer";
+      return false;
+    }
+
+    if (!test_server_->GetAddressList(&addr_)) {
+      LOG(ERROR) << "Could not get TestServer address list";
+      return false;
+    }
+
+    transport_.reset(new net::TCPClientSocket(
+        addr_, &log_, net::NetLog::Source()));
+    int rv = transport_->Connect(callback_.callback());
+    if (rv == net::ERR_IO_PENDING)
+      rv = callback_.WaitForResult();
+    if (rv != net::OK) {
+      LOG(ERROR) << "Could not connect to TestServer";
+      return false;
+    }
+    return true;
+  }
+
+  bool RecordPrivateKey(net::SSLConfig& ssl_config,
+                        EVP_PKEY* private_key) {
+    return key_store_->RecordClientCertPrivateKey(
+        ssl_config.client_cert.get(), private_key);
+  }
+
+  bool CreateAndConnectSSLClientSocket(net::SSLConfig& ssl_config,
+                                       int* result) {
+    sock_.reset(CreateSSLClientSocket(transport_.release(),
+                                      test_server_->host_port_pair(),
+                                      ssl_config));
+
+    if (sock_->IsConnected()) {
+      LOG(ERROR) << "SSL Socket prematurely connected";
+      return false;
+    }
+
+    int rv = sock_->Connect(callback_.callback());
+
+    net::CapturingNetLog::CapturedEntryList entries;
+    log_.GetEntries(&entries);
+    if (!net::LogContainsBeginEvent(
+            entries, 5, net::NetLog::TYPE_SSL_CONNECT)) {
+      LOG(ERROR) << "SSL connection not started in logs";
+      return false;
+    }
+    if (rv == net::ERR_IO_PENDING)
+      rv = callback_.WaitForResult();
+
+    *result = rv;
+    return true;
+  }
+
+
+  bool CheckSSLClientSocketSentCert() {
+    net::CapturingNetLog::CapturedEntryList entries;
+    log_.GetEntries(&entries);
+    if (!LogContainsSSLConnectEndEvent(entries, -1)) {
+      LOG(ERROR) << "!LogContainsSSLConnectEndEvent()";
+      return false;
+    }
+
+    // Check that the client certificate was sent.
+    net::SSLInfo ssl_info;
+    sock_->GetSSLInfo(&ssl_info);
+    return ssl_info.client_cert_sent;
+  }
+
+  net::ClientSocketFactory* socket_factory_;
+  scoped_ptr<net::MockCertVerifier> cert_verifier_;
+  net::SSLClientSocketContext context_;
+  net::OpenSSLPrivateKeyStore* key_store_;
+  scoped_ptr<net::TestServer> test_server_;
+  net::AddressList addr_;
+  net::TestCompletionCallback callback_;
+  net::CapturingNetLog log_;
+  scoped_ptr<net::StreamSocket> transport_;
+  scoped_ptr<net::SSLClientSocket> sock_;
+};
+
+//-----------------------------------------------------------------------------

[Ryan Sleevi, 2013/02/13 23:25:55]

Remove this

[digit1, 2013/02/14 08:24:39]

Done.

+
+// Connect to a server requesting client authentication, do not send
+// any client certificates. It should refuse the connection.
+TEST_F(SSLClientSocketOpenSSLClientAuthTest, NoCert) {
+  net::TestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = net::GetTestCertsDirectory();
+  net::SSLConfig ssl_config = kDefaultSSLConfig;
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+
+  EXPECT_EQ(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED, rv);
+  EXPECT_FALSE(sock_->IsConnected());
+}
+
+// Connect to a server requesting client authentication, and send it
+// an empty certificate. It should refuse the connection.
+TEST_F(SSLClientSocketOpenSSLClientAuthTest, SendEmptyCert) {
+  net::TestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = net::GetTestCertsDirectory();
+  net::SSLConfig ssl_config = kDefaultSSLConfig;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = NULL;
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+
+  EXPECT_EQ(net::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) {
+  net::TestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+  ssl_options.client_authorities.push_back(
+      CertDirectory().AppendASCII("client_1_root.pem"));
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = net::GetTestCertsDirectory();
+  net::SSLConfig ssl_config = kDefaultSSLConfig;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = net::ImportCertFromFile(certs_dir,
+                                                   "client_1.pem");
+
+  // This is required to ensure that signing works with the client
+  // certificate's private key.
+  net::OpenSSLPrivateKeyStore::ScopedEVP_PKEY client_private_key;
+  ASSERT_TRUE(LoadPrivateKeyOpenSSL(certs_dir.AppendASCII("client_1.key"),
+                                    &client_private_key));
+  EXPECT_TRUE(RecordPrivateKey(ssl_config, client_private_key.get()));
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+
+  EXPECT_EQ(net::OK, rv);
+  EXPECT_TRUE(sock_->IsConnected());
+
+  EXPECT_TRUE(CheckSSLClientSocketSentCert());
+
+  sock_->Disconnect();
+  EXPECT_FALSE(sock_->IsConnected());
+}
+
+// Connect to a server requesting client authentication. Send it a
+// non-matching certificate. It should not allow the connection.
+// NOTE: Disabled because our TestServer never verifies that the client
+//       certificate matches the required CA authorities. Thus is always
+//       accepts the connection.
+TEST_F(SSLClientSocketOpenSSLClientAuthTest, DISABLED_SendBadCert) {
+  net::TestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+  ssl_options.client_authorities.push_back(
+      CertDirectory().AppendASCII("client_1_root.pem"));
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = net::GetTestCertsDirectory();
+  net::SSLConfig ssl_config = kDefaultSSLConfig;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = net::ImportCertFromFile(certs_dir,
+                                                   "client_2.pem");
+
+  net::OpenSSLPrivateKeyStore::ScopedEVP_PKEY client_private_key;
+  ASSERT_TRUE(LoadPrivateKeyOpenSSL(certs_dir.AppendASCII("client_2.key"),
+                                    &client_private_key));
+  EXPECT_TRUE(RecordPrivateKey(ssl_config, client_private_key.get()));
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+
+  EXPECT_EQ(net::ERR_BAD_SSL_CLIENT_AUTH_CERT, rv);
+  EXPECT_FALSE(sock_->IsConnected());
+
+  EXPECT_TRUE(CheckSSLClientSocketSentCert());
+}
+
+// Connect to a server requesting client authentication. Send it a
+// matching certificate, but do not sign with the right private key.
+// It should not allow the connection.

[Ryan Sleevi, 2013/02/13 23:25:55]

What is this really testing? BadSignature is a server issue, but you're testing
a client socket.

You could just as well have the server 'lie' for a perfectly good signature.

[digit1, 2013/02/14 06:23:50]

Ok, I'll remove this test, this gets rid of the problematic
CreateRandomPrivateKey() function too.

+TEST_F(SSLClientSocketOpenSSLClientAuthTest, SendBadSignature) {
+  net::TestServer::SSLOptions ssl_options;
+  ssl_options.request_client_certificate = true;
+  ssl_options.client_authorities.push_back(
+      CertDirectory().AppendASCII("client_1_root.pem"));
+
+  ASSERT_TRUE(ConnectToTestServer(ssl_options));
+
+  base::FilePath certs_dir = net::GetTestCertsDirectory();
+  net::SSLConfig ssl_config = kDefaultSSLConfig;
+  ssl_config.send_client_cert = true;
+  ssl_config.client_cert = net::ImportCertFromFile(certs_dir,
+                                                   "client_1.pem");
+
+  // Instead of the matching private key, generate a new random one.
+  // This one is specially crafted to bypass OpenSSL checks.
+  // IMPORTANT: Use a key size that matches the certificate public key's,
+  // otherwise the TestServer will abort violently with an exception
+  // during the verification phase.
+  ScopedEVP_PKEY client_private_key;
+  ASSERT_TRUE(CreateRandomPrivateKey(2048, &client_private_key));
+  EXPECT_TRUE(RecordPrivateKey(ssl_config, client_private_key.get()));
+
+  int rv;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+
+  EXPECT_FALSE(sock_->IsConnected());
+  EXPECT_EQ(net::ERR_SSL_PROTOCOL_ERROR, rv);
+  EXPECT_TRUE(CheckSSLClientSocketSentCert());
+}