[WebCrypto] fix ml-kem/ml-dsa CryptoKey serialization Add a new tag for key serialization (kNoParamsWithKeyTypeKeyTag) so that no code modification is needed for serialization of new algorithms with no key parameters. Use this to fix ML-KEM and ML-DSA key serialization/deserialization, and change ChaCha20-Poly1305 serialization to use the new tag as well. It would be nice to get rid of the Ed25519/X25519 specific CryptoKeySubTags, but removing those would break currently serialized Ed25519/X25519 tags. Bug: 512509718 Change-Id: If37e0a11af116c9c1bb56f4c21bf4cf78e37b0e5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7846315 Reviewed-by: David Benjamin <davidben@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Hubert Chao <hchao@chromium.org> Cr-Commit-Position: refs/heads/main@{#1631817} NOKEYCHECK=True GitOrigin-RevId: 23479ab6594302233dc8e96fc027a102c9cbaef3
diff --git a/blink/renderer/bindings/modules/v8/serialization/v8_script_value_deserializer_for_modules.cc b/blink/renderer/bindings/modules/v8/serialization/v8_script_value_deserializer_for_modules.cc index 365d31d..d213793 100644 --- a/blink/renderer/bindings/modules/v8/serialization/v8_script_value_deserializer_for_modules.cc +++ b/blink/renderer/bindings/modules/v8/serialization/v8_script_value_deserializer_for_modules.cc
@@ -218,15 +218,17 @@ return false; } -bool AsymmetricKeyTypeFromWireFormat(uint32_t raw_key_type, - WebCryptoKeyType* key_type) { - switch (static_cast<AsymmetricCryptoKeyType>(raw_key_type)) { +bool KeyTypeFromWireFormat(uint32_t raw_key_type, WebCryptoKeyType* key_type) { + switch (static_cast<CryptoKeyType>(raw_key_type)) { case kPublicKeyType: *key_type = kWebCryptoKeyTypePublic; return true; case kPrivateKeyType: *key_type = kWebCryptoKeyTypePrivate; return true; + case kSecretKeyType: + *key_type = kWebCryptoKeyTypeSecret; + return true; } return false; } @@ -339,7 +341,7 @@ WebCryptoAlgorithmId hash; if (!ReadUint32(&raw_id) || !AlgorithmIdFromWireFormat(raw_id, &id) || !ReadUint32(&raw_key_type) || - !AsymmetricKeyTypeFromWireFormat(raw_key_type, &key_type) || + !KeyTypeFromWireFormat(raw_key_type, &key_type) || !ReadUint32(&modulus_length_bits) || !ReadUint32(&public_exponent_size) || !ReadRawBytesToSpan(public_exponent_size, &public_exponent) || @@ -359,10 +361,11 @@ WebCryptoNamedCurve named_curve; if (!ReadUint32(&raw_id) || !AlgorithmIdFromWireFormat(raw_id, &id) || !ReadUint32(&raw_key_type) || - !AsymmetricKeyTypeFromWireFormat(raw_key_type, &key_type) || + !KeyTypeFromWireFormat(raw_key_type, &key_type) || !ReadUint32(&raw_named_curve) || - !NamedCurveFromWireFormat(raw_named_curve, &named_curve)) + !NamedCurveFromWireFormat(raw_named_curve, &named_curve)) { return nullptr; + } algorithm = WebCryptoKeyAlgorithm::CreateEc(id, named_curve); break; } @@ -373,8 +376,9 @@ uint32_t raw_key_type; if (!ReadUint32(&raw_id) || !AlgorithmIdFromWireFormat(raw_id, &id) || !ReadUint32(&raw_key_type) || - !AsymmetricKeyTypeFromWireFormat(raw_key_type, &key_type)) + !KeyTypeFromWireFormat(raw_key_type, &key_type)) { return nullptr; + } algorithm = raw_key_byte == kEd25519KeyTag ? WebCryptoKeyAlgorithm::CreateEd25519(id) : WebCryptoKeyAlgorithm::CreateX25519(id); @@ -388,6 +392,18 @@ algorithm = WebCryptoKeyAlgorithm::CreateWithoutParams(id); break; } + case kNoParamsWithKeyTypeKeyTag: { + uint32_t raw_id; + WebCryptoAlgorithmId id; + uint32_t raw_key_type; + if (!ReadUint32(&raw_id) || !AlgorithmIdFromWireFormat(raw_id, &id) || + !ReadUint32(&raw_key_type) || + !KeyTypeFromWireFormat(raw_key_type, &key_type)) { + return nullptr; + } + algorithm = WebCryptoKeyAlgorithm::CreateWithoutParams(id); + break; + } } if (algorithm.IsNull()) return nullptr;
diff --git a/blink/renderer/bindings/modules/v8/serialization/v8_script_value_serializer_for_modules.cc b/blink/renderer/bindings/modules/v8/serialization/v8_script_value_serializer_for_modules.cc index 38c1569..221a807 100644 --- a/blink/renderer/bindings/modules/v8/serialization/v8_script_value_serializer_for_modules.cc +++ b/blink/renderer/bindings/modules/v8/serialization/v8_script_value_serializer_for_modules.cc
@@ -442,16 +442,16 @@ NOTREACHED() << "Unknown algorithm ID " << id; } -uint32_t AsymmetricKeyTypeForWireFormat(WebCryptoKeyType key_type) { +uint32_t KeyTypeForWireFormat(WebCryptoKeyType key_type) { switch (key_type) { case kWebCryptoKeyTypePublic: return kPublicKeyType; case kWebCryptoKeyTypePrivate: return kPrivateKeyType; case kWebCryptoKeyTypeSecret: - break; + return kSecretKeyType; } - NOTREACHED() << "Unknown asymmetric key type " << key_type; + NOTREACHED() << "Unknown key type " << key_type; } uint32_t NamedCurveForWireFormat(WebCryptoNamedCurve named_curve) { @@ -535,7 +535,7 @@ const auto& params = *algorithm.RsaHashedParams(); WriteOneByte(kRsaHashedKeyTag); WriteUint32(AlgorithmIdForWireFormat(algorithm.Id())); - WriteUint32(AsymmetricKeyTypeForWireFormat(key.GetType())); + WriteUint32(KeyTypeForWireFormat(key.GetType())); WriteUint32(params.ModulusLengthBits()); if (params.PublicExponent().size() > @@ -555,11 +555,14 @@ const auto& params = *algorithm.EcParams(); WriteOneByte(kEcKeyTag); WriteUint32(AlgorithmIdForWireFormat(algorithm.Id())); - WriteUint32(AsymmetricKeyTypeForWireFormat(key.GetType())); + WriteUint32(KeyTypeForWireFormat(key.GetType())); WriteUint32(NamedCurveForWireFormat(params.NamedCurve())); break; } case kWebCryptoKeyAlgorithmParamsTypeNone: + // Ed25519, X25519, HKDF, and PBKDF2 are special-cased because they + // pre-dated the kNoParamsWithKeyTypeKeyTag. New algorithms that have no + // params for the key should use the default case. switch (algorithm.Id()) { case kWebCryptoAlgorithmIdEd25519: case kWebCryptoAlgorithmIdX25519: { @@ -568,17 +571,21 @@ : kX25519KeyTag; WriteOneByte(tag); WriteUint32(AlgorithmIdForWireFormat(algorithm.Id())); - WriteUint32(AsymmetricKeyTypeForWireFormat(key.GetType())); + WriteUint32(KeyTypeForWireFormat(key.GetType())); break; } - default: - // TODO(crbug.com/512509718): fix ML-KEM and ML-DSA serialization. - DCHECK(WebCryptoAlgorithm::IsKdf(algorithm.Id()) || - WebCryptoAlgorithm::IsMlDsa(algorithm.Id()) || - WebCryptoAlgorithm::IsMlKem(algorithm.Id()) || - algorithm.Id() == kWebCryptoAlgorithmIdChaCha20Poly1305); + case kWebCryptoAlgorithmIdHkdf: + case kWebCryptoAlgorithmIdPbkdf2: { WriteOneByte(kNoParamsKeyTag); WriteUint32(AlgorithmIdForWireFormat(algorithm.Id())); + break; + } + default: { + WriteOneByte(kNoParamsWithKeyTypeKeyTag); + WriteUint32(AlgorithmIdForWireFormat(algorithm.Id())); + WriteUint32(KeyTypeForWireFormat(key.GetType())); + break; + } } break; }
diff --git a/blink/renderer/bindings/modules/v8/serialization/web_crypto_sub_tags.h b/blink/renderer/bindings/modules/v8/serialization/web_crypto_sub_tags.h index 34d22cb..0b43905 100644 --- a/blink/renderer/bindings/modules/v8/serialization/web_crypto_sub_tags.h +++ b/blink/renderer/bindings/modules/v8/serialization/web_crypto_sub_tags.h
@@ -71,15 +71,19 @@ // ID 3 was used by RsaKeyTag, while still behind experimental flag. kRsaHashedKeyTag = 4, kEcKeyTag = 5, + // New algorithms with no params should use NoParamsWithKeyTypeKeyTag. kNoParamsKeyTag = 6, + // kEd25519KeyTag and kX25519KeyTag are separate for historical reasons. kEd25519KeyTag = 7, kX25519KeyTag = 8, + kNoParamsWithKeyTypeKeyTag = 9, // Maximum allowed value is 255 }; -enum AsymmetricCryptoKeyType : uint32_t { +enum CryptoKeyType : uint32_t { kPublicKeyType = 1, kPrivateKeyType = 2, + kSecretKeyType = 3, // Maximum allowed value is 2^32-1 };
diff --git a/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mldsa.tentative.https.window-expected.txt b/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mldsa.tentative.https.window-expected.txt index d5b5aa4..5b37deb 100644 --- a/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mldsa.tentative.https.window-expected.txt +++ b/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mldsa.tentative.https.window-expected.txt
@@ -1,15 +1,4 @@ This is a testharness.js-based test. -[FAIL] serialization test ML-DSA-44 - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test {name: ML-DSA-44} - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test ML-DSA-65 - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test {name: ML-DSA-65} - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test ML-DSA-87 - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test {name: ML-DSA-87} - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" +All subtests passed and are omitted for brevity. +See https://chromium.googlesource.com/chromium/src/+/HEAD/docs/testing/writing_web_tests.md#Text-Test-Baselines for details. Harness: the test ran to completion. -
diff --git a/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mlkem.tentative.https.window-expected.txt b/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mlkem.tentative.https.window-expected.txt index 4baa7a2..86515a7 100644 --- a/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mlkem.tentative.https.window-expected.txt +++ b/blink/web_tests/virtual/webcrypto-pqc/external/wpt/WebCryptoAPI/serialization/mlkem.tentative.https.window-expected.txt
@@ -3,13 +3,5 @@ promise_test: Unhandled rejection with value: object "NotSupportedError: Failed to execute 'generateKey' on 'SubtleCrypto': Algorithm: Unrecognized name" [FAIL] serialization test {name: ML-KEM-512} promise_test: Unhandled rejection with value: object "NotSupportedError: Failed to execute 'generateKey' on 'SubtleCrypto': Algorithm: Unrecognized name" -[FAIL] serialization test ML-KEM-768 - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test {name: ML-KEM-768} - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test ML-KEM-1024 - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" -[FAIL] serialization test {name: ML-KEM-1024} - promise_test: Unhandled rejection with value: object "TypeError: Cannot read properties of null (reading 'publicKey')" Harness: the test ran to completion.