Skip to content

Commit 5b9986d

Browse files
bartlomiejuclaude
andauthored
fix(ext/node): improve crypto.generateKeyPair validation (#32620)
## Summary - Restructures `generateKeyPair` to call `createJob` synchronously so validation errors throw synchronously (matching Node.js behavior) - Adds key encoding validation (format, type, cipher, passphrase) with proper Node.js error codes (`ERR_INVALID_ARG_VALUE`, `ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS`, `ERR_CRYPTO_UNKNOWN_CIPHER`) - Adds DH group name validation (`ERR_CRYPTO_UNKNOWN_DH_GROUP`) - Adds RSA-PSS hash algorithm validation (`ERR_CRYPTO_INVALID_DIGEST`) - Fixes RSA key generation to return `Result` instead of panicking on invalid parameters, and validates public exponents to prevent infinite loops with even/small exponents - Fixes EC curve error message to match Node.js - Supports parsing private key PEM/DER in `publicEncrypt` to extract the public key (matching Node.js behavior) - Rejects encrypt/decrypt operations on non-RSA key types (rsa-pss, dsa, ec, etc.) with "operation not supported for this keytype" - Rejects PKCS1 padding for RSA-PSS keys in sign/verify with "illegal or unsupported padding mode" - Enables 6 new Node compat tests: `test-crypto-keygen`, `test-crypto-keygen-sync`, `test-crypto-keygen-rsa-pss`, `test-crypto-keygen-eddsa`, `test-crypto-keygen-key-object-without-encoding`, `test-crypto-keygen-promisify` --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c0c896b commit 5b9986d

File tree

7 files changed

+286
-14
lines changed

7 files changed

+286
-14
lines changed

ext/node/polyfills/internal/crypto/cipher.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,10 +594,30 @@ function _lazyInitDecipherDecoder(self: any, encoding: string) {
594594
}
595595
}
596596

597+
const ENCRYPT_UNSUPPORTED_KEY_TYPES = new Set([
598+
"rsa-pss",
599+
"dsa",
600+
"ec",
601+
"ed25519",
602+
"ed448",
603+
"x25519",
604+
"x448",
605+
]);
606+
607+
function checkUnsupportedKeyType(key) {
608+
const keyType = isKeyObject(key)
609+
? key.asymmetricKeyType
610+
: key?.key?.asymmetricKeyType;
611+
if (keyType && ENCRYPT_UNSUPPORTED_KEY_TYPES.has(keyType)) {
612+
throw new Error("operation not supported for this keytype");
613+
}
614+
}
615+
597616
export function privateEncrypt(
598617
privateKey: ArrayBufferView | string | KeyObject,
599618
buffer: ArrayBufferView | string | KeyObject,
600619
): Buffer {
620+
checkUnsupportedKeyType(privateKey);
601621
const { data } = prepareKey(privateKey);
602622
const padding = privateKey.padding || 1;
603623

@@ -609,6 +629,7 @@ export function privateDecrypt(
609629
privateKey: ArrayBufferView | string | KeyObject,
610630
buffer: ArrayBufferView | string | KeyObject,
611631
): Buffer {
632+
checkUnsupportedKeyType(privateKey);
612633
const { data } = prepareKey(privateKey);
613634
const padding = privateKey.padding || 1;
614635

@@ -620,6 +641,7 @@ export function publicEncrypt(
620641
publicKey: ArrayBufferView | string | KeyObject,
621642
buffer: ArrayBufferView | string | KeyObject,
622643
): Buffer {
644+
checkUnsupportedKeyType(publicKey);
623645
const { data } = prepareKey(publicKey);
624646
const padding = publicKey.padding || 1;
625647

ext/node/polyfills/internal/crypto/keygen.ts

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,16 @@ import {
1212
SecretKeyObject,
1313
} from "ext:deno_node/internal/crypto/keys.ts";
1414
import {
15+
ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS,
16+
ERR_CRYPTO_INVALID_DIGEST,
17+
ERR_CRYPTO_UNKNOWN_CIPHER,
18+
ERR_CRYPTO_UNKNOWN_DH_GROUP,
1519
ERR_INCOMPATIBLE_OPTION_PAIR,
1620
ERR_INVALID_ARG_VALUE,
1721
ERR_MISSING_OPTION,
1822
} from "ext:deno_node/internal/errors.ts";
23+
import { getCiphers } from "ext:deno_node/internal/crypto/util.ts";
24+
import { getHashes } from "ext:deno_node/internal/crypto/hash.ts";
1925
import {
2026
validateBuffer,
2127
validateFunction,
@@ -578,6 +584,8 @@ export function generateKeyPair(
578584
callback = options;
579585
options = undefined;
580586
}
587+
validateFunction(callback, "callback");
588+
581589
_generateKeyPair(type, options)
582590
.then(
583591
(res) => callback!(null, res.publicKey, res.privateKey),
@@ -819,9 +827,188 @@ export function generateKeyPairSync(
819827
const kSync = 0;
820828
const kAsync = 1;
821829

830+
function parseKeyFormat(
831+
formatStr: string | undefined,
832+
defaultFormat: string | undefined,
833+
optionName: string,
834+
): string | undefined {
835+
if (formatStr === undefined && defaultFormat !== undefined) {
836+
return defaultFormat;
837+
} else if (formatStr === "pem") {
838+
return "pem";
839+
} else if (formatStr === "der") {
840+
return "der";
841+
} else if (formatStr === "jwk") {
842+
return "jwk";
843+
}
844+
throw new ERR_INVALID_ARG_VALUE(optionName, formatStr);
845+
}
846+
847+
function parseKeyType(
848+
typeStr: string | undefined,
849+
required: boolean,
850+
keyType: string | undefined,
851+
isPublic: boolean | undefined,
852+
optionName: string,
853+
): string | undefined {
854+
if (typeStr === undefined && !required) {
855+
return undefined;
856+
} else if (typeStr === "pkcs1") {
857+
if (keyType !== undefined && keyType !== "rsa") {
858+
throw new ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(
859+
typeStr,
860+
"can only be used for RSA keys",
861+
);
862+
}
863+
return "pkcs1";
864+
} else if (typeStr === "spki" && isPublic !== false) {
865+
return "spki";
866+
} else if (typeStr === "pkcs8" && isPublic !== true) {
867+
return "pkcs8";
868+
} else if (typeStr === "sec1" && isPublic !== true) {
869+
if (keyType !== undefined && keyType !== "ec") {
870+
throw new ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(
871+
typeStr,
872+
"can only be used for EC keys",
873+
);
874+
}
875+
return "sec1";
876+
}
877+
878+
throw new ERR_INVALID_ARG_VALUE(optionName, typeStr);
879+
}
880+
881+
function option(name: string, objName?: string): string {
882+
return objName === undefined
883+
? `options.${name}`
884+
: `options.${objName}.${name}`;
885+
}
886+
887+
function isStringOrBuffer(val: unknown): boolean {
888+
return typeof val === "string" ||
889+
ArrayBuffer.isView(val) ||
890+
val instanceof ArrayBuffer ||
891+
val instanceof SharedArrayBuffer;
892+
}
893+
894+
function parseKeyFormatAndType(
895+
enc: any,
896+
keyType: string | undefined,
897+
isPublic: boolean | undefined,
898+
objName?: string,
899+
) {
900+
const { format: formatStr, type: typeStr } = enc;
901+
902+
const isInput = keyType === undefined;
903+
const format = parseKeyFormat(
904+
formatStr,
905+
isInput ? "pem" : undefined,
906+
option("format", objName),
907+
);
908+
909+
const isRequired = (!isInput || format === "der") && format !== "jwk";
910+
const type = parseKeyType(
911+
typeStr,
912+
isRequired,
913+
keyType,
914+
isPublic,
915+
option("type", objName),
916+
);
917+
return { format, type };
918+
}
919+
920+
function parsePublicKeyEncoding(
921+
enc: any,
922+
keyType: string | undefined,
923+
objName: string,
924+
) {
925+
validateObject(enc, "options");
926+
927+
const { format, type } = parseKeyFormatAndType(
928+
enc,
929+
keyType,
930+
keyType ? true : undefined,
931+
objName,
932+
);
933+
934+
return { format, type };
935+
}
936+
937+
function parsePrivateKeyEncoding(
938+
enc: any,
939+
keyType: string | undefined,
940+
objName: string,
941+
) {
942+
validateObject(enc, "options");
943+
944+
const { format, type } = parseKeyFormatAndType(enc, keyType, false, objName);
945+
946+
const { cipher, passphrase } = enc;
947+
948+
if (cipher != null) {
949+
if (typeof cipher !== "string") {
950+
throw new ERR_INVALID_ARG_VALUE(option("cipher", objName), cipher);
951+
}
952+
if (!getCiphers().includes(cipher)) {
953+
throw new ERR_CRYPTO_UNKNOWN_CIPHER();
954+
}
955+
if (
956+
format === "der" &&
957+
(type === "pkcs1" || type === "sec1")
958+
) {
959+
throw new ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS(
960+
type,
961+
"does not support encryption",
962+
);
963+
}
964+
} else if (passphrase !== undefined) {
965+
throw new ERR_INVALID_ARG_VALUE(option("cipher", objName), cipher);
966+
}
967+
968+
if (cipher != null && !isStringOrBuffer(passphrase)) {
969+
throw new ERR_INVALID_ARG_VALUE(option("passphrase", objName), passphrase);
970+
}
971+
972+
return { format, type, cipher, passphrase };
973+
}
974+
975+
function validateKeyEncoding(keyType: string, options: any) {
976+
if (options == null || typeof options !== "object") return;
977+
978+
const { publicKeyEncoding, privateKeyEncoding } = options;
979+
980+
if (publicKeyEncoding != null) {
981+
if (typeof publicKeyEncoding === "object") {
982+
parsePublicKeyEncoding(publicKeyEncoding, keyType, "publicKeyEncoding");
983+
} else {
984+
throw new ERR_INVALID_ARG_VALUE(
985+
"options.publicKeyEncoding",
986+
publicKeyEncoding,
987+
);
988+
}
989+
}
990+
991+
if (privateKeyEncoding != null) {
992+
if (typeof privateKeyEncoding === "object") {
993+
parsePrivateKeyEncoding(
994+
privateKeyEncoding,
995+
keyType,
996+
"privateKeyEncoding",
997+
);
998+
} else {
999+
throw new ERR_INVALID_ARG_VALUE(
1000+
"options.privateKeyEncoding",
1001+
privateKeyEncoding,
1002+
);
1003+
}
1004+
}
1005+
}
1006+
8221007
function createJob(mode, type, options) {
8231008
validateString(type, "type");
8241009

1010+
validateKeyEncoding(type, options);
1011+
8251012
if (options !== undefined) {
8261013
validateObject(options, "options");
8271014
}
@@ -867,9 +1054,15 @@ function createJob(mode, type, options) {
8671054
}
8681055
if (hashAlgorithm !== undefined) {
8691056
validateString(hashAlgorithm, "options.hashAlgorithm");
1057+
if (!getHashes().includes(hashAlgorithm)) {
1058+
throw new ERR_CRYPTO_INVALID_DIGEST(hashAlgorithm);
1059+
}
8701060
}
8711061
if (mgf1HashAlgorithm !== undefined) {
8721062
validateString(mgf1HashAlgorithm, "options.mgf1HashAlgorithm");
1063+
if (!getHashes().includes(mgf1HashAlgorithm)) {
1064+
throw new ERR_CRYPTO_INVALID_DIGEST(mgf1HashAlgorithm, "MGF1");
1065+
}
8731066
}
8741067
if (hash !== undefined) {
8751068
process.emitWarning(
@@ -995,6 +1188,13 @@ function createJob(mode, type, options) {
9951188

9961189
validateString(group, "options.group");
9971190

1191+
if (
1192+
group !== "modp5" && group !== "modp14" && group !== "modp15" &&
1193+
group !== "modp16" && group !== "modp17" && group !== "modp18"
1194+
) {
1195+
throw new ERR_CRYPTO_UNKNOWN_DH_GROUP();
1196+
}
1197+
9981198
if (mode === kSync) {
9991199
return op_node_generate_dh_group_key(group);
10001200
} else {

ext/node/polyfills/internal/errors.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,15 @@ export class ERR_CRYPTO_UNKNOWN_DH_GROUP extends NodeError {
10001000
}
10011001
}
10021002

1003+
export class ERR_CRYPTO_UNKNOWN_CIPHER extends NodeError {
1004+
constructor() {
1005+
super(
1006+
"ERR_CRYPTO_UNKNOWN_CIPHER",
1007+
"Unknown cipher",
1008+
);
1009+
}
1010+
}
1011+
10031012
export class ERR_CRYPTO_ENGINE_UNKNOWN extends NodeError {
10041013
constructor(x: string) {
10051014
super("ERR_CRYPTO_ENGINE_UNKNOWN", `Engine "${x}" was not found`);
@@ -1052,8 +1061,11 @@ export class ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS extends NodeError {
10521061
}
10531062

10541063
export class ERR_CRYPTO_INVALID_DIGEST extends NodeTypeError {
1055-
constructor(x: string) {
1056-
super("ERR_CRYPTO_INVALID_DIGEST", `Invalid digest: ${x}`);
1064+
constructor(x: string, prefix?: string) {
1065+
super(
1066+
"ERR_CRYPTO_INVALID_DIGEST",
1067+
prefix ? `Invalid ${prefix} digest: ${x}` : `Invalid digest: ${x}`,
1068+
);
10571069
}
10581070
}
10591071

ext/node_crypto/keys.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2536,26 +2536,33 @@ pub fn op_node_get_private_key_from_pair(
25362536
fn generate_rsa(
25372537
modulus_length: usize,
25382538
public_exponent: usize,
2539-
) -> KeyObjectHandlePair {
2539+
) -> Result<KeyObjectHandlePair, JsErrorBox> {
2540+
if public_exponent <= 1 || public_exponent.is_multiple_of(2) {
2541+
return Err(JsErrorBox::generic(format!(
2542+
"invalid RSA public exponent: {}",
2543+
public_exponent
2544+
)));
2545+
}
2546+
25402547
let private_key = RsaPrivateKey::new_with_exp(
25412548
&mut thread_rng(),
25422549
modulus_length,
25432550
&rsa::BigUint::from_usize(public_exponent).unwrap(),
25442551
)
2545-
.unwrap();
2552+
.map_err(|e| JsErrorBox::generic(e.to_string()))?;
25462553

25472554
let private_key = AsymmetricPrivateKey::Rsa(private_key);
25482555
let public_key = private_key.to_public_key();
25492556

2550-
KeyObjectHandlePair::new(private_key, public_key)
2557+
Ok(KeyObjectHandlePair::new(private_key, public_key))
25512558
}
25522559

25532560
#[op2]
25542561
#[cppgc]
25552562
pub fn op_node_generate_rsa_key(
25562563
#[smi] modulus_length: usize,
25572564
#[smi] public_exponent: usize,
2558-
) -> KeyObjectHandlePair {
2565+
) -> Result<KeyObjectHandlePair, JsErrorBox> {
25592566
generate_rsa(modulus_length, public_exponent)
25602567
}
25612568

@@ -2564,7 +2571,7 @@ pub fn op_node_generate_rsa_key(
25642571
pub async fn op_node_generate_rsa_key_async(
25652572
#[smi] modulus_length: usize,
25662573
#[smi] public_exponent: usize,
2567-
) -> KeyObjectHandlePair {
2574+
) -> Result<KeyObjectHandlePair, JsErrorBox> {
25682575
spawn_blocking(move || generate_rsa(modulus_length, public_exponent))
25692576
.await
25702577
.unwrap()
@@ -2747,10 +2754,7 @@ fn ec_generate(named_curve: &str) -> Result<KeyObjectHandlePair, JsErrorBox> {
27472754
AsymmetricPrivateKey::Ec(EcPrivateKey::Secp256k1(key))
27482755
}
27492756
_ => {
2750-
return Err(JsErrorBox::type_error(format!(
2751-
"unsupported named curve: {}",
2752-
named_curve
2753-
)));
2757+
return Err(JsErrorBox::type_error("Invalid EC curve name"));
27542758
}
27552759
};
27562760
let public_key = private_key.to_public_key();

0 commit comments

Comments
 (0)