tls: fix crash in SNICallback

`tls_wrap.cc` was crashing in an `Unwrap` call, when non
`SecureContext` object was passed to it. Check that the passed object
is a `SecureContext` instance before unwrapping it.

fix #7008

diff --git a/src/env.h b/src/env.h
index 26630bb..68528d0 100644 (file)
--- a/src/env.h
+++ b/src/env.h
@@ -120,6 +120,7 @@ namespace node {
   V(should_keep_alive_string, "shouldKeepAlive")                              \
   V(size_string, "size")                                                      \
   V(smalloc_p_string, "_smalloc_p")                                           \
+  V(sni_context_err_string, "Invalid SNI context")                            \
   V(sni_context_string, "sni_context")                                        \
   V(status_code_string, "statusCode")                                         \
   V(status_message_string, "statusMessage")                                   \

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index 92febc1..4475f7b 100644 (file)
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -747,29 +747,37 @@ void TLSCallbacks::SetServername(const FunctionCallbackInfo<Value>& args) {
 
 
 int TLSCallbacks::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
-  HandleScope scope(node_isolate);
-
   TLSCallbacks* p = static_cast<TLSCallbacks*>(arg);
   Environment* env = p->env();
 
   const char* servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name);
 
-  if (servername != NULL) {
-    // Call the SNI callback and use its return value as context
-    Local<Object> object = p->object();
-    Local<Value> ctx = object->Get(env->sni_context_string());
+  if (servername == NULL)
+    return SSL_TLSEXT_ERR_OK;
 
-    if (!ctx->IsObject())
-      return SSL_TLSEXT_ERR_NOACK;
+  HandleScope scope(env->isolate());
+  // Call the SNI callback and use its return value as context
+  Local<Object> object = p->object();
+  Local<Value> ctx = object->Get(env->sni_context_string());
 
-    p->sni_context_.Dispose();
-    p->sni_context_.Reset(node_isolate, ctx);
+  // Not an object, probably undefined or null
+  if (!ctx->IsObject())
+    return SSL_TLSEXT_ERR_NOACK;
 
-    SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
-    InitNPN(sc, p);
-    SSL_set_SSL_CTX(s, sc->ctx_);
+  Local<FunctionTemplate> cons = env->secure_context_constructor_template();
+  if (!cons->HasInstance(ctx)) {
+    // Failure: incorrect SNI context object
+    Local<Value> err = Exception::TypeError(env->sni_context_err_string());
+    p->MakeCallback(env->onerror_string(), 1, &err);
+    return SSL_TLSEXT_ERR_NOACK;
   }
 
+  p->sni_context_.Dispose();
+  p->sni_context_.Reset(node_isolate, ctx);
+
+  SecureContext* sc = Unwrap<SecureContext>(ctx.As<Object>());
+  InitNPN(sc, p);
+  SSL_set_SSL_CTX(s, sc->ctx_);
   return SSL_TLSEXT_ERR_OK;
 }
 #endif  // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB

diff --git a/test/simple/test-tls-sni-option.js b/test/simple/test-tls-sni-option.js
index aaf37c7..7de7dea 100644 (file)
--- a/test/simple/test-tls-sni-option.js
+++ b/test/simple/test-tls-sni-option.js
@@ -47,10 +47,14 @@ var serverOptions = {
 
     // Just to test asynchronous callback
     setTimeout(function() {
-      if (credentials)
-        callback(null, crypto.createCredentials(credentials).context);
-      else
+      if (credentials) {
+        if (credentials.emptyRegression)
+          callback(null, {});
+        else
+          callback(null, crypto.createCredentials(credentials).context);
+      } else {
         callback(null, null);
+      }
     }, 100);
   }
 };
@@ -63,6 +67,9 @@ var SNIContexts = {
   'b.example.com': {
     key: loadPEM('agent3-key'),
     cert: loadPEM('agent3-cert')
+  },
+  'c.another.com': {
+    emptyRegression: true
   }
 };
 
@@ -89,39 +96,73 @@ var clientsOptions = [{
   ca: [loadPEM('ca1-cert')],
   servername: 'c.wrong.com',
   rejectUnauthorized: false
+}, {
+  port: serverPort,
+  key: loadPEM('agent3-key'),
+  cert: loadPEM('agent3-cert'),
+  ca: [loadPEM('ca1-cert')],
+  servername: 'c.another.com',
+  rejectUnauthorized: false
 }];
 
 var serverResults = [],
-    clientResults = [];
+    clientResults = [],
+    serverErrors = [],
+    clientErrors = [],
+    serverError,
+    clientError;
 
 var server = tls.createServer(serverOptions, function(c) {
   serverResults.push(c.servername);
 });
 
+server.on('clientError', function(err) {
+  serverResults.push(null);
+  serverError = err.message;
+});
+
 server.listen(serverPort, startTest);
 
 function startTest() {
-  function connectClient(options, callback) {
+  function connectClient(i, callback) {
+    var options = clientsOptions[i];
+    clientError = null;
+    serverError = null;
+
     var client = tls.connect(options, function() {
       clientResults.push(
           /Hostname\/IP doesn't/.test(client.authorizationError || ''));
       client.destroy();
 
-      callback();
+      next();
     });
-  };
 
-  connectClient(clientsOptions[0], function() {
-    connectClient(clientsOptions[1], function() {
-      connectClient(clientsOptions[2], function() {
-        server.close();
-      });
+    client.on('error', function(err) {
+      clientResults.push(false);
+      clientError = err.message;
+      next();
     });
+
+    function next() {
+      clientErrors.push(clientError);
+      serverErrors.push(serverError);
+
+      if (i === clientsOptions.length - 1)
+        callback();
+      else
+        connectClient(i + 1, callback);
+    }
+  };
+
+  connectClient(0, function() {
+    server.close();
   });
 }
 
 process.on('exit', function() {
   assert.deepEqual(serverResults, ['a.example.com', 'b.example.com',
-                                   'c.wrong.com']);
-  assert.deepEqual(clientResults, [true, true, false]);
+                                   '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"]);
 });