Skip to content

crypto: stop clearing OpenSSL compression methods#64683

Draft
BridgeAR wants to merge 1 commit into
nodejs:mainfrom
BridgeAR:BridgeAR/2026-07-22-fix-openssl-compression-leak
Draft

crypto: stop clearing OpenSSL compression methods#64683
BridgeAR wants to merge 1 commit into
nodejs:mainfrom
BridgeAR:BridgeAR/2026-07-22-fix-openssl-compression-leak

Conversation

@BridgeAR

Copy link
Copy Markdown
Member

Summary

Node 26's OpenSSL build now includes compression support. During startup, Node clears OpenSSL's global record-compression method stack with sk_SSL_COMP_zero(), which drops the only pointers to the allocated SSL_COMP entries. LeakSanitizer reports the lost 24-byte allocation on every process.

OpenSSL sets SSL_OP_NO_COMPRESSION on every new SSL_CTX, and Node only exposes SSL_CTX_set_options(). Removing the global stack clear keeps record compression disabled without discarding OpenSSL-owned pointers.

The existing LEAK_SANITIZER check in NodeMainInstance::Run() covers this on sanitizer builds. Every test process exercises the path, so a dedicated JavaScript test would not add coverage.

Refs: #62217

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@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. labels Jul 22, 2026
OpenSSL allocates the built-in record-compression methods while creating
its global context. Clearing the stack drops the only pointers without
freeing the entries, so LeakSanitizer reports a 24-byte leak at process
shutdown.

OpenSSL disables record compression on every new SSL_CTX by default.
Node does not expose SSL_CTX_clear_options(), so the extra process-wide
clearing is not needed to protect against CRIME.

Refs: nodejs#62217
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR
BridgeAR force-pushed the BridgeAR/2026-07-22-fix-openssl-compression-leak branch from 8ef0f45 to 4ad0562 Compare July 22, 2026 20:58
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants