From ae4eaa88395029d852c15f85a822ecb00573a315 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Fern=C3=A1ndez=20Garc=C3=ADa-Boente?= Date: Tue, 28 Nov 2023 21:03:45 +0100 Subject: [PATCH] Test the behavior of deriveBits with extreme length values The PR#345 to the WebCrypto API spec resolves an inconsistency on the 'length' parameter in the deriveBits operations of several algorithms, such as ECDH, HKDF, PBKDF2 or X25519. These cange adds a few tests to evaluate the behavior of these algorithms with extreme values, like 0, null or undefined. --- .../derive_bits_keys/cfrg_curves_bits.js | 19 ---------- .../derived_bits_length.https.any.js | 11 ++++++ .../derive_bits_keys/derived_bits_length.js | 36 +++++++++++++++++++ .../derived_bits_length_testcases.js | 30 ++++++++++++++++ .../derived_bits_length_vectors.js | 33 +++++++++++++++++ WebCryptoAPI/derive_bits_keys/ecdh_bits.js | 19 ---------- WebCryptoAPI/derive_bits_keys/hkdf.js | 19 ---------- WebCryptoAPI/derive_bits_keys/pbkdf2.js | 20 ----------- 8 files changed, 110 insertions(+), 77 deletions(-) create mode 100644 WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js create mode 100644 WebCryptoAPI/derive_bits_keys/derived_bits_length.js create mode 100644 WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js create mode 100644 WebCryptoAPI/derive_bits_keys/derived_bits_length_vectors.js diff --git a/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits.js b/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits.js index 422a230bd166754..da809278a87aa51 100644 --- a/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits.js +++ b/WebCryptoAPI/derive_bits_keys/cfrg_curves_bits.js @@ -57,25 +57,6 @@ function define_tests() { }); }, algorithmName + " mixed case parameters"); - // Null length - // "Null" is not valid per the current spec - // - https://github.com/w3c/webcrypto/issues/322 - // - https://github.com/w3c/webcrypto/issues/329 - // - // Proposal for a spec change: - // - https://github.com/w3c/webcrypto/pull/345 - // - // This test case may be replaced by these new tests: - // - https://github.com/web-platform-tests/wpt/pull/43400 - promise_test(function(test) { - return subtle.deriveBits({name: algorithmName, public: publicKeys[algorithmName]}, privateKeys[algorithmName], null) - .then(function(derivation) { - assert_true(equalBuffers(derivation, derivations[algorithmName]), "Derived correct bits"); - }, function(err) { - assert_unreached("deriveBits failed with error " + err.name + ": " + err.message); - }); - }, algorithmName + " with null length"); - // Shorter than entire derivation per algorithm promise_test(function(test) { return subtle.deriveBits({name: algorithmName, public: publicKeys[algorithmName]}, privateKeys[algorithmName], 8 * sizes[algorithmName] - 32) diff --git a/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js b/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js new file mode 100644 index 000000000000000..0aee2e3c172d30a --- /dev/null +++ b/WebCryptoAPI/derive_bits_keys/derived_bits_length.https.any.js @@ -0,0 +1,11 @@ +// META: title=WebCryptoAPI: deriveBits() tests for the 'length' parameter +// META: script=derived_bits_length.js +// META: script=derived_bits_length_vectors.js +// META: script=derived_bits_length_testcases.js + +// Define subtests from a `promise_test` to ensure the harness does not +// complete before the subtests are available. `explicit_done` cannot be used +// for this purpose because the global `done` function is automatically invoked +// by the WPT infrastructure in dedicated worker tests defined using the +// "multi-global" pattern. +promise_test(define_tests, 'setup - define tests'); diff --git a/WebCryptoAPI/derive_bits_keys/derived_bits_length.js b/WebCryptoAPI/derive_bits_keys/derived_bits_length.js new file mode 100644 index 000000000000000..5a7ed7eb50a0a0b --- /dev/null +++ b/WebCryptoAPI/derive_bits_keys/derived_bits_length.js @@ -0,0 +1,36 @@ +function define_tests() { + // May want to test prefixed implementations. + var subtle = self.crypto.subtle; + + Object.keys(testCases).forEach(algorithm => { + let testData = algorithms[algorithm]; + testCases[algorithm].forEach(testParam => { + promise_test(async() => { + let derivedBits, privateKey, publicKey; + try { + privateKey = await subtle.importKey(testData.privateKey.format, testData.privateKey.data, testData.importAlg, false, ["deriveBits"]); + if (testData.deriveAlg.public !== undefined) { + publicKey = await subtle.importKey(testData.publicKey.format, testData.publicKey.data, testData.importAlg, false, []); + testData.deriveAlg.public = publicKey; + } + if (testParam.length === "omitted") + derivedBits = await subtle.deriveBits(testData.deriveAlg, privateKey); + else + derivedBits = await subtle.deriveBits(testData.deriveAlg, privateKey, testParam.length); + if (testParam.expected === undefined) { + assert_unreached("deriveBits should have thrown an OperationError exception."); + } + assert_array_equals(new Uint8Array(derivedBits), testParam.expected, "Derived bits do not match the expected result."); + } catch (err) { + if (err instanceof AssertionError || testParam.expected !== undefined) { + throw err; + } + assert_true(privateKey !== undefined, "Key should be valid."); + assert_equals(err.name, "OperationError", "deriveBits correctly threw OperationError: " + err.message); + } + }, algorithm + " derivation with " + testParam.length + " as 'length' parameter"); + }); + }); + + return Promise.resolve("define_tests"); +} diff --git a/WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js b/WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js new file mode 100644 index 000000000000000..248a29eed36ec63 --- /dev/null +++ b/WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js @@ -0,0 +1,30 @@ +var testCases = { + "HKDF": [ + {length: 256, expected: algorithms["HKDF"].derivation}, + {length: 0, expected: undefined}, // not a multiple of 8 + {length: null, expected: undefined }, // should throw an exception + {length: undefined, expected: undefined }, // should throw an exception + {length: "omitted", expected: undefined }, // default value is null, so should throw + ], + "PBKDF2": [ + {length: 256, expected: algorithms["PBKDF2"].derivation}, + {length: 0, expected: undefined}, // not a multiple of 8 + {length: null, expected: undefined }, // should throw an exception + {length: undefined, expected: undefined }, // should throw an exception + {length: "omitted", expected: undefined }, // default value is null, so should throw + ], + "ECDH": [ + {length: 256, expected: algorithms["ECDH"].derivation}, + {length: 0, expected: emptyArray}, + {length: null, expected: algorithms["ECDH"].derivation}, + {length: undefined, expected: algorithms["ECDH"].derivation}, + {length: "omitted", expected: algorithms["ECDH"].derivation }, // default value is null + ], + "X25519": [ + {length: 256, expected: algorithms["X25519"].derivation}, + {length: 0, expected: emptyArray}, + {length: null, expected: algorithms["X25519"].derivation}, + {length: undefined, expected: algorithms["X25519"].derivation}, + {length: "omitted", expected: algorithms["X25519"].derivation }, // default value is null + ], +} diff --git a/WebCryptoAPI/derive_bits_keys/derived_bits_length_vectors.js b/WebCryptoAPI/derive_bits_keys/derived_bits_length_vectors.js new file mode 100644 index 000000000000000..fa51f7d3f2b195d --- /dev/null +++ b/WebCryptoAPI/derive_bits_keys/derived_bits_length_vectors.js @@ -0,0 +1,33 @@ +const emptyArray = new Uint8Array([]); +const rawKey = new Uint8Array([85, 115, 101, 114, 115, 32, 115, 104, 111, 117, 108, 100, 32, 112, 105, 99, 107, 32, 108, 111, 110, 103, 32, 112, 97, 115, 115, 112, 104, 114, 97, 115, 101, 115, 32, 40, 110, 111, 116, 32, 117, 115, 101, 32, 115, 104, 111, 114, 116, 32, 112, 97, 115, 115, 119, 111, 114, 100, 115, 41, 33]); +const salt = new Uint8Array([83, 111, 100, 105, 117, 109, 32, 67, 104, 108, 111, 114, 105, 100, 101, 32, 99, 111, 109, 112, 111, 117, 110, 100]); +const info = new Uint8Array([72, 75, 68, 70, 32, 101, 120, 116, 114, 97, 32, 105, 110, 102, 111]); + +var algorithms = { + "HKDF": { + importAlg: {name: "HKDF"}, + privateKey: {format: "raw", data: rawKey}, + deriveAlg: {name: "HKDF", salt: salt, hash: "SHA-256", info: info}, + derivation: new Uint8Array([49, 183, 214, 133, 48, 168, 99, 231, 23, 192, 129, 202, 105, 23, 182, 134, 80, 179, 221, 154, 41, 243, 6, 6, 226, 202, 209, 153, 190, 193, 77, 19]), + }, + "PBKDF2": { + importAlg: {name: "PBKDF2"}, + privateKey: {format: "raw", data: rawKey}, + deriveAlg: {name: "PBKDF2", salt: salt, hash: "SHA-256", iterations: 100000}, + derivation: new Uint8Array([17, 153, 45, 139, 129, 51, 17, 36, 76, 84, 75, 98, 41, 41, 69, 226, 8, 212, 3, 206, 189, 107, 149, 82, 161, 165, 98, 6, 93, 153, 88, 234]), + }, + "ECDH": { + importAlg: {name: "ECDH", namedCurve: "P-256"}, + privateKey: {format: "pkcs8", data: new Uint8Array([48, 129, 135, 2, 1, 0, 48, 19, 6, 7, 42, 134, 72, 206, 61, 2, 1, 6, 8, 42, 134, 72, 206, 61, 3, 1, 7, 4, 109, 48, 107, 2, 1, 1, 4, 32, 15, 247, 79, 232, 241, 202, 175, 97, 92, 206, 241, 29, 217, 53, 114, 87, 98, 217, 216, 65, 236, 186, 185, 94, 170, 38, 68, 123, 52, 100, 245, 113, 161, 68, 3, 66, 0, 4, 140, 96, 11, 44, 102, 25, 45, 97, 158, 39, 210, 37, 107, 59, 151, 118, 178, 141, 30, 5, 246, 13, 234, 189, 98, 174, 123, 154, 211, 157, 224, 217, 59, 4, 102, 109, 199, 119, 14, 126, 207, 13, 211, 203, 203, 211, 110, 221, 107, 94, 220, 153, 81, 7, 55, 161, 237, 104, 46, 205, 112, 244, 10, 47])}, + publicKey: {format: "spki", data: new Uint8Array([48, 89, 48, 19, 6, 7, 42, 134, 72, 206, 61, 2, 1, 6, 8, 42, 134, 72, 206, 61, 3, 1, 7, 3, 66, 0, 4, 154, 116, 32, 120, 126, 95, 77, 105, 211, 232, 34, 114, 115, 1, 109, 56, 224, 71, 129, 133, 223, 127, 238, 156, 142, 103, 60, 202, 211, 79, 126, 128, 254, 49, 141, 182, 221, 107, 119, 218, 99, 32, 165, 246, 151, 89, 9, 68, 23, 177, 52, 239, 138, 139, 116, 193, 101, 4, 57, 198, 115, 0, 90, 61])}, + deriveAlg: {name: "ECDH", public: new Uint8Array ([])}, + derivation: new Uint8Array([14, 143, 60, 77, 177, 178, 162, 131, 115, 90, 0, 220, 87, 31, 26, 232, 151, 28, 227, 35, 250, 17, 131, 137, 203, 95, 65, 196, 59, 61, 181, 161]), + }, + "X25519": { + importAlg: {name: "X25519"}, + privateKey: {format: "pkcs8", data: new Uint8Array([48, 46, 2, 1, 0, 48, 5, 6, 3, 43, 101, 110, 4, 34, 4, 32, 200, 131, 142, 118, 208, 87, 223, 183, 216, 201, 90, 105, 225, 56, 22, 10, 221, 99, 115, 253, 113, 164, 210, 118, 187, 86, 227, 168, 27, 100, 255, 97])}, + publicKey: {format: "spki", data: new Uint8Array([48, 42, 48, 5, 6, 3, 43, 101, 110, 3, 33, 0, 28, 242, 177, 230, 2, 46, 197, 55, 55, 30, 215, 245, 62, 84, 250, 17, 84, 216, 62, 152, 235, 100, 234, 81, 250, 229, 179, 48, 124, 254, 151, 6])}, + deriveAlg: {name: "X25519", public: new Uint8Array ([])}, + derivation: new Uint8Array([39, 104, 64, 157, 250, 185, 158, 194, 59, 140, 137, 185, 63, 245, 136, 2, 149, 247, 97, 118, 8, 143, 137, 228, 61, 254, 190, 126, 161, 149, 0, 8]), + } +}; diff --git a/WebCryptoAPI/derive_bits_keys/ecdh_bits.js b/WebCryptoAPI/derive_bits_keys/ecdh_bits.js index cb9747a529fd535..36b29c20a282abb 100644 --- a/WebCryptoAPI/derive_bits_keys/ecdh_bits.js +++ b/WebCryptoAPI/derive_bits_keys/ecdh_bits.js @@ -55,25 +55,6 @@ function define_tests() { }); }, namedCurve + " mixed case parameters"); - // Null length - // "Null" is not valid per the current spec - // - https://github.com/w3c/webcrypto/issues/322 - // - https://github.com/w3c/webcrypto/issues/329 - // - // Proposal for a spec change: - // - https://github.com/w3c/webcrypto/pull/345 - // - // This test case may be replaced by these new tests: - // - https://github.com/web-platform-tests/wpt/pull/43400 - promise_test(function(test) { - return subtle.deriveBits({name: "ECDH", public: publicKeys[namedCurve]}, privateKeys[namedCurve], null) - .then(function(derivation) { - assert_true(equalBuffers(derivation, derivations[namedCurve]), "Derived correct bits"); - }, function(err) { - assert_unreached("deriveBits failed with error " + err.name + ": " + err.message); - }); - }, namedCurve + " with null length"); - // Shorter than entire derivation per algorithm promise_test(function(test) { return subtle.deriveBits({name: "ECDH", public: publicKeys[namedCurve]}, privateKeys[namedCurve], 8 * sizes[namedCurve] - 32) diff --git a/WebCryptoAPI/derive_bits_keys/hkdf.js b/WebCryptoAPI/derive_bits_keys/hkdf.js index 3903da5cddff941..b2dfda0257bc81b 100644 --- a/WebCryptoAPI/derive_bits_keys/hkdf.js +++ b/WebCryptoAPI/derive_bits_keys/hkdf.js @@ -139,25 +139,6 @@ function define_tests() { }); }, testName + " with missing info"); - // length null (OperationError) - // "Null" is not valid per the current spec - // - https://github.com/w3c/webcrypto/issues/322 - // - https://github.com/w3c/webcrypto/issues/329 - // - // Proposal for a spec change: - // - https://github.com/w3c/webcrypto/pull/345 - // - // This test case may be replaced by these new tests: - // - https://github.com/web-platform-tests/wpt/pull/43400 - subsetTest(promise_test, function(test) { - return subtle.deriveBits(algorithm, baseKeys[derivedKeySize], null) - .then(function(derivation) { - assert_unreached("null length should have thrown an OperationError"); - }, function(err) { - assert_equals(err.name, "OperationError", "deriveBits with null length correctly threw OperationError: " + err.message); - }); - }, testName + " with null length"); - // length not multiple of 8 (OperationError) subsetTest(promise_test, function(test) { return subtle.deriveBits(algorithm, baseKeys[derivedKeySize], 44) diff --git a/WebCryptoAPI/derive_bits_keys/pbkdf2.js b/WebCryptoAPI/derive_bits_keys/pbkdf2.js index 4e4ae79d800a402..090806ceb6b3eaa 100644 --- a/WebCryptoAPI/derive_bits_keys/pbkdf2.js +++ b/WebCryptoAPI/derive_bits_keys/pbkdf2.js @@ -103,26 +103,6 @@ function define_tests() { }); - // Test various error conditions for deriveBits below: - // length null (OperationError) - // "Null" is not valid per the current spec - // - https://github.com/w3c/webcrypto/issues/322 - // - https://github.com/w3c/webcrypto/issues/329 - // - // Proposal for a spec change: - // - https://github.com/w3c/webcrypto/pull/345 - // - // This test case may be replaced by these new tests: - // - https://github.com/web-platform-tests/wpt/pull/43400 - subsetTest(promise_test, function(test) { - return subtle.deriveBits({name: "PBKDF2", salt: salts[saltSize], hash: hashName, iterations: parseInt(iterations)}, baseKeys[passwordSize], null) - .then(function(derivation) { - assert_unreached("null length should have thrown an OperationError"); - }, function(err) { - assert_equals(err.name, "OperationError", "deriveBits with null length correctly threw OperationError: " + err.message); - }); - }, testName + " with null length"); - // 0 length (OperationError) subsetTest(promise_test, function(test) { return subtle.deriveBits({name: "PBKDF2", salt: salts[saltSize], hash: hashName, iterations: parseInt(iterations)}, baseKeys[passwordSize], 0)