Skip to content

Commit

Permalink
tls: copy client CAs and cert store on CertCb
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
indutny authored and Fishrock123 committed Nov 17, 2015
1 parent 2e67db3 commit 05f0549
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 11 deletions.
31 changes: 29 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ template class SSLWrap<TLSWrap>;
template void SSLWrap<TLSWrap>::AddMethods(Environment* env,
Local<FunctionTemplate> t);
template void SSLWrap<TLSWrap>::InitNPN(SecureContext* sc);
template void SSLWrap<TLSWrap>::SetSNIContext(SecureContext* sc);
template int SSLWrap<TLSWrap>::SetCACerts(SecureContext* sc);
template SSL_SESSION* SSLWrap<TLSWrap>::GetSessionCallback(
SSL* s,
unsigned char* key,
Expand Down Expand Up @@ -2264,6 +2266,8 @@ void SSLWrap<Base>::CertCbDone(const FunctionCallbackInfo<Value>& args) {
rv = SSL_use_PrivateKey(w->ssl_, pkey);
if (rv && chain != nullptr)
rv = SSL_set1_chain(w->ssl_, chain);
if (rv)
rv = w->SetCACerts(sc);
if (!rv) {
unsigned long err = ERR_get_error();
if (!err)
Expand Down Expand Up @@ -2314,6 +2318,30 @@ void SSLWrap<Base>::DestroySSL() {
}


template <class Base>
void SSLWrap<Base>::SetSNIContext(SecureContext* sc) {
InitNPN(sc);
CHECK_EQ(SSL_set_SSL_CTX(ssl_, sc->ctx_), sc->ctx_);

SetCACerts(sc);
}


template <class Base>
int SSLWrap<Base>::SetCACerts(SecureContext* sc) {
int err = SSL_set1_verify_cert_store(ssl_, SSL_CTX_get_cert_store(sc->ctx_));
if (err != 1)
return err;

STACK_OF(X509_NAME)* list = SSL_dup_CA_list(
SSL_CTX_get_client_CA_list(sc->ctx_));

// NOTE: `SSL_set_client_CA_list` takes the ownership of `list`
SSL_set_client_CA_list(ssl_, list);
return 1;
}


void Connection::OnClientHelloParseEnd(void* arg) {
Connection* conn = static_cast<Connection*>(arg);

Expand Down Expand Up @@ -2627,8 +2655,7 @@ int Connection::SelectSNIContextCallback_(SSL *s, int *ad, void* arg) {
if (secure_context_constructor_template->HasInstance(ret)) {
conn->sni_context_.Reset(env->isolate(), ret);
SecureContext* sc = Unwrap<SecureContext>(ret.As<Object>());
InitNPN(sc);
SSL_set_SSL_CTX(s, sc->ctx_);
conn->SetSNIContext(sc);
} else {
return SSL_TLSEXT_ERR_NOACK;
}
Expand Down
2 changes: 2 additions & 0 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ class SSLWrap {

void DestroySSL();
void WaitForCertCb(CertCb cb, void* arg);
void SetSNIContext(SecureContext* sc);
int SetCACerts(SecureContext* sc);

inline Environment* ssl_env() const {
return env_;
Expand Down
3 changes: 1 addition & 2 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -857,8 +857,7 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
p->sni_context_.Reset(env->isolate(), ctx);

SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
InitNPN(sc);
SSL_set_SSL_CTX(s, sc->ctx_);
p->SetSNIContext(sc);
return SSL_TLSEXT_ERR_OK;
}
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
Expand Down
31 changes: 24 additions & 7 deletions test/parallel/test-tls-sni-option.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ function loadPEM(n) {
var serverOptions = {
key: loadPEM('agent2-key'),
cert: loadPEM('agent2-cert'),
requestCert: true,
rejectUnauthorized: false,
SNICallback: function(servername, callback) {
var context = SNIContexts[servername];

Expand All @@ -46,7 +48,8 @@ var serverOptions = {
var SNIContexts = {
'a.example.com': {
key: loadPEM('agent1-key'),
cert: loadPEM('agent1-cert')
cert: loadPEM('agent1-cert'),
ca: [ loadPEM('ca2-cert') ]
},
'b.example.com': {
key: loadPEM('agent3-key'),
Expand All @@ -66,6 +69,13 @@ var clientsOptions = [{
ca: [loadPEM('ca1-cert')],
servername: 'a.example.com',
rejectUnauthorized: false
}, {
port: serverPort,
key: loadPEM('agent4-key'),
cert: loadPEM('agent4-cert'),
ca: [loadPEM('ca1-cert')],
servername: 'a.example.com',
rejectUnauthorized: false
}, {
port: serverPort,
key: loadPEM('agent2-key'),
Expand Down Expand Up @@ -97,7 +107,7 @@ var serverResults = [],
clientError;

var server = tls.createServer(serverOptions, function(c) {
serverResults.push(c.servername);
serverResults.push({ sni: c.servername, authorized: c.authorized });
});

server.on('clientError', function(err) {
Expand Down Expand Up @@ -144,9 +154,16 @@ function startTest() {
}

process.on('exit', function() {
assert.deepEqual(serverResults, ['a.example.com', 'b.example.com',
'c.wrong.com', null]);
assert.deepEqual(clientResults, [true, true, false, false]);
assert.deepEqual(clientErrors, [null, null, null, 'socket hang up']);
assert.deepEqual(serverErrors, [null, null, null, 'Invalid SNI context']);
assert.deepEqual(serverResults, [
{ sni: 'a.example.com', authorized: false },
{ sni: 'a.example.com', authorized: true },
{ sni: 'b.example.com', authorized: false },
{ sni: 'c.wrong.com', authorized: false },
null
]);
assert.deepEqual(clientResults, [true, true, true, false, false]);
assert.deepEqual(clientErrors, [null, null, null, null, 'socket hang up']);
assert.deepEqual(serverErrors, [
null, null, null, null, 'Invalid SNI context'
]);
});

0 comments on commit 05f0549

Please sign in to comment.