Skip to content

Commit

Permalink
Implement TLS client auth in the OS X OpenSSL port.
Browse files Browse the repository at this point in the history
This introduces a openssl_platform_key.h that looks up and wraps a platform
private key from the platform key store and returns an EVP_PKEY. It is
implemented on Mac and left as a stub on Windows. This will be refactored with
https://crbug.com/394131.

The USE_OPENSSL_CERTS case has been left intact to preserve the existing tests
on Linux but, possibly after the refactor, this will need to change as Linux and
CrOS will likely still use OpenSSL handles for X509Certificate but will not
likely want the OpenSSLClientKeyStore hack.

BUG=394131

Review URL: https://codereview.chromium.org/396803002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286112 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
davidben@chromium.org committed Jul 29, 2014
1 parent 1813c4f commit 97a854f
Show file tree
Hide file tree
Showing 13 changed files with 577 additions and 50 deletions.
15 changes: 13 additions & 2 deletions net/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,26 @@ component("net") {
"quic/crypto/p256_key_exchange_openssl.cc",
"quic/crypto/scoped_evp_aead_ctx.cc",
"quic/crypto/scoped_evp_aead_ctx.h",
"socket/openssl_ssl_util.cc",
"socket/openssl_ssl_util.h",
"socket/ssl_client_socket_openssl.cc",
"socket/ssl_client_socket_openssl.h",
"socket/ssl_server_socket_openssl.cc",
"socket/ssl_server_socket_openssl.h",
"socket/ssl_session_cache_openssl.cc",
"socket/ssl_session_cache_openssl.h",
"ssl/openssl_platform_key.h",
"ssl/openssl_ssl_util.cc",
"ssl/openssl_ssl_util.h",
]
if (is_mac) {
sources -= [
"ssl/openssl_platform_key_mac.cc",
]
}
if (is_win) {
sources -= [
"ssl/openssl_platform_key_win.cc",
]
}
}

if (!use_openssl_certs) {
Expand Down
7 changes: 5 additions & 2 deletions net/net.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,17 @@
'quic/crypto/p256_key_exchange_openssl.cc',
'quic/crypto/scoped_evp_aead_ctx.cc',
'quic/crypto/scoped_evp_aead_ctx.h',
'socket/openssl_ssl_util.cc',
'socket/openssl_ssl_util.h',
'socket/ssl_client_socket_openssl.cc',
'socket/ssl_client_socket_openssl.h',
'socket/ssl_server_socket_openssl.cc',
'socket/ssl_server_socket_openssl.h',
'socket/ssl_session_cache_openssl.cc',
'socket/ssl_session_cache_openssl.h',
'ssl/openssl_platform_key_mac.cc',
'ssl/openssl_platform_key_win.cc',
'ssl/openssl_platform_key.h',
'ssl/openssl_ssl_util.cc',
'ssl/openssl_ssl_util.h',
],
},
],
Expand Down
7 changes: 5 additions & 2 deletions net/net.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@
'socket/client_socket_pool_histograms.h',
'socket/next_proto.cc',
'socket/next_proto.h',
'socket/openssl_ssl_util.cc',
'socket/openssl_ssl_util.h',
'socket/socket.h',
'socket/ssl_client_socket.cc',
'socket/ssl_client_socket.h',
Expand All @@ -137,6 +135,11 @@
'ssl/default_channel_id_store.h',
'ssl/openssl_client_key_store.cc',
'ssl/openssl_client_key_store.h',
'ssl/openssl_platform_key.h',
'ssl/openssl_platform_key_mac.cc',
'ssl/openssl_platform_key_win.cc',
'ssl/openssl_ssl_util.cc',
'ssl/openssl_ssl_util.h',
'ssl/signed_certificate_timestamp_and_status.cc',
'ssl/signed_certificate_timestamp_and_status.h',
'ssl/ssl_cert_request_info.cc',
Expand Down
39 changes: 25 additions & 14 deletions net/socket/ssl_client_socket_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,19 @@
#include "net/cert/cert_verifier.h"
#include "net/cert/single_request_cert_verifier.h"
#include "net/cert/x509_certificate_net_log_param.h"
#include "net/socket/openssl_ssl_util.h"
#include "net/socket/ssl_error_params.h"
#include "net/socket/ssl_session_cache_openssl.h"
#include "net/ssl/openssl_client_key_store.h"
#include "net/ssl/openssl_ssl_util.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/ssl/ssl_connection_status_flags.h"
#include "net/ssl/ssl_info.h"

#if defined(USE_OPENSSL_CERTS)
#include "net/ssl/openssl_client_key_store.h"
#else
#include "net/ssl/openssl_platform_key.h"
#endif

namespace net {

namespace {
Expand Down Expand Up @@ -1326,6 +1331,11 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
DCHECK(ssl == ssl_);
DCHECK(*x509 == NULL);
DCHECK(*pkey == NULL);

#if defined(OS_IOS)
// TODO(droger): Support client auth on iOS. See http://crbug.com/145954).
LOG(WARNING) << "Client auth is not supported";
#else // !defined(OS_IOS)
if (!ssl_config_.send_client_cert) {
// First pass: we know that a client certificate is needed, but we do not
// have one at hand.
Expand Down Expand Up @@ -1363,32 +1373,33 @@ int SSLClientSocketOpenSSL::ClientCertRequestCallback(SSL* ssl,
return -1;
}

crypto::ScopedEVP_PKEY privkey;
// TODO(davidben): With Linux client auth support, this should be
// conditioned on OS_ANDROID and then, with https://crbug.com/394131,
// removed altogether. OpenSSLClientKeyStore is mostly an artifact of the
// net/ client auth API lacking a private key handle.
#if defined(USE_OPENSSL_CERTS)
// A note about ownership: FetchClientCertPrivateKey() increments
// the reference count of the EVP_PKEY. Ownership of this reference
// is passed directly to OpenSSL, which will release the reference
// using EVP_PKEY_free() when the SSL object is destroyed.
if (!OpenSSLClientKeyStore::GetInstance()->FetchClientCertPrivateKey(
ssl_config_.client_cert.get(), &privkey)) {
crypto::ScopedEVP_PKEY privkey =
OpenSSLClientKeyStore::GetInstance()->FetchClientCertPrivateKey(
ssl_config_.client_cert.get());
#else // !defined(USE_OPENSSL_CERTS)
crypto::ScopedEVP_PKEY privkey =
FetchClientCertPrivateKey(ssl_config_.client_cert.get());
#endif // defined(USE_OPENSSL_CERTS)
if (!privkey) {
// Could not find the private key. Fail the handshake and surface an
// appropriate error to the caller.
LOG(WARNING) << "Client cert found without private key";
OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_CERT_NO_PRIVATE_KEY);
return -1;
}
#else // !defined(USE_OPENSSL_CERTS)
// OS handling of private keys is not yet implemented.
NOTIMPLEMENTED();
return 0;
#endif // defined(USE_OPENSSL_CERTS)

// TODO(joth): (copied from NSS) We should wait for server certificate
// verification before sending our credentials. See http://crbug.com/13934
*x509 = leaf_x509.release();
*pkey = privkey.release();
return 1;
}
#endif // defined(OS_IOS)

// Send no client certificate.
return 0;
Expand Down
2 changes: 1 addition & 1 deletion net/socket/ssl_server_socket_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#include "crypto/rsa_private_key.h"
#include "crypto/scoped_openssl_types.h"
#include "net/base/net_errors.h"
#include "net/socket/openssl_ssl_util.h"
#include "net/socket/ssl_error_params.h"
#include "net/ssl/openssl_ssl_util.h"

#define GotoState(s) next_handshake_state_ = s

Expand Down
14 changes: 6 additions & 8 deletions net/ssl/openssl_client_key_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,22 +107,20 @@ bool OpenSSLClientKeyStore::RecordClientCertPrivateKey(
return true;
}

bool OpenSSLClientKeyStore::FetchClientCertPrivateKey(
const X509Certificate* client_cert,
crypto::ScopedEVP_PKEY* private_key) {
crypto::ScopedEVP_PKEY OpenSSLClientKeyStore::FetchClientCertPrivateKey(
const X509Certificate* client_cert) {
if (!client_cert)
return false;
return crypto::ScopedEVP_PKEY();

crypto::ScopedEVP_PKEY pub_key(GetOpenSSLPublicKey(client_cert));
if (!pub_key.get())
return false;
return crypto::ScopedEVP_PKEY();

int index = FindKeyPairIndex(pub_key.get());
if (index < 0)
return false;
return crypto::ScopedEVP_PKEY();

private_key->reset(CopyEVP_PKEY(pairs_[index].private_key));
return true;
return crypto::ScopedEVP_PKEY(CopyEVP_PKEY(pairs_[index].private_key));
}

void OpenSSLClientKeyStore::Flush() {
Expand Down
7 changes: 2 additions & 5 deletions net/ssl/openssl_client_key_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,8 @@ class NET_EXPORT OpenSSLClientKeyStore {
// Given a certificate's |public_key|, return the corresponding private
// key that has been recorded previously by RecordClientCertPrivateKey().
// |cert| is a client certificate.
// |*private_key| will be reset to its matching private key on success.
// Returns true on success, false otherwise. This increments the reference
// count of the private key on success.
bool FetchClientCertPrivateKey(const X509Certificate* cert,
crypto::ScopedEVP_PKEY* private_key);
// Returns its matching private key on success, NULL otherwise.
crypto::ScopedEVP_PKEY FetchClientCertPrivateKey(const X509Certificate* cert);

// Flush all recorded keys.
void Flush();
Expand Down
21 changes: 9 additions & 12 deletions net/ssl/openssl_client_key_store_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ TEST_F(OpenSSLClientKeyStoreTest, Flush) {

// Retrieve the private key. This should fail because the store
// was flushed.
crypto::ScopedEVP_PKEY pkey;
ASSERT_FALSE(store_->FetchClientCertPrivateKey(cert_1.get(), &pkey));
crypto::ScopedEVP_PKEY pkey = store_->FetchClientCertPrivateKey(cert_1.get());
ASSERT_FALSE(pkey.get());
}

Expand All @@ -75,8 +74,7 @@ TEST_F(OpenSSLClientKeyStoreTest, FetchEmptyPrivateKey) {

// Retrieve the private key now. This should fail because it was
// never recorded in the store.
crypto::ScopedEVP_PKEY pkey;
ASSERT_FALSE(store_->FetchClientCertPrivateKey(cert_1.get(), &pkey));
crypto::ScopedEVP_PKEY pkey = store_->FetchClientCertPrivateKey(cert_1.get());
ASSERT_FALSE(pkey.get());
}

Expand Down Expand Up @@ -110,8 +108,8 @@ TEST_F(OpenSSLClientKeyStoreTest, RecordAndFetchPrivateKey) {

// Retrieve the private key. This should increment the private key's
// reference count.
crypto::ScopedEVP_PKEY pkey2;
ASSERT_TRUE(store_->FetchClientCertPrivateKey(cert_1.get(), &pkey2));
crypto::ScopedEVP_PKEY pkey2 =
store_->FetchClientCertPrivateKey(cert_1.get());
ASSERT_EQ(pkey2.get(), priv_key.get());
ASSERT_EQ(3, EVP_PKEY_get_refcount(priv_key.get()));

Expand Down Expand Up @@ -152,12 +150,11 @@ TEST_F(OpenSSLClientKeyStoreTest, RecordAndFetchTwoPrivateKeys) {

// Retrieve the private key now. This shall succeed and increment
// the private key's reference count.
crypto::ScopedEVP_PKEY fetch_key1;
ASSERT_TRUE(store_->FetchClientCertPrivateKey(cert_1.get(),
&fetch_key1));
crypto::ScopedEVP_PKEY fetch_key2;
ASSERT_TRUE(store_->FetchClientCertPrivateKey(cert_2.get(),
&fetch_key2));
crypto::ScopedEVP_PKEY fetch_key1 =
store_->FetchClientCertPrivateKey(cert_1.get());
crypto::ScopedEVP_PKEY fetch_key2 =
store_->FetchClientCertPrivateKey(cert_2.get());

EXPECT_TRUE(fetch_key1.get());
EXPECT_TRUE(fetch_key2.get());

Expand Down
26 changes: 26 additions & 0 deletions net/ssl/openssl_platform_key.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2014 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.

#ifndef NET_SSL_OPENSSL_PLATFORM_KEY_H_
#define NET_SSL_OPENSSL_PLATFORM_KEY_H_

#include "crypto/scoped_openssl_types.h"

namespace net {

class X509Certificate;

// Looks up the private key from the platform key store corresponding
// to |certificate|'s public key. Then wraps it in an OpenSSL EVP_PKEY
// structure to be used for SSL client auth.
//
// TODO(davidben): This combines looking up a private key with
// wrapping it in an OpenSSL structure. This will be separated with
// https://crbug.com/394131
crypto::ScopedEVP_PKEY FetchClientCertPrivateKey(
const X509Certificate* certificate);

} // namespace net

#endif // NET_SSL_OPENSSL_PLATFORM_KEY_H_
Loading

0 comments on commit 97a854f

Please sign in to comment.