From 8fcfbeffe1d7017c156d13fde0d1b31e4a119ea4 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 21 Nov 2022 23:43:57 +0100 Subject: [PATCH] crypto: use DataError for webcrypto keyData import failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/45569 Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen --- lib/internal/crypto/cfrg.js | 32 ++++++++++++++++--------- lib/internal/crypto/ec.js | 40 ++++++++++++++++++++----------- lib/internal/crypto/rsa.js | 30 +++++++++++++++-------- test/wpt/status/WebCryptoAPI.json | 36 ---------------------------- 4 files changed, 67 insertions(+), 71 deletions(-) diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index 7c8348e09b20f6..b12579326b1e3a 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -109,7 +109,7 @@ function createCFRGRawKey(name, keyData, isPublic) { const keyType = isPublic ? kKeyTypePublic : kKeyTypePrivate; if (!handle.initEDRaw(name, keyData, keyType)) { - throw lazyDOMException('Failure to generate key object'); + throw lazyDOMException('Invalid keyData', 'DataError'); } return isPublic ? new PublicKeyObject(handle) : new PrivateKeyObject(handle); @@ -220,20 +220,30 @@ async function cfrgImportKey( switch (format) { case 'spki': { verifyAcceptableCfrgKeyUse(name, 'public', usagesSet); - keyObject = createPublicKey({ - key: keyData, - format: 'der', - type: 'spki' - }); + try { + keyObject = createPublicKey({ + key: keyData, + format: 'der', + type: 'spki' + }); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } break; } case 'pkcs8': { verifyAcceptableCfrgKeyUse(name, 'private', usagesSet); - keyObject = createPrivateKey({ - key: keyData, - format: 'der', - type: 'pkcs8' - }); + try { + keyObject = createPrivateKey({ + key: keyData, + format: 'der', + type: 'pkcs8' + }); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } break; } case 'jwk': { diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index d2c21da7fe762e..a57a1519a24e8f 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -80,8 +80,12 @@ function verifyAcceptableEcKeyUse(name, type, usages) { function createECPublicKeyRaw(namedCurve, keyData) { const handle = new KeyObjectHandle(); keyData = getArrayBufferOrView(keyData, 'keyData'); - if (handle.initECRaw(kNamedCurveAliases[namedCurve], keyData)) - return new PublicKeyObject(handle); + + if (!handle.initECRaw(kNamedCurveAliases[namedCurve], keyData)) { + throw lazyDOMException('Invalid keyData', 'DataError'); + } + + return new PublicKeyObject(handle); } async function ecGenerateKey(algorithm, extractable, keyUsages) { @@ -176,20 +180,30 @@ async function ecImportKey( switch (format) { case 'spki': { verifyAcceptableEcKeyUse(name, 'public', usagesSet); - keyObject = createPublicKey({ - key: keyData, - format: 'der', - type: 'spki' - }); + try { + keyObject = createPublicKey({ + key: keyData, + format: 'der', + type: 'spki' + }); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } break; } case 'pkcs8': { verifyAcceptableEcKeyUse(name, 'private', usagesSet); - keyObject = createPrivateKey({ - key: keyData, - format: 'der', - type: 'pkcs8' - }); + try { + keyObject = createPrivateKey({ + key: keyData, + format: 'der', + type: 'pkcs8' + }); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } break; } case 'jwk': { @@ -246,8 +260,6 @@ async function ecImportKey( case 'raw': { verifyAcceptableEcKeyUse(name, 'public', usagesSet); keyObject = createECPublicKeyRaw(namedCurve, keyData); - if (keyObject === undefined) - throw lazyDOMException('Unable to import EC key', 'OperationError'); break; } } diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index 63542b4ae47207..03930d2bf19ac8 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -245,20 +245,30 @@ async function rsaImportKey( switch (format) { case 'spki': { verifyAcceptableRsaKeyUse(algorithm.name, 'public', usagesSet); - keyObject = createPublicKey({ - key: keyData, - format: 'der', - type: 'spki' - }); + try { + keyObject = createPublicKey({ + key: keyData, + format: 'der', + type: 'spki' + }); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } break; } case 'pkcs8': { verifyAcceptableRsaKeyUse(algorithm.name, 'private', usagesSet); - keyObject = createPrivateKey({ - key: keyData, - format: 'der', - type: 'pkcs8' - }); + try { + keyObject = createPrivateKey({ + key: keyData, + format: 'der', + type: 'pkcs8' + }); + } catch (err) { + throw lazyDOMException( + 'Invalid keyData', { name: 'DataError', cause: err }); + } break; } case 'jwk': { diff --git a/test/wpt/status/WebCryptoAPI.json b/test/wpt/status/WebCryptoAPI.json index 365b262c0d1552..7f61a1f0364cb5 100644 --- a/test/wpt/status/WebCryptoAPI.json +++ b/test/wpt/status/WebCryptoAPI.json @@ -11,14 +11,6 @@ "import_export/okp_importKey_failures_Ed25519.https.any.js": { "fail": { "expected": [ - "Bad key length: importKey(spki, {name: Ed25519}, true, [verify])", - "Bad key length: importKey(spki, {name: Ed25519}, false, [verify])", - "Bad key length: importKey(spki, {name: Ed25519}, true, [verify, verify])", - "Bad key length: importKey(spki, {name: Ed25519}, false, [verify, verify])", - "Bad key length: importKey(pkcs8, {name: Ed25519}, true, [sign])", - "Bad key length: importKey(pkcs8, {name: Ed25519}, false, [sign])", - "Bad key length: importKey(pkcs8, {name: Ed25519}, true, [sign, sign])", - "Bad key length: importKey(pkcs8, {name: Ed25519}, false, [sign, sign])", "Bad key length: importKey(jwk(private), {name: Ed25519}, true, [sign])", "Bad key length: importKey(jwk(private), {name: Ed25519}, false, [sign])", "Bad key length: importKey(jwk(private), {name: Ed25519}, true, [sign, sign])", @@ -35,14 +27,6 @@ "import_export/okp_importKey_failures_Ed448.https.any.js": { "fail": { "expected": [ - "Bad key length: importKey(spki, {name: Ed448}, true, [verify])", - "Bad key length: importKey(spki, {name: Ed448}, false, [verify])", - "Bad key length: importKey(spki, {name: Ed448}, true, [verify, verify])", - "Bad key length: importKey(spki, {name: Ed448}, false, [verify, verify])", - "Bad key length: importKey(pkcs8, {name: Ed448}, true, [sign])", - "Bad key length: importKey(pkcs8, {name: Ed448}, false, [sign])", - "Bad key length: importKey(pkcs8, {name: Ed448}, true, [sign, sign])", - "Bad key length: importKey(pkcs8, {name: Ed448}, false, [sign, sign])", "Bad key length: importKey(jwk(private), {name: Ed448}, true, [sign])", "Bad key length: importKey(jwk(private), {name: Ed448}, false, [sign])", "Bad key length: importKey(jwk(private), {name: Ed448}, true, [sign, sign])", @@ -59,16 +43,6 @@ "import_export/okp_importKey_failures_X25519.https.any.js": { "fail": { "expected": [ - "Bad key length: importKey(spki, {name: X25519}, true, [])", - "Bad key length: importKey(spki, {name: X25519}, false, [])", - "Bad key length: importKey(pkcs8, {name: X25519}, true, [deriveKey])", - "Bad key length: importKey(pkcs8, {name: X25519}, false, [deriveKey])", - "Bad key length: importKey(pkcs8, {name: X25519}, true, [deriveBits, deriveKey])", - "Bad key length: importKey(pkcs8, {name: X25519}, false, [deriveBits, deriveKey])", - "Bad key length: importKey(pkcs8, {name: X25519}, true, [deriveBits])", - "Bad key length: importKey(pkcs8, {name: X25519}, false, [deriveBits])", - "Bad key length: importKey(pkcs8, {name: X25519}, true, [deriveKey, deriveBits, deriveKey, deriveBits])", - "Bad key length: importKey(pkcs8, {name: X25519}, false, [deriveKey, deriveBits, deriveKey, deriveBits])", "Bad key length: importKey(jwk (public) , {name: X25519}, true, [])", "Bad key length: importKey(jwk (public) , {name: X25519}, false, [])", "Bad key length: importKey(jwk(private), {name: X25519}, true, [deriveKey])", @@ -89,16 +63,6 @@ "import_export/okp_importKey_failures_X448.https.any.js": { "fail": { "expected": [ - "Bad key length: importKey(spki, {name: X448}, true, [])", - "Bad key length: importKey(spki, {name: X448}, false, [])", - "Bad key length: importKey(pkcs8, {name: X448}, true, [deriveKey])", - "Bad key length: importKey(pkcs8, {name: X448}, false, [deriveKey])", - "Bad key length: importKey(pkcs8, {name: X448}, true, [deriveBits, deriveKey])", - "Bad key length: importKey(pkcs8, {name: X448}, false, [deriveBits, deriveKey])", - "Bad key length: importKey(pkcs8, {name: X448}, true, [deriveBits])", - "Bad key length: importKey(pkcs8, {name: X448}, false, [deriveBits])", - "Bad key length: importKey(pkcs8, {name: X448}, true, [deriveKey, deriveBits, deriveKey, deriveBits])", - "Bad key length: importKey(pkcs8, {name: X448}, false, [deriveKey, deriveBits, deriveKey, deriveBits])", "Bad key length: importKey(jwk(private), {name: X448}, true, [deriveKey])", "Bad key length: importKey(jwk(private), {name: X448}, false, [deriveKey])", "Bad key length: importKey(jwk(private), {name: X448}, true, [deriveBits, deriveKey])",