Skip to content

Commit

Permalink
tls: accept array of protocols in TLSSocket
Browse files Browse the repository at this point in the history
Brings the ALPNProtocols & NPNProtocols options of TLSSocket in line
with the documentation. i.e. an array of strings for protocols may be
used, not only a buffer.

PR-URL: #16655
Fixes: https://github.com/node/issues/16643
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
  • Loading branch information
qubyte authored and apapirovski committed Nov 4, 2017
1 parent 7eb5ee3 commit 291ff72
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 11 deletions.
22 changes: 11 additions & 11 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,15 @@ function initRead(tls, wrapped) {
* Provides a wrap of socket stream to do encrypted communication.
*/

function TLSSocket(socket, options) {
if (options === undefined)
this._tlsOptions = {};
else
this._tlsOptions = options;
function TLSSocket(socket, opts) {
const tlsOptions = Object.assign({}, opts);

if (tlsOptions.NPNProtocols)
tls.convertNPNProtocols(tlsOptions.NPNProtocols, tlsOptions);
if (tlsOptions.ALPNProtocols)
tls.convertALPNProtocols(tlsOptions.ALPNProtocols, tlsOptions);

this._tlsOptions = tlsOptions;
this._secureEstablished = false;
this._securePending = false;
this._newSessionPending = false;
Expand Down Expand Up @@ -1099,11 +1103,7 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
'options.minDHSize is not a positive number: ' +
options.minDHSize);

const NPN = {};
const ALPN = {};
const context = options.secureContext || tls.createSecureContext(options);
tls.convertNPNProtocols(options.NPNProtocols, NPN);
tls.convertALPNProtocols(options.ALPNProtocols, ALPN);

var socket = new TLSSocket(options.socket, {
pipe: !!options.path,
Expand All @@ -1112,8 +1112,8 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
requestCert: true,
rejectUnauthorized: options.rejectUnauthorized !== false,
session: options.session,
NPNProtocols: NPN.NPNProtocols,
ALPNProtocols: ALPN.ALPNProtocols,
NPNProtocols: options.NPNProtocols,
ALPNProtocols: options.ALPNProtocols,
requestOCSP: options.requestOCSP
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
'use strict';

// Test that TLSSocket can take arrays of strings for ALPNProtocols and
// NPNProtocols.

const common = require('../common');

if (!common.hasCrypto)
common.skip('missing crypto');

const tls = require('tls');

new tls.TLSSocket(null, {
ALPNProtocols: ['http/1.1'],
NPNProtocols: ['http/1.1']
});

if (!process.features.tls_npn)
common.skip('node compiled without NPN feature of OpenSSL');

if (!process.features.tls_alpn)
common.skip('node compiled without ALPN feature of OpenSSL');

const assert = require('assert');
const net = require('net');
const fixtures = require('../common/fixtures');

const key = fixtures.readKey('agent1-key.pem');
const cert = fixtures.readKey('agent1-cert.pem');

const protocols = [];

const server = net.createServer(common.mustCall((s) => {
const tlsSocket = new tls.TLSSocket(s, {
isServer: true,
server,
key,
cert,
ALPNProtocols: ['http/1.1'],
NPNProtocols: ['http/1.1']
});

tlsSocket.on('secure', common.mustCall(() => {
protocols.push({
alpnProtocol: tlsSocket.alpnProtocol,
npnProtocol: tlsSocket.npnProtocol
});
tlsSocket.end();
}));
}, 2));

server.listen(0, common.mustCall(() => {
const alpnOpts = {
port: server.address().port,
rejectUnauthorized: false,
ALPNProtocols: ['h2', 'http/1.1']
};
const npnOpts = {
port: server.address().port,
rejectUnauthorized: false,
NPNProtocols: ['h2', 'http/1.1']
};

tls.connect(alpnOpts, function() {
this.end();

tls.connect(npnOpts, function() {
this.end();

server.close();

assert.deepStrictEqual(protocols, [
{ alpnProtocol: 'http/1.1', npnProtocol: false },
{ alpnProtocol: false, npnProtocol: 'http/1.1' }
]);
});
});
}));

2 comments on commit 291ff72

@apapirovski
Copy link
Member

Choose a reason for hiding this comment

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

Typo in commit message. Fixes: #16643

@apapirovski
Copy link
Member

@apapirovski apapirovski commented on 291ff72 Nov 4, 2017

Choose a reason for hiding this comment

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

For reference: here's the related fix for get-metadata https://github.com/joyeecheung/node-core-utils/pull/70 which should result in a new version going out on npm sometime soon.

Please sign in to comment.