fix(ext/node): support ieee-p1363 ECDSA signatures and pss salt len (#24981)

Fixes https://github.com/denoland/deno/issues/22919

6 files changed, 158 insertions, 13 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 6a758ce09..dfbd3e49d 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 a0191b025..3eee1a03f 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 05501fa87..738437577 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<u32>,
+  #[smi] dsa_signature_encoding: u32,
 ) -> Result<Box<[u8]>, 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<u32>,
+  #[smi] dsa_signature_encoding: u32,
 ) -> Result<bool, AnyError> {
-  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 2dba15aba..b7779a5d8 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<C: elliptic_curve::PrimeCurve>(
+  encoding: u32,
+  signature: ecdsa::Signature<C>,
+) -> Result<Box<[u8]>, AnyError>
+where
+  MaxSize<C>: ArrayLength<u8>,
+  <FieldBytesSize<C> as Add>::Output: Add<MaxOverhead> + ArrayLength<u8>,
+{
+  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<u32>,
+    dsa_signature_encoding: u32,
   ) -> Result<Box<[u8]>, 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 <D>(algorithm: Option<RsaPssHashAlgorithm>) {
@@ -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<u32>,
+    dsa_signature_encoding: u32,
   ) -> Result<bool, AnyError> {
     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 <D>(algorithm: Option<RsaPssHashAlgorithm>) {
@@ -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 3dd6b7c58..bcbcb469b 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 557eea08e..c33c9758f 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"));
+});