Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
[webcrypto] Don't disallow RSA public key import using SPKI format wh…
Browse files Browse the repository at this point in the history
…en on Linux.

BUG=406532

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

Cr-Commit-Position: refs/heads/master@{#291595}
  • Loading branch information
eroman authored and Commit bot committed Aug 23, 2014
1 parent ed1bba3 commit 24f0da6
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 33 deletions.
24 changes: 10 additions & 14 deletions content/child/webcrypto/nss/rsa_key_nss.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,24 @@ bool CreatePrivateKeyAlgorithm(const blink::WebCryptoAlgorithm& algorithm,
}

#if defined(USE_NSS) && !defined(OS_CHROMEOS)
Status ErrorRsaKeyImportNotSupported() {
Status ErrorRsaPrivateKeyImportNotSupported() {
return Status::ErrorUnsupported(
"NSS version must be at least 3.16.2 for RSA key import. See "
"NSS version must be at least 3.16.2 for RSA private key import. See "
"http://crbug.com/380424");
}

// 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.
Status NssSupportsRsaKeyImport() {
Status NssSupportsRsaPrivateKeyImport() {
if (!NSS_VersionCheck("3.16.2"))
return ErrorRsaKeyImportNotSupported();
return ErrorRsaPrivateKeyImportNotSupported();

// 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();
return ErrorRsaPrivateKeyImportNotSupported();

// CK_SLOT_INFO.hardwareVersion contains the major.minor
// version info for Softoken in the corresponding .major/.minor
Expand All @@ -86,10 +86,10 @@ Status NssSupportsRsaKeyImport() {
return Status::Success();
}

return ErrorRsaKeyImportNotSupported();
return ErrorRsaPrivateKeyImportNotSupported();
}
#else
Status NssSupportsRsaKeyImport() {
Status NssSupportsRsaPrivateKeyImport() {
return Status::Success();
}
#endif
Expand Down Expand Up @@ -346,7 +346,7 @@ Status ImportRsaPrivateKey(const blink::WebCryptoAlgorithm& algorithm,
blink::WebCryptoKeyUsageMask usage_mask,
const JwkRsaInfo& params,
blink::WebCryptoKey* key) {
Status status = NssSupportsRsaKeyImport();
Status status = NssSupportsRsaPrivateKeyImport();
if (status.IsError())
return status;

Expand Down Expand Up @@ -649,7 +649,7 @@ Status RsaHashedAlgorithm::ImportKeyPkcs8(
bool extractable,
blink::WebCryptoKeyUsageMask usage_mask,
blink::WebCryptoKey* key) const {
Status status = NssSupportsRsaKeyImport();
Status status = NssSupportsRsaPrivateKeyImport();
if (status.IsError())
return status;

Expand Down Expand Up @@ -709,10 +709,6 @@ Status RsaHashedAlgorithm::ImportKeySpki(
bool extractable,
blink::WebCryptoKeyUsageMask usage_mask,
blink::WebCryptoKey* key) const {
Status status = NssSupportsRsaKeyImport();
if (status.IsError())
return status;

if (!key_data.byte_length())
return Status::ErrorImportEmptyKeyData();

Expand Down Expand Up @@ -740,7 +736,7 @@ Status RsaHashedAlgorithm::ImportKeySpki(

// TODO(eroman): This is probably going to be the same as the input.
std::vector<uint8_t> spki_data;
status = ExportKeySpkiNss(sec_public_key.get(), &spki_data);
Status status = ExportKeySpkiNss(sec_public_key.get(), &spki_data);
if (status.IsError())
return status;

Expand Down
29 changes: 10 additions & 19 deletions content/child/webcrypto/shared_crypto_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ bool SupportsRsaOaep() {
#endif
}

bool SupportsRsaKeyImport() {
bool SupportsRsaPrivateKeyImport() {
// TODO(eroman): Exclude version test for OS_CHROMEOS
#if defined(USE_NSS)
crypto::EnsureNSSInit();
Expand Down Expand Up @@ -1841,9 +1841,6 @@ TEST(WebCryptoAesCbcTest, ImportJwkInconsistentKLength2) {
}

TEST(WebCryptoRsaSsaTest, ImportExportJwkRsaPublicKey) {
if (!SupportsRsaKeyImport())
return;

struct TestCase {
const blink::WebCryptoAlgorithmId hash;
const blink::WebCryptoKeyUsageMask usage;
Expand Down Expand Up @@ -1907,9 +1904,6 @@ TEST(WebCryptoRsaSsaTest, ImportExportJwkRsaPublicKey) {
}

TEST(WebCryptoRsaOaepTest, ImportExportJwkRsaPublicKey) {
if (!SupportsRsaKeyImport())
return;

if (!SupportsRsaOaep()) {
LOG(WARNING) << "RSA-OAEP support not present; skipping.";
return;
Expand Down Expand Up @@ -2395,9 +2389,6 @@ TEST(WebCryptoHmacTest, ExportJwkEmptyKey) {
}

TEST(WebCryptoRsaSsaTest, ImportExportSpki) {
if (!SupportsRsaKeyImport())
return;

// Passing case: Import a valid RSA key in SPKI format.
blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
ASSERT_EQ(Status::Success(),
Expand Down Expand Up @@ -2484,7 +2475,7 @@ TEST(WebCryptoRsaSsaTest, ImportExportSpki) {
}

TEST(WebCryptoRsaSsaTest, ImportExportPkcs8) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

// Passing case: Import a valid RSA key in PKCS#8 format.
Expand Down Expand Up @@ -2551,7 +2542,7 @@ TEST(WebCryptoRsaSsaTest, ImportExportPkcs8) {

// Tests importing of PKCS8 data that does not define a valid RSA key.
TEST(WebCryptoRsaSsaTest, ImportInvalidPkcs8) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

// kPrivateKeyPkcs8DerHex defines an RSA private key in PKCS8 format, whose
Expand Down Expand Up @@ -2605,7 +2596,7 @@ TEST(WebCryptoRsaSsaTest, ImportInvalidPkcs8) {
//
// PKCS8 --> JWK --> PKCS8
TEST(WebCryptoRsaSsaTest, ImportRsaPrivateKeyJwkToPkcs8RoundTrip) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
Expand Down Expand Up @@ -2673,7 +2664,7 @@ TEST(WebCryptoRsaSsaTest, ImportRsaPrivateKeyJwkToPkcs8RoundTrip) {
// be imported correctly, however every key after that would actually import
// the first key.
TEST(WebCryptoRsaSsaTest, ImportMultipleRSAPrivateKeysJwk) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

scoped_ptr<base::ListValue> key_list;
Expand Down Expand Up @@ -2837,7 +2828,7 @@ TEST(WebCryptoRsaSsaTest, ImportRsaPrivateKeyJwkMissingOptionalParams) {
// accept them, but are not required to. Chromium's WebCrypto does
// not allow such degenerate keys.
TEST(WebCryptoRsaSsaTest, ImportRsaPrivateKeyJwkIncorrectOptionalEmpty) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
Expand Down Expand Up @@ -2935,7 +2926,7 @@ TEST(WebCryptoRsaSsaTest, GenerateKeyPairRsa) {
Status::Success(),
ExportKey(blink::WebCryptoKeyFormatSpki, public_key, &public_key_spki));

if (SupportsRsaKeyImport()) {
if (SupportsRsaPrivateKeyImport()) {
public_key = blink::WebCryptoKey::createNull();
EXPECT_EQ(Status::Success(),
ImportKey(blink::WebCryptoKeyFormatSpki,
Expand Down Expand Up @@ -3139,7 +3130,7 @@ TEST(WebCryptoRsaSsaTest, GenerateKeyPairRsaBadExponent) {
}

TEST(WebCryptoRsaSsaTest, SignVerifyFailures) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

// Import a key pair.
Expand Down Expand Up @@ -3271,7 +3262,7 @@ TEST(WebCryptoRsaSsaTest, SignVerifyFailures) {
}

TEST(WebCryptoRsaSsaTest, SignVerifyKnownAnswer) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

scoped_ptr<base::ListValue> tests;
Expand Down Expand Up @@ -4750,7 +4741,7 @@ TEST(WebCryptoRsaSsaTest, GenerateKeyPairIntersectUsages) {
// 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.
TEST(WebCryptoAesCbcTest, WrapUnwrapRoundtripSpkiPkcs8) {
if (!SupportsRsaKeyImport())
if (!SupportsRsaPrivateKeyImport())
return;

// Generate the wrapping key.
Expand Down

0 comments on commit 24f0da6

Please sign in to comment.