Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls: use the most recently added matching SecureContext in default SN… #36072

Merged
merged 1 commit into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,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 is
used.

### `server.address()`
<!-- YAML
added: v0.6.0
Expand Down
3 changes: 2 additions & 1 deletion lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -1458,7 +1458,8 @@ Server.prototype[EE.captureRejectionSymbol] = function(
function SNICallback(servername, callback) {
const contexts = this.server._contexts;

for (const elem of contexts) {
for (let i = contexts.length - 1; i >= 0; --i) {
const elem = contexts[i];
if (RegExpPrototypeTest(elem[0], servername)) {
callback(null, elem[1]);
return;
Expand Down
99 changes: 99 additions & 0 deletions test/parallel/test-tls-secure-context-usage-order.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
'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 established, 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: we could be more strict here and assert that these are equal depending on the current step, same for the other conditional below

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(() => {
// 5. Make another connection using servername 'b.example.com' to ensure
// that the array of secure contexts is not reversed in place with each
// SNICallback call, as someone might be tempted to refactor this piece of
// code by using Array.prototype.reverse() method.
const onemore = tls.connect(options, () => {
onemore.end();
});

onemore.on('close', common.mustCall(() => {
server.close();
}));
}));
}));
});