From d6f662ac8280511fb4ef0f81777a0a6c5c08c0fa Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Sun, 11 Aug 2024 02:29:53 -0700 Subject: [PATCH] fix(ext/node): support ieee-p1363 ECDSA signatures and pss salt len (#24981) Fixes https://github.com/denoland/deno/issues/22919 --- Cargo.lock | 1 + ext/node/Cargo.toml | 1 + ext/node/ops/crypto/mod.rs | 21 ++++++- ext/node/ops/crypto/sign.rs | 71 +++++++++++++++++++--- ext/node/polyfills/internal/crypto/sig.ts | 47 ++++++++++++++ tests/unit_node/crypto/crypto_sign_test.ts | 30 ++++++++- 6 files changed, 158 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6a758ce09b..dfbd3e49d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1781,6 +1781,7 @@ dependencies = [ "digest", "dsa", "ecb", + "ecdsa", "ed25519-dalek", "elliptic-curve", "errno 0.2.8", diff --git a/ext/node/Cargo.toml b/ext/node/Cargo.toml index a0191b0255..3eee1a03fe 100644 --- a/ext/node/Cargo.toml +++ b/ext/node/Cargo.toml @@ -40,6 +40,7 @@ der = { version = "0.7.9", features = ["derive"] } digest = { version = "0.10.5", features = ["core-api", "std"] } dsa = "0.6.1" ecb.workspace = true +ecdsa = "0.16.9" ed25519-dalek = { version = "2.1.1", features = ["digest", "pkcs8", "rand_core", "signature"] } elliptic-curve.workspace = true errno = "0.2.8" diff --git a/ext/node/ops/crypto/mod.rs b/ext/node/ops/crypto/mod.rs index 05501fa87b..7384375773 100644 --- a/ext/node/ops/crypto/mod.rs +++ b/ext/node/ops/crypto/mod.rs @@ -362,18 +362,33 @@ pub fn op_node_sign( #[cppgc] handle: &KeyObjectHandle, #[buffer] digest: &[u8], #[string] digest_type: &str, + #[smi] pss_salt_length: Option, + #[smi] dsa_signature_encoding: u32, ) -> Result, AnyError> { - handle.sign_prehashed(digest_type, digest) + handle.sign_prehashed( + digest_type, + digest, + pss_salt_length, + dsa_signature_encoding, + ) } -#[op2(fast)] +#[op2] pub fn op_node_verify( #[cppgc] handle: &KeyObjectHandle, #[buffer] digest: &[u8], #[string] digest_type: &str, #[buffer] signature: &[u8], + #[smi] pss_salt_length: Option, + #[smi] dsa_signature_encoding: u32, ) -> Result { - handle.verify_prehashed(digest_type, digest, signature) + handle.verify_prehashed( + digest_type, + digest, + signature, + pss_salt_length, + dsa_signature_encoding, + ) } fn pbkdf2_sync( diff --git a/ext/node/ops/crypto/sign.rs b/ext/node/ops/crypto/sign.rs index 2dba15abad..b7779a5d80 100644 --- a/ext/node/ops/crypto/sign.rs +++ b/ext/node/ops/crypto/sign.rs @@ -17,12 +17,36 @@ use super::keys::EcPrivateKey; use super::keys::EcPublicKey; use super::keys::KeyObjectHandle; use super::keys::RsaPssHashAlgorithm; +use core::ops::Add; +use ecdsa::der::MaxOverhead; +use ecdsa::der::MaxSize; +use elliptic_curve::generic_array::ArrayLength; +use elliptic_curve::FieldBytesSize; + +fn dsa_signature( + encoding: u32, + signature: ecdsa::Signature, +) -> Result, AnyError> +where + MaxSize: ArrayLength, + as Add>::Output: Add + ArrayLength, +{ + match encoding { + // DER + 0 => Ok(signature.to_der().to_bytes().to_vec().into_boxed_slice()), + // IEEE P1363 + 1 => Ok(signature.to_bytes().to_vec().into_boxed_slice()), + _ => Err(type_error("invalid DSA signature encoding")), + } +} impl KeyObjectHandle { pub fn sign_prehashed( &self, digest_type: &str, digest: &[u8], + pss_salt_length: Option, + dsa_signature_encoding: u32, ) -> Result, AnyError> { let private_key = self .as_private_key() @@ -67,6 +91,9 @@ impl KeyObjectHandle { } None => {} }; + if let Some(s) = pss_salt_length { + salt_length = Some(s as usize); + } let pss = match_fixed_digest_with_oid!( digest_type, fn (algorithm: Option) { @@ -120,21 +147,24 @@ impl KeyObjectHandle { let signature: p224::ecdsa::Signature = signing_key .sign_prehash(digest) .map_err(|_| type_error("failed to sign digest"))?; - Ok(signature.to_der().to_bytes()) + + dsa_signature(dsa_signature_encoding, signature) } EcPrivateKey::P256(key) => { let signing_key = p256::ecdsa::SigningKey::from(key); let signature: p256::ecdsa::Signature = signing_key .sign_prehash(digest) .map_err(|_| type_error("failed to sign digest"))?; - Ok(signature.to_der().to_bytes()) + + dsa_signature(dsa_signature_encoding, signature) } EcPrivateKey::P384(key) => { let signing_key = p384::ecdsa::SigningKey::from(key); let signature: p384::ecdsa::Signature = signing_key .sign_prehash(digest) .map_err(|_| type_error("failed to sign digest"))?; - Ok(signature.to_der().to_bytes()) + + dsa_signature(dsa_signature_encoding, signature) } }, AsymmetricPrivateKey::X25519(_) => { @@ -154,6 +184,8 @@ impl KeyObjectHandle { digest_type: &str, digest: &[u8], signature: &[u8], + pss_salt_length: Option, + dsa_signature_encoding: u32, ) -> Result { let public_key = self .as_public_key() @@ -195,6 +227,9 @@ impl KeyObjectHandle { } None => {} }; + if let Some(s) = pss_salt_length { + salt_length = Some(s as usize); + } let pss = match_fixed_digest_with_oid!( digest_type, fn (algorithm: Option) { @@ -229,20 +264,38 @@ impl KeyObjectHandle { AsymmetricPublicKey::Ec(key) => match key { EcPublicKey::P224(key) => { let verifying_key = p224::ecdsa::VerifyingKey::from(key); - let signature = p224::ecdsa::Signature::from_der(signature) - .map_err(|_| type_error("Invalid ECDSA signature"))?; + let signature = if dsa_signature_encoding == 0 { + p224::ecdsa::Signature::from_der(signature) + } else { + p224::ecdsa::Signature::from_bytes(signature.into()) + }; + let Ok(signature) = signature else { + return Ok(false); + }; Ok(verifying_key.verify_prehash(digest, &signature).is_ok()) } EcPublicKey::P256(key) => { let verifying_key = p256::ecdsa::VerifyingKey::from(key); - let signature = p256::ecdsa::Signature::from_der(signature) - .map_err(|_| type_error("Invalid ECDSA signature"))?; + let signature = if dsa_signature_encoding == 0 { + p256::ecdsa::Signature::from_der(signature) + } else { + p256::ecdsa::Signature::from_bytes(signature.into()) + }; + let Ok(signature) = signature else { + return Ok(false); + }; Ok(verifying_key.verify_prehash(digest, &signature).is_ok()) } EcPublicKey::P384(key) => { let verifying_key = p384::ecdsa::VerifyingKey::from(key); - let signature = p384::ecdsa::Signature::from_der(signature) - .map_err(|_| type_error("Invalid ECDSA signature"))?; + let signature = if dsa_signature_encoding == 0 { + p384::ecdsa::Signature::from_der(signature) + } else { + p384::ecdsa::Signature::from_bytes(signature.into()) + }; + let Ok(signature) = signature else { + return Ok(false); + }; Ok(verifying_key.verify_prehash(digest, &signature).is_ok()) } }, diff --git a/ext/node/polyfills/internal/crypto/sig.ts b/ext/node/polyfills/internal/crypto/sig.ts index 3dd6b7c589..bcbcb469b9 100644 --- a/ext/node/polyfills/internal/crypto/sig.ts +++ b/ext/node/polyfills/internal/crypto/sig.ts @@ -58,6 +58,35 @@ export interface VerifyKeyObjectInput extends SigningOptions { key: KeyObject; } +function getSaltLength(options) { + return getIntOption("saltLength", options); +} + +function getDSASignatureEncoding(options) { + if (typeof options === "object") { + const { dsaEncoding = "der" } = options; + if (dsaEncoding === "der") { + return 0; + } else if (dsaEncoding === "ieee-p1363") { + return 1; + } + throw new ERR_INVALID_ARG_VALUE("options.dsaEncoding", dsaEncoding); + } + + return 0; +} + +function getIntOption(name, options) { + const value = options[name]; + if (value !== undefined) { + if (value === value >> 0) { + return value; + } + throw new ERR_INVALID_ARG_VALUE(`options.${name}`, value); + } + return undefined; +} + export type KeyLike = string | Buffer | KeyObject; export class SignImpl extends Writable { @@ -86,6 +115,13 @@ export class SignImpl extends Writable { encoding?: BinaryToTextEncoding, ): Buffer | string { const res = prepareAsymmetricKey(privateKey, kConsumePrivate); + + // Options specific to RSA-PSS + const pssSaltLength = getSaltLength(privateKey); + + // Options specific to (EC)DSA + const dsaSigEnc = getDSASignatureEncoding(privateKey); + let handle; if ("handle" in res) { handle = res.handle; @@ -101,6 +137,8 @@ export class SignImpl extends Writable { handle, this.hash.digest(), this.#digestType, + pssSaltLength, + dsaSigEnc, )); return encoding ? ret.toString(encoding) : ret; } @@ -152,6 +190,13 @@ export class VerifyImpl extends Writable { encoding?: BinaryToTextEncoding, ): boolean { const res = prepareAsymmetricKey(publicKey, kConsumePublic); + + // Options specific to RSA-PSS + const pssSaltLength = getSaltLength(publicKey); + + // Options specific to (EC)DSA + const dsaSigEnc = getDSASignatureEncoding(publicKey); + let handle; if ("handle" in res) { handle = res.handle; @@ -168,6 +213,8 @@ export class VerifyImpl extends Writable { this.hash.digest(), this.#digestType, Buffer.from(signature, encoding), + pssSaltLength, + dsaSigEnc, ); } } diff --git a/tests/unit_node/crypto/crypto_sign_test.ts b/tests/unit_node/crypto/crypto_sign_test.ts index 557eea08ed..c33c9758f4 100644 --- a/tests/unit_node/crypto/crypto_sign_test.ts +++ b/tests/unit_node/crypto/crypto_sign_test.ts @@ -1,7 +1,13 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. import { assert, assertEquals } from "@std/assert"; -import { createSign, createVerify, sign, verify } from "node:crypto"; +import { + createSign, + createVerify, + generateKeyPairSync, + sign, + verify, +} from "node:crypto"; import { Buffer } from "node:buffer"; import fixtures from "../testdata/crypto_digest_fixtures.json" with { type: "json", @@ -179,3 +185,25 @@ Deno.test("crypto.createVerify|verify - compare with node", async (t) => { }); } }); + +Deno.test("crypto sign|verify dsaEncoding", () => { + const { privateKey, publicKey } = generateKeyPairSync("ec", { + namedCurve: "P-256", + }); + + const sign = createSign("SHA256"); + sign.write("some data to sign"); + sign.end(); + + // @ts-ignore FIXME: types dont allow this + privateKey.dsaEncoding = "ieee-p1363"; + const signature = sign.sign(privateKey, "hex"); + + const verify = createVerify("SHA256"); + verify.write("some data to sign"); + verify.end(); + + // @ts-ignore FIXME: types dont allow this + publicKey.dsaEncoding = "ieee-p1363"; + assert(verify.verify(publicKey, signature, "hex")); +});