Skip to content

fix(ext/node): improve crypto compat - sign/verify, GCM, ChaCha20-Poly1305, CCM#32745

Closed
bartlomieju wants to merge 6 commits intomainfrom
fix/crypto-compat-tests
Closed

fix(ext/node): improve crypto compat - sign/verify, GCM, ChaCha20-Poly1305, CCM#32745
bartlomieju wants to merge 6 commits intomainfrom
fix/crypto-compat-tests

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

Summary

  • Sign/verify fixes: Support private keys in DER/pkcs8 format for verify, handle RSA-PSS special salt length constants (-1, -2), add traditional DSA PEM key parsing, pass errors to callbacks, better error messages
  • GCM cipher fixes: Validate empty IV, fix auth failure error message, fix encoding change in Decipheriv.final(), add state checks for setAAD/setAuthTag
  • ChaCha20-Poly1305: Full streaming AEAD implementation per RFC 8439 using chacha20 + poly1305 crates
  • AES-CCM: Full implementation of RFC 3610 (AES-CTR + CBC-MAC) for 128/192/256-bit keys with streaming support
  • Enable test-crypto-async-sign-verify.js and test-crypto-authenticated.js compat tests

Test plan

  • ./x test-compat test-crypto-async-sign-verify.js passes
  • ./x test-compat test-crypto-authenticated.js passes
  • ./tools/format.js clean
  • cargo clippy -p deno_node_crypto clean

🤖 Generated with Claude Code

bartlomieju and others added 6 commits March 15, 2026 08:59
- Fix verify with private keys in DER/pkcs8 format by deriving public key
- Handle RSA-PSS special salt length constants (-1, -2) correctly
- Add traditional DSA private key PEM parsing ("DSA PRIVATE KEY" label)
- Pass errors to callbacks in sign/verify oneshot functions
- Add "digest too big for rsa key" error for undersized RSA keys
- Return false instead of erroring on unparseable DSA signatures
- Validate empty IV for GCM ciphers
- Fix auth failure error message to match Node.js
- Fix encoding change assertion in Decipheriv.final()
- Add state checks for setAAD after final and duplicate setAuthTag
- Validate GCM auth tag lengths in setAuthTag without explicit option
- Enable test-crypto-async-sign-verify.js compat test
- Ignore test-crypto-argon2-unsupported.js (Deno supports argon2)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lure

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Implement full streaming ChaCha20-Poly1305 AEAD (RFC 8439) with
  encrypt/decrypt/setAAD/getAuthTag/setAuthTag support
- Add stub AES-CCM cipher variants (create + validation, final throws)
- Add chacha20 and poly1305 crate dependencies
- Add chacha20-poly1305 and aes-ccm to getCiphers/getCipherInfo
- Enable test-crypto-authenticated.js compat test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Implement RFC 3610 AES-CCM with streaming CTR encryption and CBC-MAC
  authentication for 128/192/256-bit key sizes
- Add AesBlock enum for dynamic AES key size dispatch
- Add CCM-specific JS validation: plaintextLength in setAAD, message
  size limits, authTagLength requirements
- Update set_aad ops to accept plaintextLength parameter for CCM
- Add aes-128/192/256-ccm to getCiphers()
- Enable test-crypto-authenticated.js compat test (fully passing)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix formatting in cipher.rs (indentation in cbc_mac)
- Fix getCiphers test: skip AEAD ciphers (CCM, chacha20-poly1305) that
  require special setup (authTagLength, setAAD with plaintext length)
- Fix GCM auth tag test: match actual error message "Unsupported state
  or unable to authenticate data" (consistent with Node.js compat test)
- Fix Decipheriv encoding test: unlike Cipheriv, Decipheriv allows
  changing encoding in final() (verified against Node.js compat test
  test-crypto-authenticated.js)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju added a commit that referenced this pull request Mar 30, 2026
- Validate empty IV for AES-GCM (createCipheriv/createDecipheriv with
  empty Buffer should throw "Invalid initialization vector")
- Validate auth tag length in setAuthTag for GCM when no authTagLength
  option was specified at construction (reject lengths 0,1,2,6,9,10,11,17)
- Add state check for setAuthTag (can't call twice)
- Add state check for setAAD (can't call after finalized)
- Fix _lazyInitDecipherDecoder to allow encoding changes instead of
  asserting
- Update auth failure error message to match Node.js: "Unsupported state
  or unable to authenticate data"

These are the GCM-only fixes extracted from #32745 to make review easier.
The remaining changes (sign/verify, ChaCha20-Poly1305, AES-CCM) will
follow in separate PRs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju added a commit that referenced this pull request Mar 30, 2026
## Summary

GCM-only fixes extracted from #32745 for easier review:

- **Empty IV validation**: `createCipheriv`/`createDecipheriv` with
empty IV now throws "Invalid initialization vector" (was silently
accepted)
- **Auth tag length validation**: `setAuthTag` now validates GCM tag
lengths (must be 4, 8, 12-16) even when `authTagLength` option wasn't
set at construction
- **State checks**: `setAuthTag` can't be called twice; `setAAD` can't
be called after `final()`
- **Error message**: Auth failure now says "Unsupported state or unable
to authenticate data" matching Node.js

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju added a commit that referenced this pull request Mar 30, 2026
- Support DSA private key PEM format ("DSA PRIVATE KEY") by parsing
  the traditional ASN.1 structure
- Derive public keys from private keys for verification (pkcs8, sec1)
- Handle RSA-PSS salt length constants (-1 = digest, -2 = max/auto)
  via new resolve_pss_salt_length() helper
- Change pss_salt_length from u32 to i32 to support negative constants
- Distinguish rsa::Error::MessageTooLong as "digest too big for rsa key"
- Return false instead of error for invalid DSA signature format
- Wrap signOneShot/verifyOneShot in try-catch for callback error handling
- Reject unsupported key types (x25519, x448, dh) in verify

Extracted from #32745 for easier review. Enables the
test-crypto-async-sign-verify.js node compat test.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju added a commit that referenced this pull request Mar 30, 2026
Implements the ChaCha20-Poly1305 AEAD cipher (RFC 8439) for node:crypto
createCipheriv/createDecipheriv using the chacha20 and poly1305 crates.

The implementation:
- Derives the Poly1305 key from the first 32 bytes of ChaCha20 keystream
- Encrypts/decrypts with ChaCha20 starting at counter 1
- Computes authentication tag per RFC 8439 (AAD || pad || CT || pad ||
  len(AAD) || len(CT))
- Supports streaming encrypt/decrypt with setAAD and getAuthTag

Closes #28411
Extracted from #32745

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju added a commit that referenced this pull request Mar 31, 2026
## 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"

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bartlomieju
Copy link
Copy Markdown
Member Author

Closing in favor of #33089

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.

1 participant