FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: support --use-system-ca on Windows · nodejs/node@3e207bd · GitHub

/ node Public

Commit 3e207bd

Browse files
joyeecheung
authored and
targos
committed
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. PR-URL: #56833 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 9e29416 commit 3e207bd

6 files changed

Lines changed: 259 additions & 51 deletions

File tree

‎doc/api/cli.md‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2840,7 +2840,37 @@ The following values are valid for `mode`:
28402840
Node.js uses the trusted CA certificates present in the system store along with
28412841
the `--use-bundled-ca`, `--use-openssl-ca` options.
28422842

2843-
This option is available to macOS only.
2843+
This option is only supported on Windows and macOS, and the certificate trust policy
2844+
is planned to follow [Chromium's policy for locally trusted certificates][]:
2845+
2846+
On macOS, the following certifcates are trusted:
2847+
2848+
* Default and System Keychains
2849+
* Trust:
2850+
* Any certificate where the “When using this certificate” flag is set to “Always Trust” or
2851+
* Any certificate where the “Secure Sockets Layer (SSL)” flag is set to “Always Trust.”
2852+
* Distrust:
2853+
* Any certificate where the “When using this certificate” flag is set to “Never Trust” or
2854+
* Any certificate where the “Secure Sockets Layer (SSL)” flag is set to “Never Trust.”
2855+
2856+
On Windows, the following certificates are currently trusted (unlike
2857+
Chromium's policy, distrust is not currently supported):
2858+
2859+
* Local Machine (accessed via `certlm.msc`)
2860+
* Trust:
2861+
* Trusted Root Certification Authorities
2862+
* Trusted People
2863+
* Enterprise Trust -> Enterprise -> Trusted Root Certification Authorities
2864+
* Enterprise Trust -> Enterprise -> Trusted People
2865+
* Enterprise Trust -> Group Policy -> Trusted Root Certification Authorities
2866+
* Enterprise Trust -> Group Policy -> Trusted People
2867+
* Current User (accessed via `certmgr.msc`)
2868+
* Trust:
2869+
* Trusted Root Certification Authorities
2870+
* Enterprise Trust -> Group Policy -> Trusted Root Certification Authorities
2871+
2872+
On any supported system, Node.js would check that the certificate's key usage and extended key
2873+
usage are consistent with TLS use cases before using it for server authentication.
28442874

28452875
### `--v8-options`
28462876

@@ -3661,6 +3691,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
36613691

36623692
[#42511]: https://github.com/nodejs/node/issues/42511
36633693
[Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/
3694+
[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
36643695
[CommonJS]: modules.md
36653696
[CommonJS module]: modules.md
36663697
[DEP0025 warning]: deprecations.md#dep0025-requirenodesys

‎src/crypto/crypto_context.cc‎

Lines changed: 158 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
#include <Security/Security.h>
2323
#endif
2424

25+
#ifdef _WIN32
26+
#include <Windows.h>
27+
#include <wincrypt.h>
28+
#endif
29+
2530
namespace node {
2631

2732
using ncrypto::BignumPointer;
@@ -285,13 +290,15 @@ void X509VectorToPEMVector(const std::vector<X509Pointer>& src,
285290
}
286291
}
287292

288-
#ifdef __APPLE__
289-
// This code is loosely based on
293+
// The following code is loosely based on
290294
// https://github.com/chromium/chromium/blob/54bd8e3/net/cert/internal/trust_store_mac.cc
295+
// and
296+
// https://github.com/chromium/chromium/blob/0192587/net/cert/internal/trust_store_win.cc
291297
// Copyright 2015 The Chromium Authors
292298
// Licensed under a BSD-style license
293299
// See https://chromium.googlesource.com/chromium/src/+/HEAD/LICENSE for
294300
// details.
301+
#ifdef __APPLE__
295302
TrustStatus IsTrustDictionaryTrustedForPolicy(CFDictionaryRef trust_dict,
296303
bool is_self_issued) {
297304
// Trust settings may be scoped to a single application
@@ -524,11 +531,160 @@ void ReadMacOSKeychainCertificates(
524531
}
525532
#endif // __APPLE__
526533

534+
#ifdef _WIN32
535+
536+
// Returns true if the cert can be used for server authentication, based on
537+
// certificate properties.
538+
//
539+
// While there are a variety of certificate properties that can affect how
540+
// trust is computed, the main property is CERT_ENHKEY_USAGE_PROP_ID, which
541+
// is intersected with the certificate's EKU extension (if present).
542+
// The intersection is documented in the Remarks section of
543+
// CertGetEnhancedKeyUsage, and is as follows:
544+
// - No EKU property, and no EKU extension = Trusted for all purpose
545+
// - Either an EKU property, or EKU extension, but not both = Trusted only
546+
// for the listed purposes
547+
// - Both an EKU property and an EKU extension = Trusted for the set
548+
// intersection of the listed purposes
549+
// CertGetEnhancedKeyUsage handles this logic, and if an empty set is
550+
// returned, the distinction between the first and third case can be
551+
// determined by GetLastError() returning CRYPT_E_NOT_FOUND.
552+
//
553+
// See:
554+
// https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certgetenhancedkeyusage
555+
//
556+
// If we run into any errors reading the certificate properties, we fail
557+
// closed.
558+
bool IsCertTrustedForServerAuth(PCCERT_CONTEXT cert) {
559+
DWORD usage_size = 0;
560+
561+
if (!CertGetEnhancedKeyUsage(cert, 0, nullptr, &usage_size)) {
562+
return false;
563+
}
564+
565+
std::vector<BYTE> usage_bytes(usage_size);
566+
CERT_ENHKEY_USAGE* usage =
567+
reinterpret_cast<CERT_ENHKEY_USAGE*>(usage_bytes.data());
568+
if (!CertGetEnhancedKeyUsage(cert, 0, usage, &usage_size)) {
569+
return false;
570+
}
571+
572+
if (usage->cUsageIdentifier == 0) {
573+
// check GetLastError
574+
HRESULT error_code = GetLastError();
575+
576+
switch (error_code) {
577+
case CRYPT_E_NOT_FOUND:
578+
return true;
579+
case S_OK:
580+
return false;
581+
default:
582+
return false;
583+
}
584+
}
585+
586+
// SAFETY: `usage->rgpszUsageIdentifier` is an array of LPSTR (pointer to null
587+
// terminated string) of length `usage->cUsageIdentifier`.
588+
for (DWORD i = 0; i < usage->cUsageIdentifier; ++i) {
589+
std::string_view eku(usage->rgpszUsageIdentifier[i]);
590+
if ((eku == szOID_PKIX_KP_SERVER_AUTH) ||
591+
(eku == szOID_ANY_ENHANCED_KEY_USAGE)) {
592+
return true;
593+
}
594+
}
595+
596+
return false;
597+
}
598+
599+
void GatherCertsForLocation(std::vector<X509Pointer>* vector,
600+
DWORD location,
601+
LPCWSTR store_name) {
602+
if (!(location == CERT_SYSTEM_STORE_LOCAL_MACHINE ||
603+
location == CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY ||
604+
location == CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE ||
605+
location == CERT_SYSTEM_STORE_CURRENT_USER ||
606+
location == CERT_SYSTEM_STORE_CURRENT_USER_GROUP_POLICY)) {
607+
return;
608+
}
609+
610+
DWORD flags =
611+
location | CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG;
612+
613+
HCERTSTORE opened_store(
614+
CertOpenStore(CERT_STORE_PROV_SYSTEM,
615+
0,
616+
// The Windows API only accepts NULL for hCryptProv.
617+
NULL, /* NOLINT (readability/null_usage) */
618+
flags,
619+
store_name));
620+
if (!opened_store) {
621+
return;
622+
}
623+
624+
auto cleanup = OnScopeLeave(
625+
[opened_store]() { CHECK_EQ(CertCloseStore(opened_store, 0), TRUE); });
626+
627+
PCCERT_CONTEXT cert_from_store = nullptr;
628+
while ((cert_from_store = CertEnumCertificatesInStore(
629+
opened_store, cert_from_store)) != nullptr) {
630+
if (!IsCertTrustedForServerAuth(cert_from_store)) {
631+
continue;
632+
}
633+
const unsigned char* cert_data =
634+
reinterpret_cast<const unsigned char*>(cert_from_store->pbCertEncoded);
635+
const size_t cert_size = cert_from_store->cbCertEncoded;
636+
637+
vector->emplace_back(d2i_X509(nullptr, &cert_data, cert_size));
638+
}
639+
}
640+
641+
void ReadWindowsCertificates(
642+
std::vector<std::string>* system_root_certificates) {
643+
std::vector<X509Pointer> system_root_certificates_X509;
644+
// TODO(joyeecheung): match Chromium's policy, collect more certificates
645+
// from user-added CAs and support disallowed (revoked) certificates.
646+
647+
// Grab the user-added roots.
648+
GatherCertsForLocation(
649+
&system_root_certificates_X509, CERT_SYSTEM_STORE_LOCAL_MACHINE, L"ROOT");
650+
GatherCertsForLocation(&system_root_certificates_X509,
651+
CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY,
652+
L"ROOT");
653+
GatherCertsForLocation(&system_root_certificates_X509,
654+
CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE,
655+
L"ROOT");
656+
GatherCertsForLocation(
657+
&system_root_certificates_X509, CERT_SYSTEM_STORE_CURRENT_USER, L"ROOT");
658+
GatherCertsForLocation(&system_root_certificates_X509,
659+
CERT_SYSTEM_STORE_CURRENT_USER_GROUP_POLICY,
660+
L"ROOT");
661+
662+
// Grab the user-added trusted server certs. Trusted end-entity certs are
663+
// only allowed for server auth in the "local machine" store, but not in the
664+
// "current user" store.
665+
GatherCertsForLocation(&system_root_certificates_X509,
666+
CERT_SYSTEM_STORE_LOCAL_MACHINE,
667+
L"TrustedPeople");
668+
GatherCertsForLocation(&system_root_certificates_X509,
669+
CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY,
670+
L"TrustedPeople");
671+
GatherCertsForLocation(&system_root_certificates_X509,
672+
CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPRISE,
673+
L"TrustedPeople");
674+
675+
X509VectorToPEMVector(system_root_certificates_X509,
676+
system_root_certificates);
677+
}
678+
#endif
679+
527680
void ReadSystemStoreCertificates(
528681
std::vector<std::string>* system_root_certificates) {
529682
#ifdef __APPLE__
530683
ReadMacOSKeychainCertificates(system_root_certificates);
531684
#endif
685+
#ifdef _WIN32
686+
ReadWindowsCertificates(system_root_certificates);
687+
#endif
532688
}
533689

534690
std::vector<std::string> getCombinedRootCertificates() {

‎test/fixtures/keys/fake-startcom-root-cert.cer‎

915 Bytes
Binary file not shown.

‎test/parallel/parallel.status‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ test-fs-read-stream-concurrent-reads: PASS, FLAKY
2020
test-snapshot-incompatible: SKIP
2121

2222
# Requires manual setup for certificates to be trusted by the system
23-
test-native-certs-macos: SKIP
23+
test-native-certs: SKIP
2424

2525
[$system==win32]
2626
# https://github.com/nodejs/node/issues/54807

‎test/parallel/test-native-certs-macos.mjs‎

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎test/parallel/test-native-certs.mjs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Flags: --use-system-ca
2+
3+
import * as common from '../common/index.mjs';
4+
import assert from 'node:assert/strict';
5+
import https from 'node:https';
6+
import fixtures from '../common/fixtures.js';
7+
import { it, beforeEach, afterEach, describe } from 'node:test';
8+
import { once } from 'events';
9+
10+
if (!common.isMacOS && !common.isWindows) {
11+
common.skip('--use-system-ca is only supported on macOS and Windows');
12+
}
13+
14+
if (!common.hasCrypto) {
15+
common.skip('requires crypto');
16+
}
17+
18+
// To run this test, the system needs to be configured to trust
19+
// the CA certificate first (which needs an interactive GUI approval, e.g. TouchID):
20+
// On macOS:
21+
// 1. To add the certificate:
22+
// $ security add-trusted-cert \
23+
// -k /Users/$USER/Library/Keychains/login.keychain-db \
24+
// test/fixtures/keys/fake-startcom-root-cert.pem
25+
// 2. To remove the certificate:
26+
// $ security delete-certificate -c 'StartCom Certification Authority' \
27+
// -t /Users/$USER/Library/Keychains/login.keychain-db
28+
//
29+
// On Windows:
30+
// 1. To add the certificate in PowerShell (remember the thumbprint printed):
31+
// $ Import-Certificate -FilePath .\test\fixtures\keys\fake-startcom-root-cert.cer \
32+
// -CertStoreLocation Cert:\CurrentUser\Root
33+
// 2. To remove the certificate by the thumbprint:
34+
// $ $thumbprint = (Get-ChildItem -Path Cert:\CurrentUser\Root | \
35+
// Where-Object { $_.Subject -match "StartCom Certification Authority" }).Thumbprint
36+
// $ Remove-Item -Path "Cert:\CurrentUser\Root\$thumbprint"
37+
const handleRequest = (req, res) => {
38+
const path = req.url;
39+
switch (path) {
40+
case '/hello-world':
41+
res.writeHead(200);
42+
res.end('hello world\n');
43+
break;
44+
default:
45+
assert(false, `Unexpected path: ${path}`);
46+
}
47+
};
48+
49+
describe('use-system-ca', function() {
50+
let server;
51+
52+
beforeEach(async function() {
53+
server = https.createServer({
54+
key: fixtures.readKey('agent8-key.pem'),
55+
cert: fixtures.readKey('agent8-cert.pem'),
56+
}, handleRequest);
57+
server.listen(0);
58+
await once(server, 'listening');
59+
});
60+
61+
it('can connect successfully with a trusted certificate', async function() {
62+
await fetch(`https://localhost:${server.address().port}/hello-world`);
63+
});
64+
65+
afterEach(async function() {
66+
server?.close();
67+
});
68+
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL