Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit aefb20a

Browse filesBrowse files
indutnyMyles Borins
indutny
authored and
Myles Borins
committed
tls: copy client CAs and cert store on CertCb
Copy client CA certs and cert store when asynchronously selecting `SecureContext` during `SNICallback`. We already copy private key, certificate, and certificate chain, but the client CA certs were missing. Fix: #2772 PR-URL: #3537 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 607f545 commit aefb20a
Copy full SHA for aefb20a

File tree

Expand file treeCollapse file tree

4 files changed

+56
-11
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+56
-11
lines changed
Open diff view settings
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+29-2Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ template class SSLWrap<TLSWrap>;
129129
template void SSLWrap<TLSWrap>::AddMethods(Environment* env,
130130
Local<FunctionTemplate> t);
131131
template void SSLWrap<TLSWrap>::InitNPN(SecureContext* sc);
132+
template void SSLWrap<TLSWrap>::SetSNIContext(SecureContext* sc);
133+
template int SSLWrap<TLSWrap>::SetCACerts(SecureContext* sc);
132134
template SSL_SESSION* SSLWrap<TLSWrap>::GetSessionCallback(
133135
SSL* s,
134136
unsigned char* key,
@@ -2165,6 +2167,8 @@ void SSLWrap<Base>::CertCbDone(const FunctionCallbackInfo<Value>& args) {
21652167
rv = SSL_use_PrivateKey(w->ssl_, pkey);
21662168
if (rv && chain != nullptr)
21672169
rv = SSL_set1_chain(w->ssl_, chain);
2170+
if (rv)
2171+
rv = w->SetCACerts(sc);
21682172
if (!rv) {
21692173
unsigned long err = ERR_get_error();
21702174
if (!err)
@@ -2215,6 +2219,30 @@ void SSLWrap<Base>::DestroySSL() {
22152219
}
22162220

22172221

2222+
template <class Base>
2223+
void SSLWrap<Base>::SetSNIContext(SecureContext* sc) {
2224+
InitNPN(sc);
2225+
CHECK_EQ(SSL_set_SSL_CTX(ssl_, sc->ctx_), sc->ctx_);
2226+
2227+
SetCACerts(sc);
2228+
}
2229+
2230+
2231+
template <class Base>
2232+
int SSLWrap<Base>::SetCACerts(SecureContext* sc) {
2233+
int err = SSL_set1_verify_cert_store(ssl_, SSL_CTX_get_cert_store(sc->ctx_));
2234+
if (err != 1)
2235+
return err;
2236+
2237+
STACK_OF(X509_NAME)* list = SSL_dup_CA_list(
2238+
SSL_CTX_get_client_CA_list(sc->ctx_));
2239+
2240+
// NOTE: `SSL_set_client_CA_list` takes the ownership of `list`
2241+
SSL_set_client_CA_list(ssl_, list);
2242+
return 1;
2243+
}
2244+
2245+
22182246
void Connection::OnClientHelloParseEnd(void* arg) {
22192247
Connection* conn = static_cast<Connection*>(arg);
22202248

@@ -2528,8 +2556,7 @@ int Connection::SelectSNIContextCallback_(SSL *s, int *ad, void* arg) {
25282556
if (secure_context_constructor_template->HasInstance(ret)) {
25292557
conn->sni_context_.Reset(env->isolate(), ret);
25302558
SecureContext* sc = Unwrap<SecureContext>(ret.As<Object>());
2531-
InitNPN(sc);
2532-
SSL_set_SSL_CTX(s, sc->ctx_);
2559+
conn->SetSNIContext(sc);
25332560
} else {
25342561
return SSL_TLSEXT_ERR_NOACK;
25352562
}
Collapse file

‎src/node_crypto.h‎

Copy file name to clipboardExpand all lines: src/node_crypto.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,8 @@ class SSLWrap {
279279

280280
void DestroySSL();
281281
void WaitForCertCb(CertCb cb, void* arg);
282+
void SetSNIContext(SecureContext* sc);
283+
int SetCACerts(SecureContext* sc);
282284

283285
inline Environment* ssl_env() const {
284286
return env_;
Collapse file

‎src/tls_wrap.cc‎

Copy file name to clipboardExpand all lines: src/tls_wrap.cc
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -867,8 +867,7 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
867867
p->sni_context_.Reset(env->isolate(), ctx);
868868

869869
SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
870-
InitNPN(sc);
871-
SSL_set_SSL_CTX(s, sc->ctx_);
870+
p->SetSNIContext(sc);
872871
return SSL_TLSEXT_ERR_OK;
873872
}
874873
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
Collapse file

‎test/parallel/test-tls-sni-option.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-tls-sni-option.js
+24-7Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ function loadPEM(n) {
2626
var serverOptions = {
2727
key: loadPEM('agent2-key'),
2828
cert: loadPEM('agent2-cert'),
29+
requestCert: true,
30+
rejectUnauthorized: false,
2931
SNICallback: function(servername, callback) {
3032
var context = SNIContexts[servername];
3133

@@ -46,7 +48,8 @@ var serverOptions = {
4648
var SNIContexts = {
4749
'a.example.com': {
4850
key: loadPEM('agent1-key'),
49-
cert: loadPEM('agent1-cert')
51+
cert: loadPEM('agent1-cert'),
52+
ca: [ loadPEM('ca2-cert') ]
5053
},
5154
'b.example.com': {
5255
key: loadPEM('agent3-key'),
@@ -66,6 +69,13 @@ var clientsOptions = [{
6669
ca: [loadPEM('ca1-cert')],
6770
servername: 'a.example.com',
6871
rejectUnauthorized: false
72+
}, {
73+
port: serverPort,
74+
key: loadPEM('agent4-key'),
75+
cert: loadPEM('agent4-cert'),
76+
ca: [loadPEM('ca1-cert')],
77+
servername: 'a.example.com',
78+
rejectUnauthorized: false
6979
}, {
7080
port: serverPort,
7181
key: loadPEM('agent2-key'),
@@ -97,7 +107,7 @@ let serverError;
97107
let clientError;
98108

99109
var server = tls.createServer(serverOptions, function(c) {
100-
serverResults.push(c.servername);
110+
serverResults.push({ sni: c.servername, authorized: c.authorized });
101111
});
102112

103113
server.on('clientError', function(err) {
@@ -144,9 +154,16 @@ function startTest() {
144154
}
145155

146156
process.on('exit', function() {
147-
assert.deepEqual(serverResults, ['a.example.com', 'b.example.com',
148-
'c.wrong.com', null]);
149-
assert.deepEqual(clientResults, [true, true, false, false]);
150-
assert.deepEqual(clientErrors, [null, null, null, 'socket hang up']);
151-
assert.deepEqual(serverErrors, [null, null, null, 'Invalid SNI context']);
157+
assert.deepEqual(serverResults, [
158+
{ sni: 'a.example.com', authorized: false },
159+
{ sni: 'a.example.com', authorized: true },
160+
{ sni: 'b.example.com', authorized: false },
161+
{ sni: 'c.wrong.com', authorized: false },
162+
null
163+
]);
164+
assert.deepEqual(clientResults, [true, true, true, false, false]);
165+
assert.deepEqual(clientErrors, [null, null, null, null, 'socket hang up']);
166+
assert.deepEqual(serverErrors, [
167+
null, null, null, null, 'Invalid SNI context'
168+
]);
152169
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.