Skip to content

Commit f245e53

Browse files
colinhacksaduh95
authored andcommitted
src: seed V8 from the OS CSPRNG instead of OpenSSL's DRBG
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's isolate setup no longer aborts the process from the entropy callback when the heap cannot hold another per-thread DRBG. 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: 5cc36c39d2 Refs: #44493 Refs: #46237 Signed-off-by: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> PR-URL: #65796 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 6f5fb76 commit f245e53

5 files changed

Lines changed: 93 additions & 6 deletions

File tree

src/node.cc

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@
4949

5050
#if HAVE_OPENSSL
5151
#include "ncrypto.h"
52+
#if OPENSSL_VERSION_MAJOR >= 3
53+
#include <openssl/provider.h>
54+
#endif
5255
#include "node_crypto.h"
5356
#if OPENSSL_VERSION_MAJOR >= 3 && !defined(CONF_MFLAGS_IGNORE_MISSING_FILE)
5457
// OpenSSL hides this deprecated macro under OPENSSL_NO_DEPRECATED, but the
@@ -1287,15 +1290,36 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
12871290
}
12881291
crypto::InstallFipsIndicatorCallback();
12891292

1290-
// Ensure CSPRNG is properly seeded.
1291-
CHECK(ncrypto::CSPRNG(nullptr, 0));
1293+
// Activating the default provider here keeps --openssl-legacy-provider
1294+
// working. Its explicit load disables OpenSSL's fallback, and the eager
1295+
// CSPRNG check used to activate the provider as a side effect. Only
1296+
// check the seeding when that provider is missing or FIPS is on, so a
1297+
// configuration without a DRBG still aborts at startup instead of
1298+
// hanging at the first crypto call. Otherwise the DRBG is instantiated
1299+
// on first use.
1300+
#if OPENSSL_VERSION_MAJOR >= 3
1301+
const bool check_csprng = ncrypto::isFipsEnabled() ||
1302+
!OSSL_PROVIDER_available(nullptr, "default");
1303+
#else
1304+
const bool check_csprng = true;
1305+
#endif
1306+
if (check_csprng) {
1307+
CHECK(ncrypto::CSPRNG(nullptr, 0));
1308+
}
12921309

1310+
// V8 uses the entropy for hash seeds, ASLR and Math.random(), none of
1311+
// it cryptographic. Going through OpenSSL would instantiate the DRBG
1312+
// and build the default provider's algorithm tables on every startup.
1313+
// V8 falls back to very weak entropy when the source fails, so abort
1314+
// instead.
12931315
V8::SetEntropySource([](unsigned char* buffer, size_t length) {
1294-
// V8 falls back to very weak entropy when this function fails
1295-
// and /dev/urandom isn't available. That wouldn't be so bad if
1296-
// the entropy was only used for Math.random() but it's also used for
1297-
// hash table and address space layout randomization. Better to abort.
1316+
#ifdef _AIX
1317+
// uv_random() reads /dev/random on AIX, which blocks. OpenSSL seeds
1318+
// from /dev/urandom there.
12981319
CHECK(ncrypto::CSPRNG(buffer, length));
1320+
#else
1321+
CHECK_EQ(uv_random(nullptr, nullptr, buffer, length, 0, nullptr), 0);
1322+
#endif
12991323
return true;
13001324
});
13011325
#endif // !defined(OPENSSL_IS_BORINGSSL)

test/addons/openssl-providers/test-legacy-provider-option.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,6 @@ if (getFips()) {
2222
common.skip('this test cannot be run in FIPS mode');
2323
}
2424
providers.testProviderPresent('legacy');
25+
// The explicit legacy load disables OpenSSL's provider fallback, so the
26+
// default provider has to be active before it runs.
27+
providers.testProviderPresent('default');
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
nodejs_conf = nodejs_init
2+
3+
[nodejs_init]
4+
random = random_sect
5+
6+
[random_sect]
7+
random = NO-SUCH-DRBG

test/parallel/test-crypto-no-algorithm.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,21 @@ if (isMainThread) {
5757
assert(common.nodeProcessAborted(cp.status, cp.signal),
5858
`process did not abort, code:${cp.status} signal:${cp.signal}`);
5959
}
60+
61+
// AIX keeps OpenSSL as V8's entropy source, so a DRBG that cannot be
62+
// fetched still aborts at startup there.
63+
if (!common.isAIX) {
64+
// A configuration whose random section names a DRBG that cannot be
65+
// fetched starts normally; the first crypto call fails, without a hang.
66+
const fixtures = require('../common/fixtures');
67+
const { spawnSync } = require('node:child_process');
68+
const randomConf = fixtures.path('openssl3-conf', 'random_unavailable.cnf');
69+
const cp = spawnSync(process.execPath,
70+
[ `--openssl-config=${randomConf}`, '-e',
71+
'require("node:crypto").randomBytes(8)' ],
72+
{ encoding: 'utf8' });
73+
assert(!common.nodeProcessAborted(cp.status, cp.signal),
74+
`process aborted, code:${cp.status} signal:${cp.signal}`);
75+
assert.strictEqual(cp.status, 1);
76+
assert.match(cp.stderr, /unable to fetch drbg/);
77+
}

test/parallel/test-crypto-secure-heap.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,28 @@ if (process.argv[2] === 'child') {
6161
return;
6262
}
6363

64+
if (process.argv[2] === 'workers') {
65+
// Eight Workers held alive at once. A 1 KiB secure heap has room for a
66+
// few DRBGs only, so an isolate setup that drew its entropy through
67+
// OpenSSL would fail for the later Workers and abort the process.
68+
const { Worker } = require('worker_threads');
69+
const i32 = new Int32Array(new SharedArrayBuffer(4));
70+
let online = 0;
71+
for (let i = 0; i < 8; i++) {
72+
const worker = new Worker(
73+
'const { workerData } = require("worker_threads");' +
74+
'Atomics.wait(workerData.i32, 0, 0);',
75+
{ eval: true, workerData: { i32 } });
76+
worker.on('online', () => {
77+
if (++online === 8) {
78+
Atomics.store(i32, 0, 1);
79+
Atomics.notify(i32, 0);
80+
}
81+
});
82+
}
83+
return;
84+
}
85+
6486
const child = fork(
6587
process.argv[1],
6688
['child'],
@@ -70,6 +92,19 @@ child.on('exit', common.mustCall((code) => {
7092
assert.strictEqual(code, 0);
7193
}));
7294

95+
// AIX keeps OpenSSL as V8's entropy source, so a Worker's isolate setup
96+
// still draws on the secure heap there.
97+
if (!common.isAIX) {
98+
const child = fork(
99+
process.argv[1],
100+
['workers'],
101+
{ execArgv: ['--secure-heap=1024', '--secure-heap-min=4'] });
102+
child.on('exit', common.mustCall((code, signal) => {
103+
assert.strictEqual(signal, null);
104+
assert.strictEqual(code, 0);
105+
}));
106+
}
107+
73108
{
74109
const child = fork(fixtures.path('a.js'), {
75110
execArgv: ['--secure-heap=3', '--secure-heap-min=3'],

0 commit comments

Comments
 (0)