Skip to content

Commit bb69f14

Browse files
committed
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
1 parent 0debdac commit bb69f14

File tree

5 files changed

+86
-18
lines changed

5 files changed

+86
-18
lines changed

doc/api/tls.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1856,6 +1856,9 @@ argument.
18561856
<!-- YAML
18571857
added: v0.11.13
18581858
changes:
1859+
- version: REPLACEME
1860+
pr-url: https://github.com/nodejs/node/pull/54790
1861+
description: The `allowPartialTrustChain` option has been added.
18591862
- version:
18601863
- v22.4.0
18611864
- v20.16.0
@@ -1912,6 +1915,8 @@ changes:
19121915
-->
19131916

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

lib/internal/tls/secure-context.js

Lines changed: 5 additions & 0 deletions
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

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ Local<FunctionTemplate> SecureContext::GetConstructorTemplate(
314314
SetProtoMethod(isolate, tmpl, "setKey", SetKey);
315315
SetProtoMethod(isolate, tmpl, "setCert", SetCert);
316316
SetProtoMethod(isolate, tmpl, "addCACert", AddCACert);
317+
SetProtoMethod(isolate, tmpl, "setAllowPartialTrustChain", SetAllowPartialTrustChain);
317318
SetProtoMethod(isolate, tmpl, "addCRL", AddCRL);
318319
SetProtoMethod(isolate, tmpl, "addRootCerts", AddRootCerts);
319320
SetProtoMethod(isolate, tmpl, "setCipherSuites", SetCipherSuites);
@@ -753,17 +754,35 @@ void SecureContext::SetCert(const FunctionCallbackInfo<Value>& args) {
753754
USE(sc->AddCert(env, std::move(bio)));
754755
}
755756

757+
void SecureContext::SetX509StoreFlag(unsigned long flags) {
758+
X509_STORE* cert_store = GetCertStoreOwnedByThisSecureContext();
759+
CHECK_EQ(1, X509_STORE_set_flags(cert_store, flags));
760+
}
761+
762+
X509_STORE* SecureContext::GetCertStoreOwnedByThisSecureContext() {
763+
if (owned_cert_store_cached_ != nullptr) return owned_cert_store_cached_;
764+
765+
X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get());
766+
if (cert_store == GetOrCreateRootCertStore()) {
767+
cert_store = NewRootCertStore();
768+
SSL_CTX_set_cert_store(ctx_.get(), cert_store);
769+
}
770+
771+
return owned_cert_store_cached_ = cert_store;
772+
}
773+
774+
void SecureContext::SetAllowPartialTrustChain(const FunctionCallbackInfo<Value>& args) {
775+
SecureContext* sc;
776+
ASSIGN_OR_RETURN_UNWRAP(&sc, args.This());
777+
sc->SetX509StoreFlag(X509_V_FLAG_PARTIAL_CHAIN);
778+
}
779+
756780
void SecureContext::SetCACert(const BIOPointer& bio) {
757781
ClearErrorOnReturn clear_error_on_return;
758782
if (!bio) return;
759-
X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get());
760783
while (X509Pointer x509 = X509Pointer(PEM_read_bio_X509_AUX(
761784
bio.get(), nullptr, NoPasswordCallback, nullptr))) {
762-
if (cert_store == GetOrCreateRootCertStore()) {
763-
cert_store = NewRootCertStore();
764-
SSL_CTX_set_cert_store(ctx_.get(), cert_store);
765-
}
766-
CHECK_EQ(1, X509_STORE_add_cert(cert_store, x509.get()));
785+
CHECK_EQ(1, X509_STORE_add_cert(GetCertStoreOwnedByThisSecureContext(), x509.get()));
767786
CHECK_EQ(1, SSL_CTX_add_client_CA(ctx_.get(), x509.get()));
768787
}
769788
}
@@ -793,11 +812,7 @@ Maybe<void> SecureContext::SetCRL(Environment* env, const BIOPointer& bio) {
793812
return Nothing<void>();
794813
}
795814

796-
X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx_.get());
797-
if (cert_store == GetOrCreateRootCertStore()) {
798-
cert_store = NewRootCertStore();
799-
SSL_CTX_set_cert_store(ctx_.get(), cert_store);
800-
}
815+
X509_STORE* cert_store = GetCertStoreOwnedByThisSecureContext();
801816

802817
CHECK_EQ(1, X509_STORE_add_crl(cert_store, crl.get()));
803818
CHECK_EQ(1,
@@ -1080,8 +1095,6 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
10801095
sc->issuer_.reset();
10811096
sc->cert_.reset();
10821097

1083-
X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_.get());
1084-
10851098
DeleteFnPtr<PKCS12, PKCS12_free> p12;
10861099
EVPKeyPointer pkey;
10871100
X509Pointer cert;
@@ -1135,11 +1148,7 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
11351148
for (int i = 0; i < sk_X509_num(extra_certs.get()); i++) {
11361149
X509* ca = sk_X509_value(extra_certs.get(), i);
11371150

1138-
if (cert_store == GetOrCreateRootCertStore()) {
1139-
cert_store = NewRootCertStore();
1140-
SSL_CTX_set_cert_store(sc->ctx_.get(), cert_store);
1141-
}
1142-
X509_STORE_add_cert(cert_store, ca);
1151+
X509_STORE_add_cert(sc->GetCertStoreOwnedByThisSecureContext(), ca);
11431152
SSL_CTX_add_client_CA(sc->ctx_.get(), ca);
11441153
}
11451154
ret = true;

src/crypto/crypto_context.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ class SecureContext final : public BaseObject {
6464
void SetCACert(const BIOPointer& bio);
6565
void SetRootCerts();
6666

67+
void SetX509StoreFlag(unsigned long flags);
68+
X509_STORE* GetCertStoreOwnedByThisSecureContext();
69+
6770
// TODO(joyeecheung): track the memory used by OpenSSL types
6871
SET_NO_MEMORY_INFO()
6972
SET_MEMORY_INFO_NAME(SecureContext)
@@ -90,6 +93,7 @@ class SecureContext final : public BaseObject {
9093
#endif // !OPENSSL_NO_ENGINE
9194
static void SetCert(const v8::FunctionCallbackInfo<v8::Value>& args);
9295
static void AddCACert(const v8::FunctionCallbackInfo<v8::Value>& args);
96+
static void SetAllowPartialTrustChain(const v8::FunctionCallbackInfo<v8::Value>& args);
9397
static void AddCRL(const v8::FunctionCallbackInfo<v8::Value>& args);
9498
static void AddRootCerts(const v8::FunctionCallbackInfo<v8::Value>& args);
9599
static void SetCipherSuites(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -142,6 +146,7 @@ class SecureContext final : public BaseObject {
142146
SSLCtxPointer ctx_;
143147
X509Pointer cert_;
144148
X509Pointer issuer_;
149+
X509_STORE* owned_cert_store_cached_ = nullptr;
145150
#ifndef OPENSSL_NO_ENGINE
146151
bool client_cert_engine_provided_ = false;
147152
ncrypto::EnginePointer private_key_engine_;
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const assert = require('assert');
8+
const { once } = require('events');
9+
const tls = require('tls');
10+
const fixtures = require('../common/fixtures');
11+
12+
// agent6-cert.pem is signed by intermediate cert of ca3.
13+
// The server has a cert chain of agent6->ca3->ca1(root) but
14+
15+
async function test() {
16+
const server = tls.createServer({
17+
ca: fixtures.readKey('ca3-cert.pem'),
18+
key: fixtures.readKey('agent6-key.pem'),
19+
cert: fixtures.readKey('agent6-cert.pem'),
20+
}, (socket) => socket.resume());
21+
server.listen(0);
22+
await once(server, 'listening');
23+
24+
const opts = {
25+
port: server.address().port,
26+
ca: fixtures.readKey('ca3-cert.pem'),
27+
checkServerIdentity() {}
28+
};
29+
30+
// Connecting succeeds with allowPartialTrustChain: true
31+
const client = tls.connect({ ...opts, allowPartialTrustChain: true });
32+
await once(client, 'secureConnect');
33+
client.destroy();
34+
35+
// Consistency check: Connecting fails without allowPartialTrustChain: true
36+
await assert.rejects(async () => {
37+
const client = tls.connect(opts);
38+
await once(client, 'secureConnect');
39+
}, { code: 'UNABLE_TO_GET_ISSUER_CERT' });
40+
41+
server.close();
42+
}
43+
44+
test().catch((err) => process.nextTick(() => { throw err; }));

0 commit comments

Comments
 (0)