Skip to content

Commit

Permalink
crypto: refactor to avoid unsafe array iteration
Browse files Browse the repository at this point in the history
PR-URL: #37364
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
  • Loading branch information
aduh95 authored and targos committed Feb 28, 2021
1 parent d2e5529 commit b634469
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 36 deletions.
7 changes: 4 additions & 3 deletions lib/internal/crypto/aes.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
ArrayPrototypePush,
MathFloor,
Promise,
ReflectApply,
SafeSet,
TypedArrayPrototypeSlice,
} = primordials;
Expand Down Expand Up @@ -252,12 +253,12 @@ async function aesImportKey(
extractable,
keyUsages) {
const { name } = algorithm;
const checkUsages = ['wrapKey', 'unwrapKey'];
const usagesSet = new SafeSet(keyUsages);
const checkUsages = [usagesSet, 'wrapKey', 'unwrapKey'];
if (name !== 'AES-KW')
ArrayPrototypePush(checkUsages, 'encrypt', 'decrypt');

const usagesSet = new SafeSet(keyUsages);
if (hasAnyNotIn(usagesSet, ...checkUsages)) {
if (ReflectApply(hasAnyNotIn, null, checkUsages)) {
throw lazyDOMException(
'Unsupported key usage for an AES key',
'SyntaxError');
Expand Down
9 changes: 5 additions & 4 deletions lib/internal/crypto/diffiehellman.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

const {
ArrayBufferPrototypeSlice,
ArrayPrototypePush,
FunctionPrototypeCall,
MathFloor,
ObjectDefineProperty,
Promise,
ReflectApply,
SafeSet,
} = primordials;

Expand Down Expand Up @@ -347,16 +349,15 @@ function deriveBitsDH(publicKey, privateKey, callback) {
}

function verifyAcceptableDhKeyUse(name, type, usages) {
let checkSet;
const args = [usages];
switch (type) {
case 'private':
checkSet = ['deriveBits', 'deriveKey'];
ArrayPrototypePush(args, 'deriveBits', 'deriveKey');
break;
case 'public':
checkSet = [];
break;
}
if (hasAnyNotIn(usages, ...checkSet)) {
if (ReflectApply(hasAnyNotIn, null, args)) {
throw lazyDOMException(
`Unsupported key usage for an ${name} key`,
'SyntaxError');
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/crypto/dsa.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,16 @@ const {
} = require('internal/crypto/util');

function verifyAcceptableDsaKeyUse(name, type, usages) {
let checkSet;
let check;
switch (type) {
case 'private':
checkSet = ['sign'];
check = 'sign';
break;
case 'public':
checkSet = ['verify'];
check = 'verify';
break;
}
if (hasAnyNotIn(usages, ...checkSet)) {
if (hasAnyNotIn(usages, check)) {
throw lazyDOMException(
`Unsupported key usage for an ${name} key`,
'SyntaxError');
Expand Down
12 changes: 7 additions & 5 deletions lib/internal/crypto/ec.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
'use strict';

const {
ArrayPrototypePush,
ObjectKeys,
Promise,
ReflectApply,
SafeSet,
} = primordials;

Expand Down Expand Up @@ -59,10 +61,10 @@ const {
} = require('internal/crypto/keys');

function verifyAcceptableEcKeyUse(name, type, usages) {
let checkSet;
const args = [usages];
switch (name) {
case 'ECDH':
checkSet = ['deriveKey', 'deriveBits'];
ArrayPrototypePush(args, 'deriveKey', 'deriveBits');
break;
case 'NODE-ED25519':
// Fall through
Expand All @@ -71,14 +73,14 @@ function verifyAcceptableEcKeyUse(name, type, usages) {
case 'ECDSA':
switch (type) {
case 'private':
checkSet = ['sign'];
ArrayPrototypePush(args, 'sign');
break;
case 'public':
checkSet = ['verify'];
ArrayPrototypePush(args, 'verify');
break;
}
}
if (hasAnyNotIn(usages, ...checkSet)) {
if (ReflectApply(hasAnyNotIn, null, args)) {
throw lazyDOMException(
`Unsupported key usage for a ${name} key`,
'SyntaxError');
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/crypto/hkdf.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function hkdfSync(hash, key, salt, info, length) {
} = validateParameters(hash, key, salt, info, length));

const job = new HKDFJob(kCryptoJobSync, hash, key, salt, info, length);
const [err, bits] = job.run();
const { 0: err, 1: bits } = job.run();
if (err !== undefined)
throw err;

Expand Down
11 changes: 6 additions & 5 deletions lib/internal/crypto/keygen.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {
FunctionPrototypeCall,
ObjectDefineProperty,
SafeArrayIterator,
} = primordials;

const {
Expand Down Expand Up @@ -75,7 +76,7 @@ function generateKeyPair(type, options, callback) {
job.ondone = (error, result) => {
if (error) return FunctionPrototypeCall(callback, job, error);
// If no encoding was chosen, return key objects instead.
let [pubkey, privkey] = result;
let { 0: pubkey, 1: privkey } = result;
pubkey = wrapKey(pubkey, PublicKeyObject);
privkey = wrapKey(privkey, PrivateKeyObject);
FunctionPrototypeCall(callback, job, null, pubkey, privkey);
Expand All @@ -97,11 +98,11 @@ function handleError(ret) {
if (ret == null)
return; // async

const [err, keys] = ret;
const { 0: err, 1: keys } = ret;
if (err !== undefined)
throw err;

const [publicKey, privateKey] = keys;
const { 0: publicKey, 1: privateKey } = keys;

// If no encoding was chosen, return key objects instead.
return {
Expand Down Expand Up @@ -156,7 +157,7 @@ function parseKeyEncoding(keyType, options = {}) {
function createJob(mode, type, options) {
validateString(type, 'type');

const encoding = parseKeyEncoding(type, options);
const encoding = new SafeArrayIterator(parseKeyEncoding(type, options));

if (options !== undefined)
validateObject(options, 'options');
Expand Down Expand Up @@ -341,7 +342,7 @@ function handleGenerateKeyError(ret) {
if (ret === undefined)
return; // async

const [err, key] = ret;
const { 0: err, 1: key } = ret;
if (err !== undefined)
throw err;

Expand Down
20 changes: 17 additions & 3 deletions lib/internal/crypto/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const {
ArrayFrom,
ArrayPrototypeSlice,
ObjectDefineProperty,
ObjectSetPrototypeOf,
Symbol,
Expand Down Expand Up @@ -160,6 +161,11 @@ const [
}

class AsymmetricKeyObject extends KeyObject {
// eslint-disable-next-line no-useless-constructor
constructor(type, handle) {
super(type, handle);
}

get asymmetricKeyType() {
return this[kAsymmetricKeyType] ||
(this[kAsymmetricKeyType] = this[kHandle].getAsymmetricKeyType());
Expand Down Expand Up @@ -390,13 +396,21 @@ function getKeyObjectHandle(key, ctx) {
}

function getKeyTypes(allowKeyObject, bufferOnly = false) {
return [
const types = [
'ArrayBuffer',
'Buffer',
'TypedArray',
'DataView',
...(!bufferOnly ? ['string'] : []),
...(!bufferOnly && allowKeyObject ? ['KeyObject', 'CryptoKey'] : [])];
'string', // Only if bufferOnly == false
'KeyObject', // Only if allowKeyObject == true && bufferOnly == false
'CryptoKey', // Only if allowKeyObject == true && bufferOnly == false
];
if (bufferOnly) {
return ArrayPrototypeSlice(types, 0, 4);
} else if (!allowKeyObject) {
return ArrayPrototypeSlice(types, 0, 5);
}
return types;
}

function prepareAsymmetricKey(key, ctx) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/crypto/pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function pbkdf2Sync(password, salt, iterations, keylen, digest) {
keylen,
digest);

const [err, result] = job.run();
const { 0: err, 1: result } = job.run();
if (err !== undefined)
throw err;

Expand Down
6 changes: 3 additions & 3 deletions lib/internal/crypto/random.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ function randomFillSync(buf, offset = 0, size) {
offset,
size);

const [ err ] = job.run();
const err = job.run()[0];
if (err)
throw err;

Expand Down Expand Up @@ -471,7 +471,7 @@ function generatePrimeSync(size, options = {}) {
validateUint32(size, 'size', true);

const job = createRandomPrimeJob(kCryptoJobSync, size, options);
const [err, prime] = job.run();
const { 0: err, 1: prime } = job.run();
if (err)
throw err;
return job.result(prime);
Expand Down Expand Up @@ -548,7 +548,7 @@ function checkPrimeSync(candidate, options = {}) {
validateUint32(checks, 'options.checks');

const job = new CheckPrimeJob(kCryptoJobSync, candidate, checks);
const [err, result] = job.run();
const { 0: err, 1: result } = job.run();
if (err)
throw err;

Expand Down
14 changes: 8 additions & 6 deletions lib/internal/crypto/rsa.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
'use strict';

const {
ArrayPrototypePush,
Promise,
ReflectApply,
SafeSet,
Uint8Array,
} = primordials;
Expand Down Expand Up @@ -71,29 +73,29 @@ const kRsaVariants = {
};

function verifyAcceptableRsaKeyUse(name, type, usages) {
let checkSet;
const args = [usages];
switch (name) {
case 'RSA-OAEP':
switch (type) {
case 'private':
checkSet = ['decrypt', 'unwrapKey'];
ArrayPrototypePush(args, 'decrypt', 'unwrapKey');
break;
case 'public':
checkSet = ['encrypt', 'wrapKey'];
ArrayPrototypePush(args, 'encrypt', 'wrapKey');
break;
}
break;
default:
switch (type) {
case 'private':
checkSet = ['sign'];
ArrayPrototypePush(args, 'sign');
break;
case 'public':
checkSet = ['verify'];
ArrayPrototypePush(args, 'verify');
break;
}
}
if (hasAnyNotIn(usages, ...checkSet)) {
if (ReflectApply(hasAnyNotIn, null, args)) {
throw lazyDOMException(
`Unsupported key usage for an ${name} key`,
'SyntaxError');
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/crypto/scrypt.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function scryptSync(password, salt, keylen, options = defaults) {
({ password, salt, keylen } = options);
const job = new ScryptJob(
kCryptoJobSync, password, salt, N, r, p, maxmem, keylen);
const [err, result] = job.run();
const { 0: err, 1: result } = job.run();

if (err !== undefined)
throw err;
Expand Down

0 comments on commit b634469

Please sign in to comment.