Skip to content

fix(ext/node): improve X509Certificate Node.js compatibility#32671

Open
bartlomieju wants to merge 5 commits intodenoland:mainfrom
bartlomieju:fix/node-crypto-x509-compat
Open

fix(ext/node): improve X509Certificate Node.js compatibility#32671
bartlomieju wants to merge 5 commits intodenoland:mainfrom
bartlomieju:fix/node-crypto-x509-compat

Conversation

@bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Mar 12, 2026

Summary

  • Fixes multiple compatibility issues with crypto.X509Certificate to pass the Node.js test-crypto-x509.js test suite
  • Adds isX509Certificate(), signatureAlgorithm, signatureAlgorithmOid getters
  • Fixes emailAddress label, date timezone format, toLegacyObject() output (Buffer raw, null-prototype objects, infoAccess as structured object, RSA modulus/exponent format)
  • Adds structured clone support for X509Certificate via MessagePort

Depends on #32672

Test plan

  • ./x test-compat test-crypto-x509 passes
  • CI

🤖 Generated with Claude Code

bartlomieju and others added 2 commits March 12, 2026 18:41
Adds infrastructure for custom JS objects to support structured cloning
via `postMessage`/`MessageChannel`. This enables objects like
`CryptoKey` and `X509Certificate` to be cloned across message ports.

The mechanism works by:
1. Objects set `[core.hostObjectBrand]` to a serializer function that
   returns `{ type: "<name>", ...data }`
2. Extensions register a deserializer via
   `core.registerCloneableResource(name, deserializerFn)`
3. During deserialization, the registry is consulted to reconstruct
   the original object

Changes:
- `libs/core/01_core.js`: Add `registerCloneableResource` /
  `getCloneableDeserializers` registry, auto-pass deserializers
  in `structuredClone`
- `libs/core/ops_builtin_v8.rs`: Mark `op_deserialize` as reentrant
  so deserializer callbacks can invoke ops
- `ext/web/13_message_port.js`: Pass cloneable deserializers during
  message deserialization

Ref: denoland#12067
Ref: denoland#12734

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes multiple compatibility issues with `crypto.X509Certificate` to pass
Node.js test-crypto-x509.js:

- Add `isX509Certificate()` to `internal/crypto/x509` module
- Fix `emailAddress` label in subject/issuer (was `Email`)
- Fix `validFrom`/`validTo` timezone suffix (`+00:00` → `GMT`)
- Add `signatureAlgorithm` and `signatureAlgorithmOid` getters
- Fix `toLegacyObject()`: return Buffer for `raw`, null-prototype
  objects for `subject`/`issuer`/`infoAccess`, add `emailAddress`
  field and `infoAccess` as structured object
- Fix RSA modulus to strip ASN.1 leading zero byte
- Fix RSA exponent format to `0x`-prefixed hex
- Add structured clone support for X509Certificate via MessagePort
  by introducing a cloneable resource registry in `Deno.core` and
  marking `op_deserialize` as reentrant

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bartlomieju bartlomieju force-pushed the fix/node-crypto-x509-compat branch from a534e21 to 3cbaef3 Compare March 12, 2026 17:43
Copy link
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.

toLegacyObject().infoAccess is at risk of being wrong or empty because the new AIA parser does not decode GeneralName correctly. It treats the accessLocation as if a URI were identified by tag value 6 alone, but X.509 encodes uniformResourceIdentifier as context-specific [6], not the universal IA5String tag. This can cause valid OCSP/CA Issuers URIs to be skipped or misread, breaking Node compatibility for certificates that rely on infoAccess.

…ding

Properly check that the ASN.1 tag class is ContextSpecific (not just
tag number 6) when decoding uniformResourceIdentifier in AIA extensions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
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.

Reviewed the changes. No issues found.

@bartlomieju bartlomieju requested a review from littledivy March 12, 2026 21:32
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.

2 participants