[webcrypto] Remove support for null import algorithms.

[webcrypto] Remove support for null import algorithms.

This is being dropped from the spec: https://www.w3.org/Bugs/Public/show_bug.cgi?id=25036

BUG=245025
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258446

[padolph, 2014-03-14 18:25:41]

On 2014/03/13 03:35:37, eroman wrote:

Please wait for https://codereview.chromium.org/184043021/ to land before
committing this. Thank you.

[eroman, 2014-03-14 18:40:14]

Will do!

I also need to wait for a LGTM and blink side to land first :)

[Ryan Sleevi, 2014-03-14 21:31:49]

LGTM, mod nit.

https://codereview.chromium.org/197223007/diff/40001/content/child/webcrypto/...
File content/child/webcrypto/jwk.cc (right):

https://codereview.chromium.org/197223007/diff/40001/content/child/webcrypto/...
content/child/webcrypto/jwk.cc:409: // JWK "alg" --> algorithm parameter
JWK "alg" remains optional

So you still have several cases to handle/document
1. JWK alg present but unrecognized: error
2. JWK alg valid and inconsistent with input algorithm: error
3. JWK alg valid and consistent with input algorithm: use input value
4. JWK alg is missing: use input value

https://codereview.chromium.org/197223007/diff/40001/content/child/webcrypto/...
content/child/webcrypto/jwk.cc:431: // input algorithm specified
unnecessary comment?

[eroman, 2014-03-18 01:25:29]

https://codereview.chromium.org/197223007/diff/40001/content/child/webcrypto/...
File content/child/webcrypto/jwk.cc (right):

https://codereview.chromium.org/197223007/diff/40001/content/child/webcrypto/...
content/child/webcrypto/jwk.cc:409: // JWK "alg" --> algorithm parameter
On 2014/03/14 21:31:50, Ryan Sleevi wrote:
> JWK "alg" remains optional
> 
> So you still have several cases to handle/document
> 1. JWK alg present but unrecognized: error
> 2. JWK alg valid and inconsistent with input algorithm: error
> 3. JWK alg valid and consistent with input algorithm: use input value
> 4. JWK alg is missing: use input value

Done.

https://codereview.chromium.org/197223007/diff/40001/content/child/webcrypto/...
content/child/webcrypto/jwk.cc:431: // input algorithm specified
On 2014/03/14 21:31:50, Ryan Sleevi wrote:
> unnecessary comment?

Done.

[eroman, 2014-03-19 23:20:32]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2014-03-19 23:20:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/eroman@chromium.org/197223007/120001

[commit-bot: I haz the power, 2014-03-20 00:33:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 00:33:43]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[eroman, 2014-03-20 00:35:09]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2014-03-20 00:36:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/eroman@chromium.org/197223007/120001

[commit-bot: I haz the power, 2014-03-20 06:32:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 06:32:56]

Failed to apply patch for content/child/webcrypto/shared_crypto_unittest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/child/webcrypto/shared_crypto_unittest.cc
  Hunk #1 succeeded at 1005 (offset 1 line).
  Hunk #2 succeeded at 1175 (offset 1 line).
  Hunk #3 succeeded at 1412 (offset 1 line).
  Hunk #4 succeeded at 1638 (offset 1 line).
  Hunk #5 succeeded at 1718 (offset 1 line).
  Hunk #6 succeeded at 2473 (offset 1 line).
  Hunk #7 succeeded at 2561 (offset 1 line).
  Hunk #8 FAILED at 2627.
  1 out of 8 hunks FAILED -- saving rejects to file
content/child/webcrypto/shared_crypto_unittest.cc.rej

Patch:       content/child/webcrypto/shared_crypto_unittest.cc
Index: content/child/webcrypto/shared_crypto_unittest.cc
diff --git a/content/child/webcrypto/shared_crypto_unittest.cc
b/content/child/webcrypto/shared_crypto_unittest.cc
index
814a26fb45ff31d140754e9e2ac4a3504cf9fdab..3d0948253b6683675b25afee5b3c8bce455c7daa
100644
--- a/content/child/webcrypto/shared_crypto_unittest.cc
+++ b/content/child/webcrypto/shared_crypto_unittest.cc
@@ -1004,19 +1004,6 @@ TEST_F(SharedCryptoTest, MAYBE(GenerateKeyHmacNoLength))
{
   EXPECT_EQ(128U, raw_key.byteLength());
 }
 
-TEST_F(SharedCryptoTest, MAYBE(ImportSecretKeyNoAlgorithm)) {
-  blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
-
-  // This fails because the algorithm is null.
-  EXPECT_STATUS(Status::ErrorMissingAlgorithmImportRawKey(),
-                ImportKey(blink::WebCryptoKeyFormatRaw,
-                          CryptoData(HexStringToBytes("00000000000000000000")),
-                          blink::WebCryptoAlgorithm::createNull(),
-                          true,
-                          blink::WebCryptoKeyUsageEncrypt,
-                          &key));
-}
-
 TEST_F(SharedCryptoTest, ImportJwkKeyUsage) {
   blink::WebCryptoKey key = blink::WebCryptoKey::createNull();
   base::DictionaryValue dict;
@@ -1187,16 +1174,6 @@ TEST_F(SharedCryptoTest, ImportJwkFailures) {
                 ImportKeyJwkFromDict(dict, algorithm, false, usage_mask,
&key));
   RestoreJwkOctDictionary(&dict);
 
-  // Fail on both JWK and input algorithm missing.
-  dict.Remove("alg", NULL);
-  EXPECT_STATUS(Status::ErrorJwkAlgorithmMissing(),
-                ImportKeyJwkFromDict(dict,
-                                     blink::WebCryptoAlgorithm::createNull(),
-                                     false,
-                                     usage_mask,
-                                     &key));
-  RestoreJwkOctDictionary(&dict);
-
   // Fail on invalid kty.
   dict.SetString("kty", "foo");
   EXPECT_STATUS(Status::ErrorJwkUnrecognizedKty(),
@@ -1434,14 +1411,6 @@ TEST_F(SharedCryptoTest,
MAYBE(ImportJwkInputConsistency)) {
                    usage_mask,
                    &key));
 
-  // Pass: JWK alg valid but input algorithm isNull: use JWK algorithm value.
-  EXPECT_STATUS_SUCCESS(ImportKeyJwk(CryptoData(json_vec),
-                                     blink::WebCryptoAlgorithm::createNull(),
-                                     extractable,
-                                     usage_mask,
-                                     &key));
-  EXPECT_EQ(blink::WebCryptoAlgorithmIdHmac, algorithm.id());
-
   // Pass: JWK alg missing but input algorithm specified: use input value
   dict.Remove("alg", NULL);
   EXPECT_STATUS_SUCCESS(ImportKeyJwkFromDict(
@@ -1668,24 +1637,14 @@ TEST_F(SharedCryptoTest, MAYBE(ImportExportSpki)) {
       "010001", CryptoData(key.algorithm().rsaParams()->publicExponent()));
 
   // Failing case: Empty SPKI data
-  EXPECT_STATUS(Status::ErrorImportEmptyKeyData(),
-                ImportKey(blink::WebCryptoKeyFormatSpki,
-                          CryptoData(std::vector<uint8>()),
-                          blink::WebCryptoAlgorithm::createNull(),
-                          true,
-                          blink::WebCryptoKeyUsageEncrypt,
-                          &key));
-
-  // Failing case: Import RSA key with NULL input algorithm. This is not
-  // allowed because the SPKI ASN.1 format for RSA keys is not specific enough
-  // to map to a Web Crypto algorithm.
-  EXPECT_STATUS(Status::Error(),
-                ImportKey(blink::WebCryptoKeyFormatSpki,
-                          CryptoData(HexStringToBytes(kPublicKeySpkiDerHex)),
-                          blink::WebCryptoAlgorithm::createNull(),
-                          true,
-                          blink::WebCryptoKeyUsageEncrypt,
-                          &key));
+  EXPECT_STATUS(
+      Status::ErrorImportEmptyKeyData(),
+      ImportKey(blink::WebCryptoKeyFormatSpki,
+                CryptoData(std::vector<uint8>()),
+                CreateAlgorithm(blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5),
+                true,
+                blink::WebCryptoKeyUsageEncrypt,
+                &key));
 
   // Failing case: Bad DER encoding.
   EXPECT_STATUS(
@@ -1758,18 +1717,9 @@ TEST_F(SharedCryptoTest, MAYBE(ImportPkcs8)) {
   EXPECT_STATUS(Status::ErrorImportEmptyKeyData(),
                 ImportKey(blink::WebCryptoKeyFormatPkcs8,
                           CryptoData(std::vector<uint8>()),
-                          blink::WebCryptoAlgorithm::createNull(),
-                          true,
-                          blink::WebCryptoKeyUsageSign,
-                          &key));
-
-  // Failing case: Import RSA key with NULL input algorithm. This is not
-  // allowed because the PKCS#8 ASN.1 format for RSA keys is not specific
enough
-  // to map to a Web Crypto algorithm.
-  EXPECT_STATUS(Status::Error(),
-                ImportKey(blink::WebCryptoKeyFormatPkcs8,
-                          CryptoData(HexStringToBytes(kPrivateKeyPkcs8DerHex)),
-                          blink::WebCryptoAlgorithm::createNull(),
+                          CreateRsaHashedImportAlgorithm(
+                              blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5,
+                              blink::WebCryptoAlgorithmIdSha1),
                           true,
                           blink::WebCryptoKeyUsageSign,
                           &key));
@@ -2522,21 +2472,10 @@ TEST_F(SharedCryptoTest,
MAYBE(AesKwRawSymkeyWrapUnwrapErrors)) {
       webcrypto::CreateAlgorithm(blink::WebCryptoAlgorithmIdAesCbc),
       blink::WebCryptoKeyUsageEncrypt);
 
-  // Unwrap with null algorithm must fail.
-  blink::WebCryptoKey unwrapped_key = blink::WebCryptoKey::createNull();
-  EXPECT_STATUS(Status::ErrorMissingAlgorithmUnwrapRawKey(),
-                UnwrapKey(blink::WebCryptoKeyFormatRaw,
-                          CryptoData(test_ciphertext),
-                          wrapping_key,
-                          wrapping_algorithm,
-                          blink::WebCryptoAlgorithm::createNull(),
-                          true,
-                          blink::WebCryptoKeyUsageEncrypt,
-                          &unwrapped_key));
-
   // Unwrap with wrapped data too small must fail.
   const std::vector<uint8> small_data(test_ciphertext.begin(),
                                       test_ciphertext.begin() + 23);
+  blink::WebCryptoKey unwrapped_key = blink::WebCryptoKey::createNull();
   EXPECT_STATUS(Status::ErrorDataTooSmall(),
                 UnwrapKey(blink::WebCryptoKeyFormatRaw,
                           CryptoData(small_data),
@@ -2621,14 +2560,15 @@ TEST_F(SharedCryptoTest,
MAYBE(AesKwJwkSymkeyUnwrapKnownData)) {
 
   // Unwrap the known wrapped key data to produce a new key
   blink::WebCryptoKey unwrapped_key = blink::WebCryptoKey::createNull();
-  ASSERT_STATUS_SUCCESS(UnwrapKey(blink::WebCryptoKeyFormatJwk,
-                                  CryptoData(wrapped_key_data),
-                                  wrapping_key,
-                                  wrapping_algorithm,
-                                  blink::WebCryptoAlgorithm::createNull(),
-                                  true,
-                                  blink::WebCryptoKeyUsageVerify,
-                                  &unwrapped_key));
+  ASSERT_STATUS_SUCCESS(
+      UnwrapKey(blink::WebCryptoKeyFormatJwk,
+                CryptoData(wrapped_key_data),
+                wrapping_key,
+                wrapping_algorithm,
+                CreateHmacImportAlgorithm(blink::WebCryptoAlgorithmIdSha256),
+                true,
+                blink::WebCryptoKeyUsageVerify,
+                &unwrapped_key));
 
   // Validate the new key's attributes.
   EXPECT_FALSE(unwrapped_key.isNull());
@@ -2687,15 +2627,16 @@ TEST_F(SharedCryptoTest,
MAYBE(AesKwJwkSymkeyUnwrapErrors)) {
 
   // Unwrap and ensure a generic error is received.
   blink::WebCryptoKey unwrapped_key = blink::WebCryptoKey::createNull();
-  EXPECT_STATUS(Status::Error(),
-                UnwrapKey(blink::WebCryptoKeyFormatJwk,
-                          CryptoData(wrapped_key_data),
-                          wrapping_key,
-                          wrapping_algorithm,
-                          blink::WebCryptoAlgorithm::createNull(),
-                          true,
-                          blink::WebCryptoKeyUsageVerify,
-                          &unwrapped_key));
+  EXPECT_STATUS(
+      Status::Error(),
+      UnwrapKey(blink::WebCryptoKeyFormatJwk,
+                CryptoData(wrapped_key_data),
+                wrapping_key,
+                wrapping_algorithm,
+                CreateHmacImportAlgorithm(blink::WebCryptoAlgorithmIdSha256),
+                true,
+                blink::WebCryptoKeyUsageVerify,
+                &unwrapped_key));
 
   // FIXME(padolph): The check above can fail if the AES-KW decryption step
   // failed, which masks the test result desired here. For now we have to just

[eroman, 2014-03-20 18:31:23]

The CQ bit was checked by eroman@chromium.org

[commit-bot: I haz the power, 2014-03-20 18:31:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/eroman@chromium.org/197223007/140001

[commit-bot: I haz the power, 2014-03-20 22:58:43]

Change committed as 258446