Commit 975d7a3b authored by eroman@chromium.org's avatar eroman@chromium.org

[webcrypto] Disable RSA key import for NSS versions less than 3.16.2.

Prior to NSS 3.16.2 there wasn't any validation of the RSA key parameters.

This has several consequences:

* Importing an RSA private key with another key's public modulus can be used to gain access to that key.
* importKey() can succeed for invalid RSA keys (invalid n, e, d, p, q etc).

This only affects Linux, since other platforms of Chromium bundle NSS/OpenSSL.

BUG=380424,378315
R=rsleevi@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278803 0039d316-1c4b-4281-b951-d872f2087c98
parent 13c4dcde
...@@ -292,6 +292,50 @@ Status NssSupportsRsaOaep() { ...@@ -292,6 +292,50 @@ Status NssSupportsRsaOaep() {
"later"); "later");
} }
#if defined(USE_NSS) && !defined(OS_CHROMEOS)
Status ErrorRsaKeyImportNotSupported() {
return Status::ErrorUnsupported(
"NSS version must be at least 3.16.2 for RSA key import. See "
"http://crbug.com/380424");
}
Status NssSupportsKeyImport(blink::WebCryptoAlgorithmId algorithm) {
// Prior to NSS 3.16.2 RSA key parameters were not validated. This is
// a security problem for RSA private key import from JWK which uses a
// CKA_ID based on the public modulus to retrieve the private key.
if (!IsAlgorithmRsa(algorithm))
return Status::Success();
if (!NSS_VersionCheck("3.16.2"))
return ErrorRsaKeyImportNotSupported();
// Also ensure that the version of Softoken is 3.16.2 or later.
crypto::ScopedPK11Slot slot(PK11_GetInternalSlot());
CK_SLOT_INFO info = {};
if (PK11_GetSlotInfo(slot.get(), &info) != SECSuccess)
return ErrorRsaKeyImportNotSupported();
// CK_SLOT_INFO.hardwareVersion contains the major.minor
// version info for Softoken in the corresponding .major/.minor
// fields, and .firmwareVersion contains the patch.build
// version info (in the .major/.minor fields)
if ((info.hardwareVersion.major > 3) ||
(info.hardwareVersion.major == 3 &&
(info.hardwareVersion.minor > 16 ||
(info.hardwareVersion.minor == 16 &&
info.firmwareVersion.major >= 2)))) {
return Status::Success();
}
return ErrorRsaKeyImportNotSupported();
}
#else
Status NssSupportsKeyImport(blink::WebCryptoAlgorithmId) {
return Status::Success();
}
#endif
// Creates a SECItem for the data in |buffer|. This does NOT make a copy, so // Creates a SECItem for the data in |buffer|. This does NOT make a copy, so
// |buffer| should outlive the SECItem. // |buffer| should outlive the SECItem.
SECItem MakeSECItemForBuffer(const CryptoData& buffer) { SECItem MakeSECItemForBuffer(const CryptoData& buffer) {
...@@ -987,6 +1031,10 @@ Status ImportKeySpki(const blink::WebCryptoAlgorithm& algorithm, ...@@ -987,6 +1031,10 @@ Status ImportKeySpki(const blink::WebCryptoAlgorithm& algorithm,
bool extractable, bool extractable,
blink::WebCryptoKeyUsageMask usage_mask, blink::WebCryptoKeyUsageMask usage_mask,
blink::WebCryptoKey* key) { blink::WebCryptoKey* key) {
Status status = NssSupportsKeyImport(algorithm.id());
if (status.IsError())
return status;
DCHECK(key); DCHECK(key);
if (!key_data.byte_length()) if (!key_data.byte_length())
...@@ -1016,7 +1064,7 @@ Status ImportKeySpki(const blink::WebCryptoAlgorithm& algorithm, ...@@ -1016,7 +1064,7 @@ Status ImportKeySpki(const blink::WebCryptoAlgorithm& algorithm,
return Status::ErrorUnexpected(); return Status::ErrorUnexpected();
scoped_ptr<PublicKey> key_handle; scoped_ptr<PublicKey> key_handle;
Status status = PublicKey::Create(sec_public_key.Pass(), &key_handle); status = PublicKey::Create(sec_public_key.Pass(), &key_handle);
if (status.IsError()) if (status.IsError())
return status; return status;
...@@ -1156,6 +1204,10 @@ Status ImportKeyPkcs8(const blink::WebCryptoAlgorithm& algorithm, ...@@ -1156,6 +1204,10 @@ Status ImportKeyPkcs8(const blink::WebCryptoAlgorithm& algorithm,
bool extractable, bool extractable,
blink::WebCryptoKeyUsageMask usage_mask, blink::WebCryptoKeyUsageMask usage_mask,
blink::WebCryptoKey* key) { blink::WebCryptoKey* key) {
Status status = NssSupportsKeyImport(algorithm.id());
if (status.IsError())
return status;
DCHECK(key); DCHECK(key);
if (!key_data.byte_length()) if (!key_data.byte_length())
...@@ -1191,8 +1243,7 @@ Status ImportKeyPkcs8(const blink::WebCryptoAlgorithm& algorithm, ...@@ -1191,8 +1243,7 @@ Status ImportKeyPkcs8(const blink::WebCryptoAlgorithm& algorithm,
return Status::ErrorUnexpected(); return Status::ErrorUnexpected();
scoped_ptr<PrivateKey> key_handle; scoped_ptr<PrivateKey> key_handle;
Status status = status = PrivateKey::Create(private_key.Pass(), key_algorithm, &key_handle);
PrivateKey::Create(private_key.Pass(), key_algorithm, &key_handle);
if (status.IsError()) if (status.IsError())
return status; return status;
...@@ -1689,6 +1740,10 @@ Status ImportRsaPrivateKey(const blink::WebCryptoAlgorithm& algorithm, ...@@ -1689,6 +1740,10 @@ Status ImportRsaPrivateKey(const blink::WebCryptoAlgorithm& algorithm,
const CryptoData& exponent2, const CryptoData& exponent2,
const CryptoData& coefficient, const CryptoData& coefficient,
blink::WebCryptoKey* key) { blink::WebCryptoKey* key) {
Status status = NssSupportsKeyImport(algorithm.id());
if (status.IsError())
return status;
CK_OBJECT_CLASS obj_class = CKO_PRIVATE_KEY; CK_OBJECT_CLASS obj_class = CKO_PRIVATE_KEY;
CK_KEY_TYPE key_type = CKK_RSA; CK_KEY_TYPE key_type = CKK_RSA;
CK_BBOOL ck_false = CK_FALSE; CK_BBOOL ck_false = CK_FALSE;
...@@ -1770,8 +1825,7 @@ Status ImportRsaPrivateKey(const blink::WebCryptoAlgorithm& algorithm, ...@@ -1770,8 +1825,7 @@ Status ImportRsaPrivateKey(const blink::WebCryptoAlgorithm& algorithm,
return Status::ErrorUnexpected(); return Status::ErrorUnexpected();
scoped_ptr<PrivateKey> key_handle; scoped_ptr<PrivateKey> key_handle;
Status status = status = PrivateKey::Create(private_key.Pass(), key_algorithm, &key_handle);
PrivateKey::Create(private_key.Pass(), key_algorithm, &key_handle);
if (status.IsError()) if (status.IsError())
return status; return status;
......
...@@ -122,6 +122,7 @@ bool SupportsRsaOaep() { ...@@ -122,6 +122,7 @@ bool SupportsRsaOaep() {
#if defined(USE_OPENSSL) #if defined(USE_OPENSSL)
return false; return false;
#else #else
// TODO(eroman): Exclude version test for OS_CHROMEOS
#if defined(USE_NSS) #if defined(USE_NSS)
if (!NSS_VersionCheck("3.16.2")) if (!NSS_VersionCheck("3.16.2"))
return false; return false;
...@@ -131,6 +132,18 @@ bool SupportsRsaOaep() { ...@@ -131,6 +132,18 @@ bool SupportsRsaOaep() {
#endif #endif
} }
bool SupportsRsaKeyImport() {
// TODO(eroman): Exclude version test for OS_CHROMEOS
#if defined(USE_NSS)
if (!NSS_VersionCheck("3.16.2")) {
LOG(WARNING) << "RSA key import is not supported by this version of NSS. "
"Skipping some tests";
return false;
}
#endif
return true;
}
blink::WebCryptoAlgorithm CreateRsaHashedKeyGenAlgorithm( blink::WebCryptoAlgorithm CreateRsaHashedKeyGenAlgorithm(
blink::WebCryptoAlgorithmId algorithm_id, blink::WebCryptoAlgorithmId algorithm_id,
const blink::WebCryptoAlgorithmId hash_id, const blink::WebCryptoAlgorithmId hash_id,
...@@ -1503,6 +1516,9 @@ TEST_F(SharedCryptoTest, ImportJwkOctFailures) { ...@@ -1503,6 +1516,9 @@ TEST_F(SharedCryptoTest, ImportJwkOctFailures) {
} }
TEST_F(SharedCryptoTest, MAYBE(ImportExportJwkRsaPublicKey)) { TEST_F(SharedCryptoTest, MAYBE(ImportExportJwkRsaPublicKey)) {
if (!SupportsRsaKeyImport())
return;
const bool supports_rsa_oaep = SupportsRsaOaep(); const bool supports_rsa_oaep = SupportsRsaOaep();
if (!supports_rsa_oaep) { if (!supports_rsa_oaep) {
LOG(WARNING) << "RSA-OAEP not supported on this platform. Skipping some" LOG(WARNING) << "RSA-OAEP not supported on this platform. Skipping some"
...@@ -1976,6 +1992,9 @@ TEST_F(SharedCryptoTest, MAYBE(ExportJwkEmptySymmetricKey)) { ...@@ -1976,6 +1992,9 @@ TEST_F(SharedCryptoTest, MAYBE(ExportJwkEmptySymmetricKey)) {
} }
TEST_F(SharedCryptoTest, MAYBE(ImportExportSpki)) { TEST_F(SharedCryptoTest, MAYBE(ImportExportSpki)) {
if (!SupportsRsaKeyImport())
return;
// Passing case: Import a valid RSA key in SPKI format. // Passing case: Import a valid RSA key in SPKI format.
blink::WebCryptoKey key = blink::WebCryptoKey::createNull(); blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
ASSERT_EQ(Status::Success(), ASSERT_EQ(Status::Success(),
...@@ -2062,6 +2081,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportExportSpki)) { ...@@ -2062,6 +2081,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportExportSpki)) {
} }
TEST_F(SharedCryptoTest, MAYBE(ImportExportPkcs8)) { TEST_F(SharedCryptoTest, MAYBE(ImportExportPkcs8)) {
if (!SupportsRsaKeyImport())
return;
// Passing case: Import a valid RSA key in PKCS#8 format. // Passing case: Import a valid RSA key in PKCS#8 format.
blink::WebCryptoKey key = blink::WebCryptoKey::createNull(); blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
ASSERT_EQ(Status::Success(), ASSERT_EQ(Status::Success(),
...@@ -2129,6 +2151,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportExportPkcs8)) { ...@@ -2129,6 +2151,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportExportPkcs8)) {
// //
// PKCS8 --> JWK --> PKCS8 // PKCS8 --> JWK --> PKCS8
TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkToPkcs8RoundTrip)) { TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkToPkcs8RoundTrip)) {
if (!SupportsRsaKeyImport())
return;
blink::WebCryptoKey key = blink::WebCryptoKey::createNull(); blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
ASSERT_EQ(Status::Success(), ASSERT_EQ(Status::Success(),
ImportKey(blink::WebCryptoKeyFormatPkcs8, ImportKey(blink::WebCryptoKeyFormatPkcs8,
...@@ -2194,6 +2219,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkToPkcs8RoundTrip)) { ...@@ -2194,6 +2219,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkToPkcs8RoundTrip)) {
// be imported correctly, however every key after that would actually import // be imported correctly, however every key after that would actually import
// the first key. // the first key.
TEST_F(SharedCryptoTest, MAYBE(ImportMultipleRSAPrivateKeysJwk)) { TEST_F(SharedCryptoTest, MAYBE(ImportMultipleRSAPrivateKeysJwk)) {
if (!SupportsRsaKeyImport())
return;
scoped_ptr<base::ListValue> key_list; scoped_ptr<base::ListValue> key_list;
ASSERT_TRUE(ReadJsonTestFileToList("rsa_private_keys.json", &key_list)); ASSERT_TRUE(ReadJsonTestFileToList("rsa_private_keys.json", &key_list));
...@@ -2355,6 +2383,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkMissingOptionalParams)) { ...@@ -2355,6 +2383,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkMissingOptionalParams)) {
// //
// TODO(eroman): http://crbug/com/374927 // TODO(eroman): http://crbug/com/374927
TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkIncorrectOptionalEmpty)) { TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkIncorrectOptionalEmpty)) {
if (!SupportsRsaKeyImport())
return;
blink::WebCryptoKey key = blink::WebCryptoKey::createNull(); blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
base::DictionaryValue dict; base::DictionaryValue dict;
...@@ -2430,36 +2461,39 @@ TEST_F(SharedCryptoTest, MAYBE(GenerateKeyPairRsa)) { ...@@ -2430,36 +2461,39 @@ TEST_F(SharedCryptoTest, MAYBE(GenerateKeyPairRsa)) {
EXPECT_EQ( EXPECT_EQ(
Status::Success(), Status::Success(),
ExportKey(blink::WebCryptoKeyFormatSpki, public_key, &public_key_spki)); ExportKey(blink::WebCryptoKeyFormatSpki, public_key, &public_key_spki));
public_key = blink::WebCryptoKey::createNull();
EXPECT_EQ(Status::Success(),
ImportKey(blink::WebCryptoKeyFormatSpki,
CryptoData(public_key_spki),
CreateRsaHashedImportAlgorithm(
blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
blink::WebCryptoAlgorithmIdSha256),
true,
usage_mask,
&public_key));
EXPECT_EQ(modulus_length,
public_key.algorithm().rsaHashedParams()->modulusLengthBits());
std::vector<uint8> private_key_pkcs8; if (SupportsRsaKeyImport()) {
EXPECT_EQ( public_key = blink::WebCryptoKey::createNull();
Status::Success(), EXPECT_EQ(Status::Success(),
ExportKey( ImportKey(blink::WebCryptoKeyFormatSpki,
blink::WebCryptoKeyFormatPkcs8, private_key, &private_key_pkcs8)); CryptoData(public_key_spki),
private_key = blink::WebCryptoKey::createNull(); CreateRsaHashedImportAlgorithm(
EXPECT_EQ(Status::Success(), blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
ImportKey(blink::WebCryptoKeyFormatPkcs8, blink::WebCryptoAlgorithmIdSha256),
CryptoData(private_key_pkcs8), true,
CreateRsaHashedImportAlgorithm( usage_mask,
blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5, &public_key));
blink::WebCryptoAlgorithmIdSha256), EXPECT_EQ(modulus_length,
true, public_key.algorithm().rsaHashedParams()->modulusLengthBits());
usage_mask,
&private_key)); std::vector<uint8> private_key_pkcs8;
EXPECT_EQ(modulus_length, EXPECT_EQ(
private_key.algorithm().rsaHashedParams()->modulusLengthBits()); Status::Success(),
ExportKey(
blink::WebCryptoKeyFormatPkcs8, private_key, &private_key_pkcs8));
private_key = blink::WebCryptoKey::createNull();
EXPECT_EQ(Status::Success(),
ImportKey(blink::WebCryptoKeyFormatPkcs8,
CryptoData(private_key_pkcs8),
CreateRsaHashedImportAlgorithm(
blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
blink::WebCryptoAlgorithmIdSha256),
true,
usage_mask,
&private_key));
EXPECT_EQ(modulus_length,
private_key.algorithm().rsaHashedParams()->modulusLengthBits());
}
// Fail with bad modulus. // Fail with bad modulus.
algorithm = algorithm =
...@@ -2602,6 +2636,9 @@ TEST_F(SharedCryptoTest, MAYBE(GenerateKeyPairRsaBadExponent)) { ...@@ -2602,6 +2636,9 @@ TEST_F(SharedCryptoTest, MAYBE(GenerateKeyPairRsaBadExponent)) {
} }
TEST_F(SharedCryptoTest, MAYBE(RsaSsaSignVerifyFailures)) { TEST_F(SharedCryptoTest, MAYBE(RsaSsaSignVerifyFailures)) {
if (!SupportsRsaKeyImport())
return;
// Import a key pair. // Import a key pair.
blink::WebCryptoAlgorithm import_algorithm = blink::WebCryptoAlgorithm import_algorithm =
CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5, CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
...@@ -2732,6 +2769,9 @@ TEST_F(SharedCryptoTest, MAYBE(RsaSsaSignVerifyFailures)) { ...@@ -2732,6 +2769,9 @@ TEST_F(SharedCryptoTest, MAYBE(RsaSsaSignVerifyFailures)) {
} }
TEST_F(SharedCryptoTest, MAYBE(RsaSignVerifyKnownAnswer)) { TEST_F(SharedCryptoTest, MAYBE(RsaSignVerifyKnownAnswer)) {
if (!SupportsRsaKeyImport())
return;
scoped_ptr<base::ListValue> tests; scoped_ptr<base::ListValue> tests;
ASSERT_TRUE(ReadJsonTestFileToList("pkcs1v15_sign.json", &tests)); ASSERT_TRUE(ReadJsonTestFileToList("pkcs1v15_sign.json", &tests));
...@@ -4214,6 +4254,9 @@ TEST_F(SharedCryptoTest, MAYBE(GenerateRsaSsaKeyPairIntersectUsages)) { ...@@ -4214,6 +4254,9 @@ TEST_F(SharedCryptoTest, MAYBE(GenerateRsaSsaKeyPairIntersectUsages)) {
// key pair (using SPKI format for public key, PKCS8 format for private key). // key pair (using SPKI format for public key, PKCS8 format for private key).
// Then unwrap the wrapped key pair and verify that the key data is the same. // Then unwrap the wrapped key pair and verify that the key data is the same.
TEST_F(SharedCryptoTest, MAYBE(WrapUnwrapRoundtripSpkiPkcs8UsingAesCbc)) { TEST_F(SharedCryptoTest, MAYBE(WrapUnwrapRoundtripSpkiPkcs8UsingAesCbc)) {
if (!SupportsRsaKeyImport())
return;
// Generate the wrapping key. // Generate the wrapping key.
blink::WebCryptoKey wrapping_key = blink::WebCryptoKey::createNull(); blink::WebCryptoKey wrapping_key = blink::WebCryptoKey::createNull();
ASSERT_EQ(Status::Success(), ASSERT_EQ(Status::Success(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment