Skip to content

Commit

Permalink
Use SecKeyCreateSignature on macOS 10.12 and later.
Browse files Browse the repository at this point in the history 
macOS 10.12 breaks the CSSM code for smartcards. Switch to the new APIs.
In the process, refactor the Android unit tests to be common on all
platforms and unit test this stuff. SecKeychainCreate finally works, so
we can unit test the entire path from certificate to key and compare
signatures against OpenSSL.

That second part probably bears repeating. Eight years into the project's
lifetime, we FINALLY have unit tests for this code!

BUG=666796,673058

Review-Url: https://codereview.chromium.org/2566273008
Cr-Commit-Position: refs/heads/master@{#438392}
  • Loading branch information
@davidben
davidben authored and Commit bot committed Dec 14, 2016
1 parent e7c804e commit ebe0803
Show file tree
Hide file tree
Showing 12 changed files with 656 additions and 182 deletions.
3 changes: 3 additions & 0 deletions net/net.gypi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2151,7 +2151,10 @@
'ssl/ssl_config_unittest.cc',
'ssl/ssl_connection_status_flags_unittest.cc',
'ssl/ssl_platform_key_android_unittest.cc',
'ssl/ssl_platform_key_mac_unittest.cc',
'ssl/ssl_platform_key_util_unittest.cc',
'ssl/ssl_private_key_test_util.cc',
'ssl/ssl_private_key_test_util.h',
'test/embedded_test_server/embedded_test_server_unittest.cc',
'test/embedded_test_server/http_request_unittest.cc',
'test/embedded_test_server/http_response_unittest.cc',
Expand Down
8 changes: 4 additions & 4 deletions net/ssl/ssl_platform_key.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ namespace net {
class SSLPrivateKey;
class X509Certificate;

// Looks up the private key from the platform key store corresponding to
// |certificate|'s public key and returns an SSLPrivateKey backed by the
// playform key.
// Returns an SSLPrivateKey backed by the platform private key that corresponds
// to |certificate|'s public key. If |keychain| is nullptr, the process's
// default search list is used instead.
NET_EXPORT scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKey(
X509Certificate* certificate);
const X509Certificate* certificate);

} // namespace net

Expand Down
2 changes: 1 addition & 1 deletion net/ssl/ssl_platform_key_android.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ scoped_refptr<SSLPrivateKey> WrapJavaPrivateKey(
}

scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKey(
X509Certificate* certificate) {
const X509Certificate* certificate) {
return OpenSSLClientKeyStore::GetInstance()->FetchClientCertPrivateKey(
certificate);
}
Expand Down
174 changes: 24 additions & 150 deletions net/ssl/ssl_platform_key_android_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,62 +2,36 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "net/ssl/ssl_platform_key_android.h"

#include <string>

#include "base/android/jni_android.h"
#include "base/android/jni_array.h"
#include "base/android/scoped_java_ref.h"
#include "base/bind.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "crypto/openssl_util.h"
#include "net/android/keystore.h"
#include "net/cert/x509_certificate.h"
#include "net/ssl/ssl_platform_key_android.h"
#include "net/ssl/ssl_private_key.h"
#include "net/ssl/ssl_private_key_test_util.h"
#include "net/test/cert_test_util.h"
#include "net/test/jni/AndroidKeyStoreTestUtil_jni.h"
#include "net/test/test_data_directory.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/boringssl/src/include/openssl/bytestring.h"
#include "third_party/boringssl/src/include/openssl/digest.h"
#include "third_party/boringssl/src/include/openssl/ecdsa.h"
#include "third_party/boringssl/src/include/openssl/err.h"
#include "third_party/boringssl/src/include/openssl/evp.h"
#include "third_party/boringssl/src/include/openssl/pem.h"
#include "third_party/boringssl/src/include/openssl/rsa.h"
#include "third_party/boringssl/src/include/openssl/x509.h"

namespace net {

namespace {

typedef base::android::ScopedJavaLocalRef<jobject> ScopedJava;

// Resize a string to |size| bytes of data, then return its data buffer address
// cast as an 'uint8_t*', as expected by OpenSSL functions.
// |str| the target string.
// |size| the number of bytes to write into the string.
// Return the string's new buffer in memory, as an 'uint8_t*' pointer.
uint8_t* OpenSSLWriteInto(std::string* str, size_t size) {
return reinterpret_cast<uint8_t*>(base::WriteInto(str, size + 1));
}

bool ReadTestFile(const char* filename, std::string* pkcs8) {
base::FilePath certs_dir = GetTestCertsDirectory();
base::FilePath file_path = certs_dir.AppendASCII(filename);
return base::ReadFileToString(file_path, pkcs8);
}

// Parses a PKCS#8 key into an OpenSSL private key object.
bssl::UniquePtr<EVP_PKEY> ImportPrivateKeyOpenSSL(const std::string& pkcs8) {
crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
CBS cbs;
CBS_init(&cbs, reinterpret_cast<const uint8_t*>(pkcs8.data()), pkcs8.size());
return bssl::UniquePtr<EVP_PKEY>(EVP_parse_private_key(&cbs));
}

// Retrieve a JNI local ref from encoded PKCS#8 data.
ScopedJava GetPKCS8PrivateKeyJava(android::PrivateKeyType key_type,
const std::string& pkcs8_key) {
Expand All @@ -73,91 +47,14 @@ ScopedJava GetPKCS8PrivateKeyJava(android::PrivateKeyType key_type,
return key;
}

bool VerifyWithOpenSSL(const EVP_MD* md,
const base::StringPiece& digest,
EVP_PKEY* key,
const base::StringPiece& signature) {
crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);

bssl::UniquePtr<EVP_PKEY_CTX> ctx(EVP_PKEY_CTX_new(key, nullptr));
if (!ctx || !EVP_PKEY_verify_init(ctx.get()) ||
!EVP_PKEY_CTX_set_signature_md(ctx.get(), md) ||
!EVP_PKEY_verify(
ctx.get(), reinterpret_cast<const uint8_t*>(signature.data()),
signature.size(), reinterpret_cast<const uint8_t*>(digest.data()),
digest.size())) {
return false;
}

return true;
}

bool SignWithOpenSSL(const EVP_MD* md,
const base::StringPiece& digest,
EVP_PKEY* key,
std::string* result) {
crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);

size_t sig_len;
bssl::UniquePtr<EVP_PKEY_CTX> ctx(EVP_PKEY_CTX_new(key, nullptr));
if (!ctx || !EVP_PKEY_sign_init(ctx.get()) ||
!EVP_PKEY_CTX_set_signature_md(ctx.get(), md) ||
!EVP_PKEY_sign(ctx.get(), OpenSSLWriteInto(result, EVP_PKEY_size(key)),
&sig_len, reinterpret_cast<const uint8_t*>(digest.data()),
digest.size())) {
return false;
}

result->resize(sig_len);
return true;
}

void OnSignComplete(base::RunLoop* loop,
Error* out_error,
std::string* out_signature,
Error error,
const std::vector<uint8_t>& signature) {
*out_error = error;
out_signature->assign(signature.begin(), signature.end());
loop->Quit();
}

void DoKeySigningWithWrapper(SSLPrivateKey* key,
SSLPrivateKey::Hash hash,
const base::StringPiece& message,
std::string* result) {
Error error;
base::RunLoop loop;

key->SignDigest(
hash, message,
base::Bind(OnSignComplete, base::Unretained(&loop),
base::Unretained(&error), base::Unretained(result)));
loop.Run();

ASSERT_EQ(OK, error);
}

static const struct {
const char* name;
int nid;
SSLPrivateKey::Hash hash;
} kHashes[] = {
{"MD5-SHA1", NID_md5_sha1, SSLPrivateKey::Hash::MD5_SHA1},
{"SHA-1", NID_sha1, SSLPrivateKey::Hash::SHA1},
{"SHA-256", NID_sha256, SSLPrivateKey::Hash::SHA256},
{"SHA-384", NID_sha384, SSLPrivateKey::Hash::SHA384},
{"SHA-512", NID_sha512, SSLPrivateKey::Hash::SHA512},
};

struct TestKey {
const char* cert_file;
const char* key_file;
android::PrivateKeyType android_key_type;
SSLPrivateKey::Type key_type;
};

static const TestKey kTestKeys[] = {
const TestKey kTestKeys[] = {
{"client_1.pem", "client_1.pk8", android::PRIVATE_KEY_TYPE_RSA,
SSLPrivateKey::Type::RSA},
{"client_4.pem", "client_4.pk8", android::PRIVATE_KEY_TYPE_ECDSA,
Expand All @@ -168,12 +65,15 @@ static const TestKey kTestKeys[] = {
SSLPrivateKey::Type::ECDSA_P521},
};

std::string TestKeyToString(const testing::TestParamInfo<TestKey>& params) {
return SSLPrivateKeyTypeToString(params.param.key_type);
}

} // namespace

class SSLPlatformKeyAndroidTest : public testing::TestWithParam<TestKey> {};

TEST_P(SSLPlatformKeyAndroidTest, SignHashes) {
crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
TEST_P(SSLPlatformKeyAndroidTest, Matches) {
const TestKey& test_key = GetParam();

scoped_refptr<X509Certificate> cert =
Expand All @@ -186,48 +86,22 @@ TEST_P(SSLPlatformKeyAndroidTest, SignHashes) {
GetPKCS8PrivateKeyJava(test_key.android_key_type, key_bytes);
ASSERT_FALSE(java_key.is_null());

scoped_refptr<SSLPrivateKey> wrapper_key =
WrapJavaPrivateKey(cert.get(), java_key);
ASSERT_TRUE(wrapper_key);

bssl::UniquePtr<EVP_PKEY> openssl_key = ImportPrivateKeyOpenSSL(key_bytes);
ASSERT_TRUE(openssl_key);

// Check that the wrapper key returns the correct length and type.
EXPECT_EQ(test_key.key_type, wrapper_key->GetType());
EXPECT_EQ(static_cast<size_t>(EVP_PKEY_size(openssl_key.get())),
wrapper_key->GetMaxSignatureLengthInBytes());

// Test signing against each hash.
for (const auto& hash : kHashes) {
// Only RSA signs MD5-SHA1.
if (test_key.key_type != SSLPrivateKey::Type::RSA &&
hash.nid == NID_md5_sha1) {
continue;
}

SCOPED_TRACE(hash.name);

const EVP_MD* md = EVP_get_digestbynid(hash.nid);
ASSERT_TRUE(md);
std::string digest(EVP_MD_size(md), 'a');

std::string signature;
DoKeySigningWithWrapper(wrapper_key.get(), hash.hash, digest, &signature);
EXPECT_TRUE(VerifyWithOpenSSL(md, digest, openssl_key.get(), signature));

// RSA signing is deterministic, so further check the signature matches.
if (test_key.key_type == SSLPrivateKey::Type::RSA) {
std::string openssl_signature;
ASSERT_TRUE(
SignWithOpenSSL(md, digest, openssl_key.get(), &openssl_signature));
EXPECT_EQ(openssl_signature, signature);
}
}
scoped_refptr<SSLPrivateKey> key = WrapJavaPrivateKey(cert.get(), java_key);
ASSERT_TRUE(key);

// All Android keys are expected to have the same hash preferences.
std::vector<SSLPrivateKey::Hash> expected_hashes = {
SSLPrivateKey::Hash::SHA512, SSLPrivateKey::Hash::SHA384,
SSLPrivateKey::Hash::SHA256, SSLPrivateKey::Hash::SHA1,
};
EXPECT_EQ(expected_hashes, key->GetDigestPreferences());

TestSSLPrivateKeyMatches(key.get(), key_bytes);
}

INSTANTIATE_TEST_CASE_P(,
SSLPlatformKeyAndroidTest,
testing::ValuesIn(kTestKeys));
testing::ValuesIn(kTestKeys),
TestKeyToString);

} // namespace net
2 changes: 1 addition & 1 deletion net/ssl/ssl_platform_key_chromecast.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class SSLPlatformKeyChromecast : public ThreadedSSLPrivateKey::Delegate {
} // namespace

scoped_refptr<SSLPrivateKey> FetchClientCertPrivateKey(
X509Certificate* certificate) {
const X509Certificate* certificate) {
crypto::ScopedSECKEYPrivateKey key(
PK11_FindKeyByAnyCert(certificate->os_cert_handle(), nullptr));
if (!key) {
Expand Down
Loading

0 comments on commit ebe0803

Please sign in to comment.