Implement verify() for HMAC using NSS

content/renderer/webcrypto_impl_nss.cc

Index: content/renderer/webcrypto_impl_nss.cc
diff --git a/content/renderer/webcrypto_impl_nss.cc b/content/renderer/webcrypto_impl_nss.cc
index caf986fce44f168fe5c390239d27336919066188..05eeefda45c5e0516de8ddcd14a593c9e32c0bbe 100644
--- a/content/renderer/webcrypto_impl_nss.cc
+++ b/content/renderer/webcrypto_impl_nss.cc
@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "crypto/nss_util.h"
 #include "crypto/scoped_nss_types.h"
+#include "crypto/secure_util.h"

[eroman, 2013/09/25 23:18:51]

this should be sorted above crypto/scroped_nss

[Bryan Eyler, 2013/09/25 23:57:05]

For alphabetical ordering?  I think this is correct...

[eroman, 2013/09/26 00:10:42]

Oops, my bad :)

 #include "third_party/WebKit/public/platform/WebArrayBuffer.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
@@ -249,4 +250,73 @@ bool WebCryptoImpl::SignInternal(
   return true;
 }
 
+bool WebCryptoImpl::VerifySignatureInternal(
+    const WebKit::WebCryptoAlgorithm& algorithm,
+    const WebKit::WebCryptoKey& key,
+    const unsigned char* signature,
+    unsigned signature_size,
+    const unsigned char* data,
+    unsigned data_size,
+    bool* signature_match) {
+  switch (algorithm.id()) {
+    case WebKit::WebCryptoAlgorithmIdHmac: {
+      const WebKit::WebCryptoHmacParams* params = algorithm.hmacParams();
+      if (!params) {
+        return false;
+      }
+
+      SymKeyHandle* sym_key = reinterpret_cast<SymKeyHandle*>(key.handle());
+
+      DCHECK_EQ(PK11_GetMechanism(sym_key->key()),

[eroman, 2013/09/25 23:18:51]

Isn't this a duplication of DigestInternal? Please share the code instead. I.e.
call a helper function, or even SignInternal directly for now.

[Bryan Eyler, 2013/09/25 23:57:05]

Yeah, I was trying to avoid creating a WebArrayBuffer, but I guess this is such
a minimal call that it's worth all this extra duped code.  Fixed and it's a lot
cleaner.

+                WebCryptoAlgorithmToHMACMechanism(params->hash()));
+      DCHECK_NE(0, key.usages() & WebKit::WebCryptoKeyUsageSign);
+
+      SECItem param_item = { siBuffer, NULL, 0 };
+      SECItem data_item = {
+        siBuffer,
+        const_cast<unsigned char*>(data),
+        data_size
+      };
+      // First call is to figure out the length.
+      SECItem signature_item = { siBuffer, NULL, 0 };
+
+      if (PK11_SignWithSymKey(sym_key->key(),
+                              PK11_GetMechanism(sym_key->key()),
+                              &param_item,
+                              &signature_item,
+                              &data_item) != SECSuccess) {
+        NOTREACHED();
+        return false;
+      }
+
+      DCHECK_NE(0u, signature_item.len);
+
+      std::vector<unsigned char> result(signature_item.len);
+      signature_item.data = result.data();
+
+      if (PK11_SignWithSymKey(sym_key->key(),
+                              PK11_GetMechanism(sym_key->key()),
+                              &param_item,
+                              &signature_item,
+                              &data_item) != SECSuccess) {
+        NOTREACHED();
+        return false;
+      }
+
+      DCHECK_EQ(signature_item.len, result.size());
+
+      // To ensure optimal security usage, do not support truncated signatures.

[eroman, 2013/09/25 23:18:51]

This comment should be in terms of the webcrypto spec instead. i.e. "webcrypto
spec doesn't allow this" or "webcrypto spec is ambiguous on this but we chose to
XXX". I am fairly sure the intent of webcrypto spec is to not verify truncated
signatures.

[Bryan Eyler, 2013/09/25 23:57:05]

Done.

+      *signature_match =
+          crypto::SecureMemEqual(result.data(), signature, signature_size) &&
+          result.size() == signature_size;
+
+      break;
+    }
+    default:
+      return false;
+  }
+
+  return true;
+}
+
 }  // namespace content