src: seed V8 from the OS CSPRNG instead of OpenSSL's DRBG - #65796
src: seed V8 from the OS CSPRNG instead of OpenSSL's DRBG#65796colinhacks wants to merge 1 commit into
Conversation
|
Review requested:
|
nodejs-github-bot
added
c++
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. |
4620049 to
043813b
Compare
| #if OPENSSL_VERSION_MAJOR >= 3 | ||
| const bool check_csprng = ncrypto::isFipsEnabled() || | ||
| conf_file != nullptr || | ||
| per_process::cli_options->openssl_shared_config; |
There was a problem hiding this comment.
Did you test this with --openssl-legacy-provider? I wonder if loading legacy first could leave the default provider unloaded.
There was a problem hiding this comment.
No, sorry. Fixed this. crypto.createHash('sha256') failed with ERR_OSSL_EVP_UNSUPPORTED under --openssl-legacy-provider while md4 worked. The explicit legacy load disables OpenSSL's provider fallback, and the unpatched build only has the default provider active by then because the eager CSPRNG() call activated it. Now gated on OSSL_PROVIDER_available(nullptr, "default"), which activates the fallback the same way. Verified on Linux x64. The addons/openssl-providers tests pass on both versions, so they don't cover this.
|
|
||
| // Ensure CSPRNG is properly seeded. | ||
| CHECK(ncrypto::CSPRNG(nullptr, 0)); | ||
| // Node's own OpenSSL configuration always activates the default provider, |
There was a problem hiding this comment.
What happens with configuration from OpenSSL's default openssl.cnf? AFAICT conf_file is still null in that case.
There was a problem hiding this comment.
Yes, conf_file is null then and OpenSSL still reads the nodejs_conf section of the file in OPENSSLDIR. With a base-only section in /etc/ssl/openssl.cnf this version started normally and failed at the first crypto call with ERR_OSSL_EVP_UNSUPPORTED, where the unpatched build aborts at startup. The predicate doesn't look at where the config came from anymore, just whether the default provider is available. That case aborts again.
| // going through OpenSSL instantiates its DRBG and constructs the default | ||
| // provider's algorithm tables on every startup. V8 falls back to very | ||
| // weak entropy when this function fails, so abort instead. | ||
| CHECK_EQ(uv_random(nullptr, nullptr, buffer, length, 0, nullptr), 0); |
There was a problem hiding this comment.
Did you test this on AIX? It looks like uv_random() uses /dev/random there, which I think can block.
There was a problem hiding this comment.
Haven't tested on AIX. uv_random() does read /dev/random there and that blocks when the pool is empty, so AIX now keeps OpenSSL's CSPRNG as V8's source. OpenSSL seeds from /dev/urandom first.
1690e99 to
4f28681
Compare
|
Ran both builds over 52 OpenSSL configurations and flag combinations on Linux and macOS. Two more differences, now in the description and covered by tests. One is an abort in the unpatched build, |
4f28681 to
ced7dc9
Compare
…ents It reported reviews, comments and the check rollup, and was blind to everything else — so a PR that developed a merge conflict, gained a 'blocked' label or had a reviewer requested said nothing at all until something else happened to it. mergeable was computed on every poll and then never read as a trigger. Three facts now wake a worker, and they share ONE line: a label edit must not get the weight of a red build. 🔔 nodejs/node#65796: now CONFLICTS with the base branch; labels +blocked, −needs-ci; review requested from richardlau. Only the actionable direction of each. A PR that STARTS conflicting is work the worker must do; one that stops conflicting stopped because somebody did that work, and telling them spends a turn on their own commit. Labels go both ways, because on a real project they are the state machine — needs-ci, blocked, author ready, commit-queue-failed — and losing one is as much news as gaining one. The baseline rule is the half that matters: the first poll RECORDS the PR's existing labels and reviewers and says nothing, exactly as the review baseline already does. A poll that could not read a field (the gh fallback fetches no labels) leaves the baseline alone rather than resetting it to 'unknown', which would make the next poll announce every label on the PR as newly added. The chat draws it as its own hairline beside the CI and review ones — one delivery, up to three parts — rather than falling through to the verbatim card. The worker contract and the pr_watch schema comments now say what the watcher actually covers, including the approval gate.
… is still working The full check reading — '1 failing, 2 in progress, 31 successful' — renders on the AWAITING CARD, which is only drawn for a thread at REST. So the surface that IS on screen while a thread works, the ops strip under the prompt box, listed a watcher as a ref and an age and nothing else: the one pair that cannot answer 'is anything wrong with it'. On 2026-09-04 that is exactly what happened. nodejs/node#65796's darwin build went red at 16:43Z on a thread that had been in one turn since 16:32, and there was nowhere on the board the maintainer could see it. The row now carries one number in the same column a shell's line count takes, chosen in the card's own severity order so the two surfaces can never lead on different numbers: '1 failed' → '9 held' → '14 running' → '29 green'. The full sentence rides its tooltip. Never polled and no-CI-at-all both render nothing rather than a fabricated zero. A FAILING counter takes Primer's danger red — the exception to this column's uniform text-muted/40, and the only one. Caught by reading back this row's own first screenshot: in the column's grey, '1 failed' was indistinguishable from a timestamp, which is the whole defect restated one surface over. Same colour the awaiting card draws the same fact in. The sidebar row was deliberately NOT touched: a rail row is its title and nothing else (2026-08-19), and this reading belongs where the strip already lists what will wake the thread. Driven in a real browser at 760px and 420px, in both fonts. The counter/·/age ink gaps measure 6.31px and 4.96px against a 4px box gap — the asymmetry tracks the trailing glyph's right bearing, varies per value, and is inherited from the shell counter's identical markup. At 420px the label truncates and the reading survives, which is the right trade.
InitializeOncePerProcessInternal() calls CSPRNG(nullptr, 0) to confirm OpenSSL's random source is seeded and installs a V8 entropy source that goes through CSPRNG() as well. The first RAND_status() of the process therefore runs before V8 starts, instantiates the DRBG, and with it constructs the default provider's algorithm and name tables (ossl_method_construct, ossl_namemap_stored): 3.7% of the samples of `node -e 0` on Linux x64, all of it before v8Start. V8 uses the entropy for hash seeds, address space layout randomization and Math.random(), none of which are cryptographic, so read the OS CSPRNG directly through uv_random(). AIX is the exception: uv_random() reads the blocking /dev/random there, so it stays on OpenSSL's DRBG, which seeds from /dev/urandom. Keep activating the default provider at startup, which the eager check did as a side effect and --openssl-legacy-provider depends on. Its explicit OSSL_PROVIDER_load() disables OpenSSL's provider fallback, so without a prior activation the default provider never loads. Run the seeding check itself only when that provider is unavailable or FIPS is in effect, the cases where an OpenSSL configuration from any source can leave the process without a DRBG and an early abort beats a hang at the first crypto call. Every crypto consumer stays on OpenSSL, and a system without a usable CSPRNG still aborts at startup, now from uv_random() failing. Two other behaviors change. A configuration whose [random] section names a DRBG that cannot be fetched used to abort at startup; it now starts and the first crypto call fails on the fetch. With --secure-heap the process DRBGs are instantiated after the secure heap exists, so they are allocated from it, and a Worker whose per-thread DRBG cannot be allocated no longer aborts the process from the entropy callback. Tests cover both, and the default provider staying active under --openssl-legacy-provider. Measured on Linux x64 against an unpatched build of the same tree, both binaries interleaved, min of 300 runs: `node -e 0` 29.18 -> 27.82 ms, nodeStart to v8Start 2.91 -> 2.11 ms. RAND_status and the provider's table construction leave the startup profile (2.8% of samples before); the provider activation that remains is 0.05%. The first crypto.randomBytes() instantiates the DRBG in 0.19 ms. The `parallel`, `sequential`, `message`, `es-module` and `addons` suites show no failure the unpatched build does not have. Refs: nodejs@5cc36c39d2 Refs: nodejs#44493 Refs: nodejs#46237 Signed-off-by: Colin McDonnell <[email protected]>
ced7dc9 to
650888d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65796 +/- ##
==========================================
- Coverage 90.17% 90.11% -0.06%
==========================================
Files 769 769
Lines 261448 261649 +201
Branches 49674 49682 +8
==========================================
+ Hits 235759 235795 +36
- Misses 16736 16861 +125
- Partials 8953 8993 +40
🚀 New features to boost your workflow:
|
Process startup calls
CSPRNG(nullptr, 0)to confirm OpenSSL's random source is seeded, and the V8 entropy source installed next to it goes throughCSPRNG()too. The firstRAND_status()of the process therefore runs before V8 starts. It instantiates the DRBG, which constructs the default provider's algorithm and name tables: 3.7% of the samples ofnode -e 0on Linux x64, all beforev8Start.V8 uses that entropy for hash seeds, address-space randomization and
Math.random(), none of it cryptographic. This patch reads the OS CSPRNG throughuv_random()for V8, except on AIX, whereuv_random()reads the blocking/dev/randomand OpenSSL's DRBG stays the source.The default provider is still activated at startup. The eager check did that as a side effect, and
--openssl-legacy-providerdepends on it. The explicitOSSL_PROVIDER_load()disables OpenSSL's provider fallback, so without a prior activation the default provider never loads. The seeding check itself now runs only whenOSSL_PROVIDER_available(nullptr, "default")is false or FIPS is in effect, the cases where an OpenSSL configuration from any source (--openssl-config,OPENSSL_CONF, the file in OPENSSLDIR) can leave the process without a DRBG and an early abort beats a hang at the first crypto call (#46237). A configuration that activates only the base provider still aborts at startup, astest-crypto-no-algorithmpins. Every crypto consumer stays on OpenSSL. A system without a usable CSPRNG still aborts at startup, fromuv_random()failing.Two more behaviors change, found by running both builds over 52 OpenSSL configurations and flag combinations on Linux and macOS. A configuration whose
[random]section names a DRBG that cannot be fetched (random = NO-SUCH-DRBG) used to abort at startup and now starts, with the first crypto call failing onunable to fetch drbg; the provider check cannot see that case without fetching the DRBG, which is the work this change defers. With--secure-heap, the process DRBGs are instantiated after the secure heap exists and take 512 bytes of it. The same laziness removes an abort. On the unpatched build,--secure-heap=1024plus eight Workers dies withAssertion failed: ncrypto::CSPRNG(buffer, length)in V8's entropy callback, because each Worker's isolate fetched its entropy through a per-thread DRBG allocated from the exhausted heap. Now the Workers' own crypto calls fail withERR_OSSL_CRYPTO_SECURE_MALLOC_FAILUREand the process continues. Tests cover the three cases, plus the default provider staying active under--openssl-legacy-provider.Measured on Linux x64 (n2-standard-64) against an unpatched build of the same tree, both binaries interleaved, min of 300 runs:
node -e 0node hello.jsnodeStarttov8Startinperformance.nodeTimingnodeStarttov8Start, macOS arm64 (M1 Max), min of 20RAND_statusand the provider's table construction leave the startup profile (2.8% of samples before); the provider activation that remains is 0.05%, and the OpenSSL config load stays. The firstcrypto.randomBytes()instantiates the DRBG in 0.19 ms. Theparallel,sequential,message,es-moduleandaddonssuites on Linux x64 show no failure the unpatched build does not have.The eager check and the abort in the entropy callback come from 5cc36c3 (CVE-2022-35255), whose target was WebCrypto drawing keying material from the V8 hook. Nothing but V8 reads the hook, and it still aborts on failure. #44493 already records that V8's own entropy is proper on every platform.
Refs: #44493
Refs: #46237
Disclosure per the AI use policy: this change was prepared with an AI coding agent directed by the author, including the investigation, the patch, the measurements above and this description, and the agent opened the pull request through the GitHub CLI on the author's instruction. The revision after review, this description and the replies below were prepared the same way and posted on the author's instruction. Verification: Node built at 791e2d2 on Linux x64 (gcc 13) and at 5323423 on macOS 15 arm64; the
parallel,sequential,message,es-moduleandaddonssuites run against an unpatched build; both builds run over the 52 configuration and flag cases behind the paragraph above, on Linux and macOS; AIX not tested.