[WebCrypto] Throw syntaxError if keyUsage is empty in ImportKey

content/child/webcrypto/webcrypto_util.cc

Index: content/child/webcrypto/webcrypto_util.cc
diff --git a/content/child/webcrypto/webcrypto_util.cc b/content/child/webcrypto/webcrypto_util.cc
index 5df16330b7cec68a5b53dc2a7f6f72f277e14145..0a4b03c2632c777d22c5531680165a39c883d1da 100644
--- a/content/child/webcrypto/webcrypto_util.cc
+++ b/content/child/webcrypto/webcrypto_util.cc
@@ -271,7 +271,11 @@ Status VerifyAesKeyLengthForImport(unsigned int keylen_bytes) {
 }
 
 Status CheckKeyCreationUsages(blink::WebCryptoKeyUsageMask all_possible_usages,
-                              blink::WebCryptoKeyUsageMask actual_usages) {
+                              blink::WebCryptoKeyUsageMask actual_usages,
+                              bool allow_empty_usages) {
+  if (allow_empty_usages && actual_usages == 0)

[eroman, 2014/12/16 01:26:45]

This naming is reversed. You are passing true for places which do NOT allow
empty usages

[Habib Virji, 2014/12/16 09:59:42]

Corrected now, it has now allow_empty_usages as true for scenarios it should not
check and false when it should check. I was trying to do reverse but naming was
not appropriate. Hopefully this is corrected now. 

+    return Status::ErrorKeyEmptyUsages();
+
   if (!ContainsKeyUsages(all_possible_usages, actual_usages))
     return Status::ErrorCreateKeyBadUsages();
   return Status::Success();
@@ -311,15 +315,17 @@ Status VerifyUsagesBeforeImportAsymmetricKey(
     blink::WebCryptoKeyUsageMask usages) {
   switch (format) {
     case blink::WebCryptoKeyFormatSpki:
-      return CheckKeyCreationUsages(all_public_key_usages, usages);
+      return CheckKeyCreationUsages(all_public_key_usages, usages, false);
     case blink::WebCryptoKeyFormatPkcs8:
-      return CheckKeyCreationUsages(all_private_key_usages, usages);
+      return CheckKeyCreationUsages(all_private_key_usages, usages, true);
     case blink::WebCryptoKeyFormatJwk: {
       // The JWK could represent either a public key or private key. The usages
       // must make sense for one of the two. The usages will be checked again by
       // ImportKeyJwk() once the key type has been determined.
-      if (CheckKeyCreationUsages(all_public_key_usages, usages).IsError() &&
-          CheckKeyCreationUsages(all_private_key_usages, usages).IsError()) {
+      if (CheckKeyCreationUsages(
+              all_public_key_usages, usages, false).IsError() &&
+          CheckKeyCreationUsages(
+              all_private_key_usages, usages, true).IsError()) {
         return Status::ErrorCreateKeyBadUsages();
       }
       return Status::Success();
@@ -387,17 +393,16 @@ Status GetUsagesForGenerateAsymmetricKey(
     blink::WebCryptoKeyUsageMask all_private_usages,
     blink::WebCryptoKeyUsageMask* public_usages,
     blink::WebCryptoKeyUsageMask* private_usages) {
+  bool allow_empty_usage =

[eroman, 2014/12/16 01:26:45]

I don't understand this, doesn't seem right to me.

It is also confusing because allow_empty_usage is in fact reversed inside of
CheckKeyCreationusages() per my other comment.

[Habib Virji, 2014/12/16 09:59:42]

Yes, i was passing true for scenario where it should check. But naming in the
function was making it mean different.

+      (combined_usages & all_private_usages) ? true : false;
   Status status = CheckKeyCreationUsages(all_public_usages | all_private_usages,
-                                         combined_usages);
+                                         combined_usages, allow_empty_usage);
   if (status.IsError())
     return status;
 
   *public_usages = combined_usages & all_public_usages;
   *private_usages = combined_usages & all_private_usages;
 
-  if (*private_usages == 0)
-    return Status::ErrorCreateKeyEmptyUsages();

[eroman, 2014/12/16 01:26:45]

The older mechanism was clearer, and correct. Just call the combined
CheckKeyCreationUsages() above with a static value (false, although depends if
you change it to actually meaning allow_empty_usage)

And then keep this line.

[Habib Virji, 2014/12/16 09:59:42]

I have updated above, please suggest if it is okay.

-
   return Status::Success();
 }