Reject JWK key import when key_ops contains duplicate values.

BUG=378074

Review URL: https://codereview.chromium.org/668313002

Cr-Commit-Position: refs/heads/master@{#301536}

content/child/webcrypto/status.cc

@@ -116,6 +116,12 @@ Status Status::ErrorJwkBigIntegerHasLeadingZero(const std::string& property) {
       "The JWK \"" + property + "\" property contained a leading zero.");
 }
 
+Status Status::ErrorJwkDuplicateKeyOps() {
+  return Status(blink::WebCryptoErrorTypeData,
+                "The \"key_ops\" property of the JWK dictionary contains "
+                "duplicate usages.");
+}
+
 Status Status::ErrorImportEmptyKeyData() {
   return Status(blink::WebCryptoErrorTypeData, "No key data was provided");
 }

content/child/webcrypto/status.h

@@ -111,6 +111,9 @@ class CONTENT_EXPORT Status {
   // violates the JWA requirement that such octet strings be minimal.
   static Status ErrorJwkBigIntegerHasLeadingZero(const std::string& property);
 
+  // The key_ops lists a usage more than once.
+  static Status ErrorJwkDuplicateKeyOps();
+
   // ------------------------------------
   // Other errors
   // ------------------------------------

content/child/webcrypto/test/aes_cbc_unittest.cc

@@ -402,6 +402,48 @@ TEST(WebCryptoAesCbcTest, ImportKeyJwkKeyOpsEncryptDecrypt) {
             key.usages());
 }
 
+// Tests that importing a JWK containing duplicate key_ops values fails.
+TEST(WebCryptoAesCbcTest, ImportKeyJwkDuplicateKeyOps) {
+  blink::WebCryptoKey key;
+  base::DictionaryValue dict;
+  dict.SetString("kty", "oct");
+  dict.SetString("k", "GADWrMRHwQfoNaXU5fZvTg==");
+  // key_ops will be owned by |dict|.
+  base::ListValue* key_ops = new base::ListValue;
+  dict.Set("key_ops", key_ops);
+
+  // The "encrypt" operation appears twice.
+  key_ops->AppendString("encrypt");
+  key_ops->AppendString("decrypt");
+  key_ops->AppendString("encrypt");
+
+  EXPECT_EQ(Status::ErrorJwkDuplicateKeyOps(),
+            ImportKeyJwkFromDict(
+                dict, CreateAlgorithm(blink::WebCryptoAlgorithmIdAesCbc), false,
+                0, &key));
+}
+
+// Tests that importing a JWK containing duplicate key_ops values fails.
+TEST(WebCryptoAesCbcTest, ImportKeyJwkDuplicateUnrecognizedKeyOps) {
+  blink::WebCryptoKey key;
+  base::DictionaryValue dict;
+  dict.SetString("kty", "oct");
+  dict.SetString("k", "GADWrMRHwQfoNaXU5fZvTg==");
+  // key_ops will be owned by |dict|.
+  base::ListValue* key_ops = new base::ListValue;
+  dict.Set("key_ops", key_ops);
+
+  // The (unknown) "foopy" operation appears twice.
+  key_ops->AppendString("foopy");
+  key_ops->AppendString("decrypt");
+  key_ops->AppendString("foopy");
+
+  EXPECT_EQ(Status::ErrorJwkDuplicateKeyOps(),
+            ImportKeyJwkFromDict(
+                dict, CreateAlgorithm(blink::WebCryptoAlgorithmIdAesCbc), false,
+                0, &key));
+}
+
 // Test failure if input usage is NOT a strict subset of the JWK usage.
 TEST(WebCryptoAesCbcTest, ImportKeyJwkKeyOpsNotSuperset) {
   blink::WebCryptoKey key;

content/child/webcrypto/webcrypto_util.cc

@@ -4,6 +4,8 @@
 
 #include "content/child/webcrypto/webcrypto_util.h"
 
+#include <set>
+
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "content/child/webcrypto/status.h"
@@ -61,12 +63,11 @@ const JwkToWebCryptoUsage kJwkWebCryptoUsageMap[] = {
     {"wrapKey", blink::WebCryptoKeyUsageWrapKey},
     {"unwrapKey", blink::WebCryptoKeyUsageUnwrapKey}};
 
-// Modifies the input usages by according to the key_op value.
 bool JwkKeyOpToWebCryptoUsage(const std::string& key_op,
-                              blink::WebCryptoKeyUsageMask* usages) {
+                              blink::WebCryptoKeyUsage* usage) {
   for (size_t i = 0; i < arraysize(kJwkWebCryptoUsageMap); ++i) {
     if (kJwkWebCryptoUsageMap[i].jwk_key_op == key_op) {
-      *usages |= kJwkWebCryptoUsageMap[i].webcrypto_usage;
+      *usage = kJwkWebCryptoUsageMap[i].webcrypto_usage;
       return true;
     }
   }
@@ -74,17 +75,32 @@ bool JwkKeyOpToWebCryptoUsage(const std::string& key_op,
 }
 
 // Composes a Web Crypto usage mask from an array of JWK key_ops values.
-Status GetWebCryptoUsagesFromJwkKeyOps(const base::ListValue* jwk_key_ops_value,
+Status GetWebCryptoUsagesFromJwkKeyOps(const base::ListValue* key_ops,
                                        blink::WebCryptoKeyUsageMask* usages) {
+  // This set keeps track of all unrecognized key_ops values.
+  std::set<std::string> unrecognized_usages;
+
   *usages = 0;
-  for (size_t i = 0; i < jwk_key_ops_value->GetSize(); ++i) {
+  for (size_t i = 0; i < key_ops->GetSize(); ++i) {
     std::string key_op;
-    if (!jwk_key_ops_value->GetString(i, &key_op)) {
+    if (!key_ops->GetString(i, &key_op)) {
       return Status::ErrorJwkPropertyWrongType(
           base::StringPrintf("key_ops[%d]", static_cast<int>(i)), "string");
     }
-    // Unrecognized key_ops are silently skipped.
-    ignore_result(JwkKeyOpToWebCryptoUsage(key_op, usages));
+
+    blink::WebCryptoKeyUsage usage;
+    if (JwkKeyOpToWebCryptoUsage(key_op, &usage)) {
+      // Ensure there are no duplicate usages.
+      if (*usages & usage)
+        return Status::ErrorJwkDuplicateKeyOps();
+      *usages |= usage;
+    }
+
+    // Reaching here means the usage was unrecognized. Such usages are skipped
+    // over, however they are kept track of in a set to ensure there were no
+    // duplicates.
+    if (!unrecognized_usages.insert(key_op).second)
+      return Status::ErrorJwkDuplicateKeyOps();
   }
   return Status::Success();
 }

content/child/webcrypto/webcrypto_util.h

@@ -19,10 +19,10 @@ namespace webcrypto {
 
 class Status;
 
-// Composes a Web Crypto usage mask from an array of JWK key_ops values.
-CONTENT_EXPORT Status GetWebCryptoUsagesFromJwkKeyOps(
-    const base::ListValue* jwk_key_ops_value,
-    blink::WebCryptoKeyUsageMask* jwk_key_ops_mask);
+// Converts a JWK "key_ops" array to the corresponding WebCrypto usages.
+CONTENT_EXPORT Status
+    GetWebCryptoUsagesFromJwkKeyOps(const base::ListValue* key_ops,
+                                    blink::WebCryptoKeyUsageMask* usages);
 
 // Composes a JWK key_ops array from a Web Crypto usage mask.
 base::ListValue* CreateJwkKeyOpsFromWebCryptoUsages(