Skip to content

fix(ext/node): support RSA PSS padding option in crypto sign/verify#32269

Merged
bartlomieju merged 3 commits intodenoland:mainfrom
bartlomieju:fix/rsa-pss-padding-sign
Mar 3, 2026
Merged

fix(ext/node): support RSA PSS padding option in crypto sign/verify#32269
bartlomieju merged 3 commits intodenoland:mainfrom
bartlomieju:fix/rsa-pss-padding-sign

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

Summary

  • Pass the padding option from crypto.createSign().sign() / crypto.createVerify().verify() through to the Rust signing ops
  • Handle RSA_PKCS1_PSS_PADDING (6) for regular RSA keys by using PSS signing/verification instead of PKCS#1 v1.5
  • Fix crypto.sign() / crypto.verify() one-shot functions to preserve padding and saltLength options from the original key object

Closes #28493

Test plan

  • Added spec test tests/specs/node/crypto_sign_rsa_pss_padding that verifies:
    • Signing with PSS padding and various salt lengths (20, 32, 64)
    • Verification of PSS signatures
    • Cross-verification fails with mismatched salt lengths
    • One-shot crypto.sign / crypto.verify with PSS padding

🤖 Generated with Claude Code

The `padding` option passed to `crypto.createSign().sign()` and
`crypto.createVerify().verify()` was silently ignored, causing RSA PSS
signing with regular RSA keys to always use PKCS#1 v1.5 padding instead.

This passes the `padding` option through to the Rust ops and handles
`RSA_PKCS1_PSS_PADDING` (6) for regular RSA keys by using PSS signing
instead of PKCS1v15. Also fixes `crypto.sign()`/`crypto.verify()`
one-shot functions to preserve padding/saltLength options.

Closes denoland#28493

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread ext/node_crypto/sign.rs Outdated
Comment thread ext/node_crypto/sign.rs Outdated
Address review feedback:
- Extract magic number 6 as RSA_PKCS1_PSS_PADDING constant
- Extract common PSS scheme construction into new_pss_scheme function
  shared by sign and verify

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@kajukitli kajukitli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

clean fix. the new_pss_scheme extraction is nice, and good that the constant got pulled out. tests cover the main cases well (various salt lengths, cross-verify failure, one-shot api).

one minor nit: the test comment says "PSS is probabilistic, so even same salt length gives different sigs" — might be worth adding an assertion that sig20 !== sig32 etc to validate they're actually different, not just same length. but not blocking.

@bartlomieju
Copy link
Copy Markdown
Member Author

Re: adding `sig20 !== sig32` assertion — since PSS is probabilistic, comparing signature bytes wouldn't actually validate that different salt lengths produce different outputs. Even two signatures with the same salt length would differ. The cross-verification test (Test 4, line 66-69) is the meaningful check here: it proves that a signature created with salt length 20 fails to verify with salt length 32, which confirms the salt length is actually being applied correctly.

Copy link
Copy Markdown
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju bartlomieju merged commit 291532f into denoland:main Mar 3, 2026
112 checks passed
@bartlomieju bartlomieju deleted the fix/rsa-pss-padding-sign branch March 3, 2026 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RSA PSS Padding signing fails in Deno but works in Node.js

3 participants