Skip to content

tls: reject chains containing blocked public keys - #66871

Open
UlisesGascon wants to merge 2 commits into
nodejs:mainfrom
UlisesGascon:fix/tls-spki-blocklist
Open

UlisesGascon wants to merge 2 commits into
nodejs:mainfrom
UlisesGascon:fix/tls-spki-blocklist

Conversation

@UlisesGascon

Copy link
Copy Markdown
Member

Node.js embeds the Mozilla root store but has no equivalent of the
browsers' CRLSet/OneCRL and no default online revocation checking. When
an intermediate CA key is compromised, a chain through it still verifies
as long as it reaches a trusted root. Removing the affected roots would
over-block, since other legitimate intermediates chain to them.

This adds a small compiled-in blocklist of SubjectPublicKeyInfo SHA-256
hashes, checked from a single SSL_CTX_set_cert_verify_callback hook on
every SSL_CTX Node.js creates (SecureContext, QUIC, DTLS). A hit is
reported as X509_V_ERR_CERT_REVOKED through the normal verification
path, so it honours rejectUnauthorized and surfaces as
authorizationError on client and server sockets. The OpenSSL 4.1 CN
name-constraint handling from #65957 moves into the same hook, since
OpenSSL allows only one.

The initial entries are the eight eMudhra/emSign intermediates whose
private keys were reported compromised in October 2026. The hashes are
byte-identical to Go's fix (CVE-2026-111943) and Electron's patch.

Known limitation

Sessions resumed from before the upgrade are not re-verified. This is
standard TLS resumption behaviour and ends with the session timeout.

Points for reviewers

  • This is the first compiled-in trust-policy list in core. Scope is
    deliberately narrow; whether it should be configurable or updatable
    outside a release is open.
  • The test-only hooks live on internalBinding('crypto') behind
    --expose-internals. A test/addons C++ addon is the alternative.
  • Needs backporting to every active release line.

AI disclosure

Assisted by Claude Code and Codex; reviewed, built and tested by me.

Refs: electron/electron#55126
Refs: golang/go#82345

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC transport implementation. labels Oct 20, 2026
@UlisesGascon
UlisesGascon force-pushed the fix/tls-spki-blocklist branch from 577a4a9 to db7cfbb Compare October 20, 2026 09:30
@UlisesGascon
UlisesGascon marked this pull request as ready for review October 20, 2026 09:31
Node.js embeds the Mozilla root store but has no equivalent of the
browsers' CRLSet/OneCRL and no default online revocation checking, so a
certificate chain through a compromised intermediate still verifies as
long as it reaches a trusted root. Removing the affected roots would
over-block, since other legitimate intermediates chain to them.

Add a small compiled-in blocklist of SubjectPublicKeyInfo SHA-256 hashes
and check every verified chain against it from a single
SSL_CTX_set_cert_verify_callback hook. A hit is reported as
X509_V_ERR_CERT_REVOKED through the normal verification path, so it
honours rejectUnauthorized and surfaces as authorizationError on both
client and server sockets. The hook is installed for every SSL_CTX
Node.js creates: SecureContext, QUIC and DTLS. The OpenSSL 4.1 CN name
constraint handling previously registered from crypto_tls.cc moves into
the same hook, since OpenSSL allows only one.

The initial entries are the eight eMudhra/emSign intermediates whose
private keys were reported compromised in October 2026. The same list is
shipped by Go (CVE-2026-111943) and Electron.

Sessions resumed from before this change are not re-verified, as is
standard for TLS resumption; the exposure ends with the session timeout.

Test-only hooks on the crypto binding, reachable with
--expose-internals, allow the test to block a fixture key and exercise
the full handshake.

Refs: electron/electron#55126
Refs: golang/go#82345
Assisted-by: Claude, Codex
Signed-off-by: Ulises Gascon <ulisesgascongonzalez@gmail.com>
@UlisesGascon
UlisesGascon force-pushed the fix/tls-spki-blocklist branch from db7cfbb to 71af0a9 Compare October 20, 2026 09:36
@UlisesGascon

UlisesGascon commented Oct 20, 2026 •

Copy link
Copy Markdown
Member Author

@aduh95 I am waiting for CI to finish in case I need to include more changes, but I have a local fix for the build issue (ref) 👍

Edit: pushed (8417eb1)

Use only X509_STORE_CTX APIs that BoringSSL also provides. BoringSSL
lacks X509_STORE_CTX_set_error_depth, X509_STORE_CTX_set_current_cert
and X509_STORE_CTX_get_verify_cb, which broke the aarch64 shared
BoringSSL build. Fetch the per-certificate callback from the SSL
object instead and skip the two setters on BoringSSL, where Node.js'
own callbacks never read them.

Assisted-by: Claude
Signed-off-by: Ulises Gascon <ulisesgascongonzalez@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC transport implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants