[webcrypto] Add raw symmetric key AES-KW wrap/unwrap for NSS.

content/renderer/webcrypto/webcrypto_impl_nss.cc

Index: content/renderer/webcrypto/webcrypto_impl_nss.cc
diff --git a/content/renderer/webcrypto/webcrypto_impl_nss.cc b/content/renderer/webcrypto/webcrypto_impl_nss.cc
index 1df84289b243df2018f9de3bcf4e1a7d15d523d5..7d15f6452b34e7ebfa895f392ed3f5a30be9a13f 100644
--- a/content/renderer/webcrypto/webcrypto_impl_nss.cc
+++ b/content/renderer/webcrypto/webcrypto_impl_nss.cc
@@ -175,6 +175,121 @@ bool AesCbcEncryptDecrypt(
   return true;
 }
 
+// The RFC 3394 AES Key Wrap / Unwrap Mechanism.
+const CK_MECHANISM_TYPE kAesKwMechanism = CKM_NSS_AES_KEY_WRAP;
+
+// Creates an NSS SECItem representing the Default IV specified for AES Key
+// Wrap. See http://www.ietf.org/rfc/rfc3394.txt Section 2.2.3.1.
+crypto::ScopedSECItem CreateAesKwIvItem() {
+  const unsigned int kAesKwIvLength = 8;

[eroman, 2013/12/20 01:58:14]

Can you instead define this in terms of:
  arraysize(kAesIv)

And let the compiler infer the size below.

[padolph, 2013/12/23 19:58:33]

Done.

+  const unsigned char kAesIv[kAesKwIvLength] = {0xA6, 0xA6, 0xA6, 0xA6,
+                                                0xA6, 0xA6, 0xA6, 0xA6};
+  SECItem iv_item =
+      {siBuffer, const_cast<unsigned char*>(kAesIv), kAesKwIvLength};
+  SECItem* param_item = PK11_ParamFromIV(kAesKwMechanism, &iv_item);

[eroman, 2013/12/20 01:58:14]

I will have to verify this, but my expectation is that NSS will NOT copy the
parameters.

In which case returning this from a function is incorrect and will lead to a
user-after-free.

If that is the case, the lifetime of kAesIv / iv_item will need to outlive that
of param_item

[padolph, 2013/12/23 19:58:33]

This function was not saving too much code duplication anyway. Moved back to in
line.

+  DCHECK(param_item);
+  return crypto::ScopedSECItem(param_item);
+}
+
+// Performs RFC 3394 AES Key Wrap (encryption) of the input data.
+bool AesKwEncrypt(
+    const blink::WebCryptoKey& key,
+    const unsigned char* data,
+    unsigned data_size,
+    blink::WebArrayBuffer* buffer) {
+  DCHECK_EQ(blink::WebCryptoAlgorithmIdAesKw, key.algorithm().id());
+  DCHECK_EQ(blink::WebCryptoKeyTypeSecret, key.type());
+  DCHECK(blink::WebCryptoKeyUsageWrapKey & key.usages());
+
+  // The data size must be at least 16 bytes and a multiple of 8 bytes.
+  // TODO(padolph): Should this be a DCHECK() instead?

[eroman, 2013/12/20 01:58:14]

This must be a runtime check since blink layer does not enforce it.

[padolph, 2013/12/23 19:58:33]

Done.

+  if (data_size < 16 || data_size % 8)
+    return false;
+  DCHECK(data);
+
+  // Turn the data to be wrapped into a PK11SymKey in order to use the NSS
+  // PK11_WrapSymKey() API. Create the PK11SymKey by importing the data as a
+  // generic secret blob, since we can't be certain what it is at this point.
+  SECItem data_item = {siBuffer, const_cast<unsigned char*>(data), data_size};
+  crypto::ScopedPK11SymKey key_to_be_wrapped(
+      PK11_ImportSymKey(PK11_GetInternalSlot(),
+                        CKK_GENERIC_SECRET,
+                        PK11_OriginGenerated,
+                        CKA_ENCRYPT,
+                        &data_item,
+                        0));
+  if (!key_to_be_wrapped)
+    return false;
+
+  // AES Key Wrap always adds 8 bytes to the input data size.
+  const unsigned int output_length = data_size + 8;

[eroman, 2013/12/20 01:58:14]

Paranoia: possibility of integer overflow? I should hook up some safe integer
helpers for use in this file. I guess in this case the defined behavior is
wraparound and it will just cause the operation to fail from insufficient
buffer.

[padolph, 2013/12/23 19:58:33]

RFC 3394 does not specify the max allowed input length, so I added code to fail
if the size exceeeds whatever fits into an unsigned. This is reasonable since we
are only ever wrapping keys, which are generally small.

+
+  *buffer = blink::WebArrayBuffer::create(output_length, 1);
+  unsigned char* buffer_data = reinterpret_cast<unsigned char*>(buffer->data());
+  SECItem wrapped_key_item = {siBuffer, buffer_data, output_length};
+
+  SymKeyHandle* wrapping_key = reinterpret_cast<SymKeyHandle*>(key.handle());
+
+  if (SECSuccess != PK11_WrapSymKey(kAesKwMechanism,
+                                    CreateAesKwIvItem().get(),
+                                    wrapping_key->key(),
+                                    key_to_be_wrapped.get(),
+                                    &wrapped_key_item)) {
+    return false;
+  }
+  DCHECK_EQ(output_length, wrapped_key_item.len);

[eroman, 2013/12/20 01:58:14]

Please make this a runtime check and fail if they don't match. Want to be as
defensive as possible when calling into libraries.

[padolph, 2013/12/23 19:58:33]

Done.

+
+  return true;
+}
+
+// Performs RFC 3394 AES Key Unwrap (decryption) of the input data.
+bool AesKwDecrypt(
+    const blink::WebCryptoKey& key,
+    const unsigned char* data,
+    unsigned data_size,
+    blink::WebArrayBuffer* buffer) {
+  DCHECK_EQ(blink::WebCryptoAlgorithmIdAesKw, key.algorithm().id());
+  DCHECK_EQ(blink::WebCryptoKeyTypeSecret, key.type());
+  DCHECK(blink::WebCryptoKeyUsageUnwrapKey & key.usages());
+
+  // The ciphertext data size must be at least 24 bytes and a multiple of
+  // 8 bytes. This is a DCHECK() because this function is not callable with
+  // arbitrary data: it only receives data representing an assumed-good internal
+  // key to be wrapped.
+  DCHECK(data_size >= 24 && data_size % 8 == 0);
+  DCHECK(data);
+
+  SymKeyHandle* wrapping_key = reinterpret_cast<SymKeyHandle*>(key.handle());
+
+  SECItem cipher_text = {siBuffer, const_cast<unsigned char*>(data), data_size};
+
+  // The plaintext length is always 64 bits less than the data size.
+  const unsigned int plaintext_length = data_size - 8;

[eroman, 2013/12/20 01:58:14]

Isn't it possible to have 0-length keys? In fact we have HMAC unittests with
0-byte keys.

In that case, subtracting 8 would be bad.

What you have as a DCHECK above should be a runtime check instead.

[padolph, 2013/12/23 19:58:33]

Done.

+
+  crypto::ScopedPK11SymKey unwrapped_key(PK11_UnwrapSymKey(
+      wrapping_key->key(),
+      kAesKwMechanism,
+      CreateAesKwIvItem().get(),
+      &cipher_text,
+      CKK_GENERIC_SECRET,  // Import the key material without knowing its kind.
+      CKA_ENCRYPT,  // A safe value since all we're doing is exporting the key.
+      plaintext_length));
+  if (!unwrapped_key)
+    return false;
+
+  if (PK11_ExtractKeyValue(unwrapped_key.get()) != SECSuccess)
+    return false;
+
+  const SECItem* key_data = PK11_GetKeyData(unwrapped_key.get());
+  if (!key_data)
+    return false;
+  DCHECK_EQ(plaintext_length, key_data->len);
+
+  *buffer = webcrypto::CreateArrayBuffer(key_data->data, key_data->len);
+
+  return true;
+}
+
 CK_MECHANISM_TYPE HmacAlgorithmToGenMechanism(
     const blink::WebCryptoAlgorithm& algorithm) {
   DCHECK_EQ(algorithm.id(), blink::WebCryptoAlgorithmIdHmac);
@@ -525,45 +640,53 @@ bool WebCryptoImpl::EncryptInternal(
   DCHECK(key.handle());
   DCHECK(buffer);
 
-  if (algorithm.id() == blink::WebCryptoAlgorithmIdAesCbc) {
-    return AesCbcEncryptDecrypt(
-        CKA_ENCRYPT, algorithm, key, data, data_size, buffer);
-  } else if (algorithm.id() == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5) {
-
-    // RSAES encryption does not support empty input
-    if (!data_size)
-      return false;
-    DCHECK(data);
+  switch (algorithm.id()) {

[eroman, 2013/12/20 01:58:14]

switching to a switch statement would best be left to a separate refactor CL. It
makes the differences hard to read here.

[padolph, 2013/12/23 19:58:33]

Reverted and added a TODO.

+    case blink::WebCryptoAlgorithmIdAesCbc: {
+      return AesCbcEncryptDecrypt(
+          CKA_ENCRYPT, algorithm, key, data, data_size, buffer);
+    }
+    case blink::WebCryptoAlgorithmIdAesKw: {
+      return AesKwEncrypt(key, data, data_size, buffer);
+    }
+    case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5: {
+      // RSAES encryption does not support empty input
+      if (!data_size)
+        return false;
+      DCHECK(data);
 
-    if (key.type() != blink::WebCryptoKeyTypePublic)
-      return false;
+      if (key.type() != blink::WebCryptoKeyTypePublic)
+        return false;
 
-    PublicKeyHandle* const public_key =
-        reinterpret_cast<PublicKeyHandle*>(key.handle());
+      PublicKeyHandle* const public_key =
+          reinterpret_cast<PublicKeyHandle*>(key.handle());
 
-    const unsigned encrypted_length_bytes =
-        SECKEY_PublicKeyStrength(public_key->key());
+      const unsigned encrypted_length_bytes =
+          SECKEY_PublicKeyStrength(public_key->key());
 
-    // RSAES can operate on messages up to a length of k - 11, where k is the
-    // octet length of the RSA modulus.
-    if (encrypted_length_bytes < 11 || encrypted_length_bytes - 11 < data_size)
-      return false;
+      // RSAES can operate on messages up to a length of k - 11, where k is the
+      // octet length of the RSA modulus.
+      if (encrypted_length_bytes < 11 ||
+          encrypted_length_bytes - 11 < data_size) {
+        return false;
+      }
 
-    *buffer = blink::WebArrayBuffer::create(encrypted_length_bytes, 1);
-    unsigned char* const buffer_data =
-        reinterpret_cast<unsigned char*>(buffer->data());
+      *buffer = blink::WebArrayBuffer::create(encrypted_length_bytes, 1);
+      unsigned char* const buffer_data =
+          reinterpret_cast<unsigned char*>(buffer->data());
 
-    if (PK11_PubEncryptPKCS1(public_key->key(),
-                             buffer_data,
-                             const_cast<unsigned char*>(data),
-                             data_size,
-                             NULL) != SECSuccess) {
+      if (PK11_PubEncryptPKCS1(public_key->key(),
+                               buffer_data,
+                               const_cast<unsigned char*>(data),
+                               data_size,
+                               NULL) != SECSuccess) {
+        return false;
+      }
+      return true;
+    }
+    default: {
       return false;
     }
-    return true;
   }
-
-  return false;
 }
 
 bool WebCryptoImpl::DecryptInternal(
@@ -577,47 +700,53 @@ bool WebCryptoImpl::DecryptInternal(
   DCHECK(key.handle());
   DCHECK(buffer);
 
-  if (algorithm.id() == blink::WebCryptoAlgorithmIdAesCbc) {
-    return AesCbcEncryptDecrypt(
-        CKA_DECRYPT, algorithm, key, data, data_size, buffer);
-  } else if (algorithm.id() == blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5) {
-
-    // RSAES decryption does not support empty input
-    if (!data_size)
-      return false;
-    DCHECK(data);
+  switch (algorithm.id()) {

[eroman, 2013/12/20 01:58:14]

Same comment.

[padolph, 2013/12/23 19:58:33]

Reverted and added a TODO.

+    case blink::WebCryptoAlgorithmIdAesCbc: {
+      return AesCbcEncryptDecrypt(
+          CKA_DECRYPT, algorithm, key, data, data_size, buffer);
+    }
+    case blink::WebCryptoAlgorithmIdAesKw: {
+      return AesKwDecrypt(key, data, data_size, buffer);
+    }
+    case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5: {
+      // RSAES decryption does not support empty input
+      if (!data_size)
+        return false;
+      DCHECK(data);
 
-    if (key.type() != blink::WebCryptoKeyTypePrivate)
-      return false;
+      if (key.type() != blink::WebCryptoKeyTypePrivate)
+        return false;
 
-    PrivateKeyHandle* const private_key =
-        reinterpret_cast<PrivateKeyHandle*>(key.handle());
+      PrivateKeyHandle* const private_key =
+          reinterpret_cast<PrivateKeyHandle*>(key.handle());
 
-    const int modulus_length_bytes =
-        PK11_GetPrivateModulusLen(private_key->key());
-    if (modulus_length_bytes <= 0)
-      return false;
-    const unsigned max_output_length_bytes = modulus_length_bytes;
-
-    *buffer = blink::WebArrayBuffer::create(max_output_length_bytes, 1);
-    unsigned char* const buffer_data =
-        reinterpret_cast<unsigned char*>(buffer->data());
-
-    unsigned output_length_bytes = 0;
-    if (PK11_PrivDecryptPKCS1(private_key->key(),
-                              buffer_data,
-                              &output_length_bytes,
-                              max_output_length_bytes,
-                              const_cast<unsigned char*>(data),
-                              data_size) != SECSuccess) {
+      const int modulus_length_bytes =
+          PK11_GetPrivateModulusLen(private_key->key());
+      if (modulus_length_bytes <= 0)
+        return false;
+      const unsigned max_output_length_bytes = modulus_length_bytes;
+
+      *buffer = blink::WebArrayBuffer::create(max_output_length_bytes, 1);
+      unsigned char* const buffer_data =
+          reinterpret_cast<unsigned char*>(buffer->data());
+
+      unsigned output_length_bytes = 0;
+      if (PK11_PrivDecryptPKCS1(private_key->key(),
+                                buffer_data,
+                                &output_length_bytes,
+                                max_output_length_bytes,
+                                const_cast<unsigned char*>(data),
+                                data_size) != SECSuccess) {
+        return false;
+      }
+      DCHECK_LE(output_length_bytes, max_output_length_bytes);
+      webcrypto::ShrinkBuffer(buffer, output_length_bytes);
+      return true;
+    }
+    default: {
       return false;
     }
-    DCHECK_LE(output_length_bytes, max_output_length_bytes);
-    webcrypto::ShrinkBuffer(buffer, output_length_bytes);
-    return true;
   }
-
-  return false;
 }
 
 bool WebCryptoImpl::DigestInternal(