Reject JWK key import when key_ops contains duplicate values.

content/child/webcrypto/webcrypto_util.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/child/webcrypto/webcrypto_util.h"
 
+#include <set>
+
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "content/child/webcrypto/status.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithm.h"
 #include "third_party/WebKit/public/platform/WebCryptoAlgorithmParams.h"
 #include "third_party/WebKit/public/platform/WebCryptoKeyAlgorithm.h"
 
 namespace content {
 
 namespace webcrypto {
@@ skipping 37 matching lines @@
 const JwkToWebCryptoUsage kJwkWebCryptoUsageMap[] = {
     {"encrypt", blink::WebCryptoKeyUsageEncrypt},
     {"decrypt", blink::WebCryptoKeyUsageDecrypt},
     {"sign", blink::WebCryptoKeyUsageSign},
     {"verify", blink::WebCryptoKeyUsageVerify},
     {"deriveKey", blink::WebCryptoKeyUsageDeriveKey},
     {"deriveBits", blink::WebCryptoKeyUsageDeriveBits},
     {"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;
     }
   }
   return false;
 }
 
 // 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.
+  // Lazily initialized since it is unlikely to be needed.
+  scoped_ptr<std::set<std::string>> unrecognized_usages;

[Ryan Sleevi, 2014/10/27 22:19:36]

This seems to be premature optimization. Are you sure the heap allocation
overhead is less than the initialization cost?

[eroman, 2014/10/27 23:29:04]

Removed in favor of stack allocation.

Agreed. I did some performance tests and the impact was irrelevant.

+
   *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.get())
+      unrecognized_usages.reset(new std::set<std::string>);
+    if (!unrecognized_usages->insert(key_op).second)
+      return Status::ErrorJwkDuplicateKeyOps();
   }
   return Status::Success();
 }
 
 // Composes a JWK key_ops List from a Web Crypto usage mask.
 // Note: Caller must assume ownership of returned instance.
 base::ListValue* CreateJwkKeyOpsFromWebCryptoUsages(
     blink::WebCryptoKeyUsageMask usages) {
   base::ListValue* jwk_key_ops = new base::ListValue();
   for (size_t i = 0; i < arraysize(kJwkWebCryptoUsageMap); ++i) {
@@ skipping 136 matching lines @@
   // avoid feeding OpenSSL data that will hang use a whitelist.
   if (*public_exponent != 3 && *public_exponent != 65537)
     return Status::ErrorGenerateKeyPublicExponent();
 
   return Status::Success();
 }
 
 }  // namespace webcrypto
 
 }  // namespace content