Skip to content

src: refactor SubtleCrypto algorithm and length validations #57273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 39 additions & 27 deletions lib/internal/crypto/aes.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ const {
generateKey: _generateKey,
} = require('internal/crypto/keygen');

const kMaxCounterLength = 128;
const kTagLengths = [32, 64, 96, 104, 112, 120, 128];
const generateKey = promisify(_generateKey);

Expand Down Expand Up @@ -109,35 +108,43 @@ function getVariant(name, length) {
}
}

function asyncAesCtrCipher(mode, key, data, { counter, length }) {
validateByteLength(counter, 'algorithm.counter', 16);
function validateAesCtrAlgorithm(algorithm) {
validateByteLength(algorithm.counter, 'algorithm.counter', 16);
// The length must specify an integer between 1 and 128. While
// there is no default, this should typically be 64.
if (length === 0 || length > kMaxCounterLength) {
if (algorithm.length === 0 || algorithm.length > 128) {
throw lazyDOMException(
'AES-CTR algorithm.length must be between 1 and 128',
'OperationError');
}
}

function asyncAesCtrCipher(mode, key, data, algorithm) {
validateAesCtrAlgorithm(algorithm);

return jobPromise(() => new AESCipherJob(
kCryptoJobAsync,
mode,
key[kKeyObject][kHandle],
data,
getVariant('AES-CTR', key.algorithm.length),
counter,
length));
algorithm.counter,
algorithm.length));
}

function validateAesCbcAlgorithm(algorithm) {
validateByteLength(algorithm.iv, 'algorithm.iv', 16);
}

function asyncAesCbcCipher(mode, key, data, { iv }) {
validateByteLength(iv, 'algorithm.iv', 16);
function asyncAesCbcCipher(mode, key, data, algorithm) {
validateAesCbcAlgorithm(algorithm);
return jobPromise(() => new AESCipherJob(
kCryptoJobAsync,
mode,
key[kKeyObject][kHandle],
data,
getVariant('AES-CBC', key.algorithm.length),
iv));
algorithm.iv));
}

function asyncAesKwCipher(mode, key, data) {
Expand All @@ -149,24 +156,25 @@ function asyncAesKwCipher(mode, key, data) {
getVariant('AES-KW', key.algorithm.length)));
}

function asyncAesGcmCipher(
mode,
key,
data,
{ iv, additionalData, tagLength = 128 }) {
if (!ArrayPrototypeIncludes(kTagLengths, tagLength)) {
return PromiseReject(lazyDOMException(
`${tagLength} is not a valid AES-GCM tag length`,
'OperationError'));
function validateAesGcmAlgorithm(algorithm) {
if (!ArrayPrototypeIncludes(kTagLengths, algorithm.tagLength)) {
throw lazyDOMException(
`${algorithm.tagLength} is not a valid AES-GCM tag length`,
'OperationError');
}

validateMaxBufferLength(iv, 'algorithm.iv');
validateMaxBufferLength(algorithm.iv, 'algorithm.iv');

if (additionalData !== undefined) {
validateMaxBufferLength(additionalData, 'algorithm.additionalData');
if (algorithm.additionalData !== undefined) {
validateMaxBufferLength(algorithm.additionalData, 'algorithm.additionalData');
}
}

const tagByteLength = MathFloor(tagLength / 8);
function asyncAesGcmCipher(mode, key, data, algorithm) {
algorithm.tagLength ??= 128;
validateAesGcmAlgorithm(algorithm);

const tagByteLength = MathFloor(algorithm.tagLength / 8);
let tag;
switch (mode) {
case kWebCryptoCipherDecrypt: {
Expand Down Expand Up @@ -198,9 +206,9 @@ function asyncAesGcmCipher(
key[kKeyObject][kHandle],
data,
getVariant('AES-GCM', key.algorithm.length),
iv,
algorithm.iv,
tag,
additionalData));
algorithm.additionalData));
}

function aesCipher(mode, key, data, algorithm) {
Expand All @@ -212,13 +220,17 @@ function aesCipher(mode, key, data, algorithm) {
}
}

async function aesGenerateKey(algorithm, extractable, keyUsages) {
const { name, length } = algorithm;
if (!ArrayPrototypeIncludes(kAesKeyLengths, length)) {
function validateAesGenerateKeyAlgorithm(algorithm) {
if (!ArrayPrototypeIncludes(kAesKeyLengths, algorithm.length)) {
throw lazyDOMException(
'AES key length must be 128, 192, or 256 bits',
'OperationError');
}
}

async function aesGenerateKey(algorithm, extractable, keyUsages) {
validateAesGenerateKeyAlgorithm(algorithm);
const { name, length } = algorithm;

const checkUsages = ['wrapKey', 'unwrapKey'];
if (name !== 'AES-KW')
Expand Down
15 changes: 9 additions & 6 deletions lib/internal/crypto/cfrg.js
Original file line number Diff line number Diff line change
Expand Up @@ -329,18 +329,21 @@ function cfrgImportKey(
extractable);
}

function eddsaSignVerify(key, data, { name, context }, signature) {
function validateEdDSASignVerifyAlgorithm(algorithm) {
if (algorithm.name === 'Ed448' && algorithm.context?.byteLength) {
throw lazyDOMException(
'Non zero-length context is not yet supported.', 'NotSupportedError');
}
}

function eddsaSignVerify(key, data, algorithm, signature) {
validateEdDSASignVerifyAlgorithm(algorithm);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the order of the checks. Previously, we first checked (key.type !== type) and then checked (name === 'Ed448' && context?.byteLength). Now, it's the other way around.

It seems like we're not really following the strict order from the specification for other algorithms either. So I guess this is fine?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the (name === 'Ed448' && context?.byteLength) check is our own because we don't support non-empty contexts. From the (early draft warning) supports method proposal this falls under

If the specified operation or algorithm (or one of its parameter values) is expected to fail (for any key and/or data) for an implementation-specific reason (e.g. known nonconformance to the specification), return false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a draft PR for the supports method in #57270 that I will further refactor this PR's validations structures in when it lands.

const mode = signature === undefined ? kSignJobModeSign : kSignJobModeVerify;
const type = mode === kSignJobModeSign ? 'private' : 'public';

if (key.type !== type)
throw lazyDOMException(`Key must be a ${type} key`, 'InvalidAccessError');

if (name === 'Ed448' && context?.byteLength) {
throw lazyDOMException(
'Non zero-length context is not yet supported.', 'NotSupportedError');
}

return jobPromise(() => new SignJob(
kCryptoJobAsync,
mode,
Expand Down
24 changes: 12 additions & 12 deletions lib/internal/crypto/diffiehellman.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,28 +298,28 @@ function diffieHellman(options) {

let masks;

function validateEcdhDeriveBitsAlgorithmAndLength(algorithm, length) {
if (algorithm.public.type !== 'public') {
throw lazyDOMException(
'algorithm.public must be a public key', 'InvalidAccessError');
}

if (algorithm.name !== algorithm.public.algorithm.name) {
throw lazyDOMException(`algorithm.public must be an ${algorithm.name} key`, 'InvalidAccessError');
}
}

// The ecdhDeriveBits function is part of the Web Crypto API and serves both
// deriveKeys and deriveBits functions.
async function ecdhDeriveBits(algorithm, baseKey, length) {
validateEcdhDeriveBitsAlgorithmAndLength(algorithm, length);
const { 'public': key } = algorithm;

if (key.type !== 'public') {
throw lazyDOMException(
'algorithm.public must be a public key', 'InvalidAccessError');
}
if (baseKey.type !== 'private') {
throw lazyDOMException(
'baseKey must be a private key', 'InvalidAccessError');
}

if (
key.algorithm.name !== 'ECDH' &&
key.algorithm.name !== 'X25519' &&
key.algorithm.name !== 'X448'
) {
throw lazyDOMException('Keys must be ECDH, X25519, or X448 keys', 'InvalidAccessError');
}

if (key.algorithm.name !== baseKey.algorithm.name) {
throw lazyDOMException(
'The public and private keys must be of the same type',
Expand Down
25 changes: 11 additions & 14 deletions lib/internal/crypto/ec.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
'use strict';

const {
ArrayPrototypeIncludes,
ObjectKeys,
ObjectPrototypeHasOwnProperty,
SafeSet,
} = primordials;

Expand Down Expand Up @@ -77,14 +76,17 @@ function createECPublicKeyRaw(namedCurve, keyData) {
return new PublicKeyObject(handle);
}

async function ecGenerateKey(algorithm, extractable, keyUsages) {
const { name, namedCurve } = algorithm;

if (!ArrayPrototypeIncludes(ObjectKeys(kNamedCurveAliases), namedCurve)) {
function validateEcKeyAlgorithm(algorithm) {
if (!ObjectPrototypeHasOwnProperty(kNamedCurveAliases, algorithm.namedCurve)) {
throw lazyDOMException(
'Unrecognized namedCurve',
'NotSupportedError');
}
}

async function ecGenerateKey(algorithm, extractable, keyUsages) {
validateEcKeyAlgorithm(algorithm);
const { name, namedCurve } = algorithm;

const usageSet = new SafeSet(keyUsages);
switch (name) {
Expand Down Expand Up @@ -154,16 +156,11 @@ function ecImportKey(
keyData,
algorithm,
extractable,
keyUsages) {

keyUsages,
) {
validateEcKeyAlgorithm(algorithm);
const { name, namedCurve } = algorithm;

if (!ArrayPrototypeIncludes(ObjectKeys(kNamedCurveAliases), namedCurve)) {
throw lazyDOMException(
'Unrecognized namedCurve',
'NotSupportedError');
}

let keyObject;
const usagesSet = new SafeSet(keyUsages);
switch (format) {
Expand Down
14 changes: 9 additions & 5 deletions lib/internal/crypto/hkdf.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,22 @@ function hkdfSync(hash, key, salt, info, length) {
}

const hkdfPromise = promisify(hkdf);
async function hkdfDeriveBits(algorithm, baseKey, length) {
const { hash, salt, info } = algorithm;

if (length === 0)
return new ArrayBuffer(0);
function validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length) {
if (length === null)
throw lazyDOMException('length cannot be null', 'OperationError');
if (length % 8) {
throw lazyDOMException(
'length must be a multiple of 8',
'OperationError');
}
}

async function hkdfDeriveBits(algorithm, baseKey, length) {
validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length);
const { hash, salt, info } = algorithm;

if (length === 0)
return new ArrayBuffer(0);

try {
return await hkdfPromise(
Expand Down
49 changes: 13 additions & 36 deletions lib/internal/crypto/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -895,12 +895,14 @@ function isCryptoKey(obj) {
}

function importGenericSecretKey(
{ name, length },
algorithm,
format,
keyData,
extractable,
keyUsages) {
keyUsages,
) {
const usagesSet = new SafeSet(keyUsages);
const { name } = algorithm;
if (extractable)
throw lazyDOMException(`${name} keys are not extractable`, 'SyntaxError');

Expand All @@ -910,47 +912,22 @@ function importGenericSecretKey(
'SyntaxError');
}

let keyObject;
switch (format) {
case 'KeyObject': {
if (hasAnyNotIn(usagesSet, ['deriveKey', 'deriveBits'])) {
throw lazyDOMException(
`Unsupported key usage for a ${name} key`,
'SyntaxError');
}

const checkLength = keyData.symmetricKeySize * 8;

// The Web Crypto spec allows for key lengths that are not multiples of
// 8. We don't. Our check here is stricter than that defined by the spec
// in that we require that algorithm.length match keyData.length * 8 if
// algorithm.length is specified.
if (length !== undefined && length !== checkLength) {
throw lazyDOMException('Invalid key length', 'DataError');
}
return new InternalCryptoKey(keyData, { name }, keyUsages, false);
keyObject = keyData;
break;
}
case 'raw': {
if (hasAnyNotIn(usagesSet, ['deriveKey', 'deriveBits'])) {
throw lazyDOMException(
`Unsupported key usage for a ${name} key`,
'SyntaxError');
}

const checkLength = keyData.byteLength * 8;

// The Web Crypto spec allows for key lengths that are not multiples of
// 8. We don't. Our check here is stricter than that defined by the spec
// in that we require that algorithm.length match keyData.length * 8 if
// algorithm.length is specified.
if (length !== undefined && length !== checkLength) {
throw lazyDOMException('Invalid key length', 'DataError');
}

const keyObject = createSecretKey(keyData);
return new InternalCryptoKey(keyObject, { name }, keyUsages, false);
keyObject = createSecretKey(keyData);
break;
}
}

if (keyObject) {
return new InternalCryptoKey(keyObject, { name }, keyUsages, false);
}

throw lazyDOMException(
`Unable to import ${name} key with format ${format}`,
'NotSupportedError');
Expand Down
Loading
Loading