tls: use the most recently added matching SecureContext in default SNICallback.

mkrawczuk · mkrawczuk · commit 36fb6f154792 · 2020-08-06T00:32:07.000+02:00
Fixes: https://github.com//issues/34110
diff --git a/doc/api/tls.md b/doc/api/tls.md
@@ -582,6 +582,9 @@ added: v0.5.3
 The `server.addContext()` method adds a secure context that will be used if
 the client request's SNI name matches the supplied `hostname` (or wildcard).
 
+When there are multiple matching contexts, the most recently added one will be
+used.
+
 ### `server.address()`
 <!-- YAML
 added: v0.6.0
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -1436,7 +1436,7 @@ Server.prototype[EE.captureRejectionSymbol] = function(
 function SNICallback(servername, callback) {
   const contexts = this.server._contexts;
 
-  for (const elem of contexts) {
+  for (const elem of contexts.reverse()) {
     if (elem[0].test(servername)) {
       callback(null, elem[1]);
       return;
diff --git a/test/parallel/test-tls-secure-context-usage-order.js b/test/parallel/test-tls-secure-context-usage-order.js
@@ -0,0 +1,89 @@
+'use strict';
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// This test ensures that when a TLS connection is estabilished, the server
+// selects the most recently added SecureContext that matches the servername.
+
+const assert = require('assert');
+const tls = require('tls');
+
+function loadPEM(n) {
+  return fixtures.readKey(`${n}.pem`);
+}
+
+const serverOptions = {
+  key: loadPEM('agent2-key'),
+  cert: loadPEM('agent2-cert'),
+  requestCert: true,
+  rejectUnauthorized: false,
+};
+
+const badSecureContext = {
+  key: loadPEM('agent1-key'),
+  cert: loadPEM('agent1-cert'),
+  ca: [ loadPEM('ca2-cert') ]
+};
+
+const goodSecureContext = {
+  key: loadPEM('agent1-key'),
+  cert: loadPEM('agent1-cert'),
+  ca: [ loadPEM('ca1-cert') ]
+};
+
+const server = tls.createServer(serverOptions, (c) => {
+  // The 'a' and 'b' subdomains are used to distinguish between client
+  // connections.
+  // Connection to subdomain 'a' is made when the 'bad' secure context is
+  // the only one in use.
+  if ('a.example.com' === c.servername) {
+    assert.strictEqual(c.authorized, false);
+  }
+  // Connection to subdomain 'b' is made after the 'good' context has been
+  // added.
+  if ('b.example.com' === c.servername) {
+    assert.strictEqual(c.authorized, true);
+  }
+});
+
+// 1. Add the 'bad' secure context. A connection using this context will not be
+// authorized.
+server.addContext('*.example.com', badSecureContext);
+
+server.listen(0, () => {
+  const options = {
+    port: server.address().port,
+    key: loadPEM('agent1-key'),
+    cert: loadPEM('agent1-cert'),
+    ca: [loadPEM('ca1-cert')],
+    servername: 'a.example.com',
+    rejectUnauthorized: false,
+  };
+
+  // 2. Make a connection using servername 'a.example.com'. Since a 'bad'
+  // secure context is used, this connection should not be authorized.
+  const client = tls.connect(options, () => {
+    client.end();
+  });
+
+  client.on('close', common.mustCall(() => {
+    // 3. Add a 'good' secure context.
+    server.addContext('*.example.com', goodSecureContext);
+
+    options.servername = 'b.example.com';
+    // 4. Make a connection using servername 'b.example.com'. This connection
+    // should be authorized because the 'good' secure context is the most
+    // recently added matching context.
+
+    const other = tls.connect(options, () => {
+      other.end();
+    });
+
+    other.on('close', common.mustCall(() => {
+      server.close();
+    }));
+  }));
+});
