[webcrypto] Add JWK symmetric key AES-KW unwrap for NSS.

content/child/webcrypto/shared_crypto.cc

Index: content/child/webcrypto/shared_crypto.cc
diff --git a/content/child/webcrypto/shared_crypto.cc b/content/child/webcrypto/shared_crypto.cc
index 1677270a86c2c94c2947fa6fafb11538a0c063d0..9fefcc25020cc22c3407e81a98c5b12a2f73a88e 100644
--- a/content/child/webcrypto/shared_crypto.cc
+++ b/content/child/webcrypto/shared_crypto.cc
@@ -21,8 +21,7 @@ namespace webcrypto {
 namespace {
 
 // TODO(eroman): Move this helper to WebCryptoKey.
-bool KeyUsageAllows(const blink::WebCryptoKey& key,
-                    const blink::WebCryptoKeyUsage usage) {
+bool KeyUsageAllows(const blink::WebCryptoKey& key, unsigned int usage) {

[eroman, 2014/03/14 22:14:53]

This is not what I expect as a caller to KeyUsageAllows().

Instead a create a new function called:

KeyUsageAllowsAnyOf(key, usages);

And also, make the usages parameter of type blink::WebCryptoKeyUsageMask rather
than unsigned int.

Otherwise I am afraid this could be used dangerously in the future.

[padolph, 2014/03/17 03:24:36]

Done.

   return ((key.usages() & usage) != 0);
 }
 
@@ -233,6 +232,100 @@ Status ImportKeyRaw(const CryptoData& key_data,
   }
 }
 
+// Validates the size of data input to AES-KW. AES-KW requires the input data
+// size to be at least 24 bytes and a multiple of 8 bytes.
+Status CheckAesKwInputSize(const CryptoData& aeskw_input_data) {
+  if (aeskw_input_data.byte_length() < 24)
+    return Status::ErrorDataTooSmall();
+  if (aeskw_input_data.byte_length() % 8)
+    return Status::ErrorInvalidAesKwDataLength();
+  return Status::Success();
+}
+
+Status UnwrapKeyRaw(const CryptoData& wrapped_key_data,
+                    const blink::WebCryptoKey& wrapping_key,
+                    const blink::WebCryptoAlgorithm& wrapping_algorithm,
+                    const blink::WebCryptoAlgorithm& algorithm_or_null,
+                    bool extractable,
+                    blink::WebCryptoKeyUsageMask usage_mask,
+                    blink::WebCryptoKey* key) {
+  // Must provide an algorithm when unwrapping a raw key
+  if (algorithm_or_null.isNull())
+    return Status::ErrorMissingAlgorithmUnwrapRawKey();
+
+  // TODO(padolph): Handle other wrapping algorithms
+  switch (wrapping_algorithm.id()) {
+    case blink::WebCryptoAlgorithmIdAesKw: {
+      platform::SymKey* platform_wrapping_key;
+      Status status = ToPlatformSymKey(wrapping_key, &platform_wrapping_key);
+      if (status.IsError())
+        return status;
+      status = CheckAesKwInputSize(wrapped_key_data);
+      if (status.IsError())
+        return status;
+      return platform::UnwrapSymKeyAesKw(wrapped_key_data,
+                                         platform_wrapping_key,
+                                         algorithm_or_null,
+                                         extractable,
+                                         usage_mask,
+                                         key);
+    }
+    case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5: {
+      platform::PrivateKey* platform_wrapping_key;
+      Status status =
+          ToPlatformPrivateKey(wrapping_key, &platform_wrapping_key);
+      if (status.IsError())
+        return status;
+      if (!wrapped_key_data.byte_length())
+        return Status::ErrorDataTooSmall();
+      return platform::UnwrapSymKeyRsaEs(wrapped_key_data,
+                                         platform_wrapping_key,
+                                         algorithm_or_null,
+                                         extractable,
+                                         usage_mask,
+                                         key);
+    }
+    default:
+      return Status::ErrorUnsupported();
+  }
+}
+
+Status UnwrapKeyDecryptAndImport(
+    blink::WebCryptoKeyFormat format,
+    const CryptoData& wrapped_key_data,
+    const blink::WebCryptoKey& wrapping_key,
+    const blink::WebCryptoAlgorithm& wrapping_algorithm,
+    const blink::WebCryptoAlgorithm& algorithm_or_null,
+    bool extractable,
+    blink::WebCryptoKeyUsageMask usage_mask,
+    blink::WebCryptoKey* key) {
+  blink::WebArrayBuffer buffer;
+  Status status =
+      Decrypt(wrapping_algorithm, wrapping_key, wrapped_key_data, &buffer);
+  if (status.IsError())
+    return status;
+  return ImportKey(format,

[eroman, 2014/03/14 22:14:53]

I don't think we should return the error from ImportKey().

Two options I suggest for moving forward on this changelist are:
 (1) Convince me that it is safe to return the error from ImportKey()
 (2) Replace any error from ImportKey() with a generic one like "Import failed
while unwrapping"

For expediency I would go with (2), and we can revisit the decision later if we
want to expose more errors.

My concern is that when unwrapping the key data, the encrypted contents should
be totally private from the web application. By returning the import error, we
may be telling the caller something about the wrapped data. Especially so with
the JWK format, where we tell the caller about unrecognized or parameters, so
they can learn something about encrypted JSON data.

I am not a crypto expert so maybe I am totally wrong it this isn't an issue...
But until I am convinced otherwise, please hide these errors.

I wasn't concerned about this issue for symkey unwrapping, since from what I
could tell the only error we return is the generic Status::Error(). However for
full blown ImportKey() we have a lot more errors.

Cheers, and thanks for thinking about this issue.

[padolph, 2014/03/17 03:24:36]

Thanks. I had seen your earlier comment in the code about this issue and meant
to account for it, but forgot in this CL.

I agree that we should not expose import failures in the second step of this
combined unwrap operation. Yes, it would allow information about the encrypted
JWK structure to leak.

Changed to report just Status::Error() for now. If we want more granularity in
the future, we can add a filter parameter to the ImportKey() function (perhaps
optional with a default) to enable switching between "chatty" and "secret" mode.

+                   CryptoData(buffer),
+                   algorithm_or_null,
+                   extractable,
+                   usage_mask,
+                   key);
+}
+
+Status DecryptAesKw(const blink::WebCryptoAlgorithm& algorithm,
+                    const blink::WebCryptoKey& key,
+                    const CryptoData& data,
+                    blink::WebArrayBuffer* buffer) {
+  platform::SymKey* sym_key;
+  Status status = ToPlatformSymKey(key, &sym_key);
+  if (status.IsError())
+    return status;
+  status = CheckAesKwInputSize(data);
+  if (status.IsError())
+    return status;
+  return platform::DecryptAesKw(sym_key, data, buffer);
+}
+
 }  // namespace
 
 void Init() { platform::Init(); }
@@ -262,18 +355,38 @@ Status Decrypt(const blink::WebCryptoAlgorithm& algorithm,
                const blink::WebCryptoKey& key,
                const CryptoData& data,
                blink::WebArrayBuffer* buffer) {
-  if (!KeyUsageAllows(key, blink::WebCryptoKeyUsageDecrypt))
-    return Status::ErrorUnexpected();
   if (algorithm.id() != key.algorithm().id())
     return Status::ErrorUnexpected();
 
   switch (algorithm.id()) {
     case blink::WebCryptoAlgorithmIdAesCbc:
+    case blink::WebCryptoAlgorithmIdAesGcm:
+      if (!KeyUsageAllows(key, blink::WebCryptoKeyUsageDecrypt))
+        return Status::ErrorUnexpected();
+      break;
+    case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5:
+      if (!KeyUsageAllows(key,
+                          blink::WebCryptoKeyUsageDecrypt |
+                              blink::WebCryptoKeyUsageUnwrapKey))
+        return Status::ErrorUnexpected();
+      break;
+    case blink::WebCryptoAlgorithmIdAesKw:
+      if (!KeyUsageAllows(key, blink::WebCryptoKeyUsageUnwrapKey))
+        return Status::ErrorUnexpected();
+      break;
+    default:
+      return Status::ErrorUnsupported();
+  }
+
+  switch (algorithm.id()) {
+    case blink::WebCryptoAlgorithmIdAesCbc:
       return EncryptDecryptAesCbc(DECRYPT, algorithm, key, data, buffer);
     case blink::WebCryptoAlgorithmIdAesGcm:
       return EncryptDecryptAesGcm(DECRYPT, algorithm, key, data, buffer);
     case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5:
       return DecryptRsaEsPkcs1v1_5(algorithm, key, data, buffer);
+    case blink::WebCryptoAlgorithmIdAesKw:
+      return DecryptAesKw(algorithm, key, data, buffer);
     default:
       return Status::ErrorUnsupported();
   }
@@ -430,8 +543,9 @@ Status ExportKey(blink::WebCryptoKeyFormat format,
         return status;
       return platform::ExportKeySpki(public_key, buffer);
     }
-    case blink::WebCryptoKeyFormatPkcs8:
     case blink::WebCryptoKeyFormatJwk:
+      return ExportKeyJwk(key, buffer);
+    case blink::WebCryptoKeyFormatPkcs8:
       // TODO(eroman):
       return Status::ErrorUnsupported();
     default:
@@ -545,50 +659,29 @@ Status UnwrapKey(blink::WebCryptoKeyFormat format,
   if (wrapping_algorithm.id() != wrapping_key.algorithm().id())
     return Status::ErrorUnexpected();
 
-  // TODO(padolph): Handle formats other than raw
-  if (format != blink::WebCryptoKeyFormatRaw)
-    return Status::ErrorUnsupported();
-
-  // Must provide an algorithm when unwrapping a raw key
-  if (format == blink::WebCryptoKeyFormatRaw && algorithm_or_null.isNull())
-    return Status::ErrorMissingAlgorithmUnwrapRawKey();
-
-  // TODO(padolph): Handle other wrapping algorithms
-  switch (wrapping_algorithm.id()) {
-    case blink::WebCryptoAlgorithmIdAesKw: {
-      platform::SymKey* platform_wrapping_key;
-      Status status = ToPlatformSymKey(wrapping_key, &platform_wrapping_key);
-      if (status.IsError())
-        return status;
-      // AES-KW requires the wrapped key data size must be at least 24 bytes and
-      // also a multiple of 8 bytes.
-      if (wrapped_key_data.byte_length() < 24)
-        return Status::ErrorDataTooSmall();
-      if (wrapped_key_data.byte_length() % 8)
-        return Status::ErrorInvalidAesKwDataLength();
-      return platform::UnwrapSymKeyAesKw(wrapped_key_data,
-                                         platform_wrapping_key,
-                                         algorithm_or_null,
-                                         extractable,
-                                         usage_mask,
-                                         key);
-    }
-    case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5: {
-      platform::PrivateKey* platform_wrapping_key;
-      Status status =
-          ToPlatformPrivateKey(wrapping_key, &platform_wrapping_key);
-      if (status.IsError())
-        return status;
-      if (!wrapped_key_data.byte_length())
-        return Status::ErrorDataTooSmall();
-      return platform::UnwrapSymKeyRsaEs(wrapped_key_data,
-                                         platform_wrapping_key,
-                                         algorithm_or_null,
-                                         extractable,
-                                         usage_mask,
-                                         key);
-    }
+  switch (format) {
+    case blink::WebCryptoKeyFormatRaw:
+      return UnwrapKeyRaw(wrapped_key_data,
+                          wrapping_key,
+                          wrapping_algorithm,
+                          algorithm_or_null,
+                          extractable,
+                          usage_mask,
+                          key);
+    case blink::WebCryptoKeyFormatJwk:
+      return UnwrapKeyDecryptAndImport(format,
+                                       wrapped_key_data,
+                                       wrapping_key,
+                                       wrapping_algorithm,
+                                       algorithm_or_null,
+                                       extractable,
+                                       usage_mask,
+                                       key);
+    case blink::WebCryptoKeyFormatSpki:
+    case blink::WebCryptoKeyFormatPkcs8:
+      return Status::ErrorUnsupported();  // TODO(padolph)
     default:
+      NOTREACHED();
       return Status::ErrorUnsupported();
   }
 }