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]>
nodejs · Nov 13, 2015 · 483a41c · 483a41c
1 parent a5cce79
commit 483a41c
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 11 deletions.
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -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,
@@ -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)
@@ -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);
 
@@ -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;
       }

diff --git 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
@@ -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

diff --git 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'
+  ]);
 });