Skip to content

Commit c841d57

Browse files
indutnyMyles Borins
authored andcommitted
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 <[email protected]>
1 parent 28e21b0 commit c841d57

File tree

4 files changed

+56
-11
lines changed

4 files changed

+56
-11
lines changed

‎src/node_crypto.cc‎

Lines 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
unsignedchar* 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
unsignedlong err = ERR_get_error();
21702174
if (!err)
@@ -2215,6 +2219,30 @@ void SSLWrap<Base>::DestroySSL(){
22152219
}
22162220

22172221

2222+
template <classBase>
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 <classBase>
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+
return1;
2243+
}
2244+
2245+
22182246
voidConnection::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
}

‎src/node_crypto.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,8 @@ class SSLWrap{
279279

280280
voidDestroySSL();
281281
voidWaitForCertCb(CertCb cb, void* arg);
282+
voidSetSNIContext(SecureContext* sc);
283+
intSetCACerts(SecureContext* sc);
282284

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

‎src/tls_wrap.cc‎

Lines 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

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ function loadPEM(n){
2626
varserverOptions={
2727
key: loadPEM('agent2-key'),
2828
cert: loadPEM('agent2-cert'),
29+
requestCert: true,
30+
rejectUnauthorized: false,
2931
SNICallback: function(servername,callback){
3032
varcontext=SNIContexts[servername];
3133

@@ -46,7 +48,8 @@ var serverOptions ={
4648
varSNIContexts={
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
letclientError;
98108

99109
varserver=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)