src: fix off-thread cert loading in bundled cert mode

src/crypto/crypto_context.cc

@@ -878,7 +878,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;
     }
   }

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());
+  }
+};

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)));
+}

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;
+    }
+  }
+);

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;
+    }
+  }
+);

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;
+    }
+  }
+);