WebCrypto: Implement importKey(), sign(), and verify() for HMAC in OpenSSL

content/renderer/webcrypto_impl_openssl.cc

Index: content/renderer/webcrypto_impl_openssl.cc
diff --git a/content/renderer/webcrypto_impl_openssl.cc b/content/renderer/webcrypto_impl_openssl.cc
index 9aa1a4253016bb10bd7859a32e42afb5c3483cd0..6a62b942ece4815015321b2c9455beef1ead7195 100644
--- a/content/renderer/webcrypto_impl_openssl.cc
+++ b/content/renderer/webcrypto_impl_openssl.cc
@@ -4,17 +4,42 @@
 
 #include "content/renderer/webcrypto_impl.h"
 
+#include <vector>

[eroman, 2013/09/25 00:00:59]

style nit: probably need to sort this after <openssl/*>

[padolph, 2013/09/25 02:00:38]

I followed the include header ordering recommendation here:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Names_...

"C library, C++ library, other libraries' .h, your project's .h"

I think I applied this correctly, but is there some other guideline I should be
using?

[eroman, 2013/09/27 00:15:52]

My mistake then!

+#include <openssl/hmac.h>
+#include <openssl/sha.h>
+
+#include "base/logging.h"
+#include "crypto/openssl_util.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 {
 
-void WebCryptoImpl::Init() {
-}
+namespace {
 
-bool WebCryptoImpl::DigestInternal(
-    const WebKit::WebCryptoAlgorithm& algorithm,
-    const unsigned char* data,
-    unsigned data_size,
-    WebKit::WebArrayBuffer* buffer) {
-  // TODO(bryaneyler): Placeholder for OpenSSL implementation.
+class SymKeyHandle : public WebKit::WebCryptoKeyHandle {
+ public:
+  explicit SymKeyHandle(const unsigned char* key_data, unsigned key_data_size)

[eroman, 2013/09/25 00:00:59]

style nit: can remove "explicit"

[padolph, 2013/09/25 02:00:38]

Done.

+      : key_(key_data, key_data + key_data_size) {}
+
+  const std::vector<unsigned char>& key() const { return key_; }
+
+ private:
+  const std::vector<unsigned char> key_;
+
+  DISALLOW_COPY_AND_ASSIGN(SymKeyHandle);
+};
+
+}  // anonymous namespace
+
+void WebCryptoImpl::Init() { crypto::EnsureOpenSSLInit(); }
+
+bool WebCryptoImpl::DigestInternal(const WebKit::WebCryptoAlgorithm& algorithm,
+                                   const unsigned char* data,
+                                   unsigned data_size,
+                                   WebKit::WebArrayBuffer* buffer) {
+  // TODO(padolph): Placeholder for OpenSSL implementation.
   // Issue http://crbug.com/267888.
   return false;
 }
@@ -24,23 +49,134 @@ bool WebCryptoImpl::ImportKeyInternal(
     const unsigned char* key_data,
     unsigned key_data_size,
     const WebKit::WebCryptoAlgorithm& algorithm,
-    WebKit::WebCryptoKeyUsageMask usage_mask,
+    WebKit::WebCryptoKeyUsageMask /*usage_mask*/,
     scoped_ptr<WebKit::WebCryptoKeyHandle>* handle,
     WebKit::WebCryptoKeyType* type) {
-  // TODO(bryaneyler): Placeholder for OpenSSL implementation.
-  // Issue http://crbug.com/267888.
-  return false;
+
+  // TODO(padolph): Support all relevant alg types and then remove this gate.
+  if (algorithm.id() != WebKit::WebCryptoAlgorithmIdHmac &&
+      algorithm.id() != WebKit::WebCryptoAlgorithmIdAesCbc) {
+    return false;
+  }
+
+  // TODO(padolph): Need to split handling for symmetric (raw or jwk format) and
+  // asymmetric (jwk, spki, or pkcs8 format) keys.
+  // Currently only supporting symmetric.
+
+  // TODO(padolph): jwk handling. Define precedence between jwk contents and
+  // this method's parameters, e.g. 'alg' in jwk vs algorithm.id(). Who wins if
+  // they differ? (jwk, probably)
+
+  // Symmetric keys are always type secret
+  *type = WebKit::WebCryptoKeyTypeSecret;
+
+  const unsigned char* raw_key_data;
+  unsigned raw_key_data_size;
+  switch (format) {
+    case WebKit::WebCryptoKeyFormatRaw:
+      raw_key_data = key_data;
+      raw_key_data_size = key_data_size;
+      break;
+    case WebKit::WebCryptoKeyFormatJwk:
+      // TODO(padolph): Handle jwk format; need simple JSON parser.
+      // break;
+      return false;
+    default:
+      return false;
+  }
+
+  scoped_ptr<SymKeyHandle> sym_key(
+      new SymKeyHandle(raw_key_data, raw_key_data_size));
+  *handle = sym_key.Pass();

[eroman, 2013/09/25 00:00:59]

style nit: You could get rid of the sym_key temporary and write this as:

handle->reset(new SymKeyHandle(...));

[padolph, 2013/09/25 02:00:38]

Done.

+
+  return true;
 }
 
-bool WebCryptoImpl::SignInternal(
-    const WebKit::WebCryptoAlgorithm& algorithm,
-    const WebKit::WebCryptoKey& key,
-    const unsigned char* data,
-    unsigned data_size,
-    WebKit::WebArrayBuffer* buffer) {
-  // TODO(bryaneyler): Placeholder for OpenSSL implementation.
-  // Issue http://crbug.com/267888.
-  return false;
+bool WebCryptoImpl::SignInternal(const WebKit::WebCryptoAlgorithm& algorithm,
+                                 const WebKit::WebCryptoKey& key,
+                                 const unsigned char* data,
+                                 unsigned data_size,
+                                 WebKit::WebArrayBuffer* buffer) {
+
+  WebKit::WebArrayBuffer result;
+
+  switch (algorithm.id()) {
+    case WebKit::WebCryptoAlgorithmIdHmac: {
+
+      DCHECK(key.algorithm().id() == WebKit::WebCryptoAlgorithmIdHmac);

[eroman, 2013/09/25 00:00:59]

style nit: You can use DCHECK_EQ() in this case (gives nicer output on failures)

[padolph, 2013/09/25 02:00:38]

Done.

+      DCHECK_NE(0, key.usages() & WebKit::WebCryptoKeyUsageSign);
+
+      const WebKit::WebCryptoHmacParams* const params = algorithm.hmacParams();
+      if (!params)
+        return false;
+
+      const EVP_MD* evp_sha = 0;
+      unsigned int hmac_expected_length = 0;
+      // Note that HMAC length is determined by the hash used.
+      switch (params->hash().id()) {
+        case WebKit::WebCryptoAlgorithmIdSha1:
+          evp_sha = EVP_sha1();
+          hmac_expected_length = SHA_DIGEST_LENGTH;
+          break;
+        case WebKit::WebCryptoAlgorithmIdSha224:
+          evp_sha = EVP_sha224();
+          hmac_expected_length = SHA224_DIGEST_LENGTH;
+          break;
+        case WebKit::WebCryptoAlgorithmIdSha256:
+          evp_sha = EVP_sha256();
+          hmac_expected_length = SHA256_DIGEST_LENGTH;
+          break;
+        case WebKit::WebCryptoAlgorithmIdSha384:
+          evp_sha = EVP_sha384();
+          hmac_expected_length = SHA384_DIGEST_LENGTH;
+          break;
+        case WebKit::WebCryptoAlgorithmIdSha512:
+          evp_sha = EVP_sha512();
+          hmac_expected_length = SHA512_DIGEST_LENGTH;
+          break;
+        default:
+          // Not a digest algorithm.
+          return false;
+      }
+
+      SymKeyHandle* const sym_key =
+          reinterpret_cast<SymKeyHandle*>(key.handle());
+      const std::vector<unsigned char>& raw_key = sym_key->key();
+
+      // OpenSSL wierdness here.
+      // First, HMAC() needs a void* for the key data, so make one up front as a
+      // cosmetic to avoid a cast. Second, OpenSSL does not like a NULL key,
+      // which will result if the raw_key vector is empty; an entirely valid

[eroman, 2013/09/25 00:00:59]

Is a 0-byte key a supported use-case? On the NSS side we are failing ImportKey
when given empty key bytes. I believe we should match that behavior here.

[padolph, 2013/09/25 02:00:38]

Done.

[padolph, 2013/09/25 02:00:38]

This code is required to pass Bryan's "Empty Sets" test in
WebCryptoImplTest::HMACSampleSets, which passes for NSS.

+      // case. Handle this specific case by pointing to an empty array.
+      const unsigned char null_key[] = {};
+      const void* const raw_key_voidp = raw_key.size() ? &raw_key[0] : null_key;
+
+      result = WebKit::WebArrayBuffer::create(hmac_expected_length, 1);
+      crypto::ScopedOpenSSLSafeSizeBuffer<EVP_MAX_MD_SIZE> hmac_result(
+          reinterpret_cast<unsigned char*>(result.data()),
+          hmac_expected_length);
+
+      crypto::OpenSSLErrStackTracer(FROM_HERE);
+
+      unsigned int hmac_actual_length;
+      unsigned char* const success = HMAC(evp_sha,
+                                          raw_key_voidp,
+                                          raw_key.size(),
+                                          data,
+                                          data_size,
+                                          hmac_result.safe_buffer(),
+                                          &hmac_actual_length);
+      if (!success || hmac_actual_length != hmac_expected_length)
+        return false;
+
+      break;
+    }
+    default:
+      return false;
+  }
+
+  *buffer = result;
+  return true;
 }
 
 }  // namespace content