From b82059f898bfb0b6990dd116cc749396fe45e0fd Mon Sep 17 00:00:00 2001 From: "Mark S. Everitt" Date: Tue, 31 Oct 2017 20:03:28 +0000 Subject: [PATCH] tls: allow TLSSocket construction with a list of ALPNProtocols Brings the ALPNProtocols option of TLSSocket into line with the documentation. i.e. a list of strings for protocols may be used, and not only a buffer. Fixes: https://github.com/nodejs/node/issues/16643 --- lib/_tls_wrap.js | 22 +++--- ...et-constructor-alpn-npn-options-parsing.js | 78 +++++++++++++++++++ 2 files changed, 89 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index d7e349b239cb05..1bd76a7f97c870 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -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; @@ -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, @@ -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 }); diff --git a/test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js b/test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js new file mode 100644 index 00000000000000..fec06c94eec11f --- /dev/null +++ b/test/parallel/test-tls-socket-constructor-alpn-npn-options-parsing.js @@ -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' } + ]); + }); + }); +}));