From 6432765a9c13929e5be8aab8e01f76924c9fe3fe Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 2 Dec 2025 17:51:06 +0800 Subject: [PATCH 1/2] src: fix off-thread cert loading in bundled cert mode https://redirect.github.com/nodejs/node/pull/59856 had an typo/mistake in the skip conditions so that it is skipping when --use-openssl-ca or --openssl-system-ca-path (configure time) are NOT used, even though they should be skipped only when those ARE used (which is not the default for default builds). This change fixes that so that the perf numbers in that PR is true for the default build. PR-URL: https://github.com/nodejs/node/pull/60764 Reviewed-By: Aditi Singh Reviewed-By: James M Snell Reviewed-By: Yagiz Nizipli Reviewed-By: Chengzhong Wu --- src/crypto/crypto_context.cc | 2 +- test/common/tls.js | 10 ++++ test/fixtures/list-certs.js | 19 +++++++ ...st-tls-off-thread-cert-loading-disabled.js | 40 +++++++++++++ ...test-tls-off-thread-cert-loading-system.js | 56 +++++++++++++++++++ .../test-tls-off-thread-cert-loading.js | 54 ++++++++++++++++++ 6 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/list-certs.js create mode 100644 test/parallel/test-tls-off-thread-cert-loading-disabled.js create mode 100644 test/parallel/test-tls-off-thread-cert-loading-system.js create mode 100644 test/parallel/test-tls-off-thread-cert-loading.js diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index af14273c53b298..0f97f8884c06fe 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -901,7 +901,7 @@ void StartLoadingCertificatesOffThread( // loading. { Mutex::ScopedLock cli_lock(node::per_process::cli_options_mutex); - if (!per_process::cli_options->ssl_openssl_cert_store) { + if (per_process::cli_options->ssl_openssl_cert_store) { return; } } diff --git a/test/common/tls.js b/test/common/tls.js index e59255191b4290..8568659d13cc33 100644 --- a/test/common/tls.js +++ b/test/common/tls.js @@ -194,6 +194,7 @@ function extractMetadata(cert) { subject: x509.subject, }; } +exports.extractMetadata = extractMetadata; // To compare two certificates, we can just compare serialNumber, issuer, // and subject like X509_comp(). We can't just compare two strings because @@ -219,3 +220,12 @@ exports.includesCert = function includesCert(certs, cert) { }; exports.TestTLSSocket = TestTLSSocket; + +// Dumps certs into a file to pass safely into test/fixtures/list-certs.js +exports.writeCerts = function writeCerts(certs, filename) { + const fs = require('fs'); + for (const cert of certs) { + const x509 = new crypto.X509Certificate(cert); + fs.appendFileSync(filename, x509.toString()); + } +}; diff --git a/test/fixtures/list-certs.js b/test/fixtures/list-certs.js new file mode 100644 index 00000000000000..7c9538df242115 --- /dev/null +++ b/test/fixtures/list-certs.js @@ -0,0 +1,19 @@ +const assert = require('assert'); +const EXPECTED_CERTS_PATH = process.env.EXPECTED_CERTS_PATH; +let expectedCerts = []; +if (EXPECTED_CERTS_PATH) { + const fs = require('fs'); + const file = fs.readFileSync(EXPECTED_CERTS_PATH, 'utf-8'); + const expectedCerts = file.split('-----END CERTIFICATE-----\n') + .filter(line => line.trim() !== '') + .map(line => line + '-----END CERTIFICATE-----\n'); +} + +const tls = require('tls'); +const { includesCert, extractMetadata } = require('../common/tls'); + +const CERTS_TYPE = process.env.CERTS_TYPE || 'default'; +const actualCerts = tls.getCACertificates(CERTS_TYPE); +for (const cert of expectedCerts) { + assert(includesCert(actualCerts, cert), 'Expected certificate not found: ' + JSON.stringify(extractMetadata(cert))); +} diff --git a/test/parallel/test-tls-off-thread-cert-loading-disabled.js b/test/parallel/test-tls-off-thread-cert-loading-disabled.js new file mode 100644 index 00000000000000..c2e466fcae1db5 --- /dev/null +++ b/test/parallel/test-tls-off-thread-cert-loading-disabled.js @@ -0,0 +1,40 @@ +'use strict'; +// This tests that when --use-openssl-ca is specified, no off-thread cert loading happens. + +const common = require('../common'); +if (!common.hasCrypto) { + common.skip('missing crypto'); +} +const { spawnSyncAndAssert } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); + +spawnSyncAndAssert( + process.execPath, + [ '--use-openssl-ca', fixtures.path('list-certs.js') ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'crypto', + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), + CERTS_TYPE: 'default', + } + }, + { + stderr(output) { + assert.doesNotMatch( + output, + /Started loading bundled root certificates off-thread/ + ); + assert.doesNotMatch( + output, + /Started loading extra root certificates off-thread/ + ); + assert.doesNotMatch( + output, + /Started loading system root certificates off-thread/ + ); + return true; + } + } +); diff --git a/test/parallel/test-tls-off-thread-cert-loading-system.js b/test/parallel/test-tls-off-thread-cert-loading-system.js new file mode 100644 index 00000000000000..b2893a3a501276 --- /dev/null +++ b/test/parallel/test-tls-off-thread-cert-loading-system.js @@ -0,0 +1,56 @@ +'use strict'; + +// This test verifies that system root certificates loading is loaded off-thread if +// --use-system-ca is used. + +const common = require('../common'); +if (!common.hasCrypto) { + common.skip('missing crypto'); +} +const tmpdir = require('../common/tmpdir'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { writeCerts } = require('../common/tls'); +const tls = require('tls'); + +tmpdir.refresh(); +writeCerts([ + // Check that the extra cert is loaded. + fixtures.readKey('fake-startcom-root-cert.pem'), + // Check that the system certs are loaded. + ...tls.getCACertificates('system'), + // Check that the bundled certs are loaded. + ...tls.getCACertificates('bundled'), +], tmpdir.resolve('check-cert.pem')); + +spawnSyncAndAssert( + process.execPath, + [ '--use-system-ca', '--use-bundled-ca', fixtures.path('list-certs.js') ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'crypto', + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), + EXPECTED_CERTS_PATH: tmpdir.resolve('check-cert.pem'), + CERTS_TYPE: 'default', + } + }, + { + stderr(output) { + assert.match( + output, + /Started loading bundled root certificates off-thread/ + ); + assert.match( + output, + /Started loading extra root certificates off-thread/ + ); + assert.match( + output, + /Started loading system root certificates off-thread/ + ); + return true; + } + } +); diff --git a/test/parallel/test-tls-off-thread-cert-loading.js b/test/parallel/test-tls-off-thread-cert-loading.js new file mode 100644 index 00000000000000..ce414ddec2e0bd --- /dev/null +++ b/test/parallel/test-tls-off-thread-cert-loading.js @@ -0,0 +1,54 @@ +'use strict'; + +// This test verifies that when --use-bundled-ca is used (default to true in default builds), +// the loading of extra and bundled root certificates happens off the main thread. + +const common = require('../common'); +if (!common.hasCrypto) { + common.skip('missing crypto'); +} +const tmpdir = require('../common/tmpdir'); +const { spawnSyncAndAssert } = require('../common/child_process'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { writeCerts } = require('../common/tls'); +const tls = require('tls'); + +tmpdir.refresh(); +writeCerts([ + // Check that the extra cert is loaded. + fixtures.readKey('fake-startcom-root-cert.pem'), + // Check that the bundled certs are loaded. + ...tls.getCACertificates('bundled'), +], tmpdir.resolve('check-cert.pem')); + +spawnSyncAndAssert( + process.execPath, + [ '--no-use-system-ca', '--use-bundled-ca', fixtures.path('list-certs.js') ], + { + env: { + ...process.env, + NODE_DEBUG_NATIVE: 'crypto', + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'fake-startcom-root-cert.pem'), + EXPECTED_CERTS_PATH: tmpdir.resolve('check-cert.pem'), + CERTS_TYPE: 'default', + } + }, + { + stderr(output) { + assert.match( + output, + /Started loading bundled root certificates off-thread/ + ); + assert.match( + output, + /Started loading extra root certificates off-thread/ + ); + assert.doesNotMatch( + output, + /Started loading system root certificates off-thread/ + ); + return true; + } + } +); From 4ea921bdbf94c11e86ef6b53aa7425c6df42876a Mon Sep 17 00:00:00 2001 From: Livia Medeiros Date: Tue, 2 Dec 2025 19:46:50 +0800 Subject: [PATCH 2/2] test: skip SEA inspect test if inspector is not available PR-URL: https://github.com/nodejs/node/pull/60872 Reviewed-By: Chengzhong Wu Reviewed-By: Joyee Cheung Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca --- test/sea/test-single-executable-application-inspect.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/sea/test-single-executable-application-inspect.js b/test/sea/test-single-executable-application-inspect.js index bd885bd91e17f8..5422159fbf3a88 100644 --- a/test/sea/test-single-executable-application-inspect.js +++ b/test/sea/test-single-executable-application-inspect.js @@ -16,6 +16,7 @@ const { } = require('../common/sea'); skipIfSingleExecutableIsNotSupported(); +common.skipIfInspectorDisabled(); const configFile = tmpdir.resolve('sea-config.json'); const seaPrepBlob = tmpdir.resolve('sea-prep.blob');