WebCrypto: Implement importKey() and sign() for HMAC in NSS

content/renderer/webcrypto_impl_unittest.cc

Index: content/renderer/webcrypto_impl_unittest.cc
diff --git a/content/renderer/webcrypto_impl_unittest.cc b/content/renderer/webcrypto_impl_unittest.cc
index b1f673bec97da56f02ced2e05c6a605cf154a98d..24d9111946dc492cd01bd43efbda42eddd248df9 100644
--- a/content/renderer/webcrypto_impl_unittest.cc
+++ b/content/renderer/webcrypto_impl_unittest.cc
@@ -14,6 +14,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/public/platform/WebArrayBuffer.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
+#include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
 
 namespace content {
 
@@ -117,4 +118,99 @@ TEST_F(WebCryptoImplTest, DigestSampleSets) {
   }
 }
 
+TEST_F(WebCryptoImplTest, HMACSampleSets) {
+  struct input_set_struct {
+    WebKit::WebCryptoAlgorithmId algorithm;
+    const char* key;
+    const char* msg;
+    const char* mac;
+  } input_set[] = {
+    // For this data, see http://csrc.nist.gov/groups/STM/cavp/index.html#07

[eroman, 2013/09/05 01:57:51]

I suggest additionally having a test for the "empty" data case. Just cuz it can
sometimes be a bit different.

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+    // Download:
+    // http://csrc.nist.gov/groups/STM/cavp/documents/mac/hmactestvectors.zip
+    // L=20 set 45
+    {
+      WebKit::WebCryptoAlgorithmIdSha1,
+      "59785928d72516e31272",
+      "a3ce8899df1022e8d2d539b47bf0e309c66f84095e21438ec355bf119ce5fdcb4e73a619cdf36f25b369d8c38ff419997f0c59830108223606e31223483fd39edeaa4d3f0d21198862d239c9fd26074130ff6c86493f5227ab895c8f244bd42c7afce5d147a20a590798c68e708e964902d124dadecdbda9dbd0051ed710e9bf",

[eroman, 2013/09/05 01:57:51]

regarding comment on line length: I think it is fine for test data, and I
wouldn't worry about it too much. To be on the safe side, you could always break
it up to fit on 80 character lines tho since it is easy enough.

i.e use C's auto-line joiner for string literals (whatever it is called)

"Yo dawg, this string is long" can be broken up as:

"Yo dawg, "
"this string is long"

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+      "3c8162589aafaee024fc9a5ca50dd2336fe3eb28",
+    },
+    // L=20 set 299
+    {
+      WebKit::WebCryptoAlgorithmIdSha1,
+      "ceb9aedf8d6efcf0ae52bea0fa99a9e26ae81bacea0cff4d5eecf201e3bca3c3577480621b818fd717ba99d6ff958ea3d59b2527b019c343bb199e648090225867d994607962f5866aa62930d75b58f6",
+      "99958aa459604657c7bf6e4cdfcc8785f0abf06ffe636b5b64ecd931bd8a456305592421fc28dbcccb8a82acea2be8e54161d7a78e0399a6067ebaca3f2510274dc9f92f2c8ae4265eec13d7d42e9f8612d7bc258f913ecb5a3a5c610339b49fb90e9037b02d684fc60da835657cb24eab352750c8b463b1a8494660d36c3ab2",
+      "4ac41ab89f625c60125ed65ffa958c6b490ea670",
+    },
+    // L=32, set 30
+    {
+      WebKit::WebCryptoAlgorithmIdSha256,
+      "9779d9120642797f1747025d5b22b7ac607cab08e1758f2f3a46c8be1e25c53b8c6a8f58ffefa176",
+      "b1689c2591eaf3c9e66070f8a77954ffb81749f1b00346f9dfe0b2ee905dcc288baf4a92de3f4001dd9f44c468c3d07d6c6ee82faceafc97c2fc0fc0601719d2dcd0aa2aec92d1b0ae933c65eb06a03c9c935c2bad0459810241347ab87e9f11adb30415424c6c7f5f22a003b8ab8de54f6ded0e3ab9245fa79568451dfa258e",
+      "769f00d3e6a6cc1fb426a14a4f76c6462e6149726e0dee0ec0cf97a16605ac8b",
+    },
+    // L=32, set 224
+    {
+      WebKit::WebCryptoAlgorithmIdSha256,
+      "4b7ab133efe99e02fc89a28409ee187d579e774f4cba6fc223e13504e3511bef8d4f638b9aca55d4a43b8fbd64cf9d74dcc8c9e8d52034898c70264ea911a3fd70813fa73b083371289b",
+      "138efc832c64513d11b9873c6fd4d8a65dbf367092a826ddd587d141b401580b798c69025ad510cff05fcfbceb6cf0bb03201aaa32e423d5200925bddfadd418d8e30e18050eb4f0618eb9959d9f78c1157d4b3e02cd5961f138afd57459939917d9144c95d8e6a94c8f6d4eef3418c17b1ef0b46c2a7188305d9811dccb3d99",
+      "4f1ee7cb36c58803a8721d4ac8c4cf8cae5d8832392eed2a96dc59694252801b",
+    },
+  };
+
+  for (size_t index = 0; index < arraysize(input_set); index++) {
+    WebKit::WebCryptoAlgorithm hash_algorithm(
+        WebKit::WebCryptoAlgorithm::adoptParamsAndCreate(
+            input_set[index].algorithm, "HASH", NULL));

[eroman, 2013/09/05 01:57:51]

Remove the "HASH" parameter. I am in the process of getting rid of that from the
Blink API (https://codereview.chromium.org/23672024/) since it was annoying to
pass in the unit tests like this :)

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+
+    scoped_ptr<WebKit::WebCryptoHmacKeyParams> key_params(

[eroman, 2013/09/05 01:57:51]

Use a  WebCryptoHmacParams instead of a WebCryptoHmacKeyParams.

Things should work fine with either, however to simulate how the code will be
getting called, better to use the HmacParams (HmacKeyParams is only used for
generateKey).

You can tell this by looking at the operations table here:

https://dvcs.w3.org/hg/webcrypto-api/raw-file/tip/spec/Overview.html#hmac

Notice how the input to importKey has "Parameters" of HmacParams. The Blink side
of the code which does algorithm parameter normalization will guarantee that the
Chromium code only gets called with the expected parameter type.

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+        new WebKit::WebCryptoHmacKeyParams(hash_algorithm, false, 0));
+
+    WebKit::WebCryptoAlgorithm hmac_key_algorithm(
+        WebKit::WebCryptoAlgorithm::adoptParamsAndCreate(
+            WebKit::WebCryptoAlgorithmIdHmac, "HMAC", key_params.release()));

[eroman, 2013/09/05 01:57:51]

Ditto. Remove the "HMAC" string, no longer needed. Yay

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+
+    WebKit::WebCryptoKeyHandle* handle;

[eroman, 2013/09/05 01:57:51]

This is being leaked.

That is why I would like the internal* methods to use scoped_ptr<>, so it is
very clear how the ownership of the returned pointer works :)

[Bryan Eyler, 2013/09/06 01:27:51]

Made into scoped_ptr.

+
+    std::vector<uint8> key_raw;
+    base::HexStringToBytes(input_set[index].key, &key_raw);
+
+    WebCryptoImpl crypto;
+
+    crypto.importKeyInternal(
+        WebKit::WebCryptoKeyFormatRaw,
+        &key_raw.front(),

[eroman, 2013/09/05 01:57:51]

It is more typical in chromium code to use .data()

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+        key_raw.size(),
+        hmac_key_algorithm,
+        false,
+        WebKit::WebCryptoKeyUsageSign,
+        &handle);
+
+    std::vector<uint8> msg_raw;
+    base::HexStringToBytes(input_set[index].msg, &msg_raw);
+
+    scoped_ptr<WebKit::WebCryptoHmacParams> sign_params(
+        new WebKit::WebCryptoHmacParams(hash_algorithm));

[eroman, 2013/09/05 01:57:51]

Optional: creating the WebCryptoAlgorithm for tests is clumsy enough, that I
suggest extracting that code to helpers. For example:

WebCryptoAlgorithm sup_dawg = HmacAlgorithm(WebCryptoAlgorithmIdSha1);

And another extractable method that might be useful for the pre-existing digest
tests:
   WebCryptoAlgorithm magnets = ShaAlgorithm(256)

(OK, maybe using an integer parameter is crazy, but just spitballing some ideas)

[Bryan Eyler, 2013/09/06 01:27:51]

This duplication is actually a result of the mistake above where I was using
HmacKeyParams instead of HmacParams.  I've removed this code and unified the
algorithm used.  Should I break it out only if it's necessary in the future?

+
+    WebKit::WebCryptoAlgorithm hmac_algorithm(
+        WebKit::WebCryptoAlgorithm::adoptParamsAndCreate(
+            WebKit::WebCryptoAlgorithmIdHmac, "HMAC", sign_params.release()));

[eroman, 2013/09/05 01:57:51]

remove "HMAC"

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+
+    WebKit::WebArrayBuffer array_buffer;
+
+    crypto.signInternal(
+        hmac_algorithm,
+        handle,
+        &msg_raw.front(),

[eroman, 2013/09/05 01:57:51]

msg_raw.data() is more concise in my opinion.

[Bryan Eyler, 2013/09/06 01:27:51]

Done.

+        msg_raw.size(),
+        &array_buffer);
+
+    // Ignore case, it's checking the hex value.
+    EXPECT_STRCASEEQ(
+        input_set[index].mac,
+        base::HexEncode(
+            array_buffer.data(), array_buffer.byteLength()).c_str());
+  }
+}
+
 }  // namespace content