Repository: nodejs/node
Pull request: crypto: support --use-system-ca on Windows

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.

Fixes: https://github.com/nodejs/node/issues/51537

---

This PR re-implements https://github.com/nodejs/node/pull/44532 but is based on the support added for macOS in https://github.com/nodejs/node/pull/56599 with the following modifications to match Chromium's policy:

1. We also collect certificates from local machine, enterprise policies and trusted people
2. We check that the extended key usage (if it exists) is suitable for TLS use.

The first commit comes from https://github.com/nodejs/node/pull/56832 to fix a leak from the macOS PR.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
3. Update documentation if relevant.
4. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: doc/api/cli.md
patch: @@ -2868,7 +2868,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 “When using this certificate” flag is set to “Always Trust” or
+    * Any certificate where the “Secure Sockets Layer (SSL)” flag is set to “Always Trust.”
+  * Distrust:
+    * Any certificate where the “When using this certificate” flag is set to “Never Trust” or
+    * Any certificate where the “Secure Sockets Layer (SSL)” flag is set to “Never Trust.”
+
+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`
 
@@ -3688,6 +3718,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

filename: src/crypto/crypto_context.cc
patch: @@ -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() {

filename: test/parallel/parallel.status
patch: @@ -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/54808

filename: test/parallel/test-native-certs-macos.mjs
patch: @@ -1,47 +0,0 @@
-// 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';
-
-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', { skip: !common.isMacOS }, 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() {
-    // Requires trusting the CA certificate first (which needs an interactive GUI approval, e.g. TouchID):
-    // security add-trusted-cert \
-    // -k /Users/$USER/Library/Keychains/login.keychain-db test/fixtures/keys/fake-startcom-root-cert.pem
-    // To remove:
-    // security delete-certificate -c 'StartCom Certification Authority' \
-    // -t /Users/$USER/Library/Keychains/login.keychain-db
-    await fetch(`https://localhost:${server.address().port}/hello-world`);
-  });
-
-  afterEach(async function() {
-    server?.close();
-  });
-});

filename: test/parallel/test-native-certs.mjs
patch: @@ -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();
+  });
+});