[webcrypto] Fix a bug with JWK private key import.

content/child/webcrypto/shared_crypto_unittest.cc

 // 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.
 
 #include "content/child/webcrypto/shared_crypto.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 72 matching lines @@
 void PrintTo(const CryptoData& data, ::std::ostream* os) {
   *os << "[" << base::HexEncode(data.bytes(), data.byte_length()) << "]";
 }
 
 bool operator==(const content::webcrypto::CryptoData& a,
                 const content::webcrypto::CryptoData& b) {
   return a.byte_length() == b.byte_length() &&
          memcmp(a.bytes(), b.bytes(), a.byte_length()) == 0;
 }
 
+bool operator!=(const content::webcrypto::CryptoData& a,
+                const content::webcrypto::CryptoData& b) {
+  return !(a == b);
+}
+
 namespace {
 
 // -----------------------------------------------------------------------------
 
 // TODO(eroman): For Linux builds using system NSS, AES-GCM support is a
 // runtime dependency. Test it by trying to import a key.
 // TODO(padolph): Consider caching the result of the import key test.
 bool SupportsAesGcm() {
   std::vector<uint8> key_raw(16, 0);
 
@@ skipping 736 matching lines @@
 
     blink::WebCryptoAlgorithm test_hash = GetDigestAlgorithm(test, "hash");
     const std::vector<uint8> test_key = GetBytesFromHexString(test, "key");
     const std::vector<uint8> test_message =
         GetBytesFromHexString(test, "message");
     const std::vector<uint8> test_mac = GetBytesFromHexString(test, "mac");
 
     blink::WebCryptoAlgorithm algorithm =
         CreateAlgorithm(blink::WebCryptoAlgorithmIdHmac);
 
-    blink::WebCryptoAlgorithm importAlgorithm =
+    blink::WebCryptoAlgorithm import_algorithm =
         CreateHmacImportAlgorithm(test_hash.id());
 
     blink::WebCryptoKey key = ImportSecretKeyFromRaw(
         test_key,
-        importAlgorithm,
+        import_algorithm,
         blink::WebCryptoKeyUsageSign | blink::WebCryptoKeyUsageVerify);
 
     EXPECT_EQ(test_hash.id(), key.algorithm().hmacParams()->hash().id());
     EXPECT_EQ(test_key.size() * 8, key.algorithm().hmacParams()->lengthBits());
 
     // Verify exported raw key is identical to the imported data
     std::vector<uint8> raw_key;
     EXPECT_EQ(Status::Success(),
               ExportKey(blink::WebCryptoKeyFormatRaw, key, &raw_key));
     EXPECT_BYTES_EQ(test_key, raw_key);
@@ skipping 1280 matching lines @@
 
   std::vector<uint8> exported_key_pkcs8;
   ASSERT_EQ(
       Status::Success(),
       ExportKey(blink::WebCryptoKeyFormatPkcs8, key, &exported_key_pkcs8));
 
   ASSERT_EQ(CryptoData(HexStringToBytes(kPrivateKeyPkcs8DerHex)),
             CryptoData(exported_key_pkcs8));
 }
 
+// Tests importing multiple RSA private keys from JWK, and then exporting to
+// PKCS8.
+//
+// This is a regression test for http://crbug.com/378315, for which importing
+// a sequence of keys from JWK could yield the wrong key. The first key would
+// be imported correctly, however every key after that would actually import
+// the first key.
+TEST_F(SharedCryptoTest, MAYBE(ImportMultipleRSAPrivateKeysJwk)) {
+  scoped_ptr<base::ListValue> key_list;
+  ASSERT_TRUE(ReadJsonTestFileToList("rsa_private_keys.json", &key_list));
+
+  // For this test to be meaningful the keys MUST be kept alive before importing
+  // new keys.
+  std::vector<blink::WebCryptoKey> live_keys;
+
+  for (size_t key_index = 0; key_index < key_list->GetSize(); ++key_index) {
+    SCOPED_TRACE(key_index);
+
+    base::DictionaryValue* key_values;
+    ASSERT_TRUE(key_list->GetDictionary(key_index, &key_values));
+
+    // Get the JWK representation of the key.
+    base::DictionaryValue* key_jwk;
+    ASSERT_TRUE(key_values->GetDictionary("jwk", &key_jwk));
+
+    // Get the PKCS8 representation of the key.
+    std::string pkcs8_hex_string;
+    ASSERT_TRUE(key_values->GetString("pkcs8", &pkcs8_hex_string));
+    std::vector<uint8> pkcs8_bytes = HexStringToBytes(pkcs8_hex_string);
+
+    // Get the modulus length for the key.
+    int modulus_length_bits = 0;
+    ASSERT_TRUE(key_values->GetInteger("modulusLength", &modulus_length_bits));
+
+    blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
+
+    // Import the key from JWK.
+    ASSERT_EQ(
+        Status::Success(),
+        ImportKeyJwkFromDict(*key_jwk,
+                             CreateRsaHashedImportAlgorithm(
+                                 blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
+                                 blink::WebCryptoAlgorithmIdSha256),
+                             true,
+                             blink::WebCryptoKeyUsageSign,
+                             &private_key));
+
+    live_keys.push_back(private_key);
+
+    EXPECT_EQ(
+        modulus_length_bits,
+        static_cast<int>(
+            private_key.algorithm().rsaHashedParams()->modulusLengthBits()));
+
+    // Export to PKCS8 and verify that it matches expectation.
+    std::vector<uint8> exported_key_pkcs8;
+    ASSERT_EQ(
+        Status::Success(),
+        ExportKey(
+            blink::WebCryptoKeyFormatPkcs8, private_key, &exported_key_pkcs8));
+
+    EXPECT_BYTES_EQ(pkcs8_bytes, exported_key_pkcs8);
+  }
+}
+
+// Import an RSA private key using JWK. Next import a JWK containing the same
+// modulus, but mismatched parameters for the rest. It should NOT be possible
+// that the second import retrieves the first key. See http://crbug.com/378315
+// for how that could happen.
+TEST_F(SharedCryptoTest, MAYBE(DISABLED_ImportJwkExistingModulusAndInvalid)) {
+  scoped_ptr<base::ListValue> key_list;
+  ASSERT_TRUE(ReadJsonTestFileToList("rsa_private_keys.json", &key_list));
+
+  // Import a 1024-bit private key.
+  base::DictionaryValue* key1_props;
+  ASSERT_TRUE(key_list->GetDictionary(1, &key1_props));
+  base::DictionaryValue* key1_jwk;
+  ASSERT_TRUE(key1_props->GetDictionary("jwk", &key1_jwk));
+
+  blink::WebCryptoKey key1 = blink::WebCryptoKey::createNull();
+  ASSERT_EQ(Status::Success(),
+            ImportKeyJwkFromDict(*key1_jwk,
+                                 CreateRsaHashedImportAlgorithm(
+                                     blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
+                                     blink::WebCryptoAlgorithmIdSha256),
+                                 true,
+                                 blink::WebCryptoKeyUsageSign,
+                                 &key1));
+
+  ASSERT_EQ(1024u, key1.algorithm().rsaHashedParams()->modulusLengthBits());
+
+  // Construct a JWK using the modulus of key1, but all the other fields from
+  // another key (also a 1024-bit private key).
+  base::DictionaryValue* key2_props;
+  ASSERT_TRUE(key_list->GetDictionary(5, &key2_props));
+  base::DictionaryValue* key2_jwk;
+  ASSERT_TRUE(key2_props->GetDictionary("jwk", &key2_jwk));
+  std::string modulus;
+  key1_jwk->GetString("n", &modulus);
+  key2_jwk->SetString("n", modulus);
+
+  // If NSS did validations of the input, this would fail. Failing would be the

[Ryan Sleevi, 2014/06/05 23:21:25]

I'm not a fan of this test, even DISABLED_, since OpenSSL will likely behave
differently.

Rather, changing this test to make sure the second import *doesn't* succeed
seems much better.

[eroman, 2014/06/05 23:43:35]

Once the NSS side rolls in, presumably I can change the import to assert a
failure (for builds using bundled NSS, and skip it on the others).

+  // right thing to do. Barring that ensure at least the key isn't bogus.
+  blink::WebCryptoKey key2 = blink::WebCryptoKey::createNull();
+  ASSERT_EQ(Status::Success(),
+            ImportKeyJwkFromDict(*key2_jwk,
+                                 CreateRsaHashedImportAlgorithm(
+                                     blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
+                                     blink::WebCryptoAlgorithmIdSha256),
+                                 true,
+                                 blink::WebCryptoKeyUsageSign,
+                                 &key2));
+  ASSERT_EQ(1024u, key2.algorithm().rsaHashedParams()->modulusLengthBits());
+
+  // Export to PKCS8 to make sure key1 != key2.
+  std::vector<uint8> exported_key1;
+  std::vector<uint8> exported_key2;
+  ASSERT_EQ(Status::Success(),
+            ExportKey(blink::WebCryptoKeyFormatPkcs8, key1, &exported_key1));
+  ASSERT_EQ(Status::Success(),
+            ExportKey(blink::WebCryptoKeyFormatPkcs8, key2, &exported_key2));
+
+  EXPECT_NE(CryptoData(exported_key1), CryptoData(exported_key2));
+}
+
 // Import a JWK RSA private key with some optional parameters missing (q, dp,
 // dq, qi).
 //
 // The only optional parameter included is "p".
 //
 // This fails because JWA says that producers must include either ALL optional
 // parameters or NONE.
 TEST_F(SharedCryptoTest, MAYBE(ImportRsaPrivateKeyJwkMissingOptionalParams)) {
   blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
 
@@ skipping 244 matching lines @@
   // This should fail since spki is for public keys.
   EXPECT_EQ(
       Status::Success(),
       GenerateKeyPair(algorithm, true, usage_mask, &public_key, &private_key));
   EXPECT_EQ(Status::ErrorUnexpectedKeyType(),
             ExportKey(blink::WebCryptoKeyFormatSpki, private_key, &output));
 }
 
 TEST_F(SharedCryptoTest, MAYBE(RsaSsaSignVerifyFailures)) {
   // Import a key pair.
-  blink::WebCryptoAlgorithm importAlgorithm =
+  blink::WebCryptoAlgorithm import_algorithm =
       CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
                                      blink::WebCryptoAlgorithmIdSha1);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
   ASSERT_NO_FATAL_FAILURE(
       ImportRsaKeyPair(HexStringToBytes(kPublicKeySpkiDerHex),
                        HexStringToBytes(kPrivateKeyPkcs8DerHex),
-                       importAlgorithm,
+                       import_algorithm,
                        false,
                        blink::WebCryptoKeyUsageVerify,
                        blink::WebCryptoKeyUsageSign,
                        &public_key,
                        &private_key));
 
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5);
 
   std::vector<uint8> signature;
@@ skipping 104 matching lines @@
                 CryptoData(data),
                 &is_match));
   EXPECT_FALSE(is_match);
 }
 
 TEST_F(SharedCryptoTest, MAYBE(RsaSignVerifyKnownAnswer)) {
   scoped_ptr<base::ListValue> tests;
   ASSERT_TRUE(ReadJsonTestFileToList("pkcs1v15_sign.json", &tests));
 
   // Import the key pair.
-  blink::WebCryptoAlgorithm importAlgorithm =
+  blink::WebCryptoAlgorithm import_algorithm =
       CreateRsaHashedImportAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
                                      blink::WebCryptoAlgorithmIdSha1);
   blink::WebCryptoKey public_key = blink::WebCryptoKey::createNull();
   blink::WebCryptoKey private_key = blink::WebCryptoKey::createNull();
   ASSERT_NO_FATAL_FAILURE(
       ImportRsaKeyPair(HexStringToBytes(kPublicKeySpkiDerHex),
                        HexStringToBytes(kPrivateKeyPkcs8DerHex),
-                       importAlgorithm,
+                       import_algorithm,
                        false,
                        blink::WebCryptoKeyUsageVerify,
                        blink::WebCryptoKeyUsageSign,
                        &public_key,
                        &private_key));
 
   blink::WebCryptoAlgorithm algorithm =
       CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5);
 
   // Validate the signatures are computed and verified as expected.
@@ skipping 1498 matching lines @@
   EXPECT_EQ(public_key_spki, unwrapped_public_key_spki);
   EXPECT_EQ(private_key_pkcs8, unwrapped_private_key_pkcs8);
 
   EXPECT_NE(public_key_spki, wrapped_public_key);
   EXPECT_NE(private_key_pkcs8, wrapped_private_key);
 }
 
 }  // namespace webcrypto
 
 }  // namespace content