bindings: Replace a Dictionary in SubtleCrypto.importKey()

Drops a use case of Dictionary in IDL.
Following the spec, this CL replaces Dictionary type with an IDL
dictionary JsonWebKey used in importKey().


Bug: 385376, 839389, 1047081
Change-Id: I531f3088301d330cb15dcba5de9fad2cd6f997f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065193Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743409}

third_party/blink/renderer/bindings/idl_in_modules.gni

@@ -142,6 +142,8 @@ static_idl_files_in_modules = get_path_info(
           "//third_party/blink/renderer/modules/credentialmanager/public_key_credential_user_entity.idl",
           "//third_party/blink/renderer/modules/crypto/crypto.idl",
           "//third_party/blink/renderer/modules/crypto/crypto_key.idl",
+          "//third_party/blink/renderer/modules/crypto/json_web_key.idl",
+          "//third_party/blink/renderer/modules/crypto/rsa_other_primes_info.idl",
           "//third_party/blink/renderer/modules/crypto/subtle_crypto.idl",
           "//third_party/blink/renderer/modules/crypto/window_crypto.idl",
           "//third_party/blink/renderer/modules/crypto/worker_global_scope_crypto.idl",

third_party/blink/renderer/bindings/modules/v8/generated.gni

@@ -19,8 +19,8 @@ bindings_modules_generated_union_type_files = [
   "$bindings_modules_v8_output_dir/animation_effect_or_animation_effect_sequence.h",
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_blob_or_usv_string_or_write_params.cc",
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_blob_or_usv_string_or_write_params.h",
-  "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_dictionary.cc",
-  "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_dictionary.h",
+  "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_json_web_key.cc",
+  "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_json_web_key.h",
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_string.cc",
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_string.h",
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_usv_string.cc",

third_party/blink/renderer/bindings/scripts/web_idl/union.py

@@ -74,7 +74,7 @@ class Union(WithIdentifier, WithCodeGeneratorInfo, WithComponent,
 
         def collect_primary_component(idl_type):
             type_definition_object = idl_type.type_definition_object
-            if type_definition_object:
+            if type_definition_object and type_definition_object.components:
                 components.add(type_definition_object.components[0])
 
         for idl_type in flattened_members:

third_party/blink/renderer/modules/crypto/idls.gni

@@ -8,6 +8,11 @@ modules_idl_files = [
   "subtle_crypto.idl",
 ]
 
+modules_dictionary_idl_files = [
+  "json_web_key.idl",
+  "rsa_other_primes_info.idl",
+]
+
 modules_dependency_idl_files = [
   "window_crypto.idl",
   "worker_global_scope_crypto.idl",

third_party/blink/renderer/modules/crypto/json_web_key.idl

+// Copyright 2020 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.
+
+// https://w3c.github.io/webcrypto/#JsonWebKey-dictionary
+
+dictionary JsonWebKey {
+  DOMString kty;
+  DOMString use;
+  sequence<DOMString> key_ops;
+  DOMString alg;
+
+  boolean ext;
+
+  DOMString crv;
+  DOMString x;
+  DOMString y;
+  DOMString d;
+  DOMString n;
+  DOMString e;
+  DOMString p;
+  DOMString q;
+  DOMString dp;
+  DOMString dq;
+  DOMString qi;
+  sequence<RsaOtherPrimesInfo> oth;
+  DOMString k;
+};

third_party/blink/renderer/modules/crypto/rsa_other_primes_info.idl

+// Copyright 2020 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.
+
+// https://w3c.github.io/webcrypto/#dfn-RsaOtherPrimesInfo
+
+dictionary RsaOtherPrimesInfo {
+  DOMString r;
+  DOMString d;
+  DOMString t;
+};

third_party/blink/renderer/modules/crypto/subtle_crypto.cc

@@ -37,6 +37,7 @@
 #include "third_party/blink/public/platform/web_crypto.h"
 #include "third_party/blink/public/platform/web_crypto_algorithm.h"
 #include "third_party/blink/renderer/bindings/core/v8/dictionary.h"
+#include "third_party/blink/renderer/bindings/modules/v8/array_buffer_or_array_buffer_view_or_json_web_key.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/frame/deprecation.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
@@ -62,29 +63,6 @@ static bool ParseAlgorithm(const AlgorithmIdentifier& raw,
   return success;
 }
 
-static bool CopyStringProperty(const char* property,
-                               const Dictionary& source,
-                               JSONObject* destination) {
-  String value;
-  if (!DictionaryHelper::Get(source, property, value))
-    return false;
-  destination->SetString(property, value);
-  return true;
-}
-
-static bool CopySequenceOfStringProperty(const char* property,
-                                         const Dictionary& source,
-                                         JSONObject* destination) {
-  Vector<String> value;
-  if (!DictionaryHelper::Get(source, property, value))
-    return false;
-  auto json_array = std::make_unique<JSONArray>();
-  for (unsigned i = 0; i < value.size(); ++i)
-    json_array->PushString(value[i]);
-  destination->SetArray(property, std::move(json_array));
-  return true;
-}
-
 // Parses a JsonWebKey dictionary. On success writes the result to
 // |jsonUtf8| as a UTF8-encoded JSON octet string and returns true.
 // On failure sets an error on |result| and returns false.
@@ -92,66 +70,51 @@ static bool CopySequenceOfStringProperty(const char* property,
 // Note: The choice of output as an octet string is to facilitate interop
 // with the non-JWK formats, but does mean there is a second parsing step.
 // This design choice should be revisited after crbug.com/614385).
-//
-// Defined by the WebCrypto spec as:
-//
-//    dictionary JsonWebKey {
-//      DOMString kty;
-//      DOMString use;
-//      sequence<DOMString> key_ops;
-//      DOMString alg;
-//
-//      boolean ext;
-//
-//      DOMString crv;
-//      DOMString x;
-//      DOMString y;
-//      DOMString d;
-//      DOMString n;
-//      DOMString e;
-//      DOMString p;
-//      DOMString q;
-//      DOMString dp;
-//      DOMString dq;
-//      DOMString qi;
-//      sequence<RsaOtherPrimesInfo> oth;
-//      DOMString k;
-//    };
-//
-//    dictionary RsaOtherPrimesInfo {
-//      DOMString r;
-//      DOMString d;
-//      DOMString t;
-//    };
-static bool ParseJsonWebKey(const Dictionary& dict,
+static bool ParseJsonWebKey(const JsonWebKey& key,
                             WebVector<uint8_t>& json_utf8,
                             CryptoResult* result) {
-  // TODO(eroman): This implementation is incomplete and not spec compliant:
-  //  * Properties need to be read in the definition order above
-  //  * Preserve the type of optional parameters (crbug.com/385376)
-  //  * Parse "oth" (crbug.com/441396)
-  //  * Fail with TypeError (not DataError) if the input does not conform
-  //    to a JsonWebKey
   auto json_object = std::make_unique<JSONObject>();
 
-  if (!CopyStringProperty("kty", dict, json_object.get())) {
-    result->CompleteWithError(kWebCryptoErrorTypeData,
-                              "The required JWK member \"kty\" was missing");
-    return false;
+  if (key.hasKty())
+    json_object->SetString("kty", key.kty());
+  if (key.hasUse())
+    json_object->SetString("use", key.use());
+  if (key.hasKeyOps()) {
+    auto json_array = std::make_unique<JSONArray>();
+    for (auto&& value : key.keyOps())
+      json_array->PushString(value);
+    json_object->SetArray("key_ops", std::move(json_array));
   }
-
-  CopyStringProperty("use", dict, json_object.get());
-  CopySequenceOfStringProperty("key_ops", dict, json_object.get());
-  CopyStringProperty("alg", dict, json_object.get());
-
-  bool ext;
-  if (DictionaryHelper::Get(dict, "ext", ext))
-    json_object->SetBoolean("ext", ext);
-
-  const char* const kPropertyNames[] = {"d",  "n",  "e", "p",   "q", "dp",
-                                        "dq", "qi", "k", "crv", "x", "y"};
-  for (unsigned i = 0; i < base::size(kPropertyNames); ++i)
-    CopyStringProperty(kPropertyNames[i], dict, json_object.get());
+  if (key.hasAlg())
+    json_object->SetString("alg", key.alg());
+  if (key.hasExt())
+    json_object->SetBoolean("ext", key.ext());
+
+  if (key.hasCrv())
+    json_object->SetString("crv", key.crv());
+  if (key.hasX())
+    json_object->SetString("x", key.x());
+  if (key.hasY())
+    json_object->SetString("y", key.y());
+  if (key.hasD())
+    json_object->SetString("d", key.d());
+  if (key.hasN())
+    json_object->SetString("n", key.n());
+  if (key.hasE())
+    json_object->SetString("e", key.e());
+  if (key.hasP())
+    json_object->SetString("p", key.p());
+  if (key.hasQ())
+    json_object->SetString("q", key.q());
+  if (key.hasDp())
+    json_object->SetString("dp", key.dp());
+  if (key.hasDq())
+    json_object->SetString("dq", key.dq());
+  if (key.hasQi())
+    json_object->SetString("qi", key.qi());
+  // TODO(eroman): Parse "oth" (crbug.com/441396)
+  if (key.hasK())
+    json_object->SetString("k", key.k());
 
   String json = json_object->ToJSONString();
   json_utf8 = WebVector<uint8_t>(json.Utf8().c_str(), json.Utf8().length());
@@ -406,7 +369,7 @@ ScriptPromise SubtleCrypto::generateKey(
 ScriptPromise SubtleCrypto::importKey(
     ScriptState* script_state,
     const String& raw_format,
-    const ArrayBufferOrArrayBufferViewOrDictionary& raw_key_data,
+    const ArrayBufferOrArrayBufferViewOrJsonWebKey& raw_key_data,
     const AlgorithmIdentifier& raw_algorithm,
     bool extractable,
     const Vector<String>& raw_key_usages) {
@@ -458,11 +421,8 @@ ScriptPromise SubtleCrypto::importKey(
     //  (2) Let keyData be the keyData parameter passed to the importKey
     //      method.
     case kWebCryptoKeyFormatJwk:
-      if (raw_key_data.IsDictionary()) {
-        // TODO(eroman): To match the spec error order, parsing of the
-        // JsonWebKey should be done earlier (at the WebIDL layer of
-        // parameter checking), regardless of the format being "jwk".
-        if (!ParseJsonWebKey(raw_key_data.GetAsDictionary(), key_data, result))
+      if (raw_key_data.IsJsonWebKey()) {
+        if (!ParseJsonWebKey(*raw_key_data.GetAsJsonWebKey(), key_data, result))
           return promise;
       } else {
         result->CompleteWithError(kWebCryptoErrorTypeType,

third_party/blink/renderer/modules/crypto/subtle_crypto.h

@@ -33,7 +33,6 @@
 
 #include "third_party/blink/renderer/bindings/core/v8/array_buffer_or_array_buffer_view.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
-#include "third_party/blink/renderer/bindings/modules/v8/array_buffer_or_array_buffer_view_or_dictionary.h"
 #include "third_party/blink/renderer/bindings/modules/v8/dictionary_or_string.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
@@ -41,10 +40,11 @@
 
 namespace blink {
 
+class ArrayBufferOrArrayBufferViewOrJsonWebKey;
 class CryptoKey;
 
-typedef ArrayBufferOrArrayBufferView BufferSource;
-typedef DictionaryOrString AlgorithmIdentifier;
+using BufferSource = ArrayBufferOrArrayBufferView;
+using AlgorithmIdentifier = DictionaryOrString;
 
 class SubtleCrypto final : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
@@ -81,7 +81,7 @@ class SubtleCrypto final : public ScriptWrappable {
                             const Vector<String>& key_usages);
   ScriptPromise importKey(ScriptState*,
                           const String&,
-                          const ArrayBufferOrArrayBufferViewOrDictionary&,
+                          const ArrayBufferOrArrayBufferViewOrJsonWebKey&,
                           const AlgorithmIdentifier&,
                           bool extractable,
                           const Vector<String>& key_usages);

third_party/blink/renderer/modules/crypto/subtle_crypto.idl

@@ -48,7 +48,7 @@ typedef (Dictionary or DOMString) AlgorithmIdentifier;
     [CallWith=ScriptState, MeasureAs=SubtleCryptoDeriveKey] Promise<any> deriveKey(AlgorithmIdentifier algorithm, CryptoKey baseKey, AlgorithmIdentifier derivedKeyType, boolean extractable, sequence<KeyUsage> keyUsages);
     [CallWith=ScriptState, MeasureAs=SubtleCryptoDeriveBits] Promise<ArrayBuffer> deriveBits(AlgorithmIdentifier algorithm, CryptoKey baseKey, unsigned long length);
 
-    [CallWith=ScriptState, MeasureAs=SubtleCryptoImportKey] Promise<CryptoKey> importKey(KeyFormat format, (ArrayBuffer or ArrayBufferView or Dictionary) keyData, AlgorithmIdentifier algorithm, boolean extractable, sequence<KeyUsage> keyUsages);
+    [CallWith=ScriptState, MeasureAs=SubtleCryptoImportKey] Promise<CryptoKey> importKey(KeyFormat format, (BufferSource or JsonWebKey) keyData, AlgorithmIdentifier algorithm, boolean extractable, sequence<KeyUsage> keyUsages);
     [CallWith=ScriptState, MeasureAs=SubtleCryptoExportKey] Promise<any> exportKey(KeyFormat format, CryptoKey key);
 
     [CallWith=ScriptState, MeasureAs=SubtleCryptoWrapKey] Promise<any> wrapKey(KeyFormat format, CryptoKey key, CryptoKey wrappingKey, AlgorithmIdentifier wrapAlgorithm);