Skip to content

Commit bc020f7

Browse files
addaleaxtargos
authored andcommitted
tls: add allowPartialTrustChain flag
This commit exposes the `X509_V_FLAG_PARTIAL_CHAIN` OpenSSL flag to users. This is behavior that has been requested repeatedly in the Github issues, and allows aligning behavior with other TLS libraries and commonly used applications (e.g. `curl`). As a drive-by, simplify the `SecureContext` source by deduplicating call sites at which a new custom certificate store was created for the `secureContext` in question. Fixes: #36453 PR-URL: #54790 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 1dfd238 commit bc020f7

File tree

5 files changed

+103
-18
lines changed

5 files changed

+103
-18
lines changed

doc/api/tls.md

+5
Original file line numberDiff line numberDiff line change
@@ -1813,6 +1813,9 @@ argument.
18131813
<!-- YAML
18141814
added: v0.11.13
18151815
changes:
1816+
- version: REPLACEME
1817+
pr-url: https://github.com/nodejs/node/pull/54790
1818+
description: The `allowPartialTrustChain` option has been added.
18161819
- version: v20.16.0
18171820
pr-url: https://github.com/nodejs/node/pull/53329
18181821
description: The `clientCertEngine`, `privateKeyEngine` and
@@ -1867,6 +1870,8 @@ changes:
18671870
-->
18681871

18691872
* `options` {Object}
1873+
* `allowPartialTrustChain` {boolean} Treat intermediate (non-self-signed)
1874+
certificates in the trust CA certificate list as trusted.
18701875
* `ca` {string|string\[]|Buffer|Buffer\[]} Optionally override the trusted CA
18711876
certificates. Default is to trust the well-known CAs curated by Mozilla.
18721877
Mozilla's CAs are completely replaced when CAs are explicitly specified

lib/internal/tls/secure-context.js

+5
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
130130
validateObject(options, name);
131131

132132
const {
133+
allowPartialTrustChain,
133134
ca,
134135
cert,
135136
ciphers = getDefaultCiphers(),
@@ -182,6 +183,10 @@ function configSecureContext(context, options = kEmptyObject, name = 'options')
182183
context.addRootCerts();
183184
}
184185

186+
if (allowPartialTrustChain) {
187+
context.setAllowPartialTrustChain();
188+
}
189+
185190
if (cert) {
186191
setCerts(context, ArrayIsArray(cert) ? cert : [cert], `${name}.cert`);
187192
}

src/crypto/crypto_context.cc

+33-18
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,8 @@ Local<FunctionTemplate> SecureContext::GetConstructorTemplate(
273273
SetProtoMethod(isolate, tmpl, "setKey", SetKey);
274274
SetProtoMethod(isolate, tmpl, "setCert", SetCert);
275275
SetProtoMethod(isolate, tmpl, "addCACert", AddCACert);
276+
SetProtoMethod(
277+
isolate, tmpl, "setAllowPartialTrustChain", SetAllowPartialTrustChain);
276278
SetProtoMethod(isolate, tmpl, "addCRL", AddCRL);
277279
SetProtoMethod(isolate, tmpl, "addRootCerts", AddRootCerts);
278280
SetProtoMethod(isolate, tmpl, "setCipherSuites", SetCipherSuites);
@@ -354,6 +356,7 @@ void SecureContext::RegisterExternalReferences(
354356
registry->Register(AddCACert);
355357
registry->Register(AddCRL);
356358
registry->Register(AddRootCerts);
359+
registry->Register(SetAllowPartialTrustChain);
357360
registry->Register(SetCipherSuites);
358361
registry->Register(SetCiphers);
359362
registry->Register(SetSigalgs);
@@ -715,17 +718,39 @@ void SecureContext::SetCert(const FunctionCallbackInfo<Value>& args) {
715718
USE(sc->AddCert(env, std::move(bio)));
716719
}
717720

721+
// NOLINTNEXTLINE(runtime/int)
722+
void SecureContext::SetX509StoreFlag(unsigned long flags) {
723+
X509_STORE* cert_store = GetCertStoreOwnedByThisSecureContext();
724+
CHECK_EQ(1, X509_STORE_set_flags(cert_store, flags));
725+
}
726+
727+
X509_STORE* SecureContext::GetCertStoreOwnedByThisSecureContext() {
728+
if (own_cert_store_cache_ != nullptr) return own_cert_store_cache_;
729+
730+
X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get());
731+
if (cert_store == GetOrCreateRootCertStore()) {
732+
cert_store = NewRootCertStore();
733+
SSL_CTX_set_cert_store(ctx_.get(), cert_store);
734+
}
735+
736+
return own_cert_store_cache_ = cert_store;
737+
}
738+
739+
void SecureContext::SetAllowPartialTrustChain(
740+
const FunctionCallbackInfo<Value>& args) {
741+
SecureContext* sc;
742+
ASSIGN_OR_RETURN_UNWRAP(&sc, args.This());
743+
sc->SetX509StoreFlag(X509_V_FLAG_PARTIAL_CHAIN);
744+
}
745+
718746
void SecureContext::SetCACert(const BIOPointer& bio) {
719747
ClearErrorOnReturn clear_error_on_return;
720748
if (!bio) return;
721-
X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get());
722749
while (X509Pointer x509 = X509Pointer(PEM_read_bio_X509_AUX(
723750
bio.get(), nullptr, NoPasswordCallback, nullptr))) {
724-
if (cert_store == GetOrCreateRootCertStore()) {
725-
cert_store = NewRootCertStore();
726-
SSL_CTX_set_cert_store(ctx_.get(), cert_store);
727-
}
728-
CHECK_EQ(1, X509_STORE_add_cert(cert_store, x509.get()));
751+
CHECK_EQ(1,
752+
X509_STORE_add_cert(GetCertStoreOwnedByThisSecureContext(),
753+
x509.get()));
729754
CHECK_EQ(1, SSL_CTX_add_client_CA(ctx_.get(), x509.get()));
730755
}
731756
}
@@ -754,11 +779,7 @@ Maybe<bool> SecureContext::SetCRL(Environment* env, const BIOPointer& bio) {
754779
return Nothing<bool>();
755780
}
756781

757-
X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get());
758-
if (cert_store == GetOrCreateRootCertStore()) {
759-
cert_store = NewRootCertStore();
760-
SSL_CTX_set_cert_store(ctx_.get(), cert_store);
761-
}
782+
X509_STORE* cert_store = GetCertStoreOwnedByThisSecureContext();
762783

763784
CHECK_EQ(1, X509_STORE_add_crl(cert_store, crl.get()));
764785
CHECK_EQ(1,
@@ -1042,8 +1063,6 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
10421063
sc->issuer_.reset();
10431064
sc->cert_.reset();
10441065

1045-
X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_.get());
1046-
10471066
DeleteFnPtr<PKCS12, PKCS12_free> p12;
10481067
EVPKeyPointer pkey;
10491068
X509Pointer cert;
@@ -1097,11 +1116,7 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
10971116
for (int i = 0; i < sk_X509_num(extra_certs.get()); i++) {
10981117
X509* ca = sk_X509_value(extra_certs.get(), i);
10991118

1100-
if (cert_store == GetOrCreateRootCertStore()) {
1101-
cert_store = NewRootCertStore();
1102-
SSL_CTX_set_cert_store(sc->ctx_.get(), cert_store);
1103-
}
1104-
X509_STORE_add_cert(cert_store, ca);
1119+
X509_STORE_add_cert(sc->GetCertStoreOwnedByThisSecureContext(), ca);
11051120
SSL_CTX_add_client_CA(sc->ctx_.get(), ca);
11061121
}
11071122
ret = true;

src/crypto/crypto_context.h

+7
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ class SecureContext final : public BaseObject {
6565
void SetCACert(const BIOPointer& bio);
6666
void SetRootCerts();
6767

68+
void SetX509StoreFlag(unsigned long flags); // NOLINT(runtime/int)
69+
X509_STORE* GetCertStoreOwnedByThisSecureContext();
70+
6871
// TODO(joyeecheung): track the memory used by OpenSSL types
6972
SET_NO_MEMORY_INFO()
7073
SET_MEMORY_INFO_NAME(SecureContext)
@@ -91,6 +94,8 @@ class SecureContext final : public BaseObject {
9194
#endif // !OPENSSL_NO_ENGINE
9295
static void SetCert(const v8::FunctionCallbackInfo<v8::Value>& args);
9396
static void AddCACert(const v8::FunctionCallbackInfo<v8::Value>& args);
97+
static void SetAllowPartialTrustChain(
98+
const v8::FunctionCallbackInfo<v8::Value>& args);
9499
static void AddCRL(const v8::FunctionCallbackInfo<v8::Value>& args);
95100
static void AddRootCerts(const v8::FunctionCallbackInfo<v8::Value>& args);
96101
static void SetCipherSuites(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -143,6 +148,8 @@ class SecureContext final : public BaseObject {
143148
SSLCtxPointer ctx_;
144149
X509Pointer cert_;
145150
X509Pointer issuer_;
151+
// Non-owning cache for SSL_CTX_get_cert_store(ctx_.get())
152+
X509_STORE* own_cert_store_cache_ = nullptr;
146153
#ifndef OPENSSL_NO_ENGINE
147154
bool client_cert_engine_provided_ = false;
148155
EnginePointer private_key_engine_;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto) { common.skip('missing crypto'); }
4+
5+
const assert = require('assert');
6+
const { once } = require('events');
7+
const fixtures = require('../common/fixtures');
8+
9+
// agent6-cert.pem is signed by intermediate cert of ca3.
10+
// The server has a cert chain of agent6->ca3->ca1(root).
11+
12+
const { it, beforeEach, afterEach, describe } = require('node:test');
13+
14+
describe('allowPartialTrustChain', { skip: !common.hasCrypto }, function() {
15+
const tls = require('tls');
16+
let server;
17+
let client;
18+
let opts;
19+
20+
beforeEach(async function() {
21+
server = tls.createServer({
22+
ca: fixtures.readKey('ca3-cert.pem'),
23+
key: fixtures.readKey('agent6-key.pem'),
24+
cert: fixtures.readKey('agent6-cert.pem'),
25+
}, (socket) => socket.resume());
26+
server.listen(0);
27+
await once(server, 'listening');
28+
29+
opts = {
30+
port: server.address().port,
31+
ca: fixtures.readKey('ca3-cert.pem'),
32+
checkServerIdentity() {}
33+
};
34+
});
35+
36+
afterEach(async function() {
37+
client?.destroy();
38+
server?.close();
39+
});
40+
41+
it('can connect successfully with allowPartialTrustChain: true', async function() {
42+
client = tls.connect({ ...opts, allowPartialTrustChain: true });
43+
await once(client, 'secureConnect'); // Should not throw
44+
});
45+
46+
it('fails without with allowPartialTrustChain: true for an intermediate cert in the CA', async function() {
47+
// Consistency check: Connecting fails without allowPartialTrustChain: true
48+
await assert.rejects(async () => {
49+
const client = tls.connect(opts);
50+
await once(client, 'secureConnect');
51+
}, { code: 'UNABLE_TO_GET_ISSUER_CERT' });
52+
});
53+
});

0 commit comments

Comments
 (0)