From b64006c0ed3c996bbce19113c8f03168fe409bc9 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 23 Sep 2024 17:28:44 +0200 Subject: [PATCH] crypto: ensure invalid SubtleCrypto JWK data import results in DataError PR-URL: https://github.com/nodejs/node/pull/55041 Reviewed-By: James M Snell Reviewed-By: Yagiz Nizipli --- lib/internal/crypto/aes.js | 7 +++- lib/internal/crypto/cfrg.js | 36 ++++++++++++------- lib/internal/crypto/ec.js | 10 ++++-- lib/internal/crypto/mac.js | 7 +++- lib/internal/crypto/rsa.js | 10 ++++-- .../test-webcrypto-export-import-cfrg.js | 9 +++++ .../test-webcrypto-export-import-ec.js | 9 +++++ .../test-webcrypto-export-import-rsa.js | 9 +++++ 8 files changed, 78 insertions(+), 19 deletions(-) diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js index bb35d2b21de..762e8ead4ac 100644 --- a/lib/internal/crypto/aes.js +++ b/lib/internal/crypto/aes.js @@ -295,7 +295,12 @@ async function aesImportKey( } const handle = new KeyObjectHandle(); - handle.initJwk(keyData); + try { + handle.initJwk(keyData); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } ({ length } = handle.keyDetail({ })); validateKeyLength(length); diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index e7141c04b3f..9c5e59ebb3b 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -17,6 +17,12 @@ const { kSignJobModeVerify, } = internalBinding('crypto'); +const { + codes: { + ERR_CRYPTO_INVALID_JWK, + }, +} = require('internal/errors'); + const { getUsagesUnion, hasAnyNotIn, @@ -277,22 +283,26 @@ async function cfrgImportKey( isPublic, usagesSet); - const publicKeyObject = createCFRGRawKey( - name, - Buffer.from(keyData.x, 'base64'), - true); - - if (isPublic) { - keyObject = publicKeyObject; - } else { - keyObject = createCFRGRawKey( + try { + const publicKeyObject = createCFRGRawKey( name, - Buffer.from(keyData.d, 'base64'), - false); + Buffer.from(keyData.x, 'base64'), + true); - if (!createPublicKey(keyObject).equals(publicKeyObject)) { - throw lazyDOMException('Invalid JWK', 'DataError'); + if (isPublic) { + keyObject = publicKeyObject; + } else { + keyObject = createCFRGRawKey( + name, + Buffer.from(keyData.d, 'base64'), + false); + + if (!createPublicKey(keyObject).equals(publicKeyObject)) { + throw new ERR_CRYPTO_INVALID_JWK(); + } } + } catch (err) { + throw lazyDOMException('Invalid keyData', { name: 'DataError', cause: err }); } break; } diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index 9f30f6e1a0b..e155a5b6610 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -240,9 +240,15 @@ async function ecImportKey( } const handle = new KeyObjectHandle(); - const type = handle.initJwk(keyData, namedCurve); + let type; + try { + type = handle.initJwk(keyData, namedCurve); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } if (type === undefined) - throw lazyDOMException('Invalid JWK', 'DataError'); + throw lazyDOMException('Invalid keyData', 'DataError'); keyObject = type === kKeyTypePrivate ? new PrivateKeyObject(handle) : new PublicKeyObject(handle); diff --git a/lib/internal/crypto/mac.js b/lib/internal/crypto/mac.js index 91f58a85a9f..112861523c6 100644 --- a/lib/internal/crypto/mac.js +++ b/lib/internal/crypto/mac.js @@ -145,7 +145,12 @@ async function hmacImportKey( } const handle = new KeyObjectHandle(); - handle.initJwk(keyData); + try { + handle.initJwk(keyData); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } keyObject = new SecretKeyObject(handle); break; } diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index 3b338ce8762..fd223d3cb63 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -275,9 +275,15 @@ async function rsaImportKey( } const handle = new KeyObjectHandle(); - const type = handle.initJwk(keyData); + let type; + try { + type = handle.initJwk(keyData); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } if (type === undefined) - throw lazyDOMException('Invalid JWK', 'DataError'); + throw lazyDOMException('Invalid keyData', 'DataError'); keyObject = type === kKeyTypePrivate ? new PrivateKeyObject(handle) : diff --git a/test/parallel/test-webcrypto-export-import-cfrg.js b/test/parallel/test-webcrypto-export-import-cfrg.js index 144424ce01d..2c2cb80a31c 100644 --- a/test/parallel/test-webcrypto-export-import-cfrg.js +++ b/test/parallel/test-webcrypto-export-import-cfrg.js @@ -322,6 +322,15 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable) extractable, [/* empty usages */]), { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); + + await assert.rejects( + subtle.importKey( + 'jwk', + { kty: jwk.kty, /* missing x */ crv: jwk.crv }, + { name }, + extractable, + publicUsages), + { name: 'DataError', message: 'Invalid keyData' }); } async function testImportRaw({ name, publicUsages }) { diff --git a/test/parallel/test-webcrypto-export-import-ec.js b/test/parallel/test-webcrypto-export-import-ec.js index 27c37715be1..35c657bbbf0 100644 --- a/test/parallel/test-webcrypto-export-import-ec.js +++ b/test/parallel/test-webcrypto-export-import-ec.js @@ -330,6 +330,15 @@ async function testImportJwk( extractable, [/* empty usages */]), { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); + + await assert.rejects( + subtle.importKey( + 'jwk', + { kty: jwk.kty, /* missing x */ y: jwk.y, crv: jwk.crv }, + { name, namedCurve }, + extractable, + publicUsages), + { name: 'DataError', message: 'Invalid keyData' }); } async function testImportRaw({ name, publicUsages }, namedCurve) { diff --git a/test/parallel/test-webcrypto-export-import-rsa.js b/test/parallel/test-webcrypto-export-import-rsa.js index fb79184afd6..303efef7bb8 100644 --- a/test/parallel/test-webcrypto-export-import-rsa.js +++ b/test/parallel/test-webcrypto-export-import-rsa.js @@ -513,6 +513,15 @@ async function testImportJwk( extractable, [/* empty usages */]), { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); + + await assert.rejects( + subtle.importKey( + 'jwk', + { kty: jwk.kty, /* missing e */ n: jwk.n }, + { name, hash }, + extractable, + publicUsages), + { name: 'DataError', message: 'Invalid keyData' }); } // combinations to test