crypto: support --use-system-ca on Windows

joyeecheung · targos · commit 3e207bd9ecb7 · 2025-02-10T15:15:02.000+01:00
This patch adds support for --use-system-ca on Windows, the certificates are collected following Chromium's policy, though the following are left as TODO and out of this patch. - Support for user-added intermediate certificates - Support for distrusted certificates Since those aren't typically supported by other runtimes/tools either, and what's implemented in this patch is sufficient for enough use cases already. PR-URL: #56833 Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -2840,7 +2840,37 @@ The following values are valid for `mode`:
 Node.js uses the trusted CA certificates present in the system store along with
 the `--use-bundled-ca`, `--use-openssl-ca` options.
 
-This option is available to macOS only.
+This option is only supported on Windows and macOS, and the certificate trust policy
+is planned to follow [Chromium's policy for locally trusted certificates][]:
+
+On macOS, the following certifcates are trusted:
+
+* Default and System Keychains
+  * Trust:
+    * Any certificate where the &ldquo;When using this certificate&rdquo; flag is set to &ldquo;Always Trust&rdquo; or
+    * Any certificate where the &ldquo;Secure Sockets Layer (SSL)&rdquo; flag is set to &ldquo;Always Trust.&rdquo;
+  * Distrust:
+    * Any certificate where the &ldquo;When using this certificate&rdquo; flag is set to &ldquo;Never Trust&rdquo; or
+    * Any certificate where the &ldquo;Secure Sockets Layer (SSL)&rdquo; flag is set to &ldquo;Never Trust.&rdquo;
+
+On Windows, the following certificates are currently trusted (unlike
+Chromium's policy, distrust is not currently supported):
+
+* Local Machine (accessed via `certlm.msc`)
+  * Trust:
+    * Trusted Root Certification Authorities
+    * Trusted People
+    * Enterprise Trust -> Enterprise -> Trusted Root Certification Authorities
+    * Enterprise Trust -> Enterprise -> Trusted People
+    * Enterprise Trust -> Group Policy -> Trusted Root Certification Authorities
+    * Enterprise Trust -> Group Policy -> Trusted People
+* Current User (accessed via `certmgr.msc`)
+  * Trust:
+    * Trusted Root Certification Authorities
+    * Enterprise Trust -> Group Policy -> Trusted Root Certification Authorities
+
+On any supported system, Node.js would check that the certificate's key usage and extended key
+usage are consistent with TLS use cases before using it for server authentication.
 
 ### `--v8-options`
 
@@ -3661,6 +3691,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 
 [#42511]: https://github.com/nodejs/node/issues/42511
 [Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/
+[Chromium's policy for locally trusted certificates]: https://chromium.googlesource.com/chromium/src/+/main/net/data/ssl/chrome_root_store/faq.md#does-the-chrome-certificate-verifier-consider-local-trust-decisions
 [CommonJS]: modules.md
 [CommonJS module]: modules.md
 [DEP0025 warning]: deprecations.md#dep0025-requirenodesys
diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
@@ -22,6 +22,11 @@
 #include <Security/Security.h>
 #endif
 
+#ifdef _WIN32
+#include <Windows.h>
+#include <wincrypt.h>
+#endif
+
 namespace node {
 
 using ncrypto::BignumPointer;
@@ -285,13 +290,15 @@ void X509VectorToPEMVector(const std::vector<X509Pointer>& src,
   }
 }
 
-#ifdef __APPLE__
-// This code is loosely based on
+// The following code is loosely based on
 // https://github.com/chromium/chromium/blob/54bd8e3/net/cert/internal/trust_store_mac.cc
+// and
+// https://github.com/chromium/chromium/blob/0192587/net/cert/internal/trust_store_win.cc
 // Copyright 2015 The Chromium Authors
 // Licensed under a BSD-style license
 // See https://chromium.googlesource.com/chromium/src/+/HEAD/LICENSE for
 // details.
+#ifdef __APPLE__
 TrustStatus IsTrustDictionaryTrustedForPolicy(CFDictionaryRef trust_dict,
                                               bool is_self_issued) {
   // Trust settings may be scoped to a single application
@@ -524,11 +531,160 @@ void ReadMacOSKeychainCertificates(
 }
 #endif  // __APPLE__
 
+#ifdef _WIN32
+
+// Returns true if the cert can be used for server authentication, based on
+// certificate properties.
+//
+// While there are a variety of certificate properties that can affect how
+// trust is computed, the main property is CERT_ENHKEY_USAGE_PROP_ID, which
+// is intersected with the certificate's EKU extension (if present).
+// The intersection is documented in the Remarks section of
+// CertGetEnhancedKeyUsage, and is as follows:
+// - No EKU property, and no EKU extension = Trusted for all purpose
+// - Either an EKU property, or EKU extension, but not both = Trusted only
+//   for the listed purposes
+// - Both an EKU property and an EKU extension = Trusted for the set
+//   intersection of the listed purposes
+// CertGetEnhancedKeyUsage handles this logic, and if an empty set is
+// returned, the distinction between the first and third case can be
+// determined by GetLastError() returning CRYPT_E_NOT_FOUND.
+//
+// See:
+// https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certgetenhancedkeyusage
+//
+// If we run into any errors reading the certificate properties, we fail
+// closed.
+bool IsCertTrustedForServerAuth(PCCERT_CONTEXT cert) {
+  DWORD usage_size = 0;
+
+  if (!CertGetEnhancedKeyUsage(cert, 0, nullptr, &usage_size)) {
+    return false;
+  }
+
+  std::vector<BYTE> usage_bytes(usage_size);
+  CERT_ENHKEY_USAGE* usage =
+      reinterpret_cast<CERT_ENHKEY_USAGE*>(usage_bytes.data());
+  if (!CertGetEnhancedKeyUsage(cert, 0, usage, &usage_size)) {
+    return false;
+  }
+
+  if (usage->cUsageIdentifier == 0) {
+    // check GetLastError
+    HRESULT error_code = GetLastError();
+
+    switch (error_code) {
+      case CRYPT_E_NOT_FOUND:
+        return true;
+      case S_OK:
+        return false;
+      default:
+        return false;
+    }
+  }
+
+  // SAFETY: `usage->rgpszUsageIdentifier` is an array of LPSTR (pointer to null
+  // terminated string) of length `usage->cUsageIdentifier`.
+  for (DWORD i = 0; i < usage->cUsageIdentifier; ++i) {
+    std::string_view eku(usage->rgpszUsageIdentifier[i]);
+    if ((eku == szOID_PKIX_KP_SERVER_AUTH) ||
+        (eku == szOID_ANY_ENHANCED_KEY_USAGE)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+void GatherCertsForLocation(std::vector<X509Pointer>* vector,
+                            DWORD location,
+                            LPCWSTR store_name) {
+  if (!(location == CERT_SYSTEM_STORE_LOCAL_MACHINE ||
+        location == CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY ||
+        location == CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE ||
+        location == CERT_SYSTEM_STORE_CURRENT_USER ||
+        location == CERT_SYSTEM_STORE_CURRENT_USER_GROUP_POLICY)) {
+    return;
+  }
+
+  DWORD flags =
+      location | CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG;
+
+  HCERTSTORE opened_store(
+      CertOpenStore(CERT_STORE_PROV_SYSTEM,
+                    0,
+                    // The Windows API only accepts NULL for hCryptProv.
+                    NULL, /* NOLINT (readability/null_usage) */
+                    flags,
+                    store_name));
+  if (!opened_store) {
+    return;
+  }
+
+  auto cleanup = OnScopeLeave(
+      [opened_store]() { CHECK_EQ(CertCloseStore(opened_store, 0), TRUE); });
+
+  PCCERT_CONTEXT cert_from_store = nullptr;
+  while ((cert_from_store = CertEnumCertificatesInStore(
+              opened_store, cert_from_store)) != nullptr) {
+    if (!IsCertTrustedForServerAuth(cert_from_store)) {
+      continue;
+    }
+    const unsigned char* cert_data =
+        reinterpret_cast<const unsigned char*>(cert_from_store->pbCertEncoded);
+    const size_t cert_size = cert_from_store->cbCertEncoded;
+
+    vector->emplace_back(d2i_X509(nullptr, &cert_data, cert_size));
+  }
+}
+
+void ReadWindowsCertificates(
+    std::vector<std::string>* system_root_certificates) {
+  std::vector<X509Pointer> system_root_certificates_X509;
+  // TODO(joyeecheung): match Chromium's policy, collect more certificates
+  // from user-added CAs and support disallowed (revoked) certificates.
+
+  // Grab the user-added roots.
+  GatherCertsForLocation(
+      &system_root_certificates_X509, CERT_SYSTEM_STORE_LOCAL_MACHINE, L"ROOT");
+  GatherCertsForLocation(&system_root_certificates_X509,
+                         CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY,
+                         L"ROOT");
+  GatherCertsForLocation(&system_root_certificates_X509,
+                         CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE,
+                         L"ROOT");
+  GatherCertsForLocation(
+      &system_root_certificates_X509, CERT_SYSTEM_STORE_CURRENT_USER, L"ROOT");
+  GatherCertsForLocation(&system_root_certificates_X509,
+                         CERT_SYSTEM_STORE_CURRENT_USER_GROUP_POLICY,
+                         L"ROOT");
+
+  // Grab the user-added trusted server certs. Trusted end-entity certs are
+  // only allowed for server auth in the "local machine" store, but not in the
+  // "current user" store.
+  GatherCertsForLocation(&system_root_certificates_X509,
+                         CERT_SYSTEM_STORE_LOCAL_MACHINE,
+                         L"TrustedPeople");
+  GatherCertsForLocation(&system_root_certificates_X509,
+                         CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY,
+                         L"TrustedPeople");
+  GatherCertsForLocation(&system_root_certificates_X509,
+                         CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE,
+                         L"TrustedPeople");
+
+  X509VectorToPEMVector(system_root_certificates_X509,
+                        system_root_certificates);
+}
+#endif
+
 void ReadSystemStoreCertificates(
     std::vector<std::string>* system_root_certificates) {
 #ifdef __APPLE__
   ReadMacOSKeychainCertificates(system_root_certificates);
 #endif
+#ifdef _WIN32
+  ReadWindowsCertificates(system_root_certificates);
+#endif
 }
 
 std::vector<std::string> getCombinedRootCertificates() {
diff --git a/test/fixtures/keys/fake-startcom-root-cert.cer b/test/fixtures/keys/fake-startcom-root-cert.cer
diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status
@@ -20,7 +20,7 @@ test-fs-read-stream-concurrent-reads: PASS, FLAKY
 test-snapshot-incompatible: SKIP
 
 # Requires manual setup for certificates to be trusted by the system
-test-native-certs-macos: SKIP
+test-native-certs: SKIP
 
 [$system==win32]
 # https://github.com/nodejs/node/issues/54807
diff --git a/test/parallel/test-native-certs-macos.mjs b/test/parallel/test-native-certs-macos.mjs
diff --git a/test/parallel/test-native-certs.mjs b/test/parallel/test-native-certs.mjs
@@ -0,0 +1,68 @@
+// Flags: --use-system-ca
+
+import * as common from '../common/index.mjs';
+import assert from 'node:assert/strict';
+import https from 'node:https';
+import fixtures from '../common/fixtures.js';
+import { it, beforeEach, afterEach, describe } from 'node:test';
+import { once } from 'events';
+
+if (!common.isMacOS && !common.isWindows) {
+  common.skip('--use-system-ca is only supported on macOS and Windows');
+}
+
+if (!common.hasCrypto) {
+  common.skip('requires crypto');
+}
+
+// To run this test, the system needs to be configured to trust
+// the CA certificate first (which needs an interactive GUI approval, e.g. TouchID):
+// On macOS:
+//   1. To add the certificate:
+//    $ security add-trusted-cert \
+//       -k /Users/$USER/Library/Keychains/login.keychain-db \
+//       test/fixtures/keys/fake-startcom-root-cert.pem
+//   2. To remove the certificate:
+//     $ security delete-certificate -c 'StartCom Certification Authority' \
+//         -t /Users/$USER/Library/Keychains/login.keychain-db
+//
+// On Windows:
+//   1. To add the certificate in PowerShell (remember the thumbprint printed):
+//    $ Import-Certificate -FilePath .\test\fixtures\keys\fake-startcom-root-cert.cer \
+//       -CertStoreLocation Cert:\CurrentUser\Root
+//   2. To remove the certificate by the thumbprint:
+//    $  $thumbprint = (Get-ChildItem -Path Cert:\CurrentUser\Root | \
+//          Where-Object { $_.Subject -match "StartCom Certification Authority" }).Thumbprint
+//    $  Remove-Item -Path "Cert:\CurrentUser\Root\$thumbprint"
+const handleRequest = (req, res) => {
+  const path = req.url;
+  switch (path) {
+    case '/hello-world':
+      res.writeHead(200);
+      res.end('hello world\n');
+      break;
+    default:
+      assert(false, `Unexpected path: ${path}`);
+  }
+};
+
+describe('use-system-ca', function() {
+  let server;
+
+  beforeEach(async function() {
+    server = https.createServer({
+      key: fixtures.readKey('agent8-key.pem'),
+      cert: fixtures.readKey('agent8-cert.pem'),
+    }, handleRequest);
+    server.listen(0);
+    await once(server, 'listening');
+  });
+
+  it('can connect successfully with a trusted certificate', async function() {
+    await fetch(`https://localhost:${server.address().port}/hello-world`);
+  });
+
+  afterEach(async function() {
+    server?.close();
+  });
+});
