WebCrypto: Implement importKey() and sign() for HMAC in 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 018037c26f597e9144b380d061112c4b15e4b9e7..b59721147fc9e39d14fccc19c8618212f9d262d8 100644
--- a/content/renderer/webcrypto_impl_nss.cc
+++ b/content/renderer/webcrypto_impl_nss.cc
@@ -4,41 +4,111 @@
 
 #include "content/renderer/webcrypto_impl.h"
 
+#include <pk11pub.h>
 #include <sechash.h>
 
 #include "base/logging.h"
 #include "crypto/nss_util.h"
+#include "crypto/scoped_nss_types.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 {
 
-bool WebCryptoImpl::DigestInternal(
-    const WebKit::WebCryptoAlgorithm& algorithm,
-    const unsigned char* data,
-    unsigned data_size,
-    WebKit::WebArrayBuffer* buffer) {
-  HASH_HashType hash_type = HASH_AlgNULL;
+namespace {
+
+class KeyHandleBase : public WebKit::WebCryptoKeyHandle {
+ public:
+  bool Initialize() {
+    slot_.reset(PK11_GetInternalSlot());
+    return slot_.get() != NULL;
+  }
+
+  CK_MECHANISM_TYPE mechanism() const { return mechanism_; }
+  PK11SlotInfo* slot() { return slot_.get(); }
+
+ protected:
+  explicit KeyHandleBase(CK_MECHANISM_TYPE mechanism)
+      : mechanism_(mechanism) {
+  }
+
+  CK_MECHANISM_TYPE mechanism_;

[Ryan Sleevi, 2013/09/12 22:01:56]

This is duplicating what the PK11SymKey already stores as well.

Any reason for that?

[Bryan Eyler, 2013/09/12 23:09:56]

None, other than I didn't realize that could be retrieved from the key. 
Removed.

+  crypto::ScopedPK11Slot slot_;
+};
+
+class SymKeyHandle : public KeyHandleBase {
+ public:
+  explicit SymKeyHandle(CK_MECHANISM_TYPE mechanism)
+      : KeyHandleBase(mechanism) {
+  }
+
+  void set_key(crypto::ScopedPK11SymKey key) {
+    DCHECK(!key_.get());
+    key_ = key.Pass();
+  }
+
+  PK11SymKey* key() { return key_.get(); }
+
+ private:
+  crypto::ScopedPK11SymKey key_;

[Ryan Sleevi, 2013/09/12 22:01:56]

The PK11SymKey holds onto its PK11SlotInfoStr. The PK11SlotInfoStr is a
ref-counted object. So you shouldn't need to independently store the key.

[Ryan Sleevi, 2013/09/12 22:15:02]

s/key/slot

[Bryan Eyler, 2013/09/12 23:09:56]

Removed tracking of slot.

With the removal of tracking of slot and mechanism, there's nothing else to
store in a KeyHandleBase class.  I believe there should eventually be something,
like a type indicating whether it's symmetric of asymmetric, but perhaps this
should be left until it's needed and the base class removed?  I removed it in
the next patch set.

+
+  DISALLOW_COPY_AND_ASSIGN(SymKeyHandle);
+};
 
+CK_FLAGS WebCryptoKeyUsageMaskToNSSFlags(
+    WebKit::WebCryptoKeyUsageMask mask) {
+  return ((mask & WebKit::WebCryptoKeyUsageEncrypt) ? CKF_ENCRYPT : 0) |
+      ((mask & WebKit::WebCryptoKeyUsageDecrypt) ? CKF_DECRYPT : 0) |
+      ((mask & WebKit::WebCryptoKeyUsageSign) ? CKF_SIGN : 0) |
+      ((mask & WebKit::WebCryptoKeyUsageVerify) ? CKF_VERIFY : 0) |
+      ((mask & WebKit::WebCryptoKeyUsageDeriveKey) ? CKF_DERIVE : 0) |
+      ((mask & WebKit::WebCryptoKeyUsageWrapKey) ? CKF_WRAP : 0) |
+      ((mask & WebKit::WebCryptoKeyUsageUnwrapKey) ? CKF_UNWRAP : 0);

[Ryan Sleevi, 2013/09/12 22:01:56]

design: Don't use this?

[Ryan Sleevi, 2013/09/12 22:15:02]

eric pointed out some confusion: We should not be tracking CKF flags on the NSS
side - or at least, generating the keys with all flags.

In the end, it won't matter - the Blink side is responsible for enforcing
usages.

The example of why we shouldn't be doing this is a key being used to unwrap
JWKs. NSS will never support JWK, so it will need to CKF_DECRYPT the JWK and
then import that, but if this is used, it'll only have the CKF_UNWRAP.

If it won't let you supply no usages, supply all usages (that are valid for that
key type) - eg: no sign/verify for AES keys.

[Bryan Eyler, 2013/09/12 23:09:56]

Done.

+}
+
+HASH_HashType WebCryptoAlgorithmToNSSHashType(
+    const WebKit::WebCryptoAlgorithm& algorithm) {
   switch (algorithm.id()) {
     case WebKit::WebCryptoAlgorithmIdSha1:
-      hash_type = HASH_AlgSHA1;
-      break;
+      return HASH_AlgSHA1;
     case WebKit::WebCryptoAlgorithmIdSha224:
-      hash_type = HASH_AlgSHA224;
-      break;
+      return HASH_AlgSHA224;
     case WebKit::WebCryptoAlgorithmIdSha256:
-      hash_type = HASH_AlgSHA256;
-      break;
+      return HASH_AlgSHA256;
     case WebKit::WebCryptoAlgorithmIdSha384:
-      hash_type = HASH_AlgSHA384;
-      break;
+      return HASH_AlgSHA384;
     case WebKit::WebCryptoAlgorithmIdSha512:
-      hash_type = HASH_AlgSHA512;
-      break;
+      return HASH_AlgSHA512;
     default:
       // Not a digest algorithm.
-      return false;
+      return HASH_AlgNULL;
+  }
+}
+
+CK_MECHANISM_TYPE WebCryptoAlgorithmToHMACMechanism(
+    const WebKit::WebCryptoAlgorithm& algorithm) {
+  switch (algorithm.id()) {
+    case WebKit::WebCryptoAlgorithmIdSha1:
+      return CKM_SHA_1_HMAC;
+    case WebKit::WebCryptoAlgorithmIdSha256:
+      return CKM_SHA256_HMAC;
+    default:
+      // Not a supported algorithm.
+      return CKM_INVALID_MECHANISM;
+  }
+}
+
+}  // namespace
+
+bool WebCryptoImpl::DigestInternal(
+    const WebKit::WebCryptoAlgorithm& algorithm,
+    const unsigned char* data,
+    unsigned data_size,
+    WebKit::WebArrayBuffer* buffer) {
+  HASH_HashType hash_type = WebCryptoAlgorithmToNSSHashType(algorithm);
+  if (hash_type == HASH_AlgNULL) {
+    return false;
   }
 
   crypto::EnsureNSSInit();
@@ -52,14 +122,14 @@ bool WebCryptoImpl::DigestInternal(
 
   HASH_Update(context, data, data_size);
 
-  size_t hash_result_length = HASH_ResultLenContext(context);
+  unsigned hash_result_length = HASH_ResultLenContext(context);
   DCHECK_LE(hash_result_length, static_cast<size_t>(HASH_LENGTH_MAX));
 
   *buffer = WebKit::WebArrayBuffer::create(hash_result_length, 1);
 
   unsigned char* digest = reinterpret_cast<unsigned char*>(buffer->data());
 
-  uint32 result_length = 0;
+  unsigned result_length = 0;
   HASH_End(context, digest, &result_length, hash_result_length);
 
   HASH_Destroy(context);
@@ -67,4 +137,151 @@ bool WebCryptoImpl::DigestInternal(
   return result_length == hash_result_length;
 }
 
+bool WebCryptoImpl::ImportKeyInternal(
+    WebKit::WebCryptoKeyFormat format,
+    const unsigned char* key_data,
+    unsigned key_data_size,
+    const WebKit::WebCryptoAlgorithm& algorithm,
+    WebKit::WebCryptoKeyUsageMask usage_mask,
+    scoped_ptr<WebKit::WebCryptoKeyHandle>* handle,
+    WebKit::WebCryptoKeyType* type) {
+  switch (algorithm.id()) {
+    case WebKit::WebCryptoAlgorithmIdHmac:
+      *type = WebKit::WebCryptoKeyTypeSecret;
+      break;
+    // TODO(bryaneyler): Support more key types.
+    default:
+      return false;
+  }
+
+  // TODO(bryaneyler): Need to split handling for symmetric and asymmetric keys.
+  // Currently only supporting symmetric.
+  scoped_ptr<SymKeyHandle> sym_key;
+
+  crypto::EnsureNSSInit();
+
+  switch(algorithm.id()) {
+    case WebKit::WebCryptoAlgorithmIdHmac: {
+      const WebKit::WebCryptoHmacParams* params = algorithm.hmacParams();
+      if (!params) {
+        return false;
+      }
+
+      CK_MECHANISM_TYPE mechanism =
+          WebCryptoAlgorithmToHMACMechanism(params->hash());
+      if (mechanism == CKM_INVALID_MECHANISM) {
+        return false;
+      }
+
+      sym_key.reset(new SymKeyHandle(mechanism));
+
+      if (!sym_key->Initialize()) {
+        return false;
+      }
+
+      break;
+    }
+    default:
+      return false;
+  }
+
+  SECItem key_item = { siBuffer, NULL, 0 };
+
+  switch (format) {
+    case WebKit::WebCryptoKeyFormatRaw:
+      key_item.data = const_cast<unsigned char*>(key_data);
+      key_item.len = key_data_size;
+      break;
+    // TODO(bryaneyler): Handle additional formats.
+    default:
+      return false;
+  }
+
+  crypto::ScopedPK11SymKey pk11_sym_key(
+      PK11_ImportSymKeyWithFlags(sym_key->slot(),
+                                 sym_key->mechanism(),
+                                 PK11_OriginUnwrap,
+                                 CKA_FLAGS_ONLY,
+                                 &key_item,
+                                 WebCryptoKeyUsageMaskToNSSFlags(usage_mask),
+                                 false,
+                                 NULL));
+  sym_key->set_key(pk11_sym_key.Pass());
+  if (!sym_key->key()) {
+    NOTREACHED();
+    return false;
+  }
+
+  *handle = sym_key.Pass();
+
+  return true;
+}
+
+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: {
+      const WebKit::WebCryptoHmacParams* params = algorithm.hmacParams();
+      if (!params) {
+        return false;
+      }
+
+      SymKeyHandle* sym_key =
+          const_cast<SymKeyHandle*>(
+              reinterpret_cast<const SymKeyHandle*>(key.handle()));
+
+      DCHECK_EQ(sym_key->mechanism(),
+                WebCryptoAlgorithmToHMACMechanism(params->hash()));
+      DCHECK_NE(0, key.usages() & WebKit::WebCryptoKeyUsageSign);

[Ryan Sleevi, 2013/09/12 22:01:56]

What's this DCHECK guarding against?

The WebKit API?

[Bryan Eyler, 2013/09/12 23:09:56]

I guess - just a sanity check that the params in match up.  Should I just remove
it?  Same as above - just validating that what's passed in is as expected.

+
+      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(),
+                              sym_key->mechanism(),
+                              &param_item,
+                              &signature_item,
+                              &data_item) != SECSuccess) {
+        NOTREACHED();
+        return false;
+      }
+
+      DCHECK_LT(0u, signature_item.len);
+
+      result = WebKit::WebArrayBuffer::create(signature_item.len, 1);
+      signature_item.data = reinterpret_cast<unsigned char*>(result.data());
+
+      if (PK11_SignWithSymKey(sym_key->key(),
+                              sym_key->mechanism(),
+                              &param_item,
+                              &signature_item,
+                              &data_item) != SECSuccess) {
+        NOTREACHED();
+        return false;
+      }
+
+      DCHECK_EQ(result.byteLength(), signature_item.len);
+
+      break;
+    }
+    default:
+      return false;
+  }
+
+  *buffer = result;
+  return true;
+}
+
 }  // namespace content