Repository: denoland/deno
Pull request: fix(ext/node): improve sign/verify crypto compat with Node.js

## Summary

Sign/verify fixes extracted from #32745 for easier review:

- **DSA private key PEM**: Parse traditional "DSA PRIVATE KEY" format (ASN.1 sequence with p, q, g, pub_key, priv_key)
- **Private-to-public key derivation**: When verify receives a private key (pkcs8, sec1), derive the public key from it instead of failing
- **RSA-PSS salt length constants**: Support `-1` (RSA_PSS_SALTLEN_DIGEST) and `-2` (RSA_PSS_SALTLEN_MAX_SIGN/AUTO) via `resolve_pss_salt_length()` helper; changed `pss_salt_length` type from `u32` to `i32`
- **Better error messages**: `rsa::Error::MessageTooLong` → "digest too big for rsa key"
- **DSA verify**: Return `false` for invalid signature format instead of throwing
- **Callback error handling**: Wrap `signOneShot`/`verifyOneShot` in try-catch to pass errors to callbacks
- **Key type validation**: Reject x25519, x448, dh keys in verify with "operation not supported for this keytype"

## Split plan from #32745

| PR | Scope | Status |
|---|---|---|
| #33079 | GCM fixes | Landed |
| This PR | Sign/verify fixes | Ready |
| TBD | ChaCha20-Poly1305 | Next |
| TBD | AES-CCM | Last |

## Test plan
- Enabled `test-crypto-async-sign-verify.js` node compat test — passes
- All 10 crypto unit test suites pass (`./x test-node crypto`)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Changed files:
filename: ext/node/polyfills/internal/crypto/sig.ts
patch: @@ -13,6 +13,7 @@ const {
 import {
   op_node_create_private_key,
   op_node_create_public_key,
+  op_node_derive_public_key_from_private_key,
   op_node_get_asymmetric_key_type,
   op_node_sign,
   op_node_sign_ed25519,
@@ -101,6 +102,13 @@ function getIntOption(name, options) {
   return undefined;
 }
 
+// Private key types that need to be converted to public keys for verification
+const PRIVATE_KEY_TYPES = ["pkcs8", "sec1"];
+
+function isPrivateKeyType(type: string | undefined): boolean {
+  return type !== undefined && PRIVATE_KEY_TYPES.includes(type);
+}
+
 export type KeyLike = string | Buffer | KeyObject;
 
 export class SignImpl extends Writable {
@@ -221,6 +229,14 @@ export class VerifyImpl extends Writable {
     let handle;
     if ("handle" in res) {
       handle = res.handle;
+    } else if (isPrivateKeyType(res.type)) {
+      const privateHandle = op_node_create_private_key(
+        res.data,
+        res.format,
+        res.type ?? "",
+        res.passphrase,
+      );
+      handle = op_node_derive_public_key_from_private_key(privateHandle);
     } else {
       handle = op_node_create_public_key(
         res.data,
@@ -265,48 +281,56 @@ export function signOneShot(
     throw new ERR_CRYPTO_SIGN_KEY_REQUIRED();
   }
 
-  const res = prepareAsymmetricKey(key, kConsumePrivate);
-  let handle;
-  if ("handle" in res) {
-    handle = res.handle;
-  } else {
-    handle = op_node_create_private_key(
-      res.data,
-      res.format,
-      res.type ?? "",
-      res.passphrase,
-    );
-  }
+  try {
+    const res = prepareAsymmetricKey(key, kConsumePrivate);
+    let handle;
+    if ("handle" in res) {
+      handle = res.handle;
+    } else {
+      handle = op_node_create_private_key(
+        res.data,
+        res.format,
+        res.type ?? "",
+        res.passphrase,
+      );
+    }
 
-  let result: Buffer;
-  const keyType = op_node_get_asymmetric_key_type(handle);
-  if (keyType === "ed25519") {
-    if (algorithm != null && algorithm !== "sha512") {
-      throw new TypeError("Only 'sha512' is supported for Ed25519 keys");
+    let result: Buffer;
+    const keyType = op_node_get_asymmetric_key_type(handle);
+    if (keyType === "ed25519") {
+      if (algorithm != null && algorithm !== "sha512") {
+        throw new TypeError("Only 'sha512' is supported for Ed25519 keys");
+      }
+      result = new FastBuffer(64);
+      op_node_sign_ed25519(handle, data, result);
+    } else if (keyType === "ed448") {
+      result = new FastBuffer(114);
+      op_node_sign_ed448(handle, data, result);
+    } else if (algorithm == null) {
+      throw new TypeError(
+        "Algorithm must be specified when using non-Ed25519 keys",
+      );
+    } else {
+      // Preserve padding/saltLength options from the original key
+      const privateKeyObject = new PrivateKeyObject(handle);
+      const signKey = typeof key === "object" && !(key instanceof KeyObject)
+        ? { ...key, key: privateKeyObject }
+        : privateKeyObject;
+      result = Sign(algorithm!).update(data)
+        .sign(signKey);
     }
-    result = new FastBuffer(64);
-    op_node_sign_ed25519(handle, data, result);
-  } else if (keyType === "ed448") {
-    result = new FastBuffer(114);
-    op_node_sign_ed448(handle, data, result);
-  } else if (algorithm == null) {
-    throw new TypeError(
-      "Algorithm must be specified when using non-Ed25519 keys",
-    );
-  } else {
-    // Preserve padding/saltLength options from the original key
-    const privateKeyObject = new PrivateKeyObject(handle);
-    const signKey = typeof key === "object" && !(key instanceof KeyObject)
-      ? { ...key, key: privateKeyObject }
-      : privateKeyObject;
-    result = Sign(algorithm!).update(data)
-      .sign(signKey);
-  }
 
-  if (callback) {
-    setTimeout(() => callback(null, result));
-  } else {
-    return result;
+    if (callback) {
+      setTimeout(() => callback(null, result));
+    } else {
+      return result;
+    }
+  } catch (err) {
+    if (callback) {
+      setTimeout(() => callback(err, Buffer.alloc(0)));
+    } else {
+      throw err;
+    }
   }
 }
 
@@ -329,46 +353,66 @@ export function verifyOneShot(
     throw new ERR_CRYPTO_SIGN_KEY_REQUIRED();
   }
 
-  const res = prepareAsymmetricKey(key, kConsumePublic);
-  let handle;
-  if ("handle" in res) {
-    handle = res.handle;
-  } else {
-    handle = op_node_create_public_key(
-      res.data,
-      res.format,
-      res.type ?? "",
-      res.passphrase,
-    );
-  }
+  try {
+    const res = prepareAsymmetricKey(key, kConsumePublic);
+    let handle;
+    if ("handle" in res) {
+      handle = res.handle;
+    } else if (isPrivateKeyType(res.type)) {
+      const privateHandle = op_node_create_private_key(
+        res.data,
+        res.format,
+        res.type ?? "",
+        res.passphrase,
+      );
+      handle = op_node_derive_public_key_from_private_key(privateHandle);
+    } else {
+      handle = op_node_create_public_key(
+        res.data,
+        res.format,
+        res.type ?? "",
+        res.passphrase,
+      );
+    }
 
-  let result: boolean;
-  const keyType = op_node_get_asymmetric_key_type(handle);
-  if (keyType === "ed25519") {
-    if (algorithm != null && algorithm !== "sha512") {
-      throw new TypeError("Only 'sha512' is supported for Ed25519 keys");
+    let result: boolean;
+    const keyType = op_node_get_asymmetric_key_type(handle);
+    if (keyType === "ed25519") {
+      if (algorithm != null && algorithm !== "sha512") {
+        throw new TypeError("Only 'sha512' is supported for Ed25519 keys");
+      }
+      result = op_node_verify_ed25519(handle, data, signature);
+    } else if (keyType === "ed448") {
+      result = op_node_verify_ed448(handle, data, signature);
+    } else if (keyType === "x25519" || keyType === "x448" || keyType === "dh") {
+      throw new TypeError(
+        "operation not supported for this keytype",
+      );
+    } else if (algorithm == null) {
+      throw new TypeError(
+        "no default digest",
+      );
+    } else {
+      // Preserve padding/saltLength options from the original key
+      const publicKeyObject = new PublicKeyObject(handle);
+      const verifyKey = typeof key === "object" && !(key instanceof KeyObject)
+        ? { ...key, key: publicKeyObject }
+        : publicKeyObject;
+      result = Verify(algorithm!).update(data)
+        .verify(verifyKey, signature);
     }
-    result = op_node_verify_ed25519(handle, data, signature);
-  } else if (keyType === "ed448") {
-    result = op_node_verify_ed448(handle, data, signature);
-  } else if (algorithm == null) {
-    throw new TypeError(
-      "Algorithm must be specified when using non-Ed25519 keys",
-    );
-  } else {
-    // Preserve padding/saltLength options from the original key
-    const publicKeyObject = new PublicKeyObject(handle);
-    const verifyKey = typeof key === "object" && !(key instanceof KeyObject)
-      ? { ...key, key: publicKeyObject }
-      : publicKeyObject;
-    result = Verify(algorithm!).update(data)
-      .verify(verifyKey, signature);
-  }
 
-  if (callback) {
-    setTimeout(() => callback(null, result));
-  } else {
-    return result;
+    if (callback) {
+      setTimeout(() => callback(null, result));
+    } else {
+      return result;
+    }
+  } catch (err) {
+    if (callback) {
+      setTimeout(() => callback(err, false));
+    } else {
+      throw err;
+    }
   }
 }
 

filename: ext/node_crypto/keys.rs
patch: @@ -872,6 +872,22 @@ impl KeyObjectHandle {
               SecretDocument::from_sec1_der(doc.as_bytes())
                 .map_err(|_| AsymmetricPrivateKeyError::InvalidSec1PrivateKey)?
             }
+            "DSA PRIVATE KEY" => {
+              // Traditional DSA private key format:
+              // DSAPrivateKey ::= SEQUENCE {
+              //   version  INTEGER,
+              //   p        INTEGER,
+              //   q        INTEGER,
+              //   g        INTEGER,
+              //   pub_key  INTEGER,
+              //   priv_key INTEGER
+              // }
+              let private_key =
+                parse_traditional_dsa_private_key(doc.as_bytes())?;
+              return Ok(KeyObjectHandle::AsymmetricPrivate(
+                AsymmetricPrivateKey::Dsa(private_key),
+              ));
+            }
             _ => {
               return Err(AsymmetricPrivateKeyError::UnsupportedPemLabel(
                 label.to_string(),
@@ -1327,7 +1343,8 @@ impl KeyObjectHandle {
           EncryptedPrivateKeyInfo::PEM_LABEL
           | PrivateKeyInfo::PEM_LABEL
           | sec1::EcPrivateKey::PEM_LABEL
-          | rsa::pkcs1::RsaPrivateKey::PEM_LABEL => {
+          | rsa::pkcs1::RsaPrivateKey::PEM_LABEL
+          | "DSA PRIVATE KEY" => {
             let handle = KeyObjectHandle::new_asymmetric_private_key_from_js(
               key, format, typ, passphrase,
             )?;
@@ -1516,6 +1533,60 @@ pub enum RsaPssParamsParseError {
   MalformedOrMissingPssMaskGenAlgorithm,
 }
 
+/// Parse a traditional DSA private key (PEM label "DSA PRIVATE KEY").
+///
+/// The traditional format is:
+/// ```asn1
+/// DSAPrivateKey ::= SEQUENCE {
+///   version  INTEGER,
+///   p        INTEGER,
+///   q        INTEGER,
+///   g        INTEGER,
+///   pub_key  INTEGER,
+///   priv_key INTEGER
+/// }
+/// ```
+fn parse_traditional_dsa_private_key(
+  der: &[u8],
+) -> Result<dsa::SigningKey, AsymmetricPrivateKeyError> {
+  use spki::der::Decode;
+  use spki::der::Reader as _;
+  use spki::der::SliceReader;
+
+  let err = || AsymmetricPrivateKeyError::InvalidDsaPrivateKey;
+
+  let mut reader = SliceReader::new(der).map_err(|_| err())?;
+  reader
+    .sequence(|seq_reader| {
+      // version
+      let _version = asn1::UintRef::decode(seq_reader)?;
+      // p
+      let p_ref = asn1::UintRef::decode(seq_reader)?;
+      // q
+      let q_ref = asn1::UintRef::decode(seq_reader)?;
+      // g
+      let g_ref = asn1::UintRef::decode(seq_reader)?;
+      // y (public key)
+      let y_ref = asn1::UintRef::decode(seq_reader)?;
+      // x (private key)
+      let x_ref = asn1::UintRef::decode(seq_reader)?;
+
+      let p = num_bigint_dig::BigUint::from_bytes_be(p_ref.as_bytes());
+      let q = num_bigint_dig::BigUint::from_bytes_be(q_ref.as_bytes());
+      let g = num_bigint_dig::BigUint::from_bytes_be(g_ref.as_bytes());
+      let y = num_bigint_dig::BigUint::from_bytes_be(y_ref.as_bytes());
+      let x = num_bigint_dig::BigUint::from_bytes_be(x_ref.as_bytes());
+
+      let components = dsa::Components::from_components(p, q, g)
+        .map_err(|_| spki::der::Tag::Sequence.value_error())?;
+      let verifying_key = dsa::VerifyingKey::from_components(components, y)
+        .map_err(|_| spki::der::Tag::Sequence.value_error())?;
+      dsa::SigningKey::from_components(verifying_key, x)
+        .map_err(|_| spki::der::Tag::Sequence.value_error())
+    })
+    .map_err(|_| err())
+}
+
 fn parse_rsa_pss_params(
   parameters: Option<AnyRef<'_>>,
 ) -> Result<Option<RsaPssDetails>, RsaPssParamsParseError> {

filename: ext/node_crypto/lib.rs
patch: @@ -665,7 +665,7 @@ pub fn op_node_sign(
   #[cppgc] handle: &KeyObjectHandle,
   #[buffer] digest: &[u8],
   #[string] digest_type: &str,
-  #[smi] pss_salt_length: Option<u32>,
+  #[smi] pss_salt_length: Option<i32>,
   #[smi] padding: Option<u32>,
   #[smi] dsa_signature_encoding: u32,
 ) -> Result<Box<[u8]>, sign::KeyObjectHandlePrehashedSignAndVerifyError> {
@@ -684,7 +684,7 @@ pub fn op_node_verify(
   #[buffer] digest: &[u8],
   #[string] digest_type: &str,
   #[buffer] signature: &[u8],
-  #[smi] pss_salt_length: Option<u32>,
+  #[smi] pss_salt_length: Option<i32>,
   #[smi] padding: Option<u32>,
   #[smi] dsa_signature_encoding: u32,
 ) -> Result<bool, sign::KeyObjectHandlePrehashedSignAndVerifyError> {

filename: ext/node_crypto/sign.rs
patch: @@ -57,6 +57,9 @@ pub enum KeyObjectHandlePrehashedSignAndVerifyError {
   #[class(generic)]
   #[error("failed to sign digest with RSA")]
   FailedToSignDigestWithRsa,
+  #[class(generic)]
+  #[error("digest too big for rsa key")]
+  DigestTooBigForRsaKey,
   #[error("digest not allowed for RSA-PSS signature: {0}")]
   DigestNotAllowedForRsaPssSignature(String),
   #[class(generic)]
@@ -103,26 +106,61 @@ pub enum KeyObjectHandlePrehashedSignAndVerifyError {
 /// Node.js's documented default of `RSA_PSS_SALTLEN_MAX_SIGN`.
 /// When `key_size_bits` is `None` (verify path), the default salt length
 /// is the digest length.
+/// OpenSSL RSA_PSS_SALTLEN_DIGEST: use digest length as salt length.
+const RSA_PSS_SALTLEN_DIGEST: i32 = -1;
+/// OpenSSL RSA_PSS_SALTLEN_MAX_SIGN: use maximum possible salt length.
+const RSA_PSS_SALTLEN_MAX_SIGN: i32 = -2;
+
+/// Resolves the effective salt length for PSS operations.
+///
+/// Handles Node.js special constants:
+/// - `-1` (RSA_PSS_SALTLEN_DIGEST): use the digest output size
+/// - `-2` (RSA_PSS_SALTLEN_MAX_SIGN / RSA_PSS_SALTLEN_AUTO): use maximum
+///   possible salt length (key_bytes - hash_len - 2)
+/// - `None`: defaults to max salt when `key_size_bits` is provided (sign),
+///   or digest length otherwise (verify)
+/// - Positive values: use as-is
+fn resolve_pss_salt_length<D: digest::Digest>(
+  pss_salt_length: Option<i32>,
+  key_size_bits: Option<usize>,
+) -> usize {
+  match pss_salt_length {
+    Some(RSA_PSS_SALTLEN_DIGEST) => <D as digest::Digest>::output_size(),
+    Some(RSA_PSS_SALTLEN_MAX_SIGN) => {
+      let hash_len = <D as digest::Digest>::output_size();
+      if let Some(key_bits) = key_size_bits {
+        let key_bytes = key_bits / 8;
+        key_bytes.saturating_sub(hash_len + 2)
+      } else {
+        hash_len
+      }
+    }
+    Some(len) if len >= 0 => len as usize,
+    Some(_) => <D as digest::Digest>::output_size(), // Unknown negative, fallback to digest length
+    None => {
+      if let Some(key_bits) = key_size_bits {
+        // Default to max salt length for signing (RSA_PSS_SALTLEN_MAX_SIGN)
+        let key_bytes = key_bits / 8;
+        let hash_len = <D as digest::Digest>::output_size();
+        key_bytes.saturating_sub(hash_len + 2)
+      } else {
+        <D as digest::Digest>::output_size()
+      }
+    }
+  }
+}
+
 fn new_pss_scheme(
   digest_type: &str,
-  pss_salt_length: Option<u32>,
+  pss_salt_length: Option<i32>,
   key_size_bits: Option<usize>,
 ) -> Result<rsa::pss::Pss, KeyObjectHandlePrehashedSignAndVerifyError> {
   let pss = match_fixed_digest_with_oid!(
     digest_type,
     fn <D>(algorithm: Option<RsaPssHashAlgorithm>) {
       let _: Option<RsaPssHashAlgorithm> = algorithm;
-      if let Some(salt_length) = pss_salt_length {
-        rsa::pss::Pss::new_with_salt::<D>(salt_length as usize)
-      } else if let Some(key_bits) = key_size_bits {
-        // Default to max salt length for signing (RSA_PSS_SALTLEN_MAX_SIGN)
-        let key_bytes = key_bits / 8;
-        let hash_len = <D as digest::Digest>::output_size();
-        let max_salt = key_bytes.saturating_sub(hash_len + 2);
-        rsa::pss::Pss::new_with_salt::<D>(max_salt)
-      } else {
-        rsa::pss::Pss::new::<D>()
-      }
+      let salt_len = resolve_pss_salt_length::<D>(pss_salt_length, key_size_bits);
+      rsa::pss::Pss::new_with_salt::<D>(salt_len)
     },
     _ => {
       return Err(KeyObjectHandlePrehashedSignAndVerifyError::DigestNotAllowedForRsaPssSignature(digest_type.to_string()));
@@ -136,7 +174,7 @@ impl KeyObjectHandle {
     &self,
     digest_type: &str,
     digest: &[u8],
-    pss_salt_length: Option<u32>,
+    pss_salt_length: Option<i32>,
     padding: Option<u32>,
     dsa_signature_encoding: u32,
   ) -> Result<Box<[u8]>, KeyObjectHandlePrehashedSignAndVerifyError> {
@@ -172,9 +210,15 @@ impl KeyObjectHandle {
           )
         };
 
-        let signature = signer
-          .sign(Some(&mut OsRng), key, digest)
-          .map_err(|_| KeyObjectHandlePrehashedSignAndVerifyError::FailedToSignDigestWithRsa)?;
+        let signature = signer.sign(Some(&mut OsRng), key, digest).map_err(
+          |e| {
+            if e == rsa::Error::MessageTooLong {
+              KeyObjectHandlePrehashedSignAndVerifyError::DigestTooBigForRsaKey
+            } else {
+              KeyObjectHandlePrehashedSignAndVerifyError::FailedToSignDigestWithRsa
+            }
+          },
+        )?;
         Ok(signature.into())
       }
       AsymmetricPrivateKey::RsaPss(key) => {
@@ -190,9 +234,6 @@ impl KeyObjectHandle {
           hash_algorithm = Some(details.hash_algorithm);
           salt_length = Some(details.salt_length as usize);
         }
-        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>) {
@@ -203,11 +244,16 @@ impl KeyObjectHandle {
                   expected: hash_algorithm.as_str().to_string(),
                 });
               }
-            if let Some(salt_length) = salt_length {
-              rsa::pss::Pss::new_with_salt::<D>(salt_length)
+            // Resolve salt length: explicit pss_salt_length takes priority,
+            // then key details, then default (digest length)
+            let resolved = if pss_salt_length.is_some() {
+              resolve_pss_salt_length::<D>(pss_salt_length, Some(key.key.n().bits()))
+            } else if let Some(sl) = salt_length {
+              sl
             } else {
-              rsa::pss::Pss::new::<D>()
-            }
+              <D as digest::Digest>::output_size()
+            };
+            rsa::pss::Pss::new_with_salt::<D>(resolved)
           },
           _ => {
             return Err(KeyObjectHandlePrehashedSignAndVerifyError::DigestNotAllowedForRsaPssSignature(digest_type.to_string()));
@@ -291,7 +337,7 @@ impl KeyObjectHandle {
     digest_type: &str,
     digest: &[u8],
     signature: &[u8],
-    pss_salt_length: Option<u32>,
+    pss_salt_length: Option<i32>,
     padding: Option<u32>,
     dsa_signature_encoding: u32,
   ) -> Result<bool, KeyObjectHandlePrehashedSignAndVerifyError> {
@@ -302,7 +348,11 @@ impl KeyObjectHandle {
     match &*public_key {
       AsymmetricPublicKey::Rsa(key) => {
         if padding == Some(RSA_PKCS1_PSS_PADDING) {
-          let pss = new_pss_scheme(digest_type, pss_salt_length, None)?;
+          let pss = new_pss_scheme(
+            digest_type,
+            pss_salt_length,
+            Some(key.n().bits()),
+          )?;
           return Ok(pss.verify(key, digest, signature).is_ok());
         }
 
@@ -335,9 +385,6 @@ impl KeyObjectHandle {
           hash_algorithm = Some(details.hash_algorithm);
           salt_length = Some(details.salt_length as usize);
         }
-        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>) {
@@ -348,11 +395,14 @@ impl KeyObjectHandle {
                   expected: hash_algorithm.as_str().to_string(),
                 });
               }
-            if let Some(salt_length) = salt_length {
-              rsa::pss::Pss::new_with_salt::<D>(salt_length)
+            let resolved = if pss_salt_length.is_some() {
+              resolve_pss_salt_length::<D>(pss_salt_length, Some(key.key.n().bits()))
+            } else if let Some(sl) = salt_length {
+              sl
             } else {
-              rsa::pss::Pss::new::<D>()
-            }
+              <D as digest::Digest>::output_size()
+            };
+            rsa::pss::Pss::new_with_salt::<D>(resolved)
           },
           _ => {
             return Err(KeyObjectHandlePrehashedSignAndVerifyError::DigestNotAllowedForRsaPssSignature(digest_type.to_string()));
@@ -361,8 +411,9 @@ impl KeyObjectHandle {
         Ok(pss.verify(&key.key, digest, signature).is_ok())
       }
       AsymmetricPublicKey::Dsa(key) => {
-        let signature = dsa::Signature::from_der(signature)
-          .map_err(|_| KeyObjectHandlePrehashedSignAndVerifyError::InvalidDsaSignature)?;
+        let Ok(signature) = dsa::Signature::from_der(signature) else {
+          return Ok(false);
+        };
         Ok(key.verify_prehash(digest, &signature).is_ok())
       }
       AsymmetricPublicKey::Ec(key) => match key {

filename: tests/node_compat/config.jsonc
patch: @@ -370,6 +370,7 @@
     "parallel/test-console-sync-write-error.js": {},
     "parallel/test-console-table.js": {},
     "parallel/test-console-with-frozen-intrinsics.js": {},
+    "parallel/test-crypto-async-sign-verify.js": {},
     "parallel/test-crypto-classes.js": {},
     "parallel/test-crypto-certificate.js": {},
     "parallel/test-crypto-cipheriv-decipheriv.js": {