From c9ea6a92619658d40295566b600d0778246287e2 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 26 Feb 2022 18:12:48 +0100 Subject: [PATCH] crypto: validate `this` value for `webcrypto.getRandomValues` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/41760 Reviewed-By: Mestery Reviewed-By: Michaël Zasso Reviewed-By: Tobias Nießen --- benchmark/crypto/webcrypto-digest.js | 8 +++----- doc/api/webcrypto.md | 8 ++++---- lib/internal/crypto/webcrypto.js | 11 ++++++++++- test/parallel/test-webcrypto-derivebits-ecdh.js | 5 +++-- test/parallel/test-webcrypto-derivekey-ecdh.js | 5 +++-- .../test-webcrypto-encrypt-decrypt-aes.js | 9 +++++---- test/parallel/test-webcrypto-encrypt-decrypt.js | 17 +++++++++-------- test/parallel/test-webcrypto-export-import.js | 9 +++++---- test/parallel/test-webcrypto-getRandomValues.js | 11 +++++++++++ test/parallel/test-webcrypto-random.js | 12 ++++++------ 10 files changed, 59 insertions(+), 36 deletions(-) create mode 100644 test/parallel/test-webcrypto-getRandomValues.js diff --git a/benchmark/crypto/webcrypto-digest.js b/benchmark/crypto/webcrypto-digest.js index 2d95f868f66f7c..4acd82878dacf2 100644 --- a/benchmark/crypto/webcrypto-digest.js +++ b/benchmark/crypto/webcrypto-digest.js @@ -3,11 +3,9 @@ const common = require('../common.js'); const { createHash, - webcrypto: { - subtle, - getRandomValues - } + webcrypto, } = require('crypto'); +const { subtle } = webcrypto; const bench = common.createBenchmark(main, { sync: ['createHash', 'subtle'], @@ -50,7 +48,7 @@ function measureSubtle(n, data, method) { } function main({ n, sync, data, method }) { - data = getRandomValues(Buffer.alloc(data)); + data = webcrypto.getRandomValues(Buffer.alloc(data)); switch (sync) { case 'createHash': return measureLegacy(n, data, method); case 'subtle': return measureSubtle(n, data, method); diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md index 3d7bf0f946ff05..dc046e846c4a85 100644 --- a/doc/api/webcrypto.md +++ b/doc/api/webcrypto.md @@ -128,14 +128,14 @@ async function generateRsaKey(modulusLength = 2048, hash = 'SHA-256') { ### Encryption and decryption ```js -const { subtle, getRandomValues } = require('crypto').webcrypto; +const crypto = require('crypto').webcrypto; async function aesEncrypt(plaintext) { const ec = new TextEncoder(); const key = await generateAesKey(); - const iv = getRandomValues(new Uint8Array(16)); + const iv = crypto.getRandomValues(new Uint8Array(16)); - const ciphertext = await subtle.encrypt({ + const ciphertext = await crypto.subtle.encrypt({ name: 'AES-CBC', iv, }, key, ec.encode(plaintext)); @@ -149,7 +149,7 @@ async function aesEncrypt(plaintext) { async function aesDecrypt(ciphertext, key, iv) { const dec = new TextDecoder(); - const plaintext = await subtle.decrypt({ + const plaintext = await crypto.subtle.decrypt({ name: 'AES-CBC', iv, }, key, ciphertext); diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js index 8e3822494be230..63dd03bd00e0f0 100644 --- a/lib/internal/crypto/webcrypto.js +++ b/lib/internal/crypto/webcrypto.js @@ -5,6 +5,7 @@ const { JSONParse, JSONStringify, ObjectDefineProperties, + ReflectApply, SafeSet, SymbolToStringTag, StringPrototypeRepeat, @@ -31,6 +32,7 @@ const { TextDecoder, TextEncoder } = require('internal/encoding'); const { codes: { ERR_INVALID_ARG_TYPE, + ERR_INVALID_THIS, } } = require('internal/errors'); @@ -64,7 +66,7 @@ const { } = require('internal/util'); const { - getRandomValues, + getRandomValues: _getRandomValues, randomUUID: _randomUUID, } = require('internal/crypto/random'); @@ -695,6 +697,13 @@ class Crypto { } const crypto = new Crypto(); +function getRandomValues(array) { + if (!(this instanceof Crypto)) { + throw new ERR_INVALID_THIS('Crypto'); + } + return ReflectApply(_getRandomValues, this, arguments); +} + ObjectDefineProperties( Crypto.prototype, { [SymbolToStringTag]: { diff --git a/test/parallel/test-webcrypto-derivebits-ecdh.js b/test/parallel/test-webcrypto-derivebits-ecdh.js index 64cbae7cec6a03..166da81e3e4e6d 100644 --- a/test/parallel/test-webcrypto-derivebits-ecdh.js +++ b/test/parallel/test-webcrypto-derivebits-ecdh.js @@ -6,7 +6,8 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); -const { subtle, getRandomValues } = require('crypto').webcrypto; +const { webcrypto } = require('crypto'); +const { subtle } = webcrypto; const kTests = [ { @@ -250,7 +251,7 @@ async function prepareKeys() { { // Public is a secret key - const keyData = getRandomValues(new Uint8Array(32)); + const keyData = webcrypto.getRandomValues(new Uint8Array(32)); const key = await subtle.importKey( 'raw', keyData, diff --git a/test/parallel/test-webcrypto-derivekey-ecdh.js b/test/parallel/test-webcrypto-derivekey-ecdh.js index bdd9bd7588a763..42c8d250f42b06 100644 --- a/test/parallel/test-webcrypto-derivekey-ecdh.js +++ b/test/parallel/test-webcrypto-derivekey-ecdh.js @@ -6,7 +6,8 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); -const { subtle, getRandomValues } = require('crypto').webcrypto; +const { webcrypto } = require('crypto'); +const { subtle } = webcrypto; const kTests = [ { @@ -226,7 +227,7 @@ async function prepareKeys() { { // Public is a secret key - const keyData = getRandomValues(new Uint8Array(32)); + const keyData = webcrypto.getRandomValues(new Uint8Array(32)); const key = await subtle.importKey( 'raw', keyData, diff --git a/test/parallel/test-webcrypto-encrypt-decrypt-aes.js b/test/parallel/test-webcrypto-encrypt-decrypt-aes.js index 7adb18918d2205..885cded906b079 100644 --- a/test/parallel/test-webcrypto-encrypt-decrypt-aes.js +++ b/test/parallel/test-webcrypto-encrypt-decrypt-aes.js @@ -6,7 +6,8 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); -const { getRandomValues, subtle } = require('crypto').webcrypto; +const { webcrypto } = require('crypto'); +const { subtle } = webcrypto; async function testEncrypt({ keyBuffer, algorithm, plaintext, result }) { // Using a copy of plaintext to prevent tampering of the original @@ -213,8 +214,8 @@ async function testDecrypt({ keyBuffer, algorithm, result }) { ['encrypt', 'decrypt'], ); - const iv = getRandomValues(new Uint8Array(12)); - const aad = getRandomValues(new Uint8Array(32)); + const iv = webcrypto.getRandomValues(new Uint8Array(12)); + const aad = webcrypto.getRandomValues(new Uint8Array(32)); const encrypted = await subtle.encrypt( { @@ -224,7 +225,7 @@ async function testDecrypt({ keyBuffer, algorithm, result }) { tagLength: 128 }, secretKey, - getRandomValues(new Uint8Array(32)) + webcrypto.getRandomValues(new Uint8Array(32)) ); await subtle.decrypt( diff --git a/test/parallel/test-webcrypto-encrypt-decrypt.js b/test/parallel/test-webcrypto-encrypt-decrypt.js index 50fa99a999cd92..cc997e7d2e6d26 100644 --- a/test/parallel/test-webcrypto-encrypt-decrypt.js +++ b/test/parallel/test-webcrypto-encrypt-decrypt.js @@ -6,14 +6,15 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); -const { subtle, getRandomValues } = require('crypto').webcrypto; +const { webcrypto } = require('crypto'); +const { subtle } = webcrypto; // This is only a partial test. The WebCrypto Web Platform Tests // will provide much greater coverage. // Test Encrypt/Decrypt RSA-OAEP { - const buf = getRandomValues(new Uint8Array(50)); + const buf = webcrypto.getRandomValues(new Uint8Array(50)); async function test() { const ec = new TextEncoder(); @@ -44,8 +45,8 @@ const { subtle, getRandomValues } = require('crypto').webcrypto; // Test Encrypt/Decrypt AES-CTR { - const buf = getRandomValues(new Uint8Array(50)); - const counter = getRandomValues(new Uint8Array(16)); + const buf = webcrypto.getRandomValues(new Uint8Array(50)); + const counter = webcrypto.getRandomValues(new Uint8Array(16)); async function test() { const key = await subtle.generateKey({ @@ -71,8 +72,8 @@ const { subtle, getRandomValues } = require('crypto').webcrypto; // Test Encrypt/Decrypt AES-CBC { - const buf = getRandomValues(new Uint8Array(50)); - const iv = getRandomValues(new Uint8Array(16)); + const buf = webcrypto.getRandomValues(new Uint8Array(50)); + const iv = webcrypto.getRandomValues(new Uint8Array(16)); async function test() { const key = await subtle.generateKey({ @@ -98,8 +99,8 @@ const { subtle, getRandomValues } = require('crypto').webcrypto; // Test Encrypt/Decrypt AES-GCM { - const buf = getRandomValues(new Uint8Array(50)); - const iv = getRandomValues(new Uint8Array(12)); + const buf = webcrypto.getRandomValues(new Uint8Array(50)); + const iv = webcrypto.getRandomValues(new Uint8Array(12)); async function test() { const key = await subtle.generateKey({ diff --git a/test/parallel/test-webcrypto-export-import.js b/test/parallel/test-webcrypto-export-import.js index d7db433b364011..b4fd26b0cda6ae 100644 --- a/test/parallel/test-webcrypto-export-import.js +++ b/test/parallel/test-webcrypto-export-import.js @@ -6,11 +6,12 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('assert'); -const { subtle, getRandomValues } = require('crypto').webcrypto; +const { webcrypto } = require('crypto'); +const { subtle } = webcrypto; { async function test() { - const keyData = getRandomValues(new Uint8Array(32)); + const keyData = webcrypto.getRandomValues(new Uint8Array(32)); await Promise.all([1, null, undefined, {}, []].map((format) => assert.rejects( subtle.importKey(format, keyData, {}, false, ['wrapKey']), { @@ -82,7 +83,7 @@ const { subtle, getRandomValues } = require('crypto').webcrypto; // Import/Export HMAC Secret Key { async function test() { - const keyData = getRandomValues(new Uint8Array(32)); + const keyData = webcrypto.getRandomValues(new Uint8Array(32)); const key = await subtle.importKey( 'raw', keyData, { @@ -112,7 +113,7 @@ const { subtle, getRandomValues } = require('crypto').webcrypto; // Import/Export AES Secret Key { async function test() { - const keyData = getRandomValues(new Uint8Array(32)); + const keyData = webcrypto.getRandomValues(new Uint8Array(32)); const key = await subtle.importKey( 'raw', keyData, { diff --git a/test/parallel/test-webcrypto-getRandomValues.js b/test/parallel/test-webcrypto-getRandomValues.js new file mode 100644 index 00000000000000..049cdcc847feb1 --- /dev/null +++ b/test/parallel/test-webcrypto-getRandomValues.js @@ -0,0 +1,11 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const { getRandomValues } = require('crypto').webcrypto; + +assert.throws(() => getRandomValues(new Uint8Array()), { code: 'ERR_INVALID_THIS' }); diff --git a/test/parallel/test-webcrypto-random.js b/test/parallel/test-webcrypto-random.js index e17cc834b6c2bf..c3fc6aaab2eb1e 100644 --- a/test/parallel/test-webcrypto-random.js +++ b/test/parallel/test-webcrypto-random.js @@ -7,7 +7,7 @@ if (!common.hasCrypto) const { Buffer } = require('buffer'); const assert = require('assert'); -const { getRandomValues } = require('crypto').webcrypto; +const { webcrypto } = require('crypto'); [ undefined, null, '', 1, {}, [], @@ -16,14 +16,14 @@ const { getRandomValues } = require('crypto').webcrypto; new DataView(new ArrayBuffer(1)), ].forEach((i) => { assert.throws( - () => getRandomValues(i), + () => webcrypto.getRandomValues(i), { name: 'TypeMismatchError', code: 17 }, ); }); { const buf = new Uint8Array(0); - getRandomValues(buf); + webcrypto.getRandomValues(buf); } const intTypedConstructors = [ @@ -41,7 +41,7 @@ const intTypedConstructors = [ for (const ctor of intTypedConstructors) { const buf = new ctor(10); const before = Buffer.from(buf.buffer).toString('hex'); - getRandomValues(buf); + webcrypto.getRandomValues(buf); const after = Buffer.from(buf.buffer).toString('hex'); assert.notStrictEqual(before, after); } @@ -49,7 +49,7 @@ for (const ctor of intTypedConstructors) { { const buf = new Uint16Array(10); const before = Buffer.from(buf).toString('hex'); - getRandomValues(buf); + webcrypto.getRandomValues(buf); const after = Buffer.from(buf).toString('hex'); assert.notStrictEqual(before, after); } @@ -63,7 +63,7 @@ for (const ctor of intTypedConstructors) { } if (kData !== undefined) { - assert.throws(() => getRandomValues(kData), { + assert.throws(() => webcrypto.getRandomValues(kData), { code: 22 }); }