From ea7b817266512b93b99032189984a3bd0f8386dc Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Fri, 2 Aug 2013 20:11:17 +0400
Subject: [PATCH] tls: fix handling of `SNICallback` server option

It shouldn't ignore it!

There're two possibile cases, which should be handled properly:

1. Having a default `SNICallback` which is using contexts, added with
  `server.addContext(...)` routine
2. Having a custom `SNICallback`.

In first case we may want to opt-out setting `.onsniselect` method (and
thus save some CPU time), if there're no contexts added. But, if custom
`SNICallback` is used, `.onsniselect` should always be set, because
server contexts don't affect it.
---
 lib/_tls_wrap.js                   |  14 +++-
 test/simple/test-tls-sni-option.js | 121 +++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 4 deletions(-)
 create mode 100644 test/simple/test-tls-sni-option.js

diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 1a5c6cb393..b432f89843 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -167,11 +167,15 @@ TLSSocket.prototype._init = function() {
     }
   };
 
+  // If custom SNICallback was given, or if
+  // there're SNI contexts to perform match against -
+  // set `.onsniselect` callback.
   if (process.features.tls_sni &&
       options.isServer &&
       options.server &&
-      options.SNICallback &&
-      options.server._contexts.length) {
+      (options.SNICallback !== SNICallback ||
+       options.server._contexts.length)) {
+    assert(typeof options.SNICallback === 'function');
     this.ssl.onsniselect = options.SNICallback;
   }
 
@@ -495,7 +499,7 @@ Server.prototype.addContext = function(servername, credentials) {
   this._contexts.push([re, crypto.createCredentials(credentials).context]);
 };
 
-Server.prototype.SNICallback = function(servername) {
+function SNICallback(servername) {
   var ctx;
 
   this._contexts.some(function(elem) {
@@ -506,7 +510,9 @@ Server.prototype.SNICallback = function(servername) {
   });
 
   return ctx;
-};
+}
+
+Server.prototype.SNICallback = SNICallback;
 
 
 // Target API:
diff --git a/test/simple/test-tls-sni-option.js b/test/simple/test-tls-sni-option.js
new file mode 100644
index 0000000000..351820227b
--- /dev/null
+++ b/test/simple/test-tls-sni-option.js
@@ -0,0 +1,121 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+if (!process.features.tls_sni) {
+  console.error('Skipping because node compiled without OpenSSL or ' +
+                'with old OpenSSL version.');
+  process.exit(0);
+}
+
+var common = require('../common'),
+    assert = require('assert'),
+    crypto = require('crypto'),
+    fs = require('fs'),
+    tls = require('tls');
+
+function filenamePEM(n) {
+  return require('path').join(common.fixturesDir, 'keys', n + '.pem');
+}
+
+function loadPEM(n) {
+  return fs.readFileSync(filenamePEM(n));
+}
+
+var serverOptions = {
+  key: loadPEM('agent2-key'),
+  cert: loadPEM('agent2-cert'),
+  SNICallback: function(servername) {
+    var credentials = SNIContexts[servername];
+    if (credentials)
+      return crypto.createCredentials(credentials).context;
+  }
+};
+
+var SNIContexts = {
+  'a.example.com': {
+    key: loadPEM('agent1-key'),
+    cert: loadPEM('agent1-cert')
+  },
+  'b.example.com': {
+    key: loadPEM('agent3-key'),
+    cert: loadPEM('agent3-cert')
+  }
+};
+
+var serverPort = common.PORT;
+
+var clientsOptions = [{
+  port: serverPort,
+  key: loadPEM('agent1-key'),
+  cert: loadPEM('agent1-cert'),
+  ca: [loadPEM('ca1-cert')],
+  servername: 'a.example.com',
+  rejectUnauthorized: false
+}, {
+  port: serverPort,
+  key: loadPEM('agent2-key'),
+  cert: loadPEM('agent2-cert'),
+  ca: [loadPEM('ca2-cert')],
+  servername: 'b.example.com',
+  rejectUnauthorized: false
+}, {
+  port: serverPort,
+  key: loadPEM('agent3-key'),
+  cert: loadPEM('agent3-cert'),
+  ca: [loadPEM('ca1-cert')],
+  servername: 'c.wrong.com',
+  rejectUnauthorized: false
+}];
+
+var serverResults = [],
+    clientResults = [];
+
+var server = tls.createServer(serverOptions, function(c) {
+  serverResults.push(c.servername);
+});
+
+server.listen(serverPort, startTest);
+
+function startTest() {
+  function connectClient(options, callback) {
+    var client = tls.connect(options, function() {
+      clientResults.push(
+          /Hostname\/IP doesn't/.test(client.authorizationError || ''));
+      client.destroy();
+
+      callback();
+    });
+  };
+
+  connectClient(clientsOptions[0], function() {
+    connectClient(clientsOptions[1], function() {
+      connectClient(clientsOptions[2], 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]);
+});