From 483a41c0ad585b88e05e3d610f8cded0efcd0e49 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 26 Oct 2015 12:06:28 -0400 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/3537 Reviewed-By: Ben Noordhuis --- src/node_crypto.cc | 31 ++++++++++++++++++++++++++-- src/node_crypto.h | 2 ++ src/tls_wrap.cc | 3 +-- test/parallel/test-tls-sni-option.js | 31 +++++++++++++++++++++------- 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index f20c9e2c1dfa33..f0569eb354ac5e 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -137,6 +137,8 @@ template class SSLWrap; template void SSLWrap::AddMethods(Environment* env, Local t); template void SSLWrap::InitNPN(SecureContext* sc); +template void SSLWrap::SetSNIContext(SecureContext* sc); +template int SSLWrap::SetCACerts(SecureContext* sc); template SSL_SESSION* SSLWrap::GetSessionCallback( SSL* s, unsigned char* key, @@ -2264,6 +2266,8 @@ void SSLWrap::CertCbDone(const FunctionCallbackInfo& 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) @@ -2314,6 +2318,30 @@ void SSLWrap::DestroySSL() { } +template +void SSLWrap::SetSNIContext(SecureContext* sc) { + InitNPN(sc); + CHECK_EQ(SSL_set_SSL_CTX(ssl_, sc->ctx_), sc->ctx_); + + SetCACerts(sc); +} + + +template +int SSLWrap::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(arg); @@ -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(ret.As()); - InitNPN(sc); - SSL_set_SSL_CTX(s, sc->ctx_); + conn->SetSNIContext(sc); } else { return SSL_TLSEXT_ERR_NOACK; } diff --git a/src/node_crypto.h b/src/node_crypto.h index 06e2ad40fba201..beeb2fb4458267 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -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_; diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index fc00893b77f84f..fe91f177132b7c 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -857,8 +857,7 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) { p->sni_context_.Reset(env->isolate(), ctx); SecureContext* sc = Unwrap(ctx.As()); - InitNPN(sc); - SSL_set_SSL_CTX(s, sc->ctx_); + p->SetSNIContext(sc); return SSL_TLSEXT_ERR_OK; } #endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB diff --git a/test/parallel/test-tls-sni-option.js b/test/parallel/test-tls-sni-option.js index 8ebc0c3af44705..84b2f506156818 100644 --- a/test/parallel/test-tls-sni-option.js +++ b/test/parallel/test-tls-sni-option.js @@ -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]; @@ -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'), @@ -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'), @@ -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) { @@ -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' + ]); });