From fc9a855084345404f7d0f6980afa8c20b1adf269 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Sun, 21 May 2017 13:00:20 +0800 Subject: [PATCH] http2: reject incompatible TLS ALPN handshakes Emit `unknownProtocol` event or silently destroy the socket PR-URL: https://github.com/nodejs/http2/pull/144 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- doc/api/http2.md | 16 ++++++++--- lib/internal/http2/core.js | 23 +++++++++------ test/parallel/test-http2-https-fallback.js | 33 +++++++++++++++++++--- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index adc0b0c118..e330f8f358 100755 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -437,7 +437,7 @@ an `Http2Stream`. #### Event: 'fetchTrailers' -The `'fetchTrailers`' event is emitted by the `Http2Stream` immediately after +The `'fetchTrailers'` event is emitted by the `Http2Stream` immediately after queuing the last chunk of payload data to be sent. The listener callback is passed a single object (with a `null` prototype) that the listener may used to specify the trailing header fields to send to the peer. @@ -696,7 +696,7 @@ an `Http2Session` object. If no listener is registered for this event, an #### Event: 'socketError' -The `'socketError`' event is emitted when an `'error'` event is emitted by +The `'socketError'` event is emitted when an `'error'` event is emitted by a `Socket` associated with the server. If no listener is registered for this event, an `'error'` event is emitted. @@ -745,10 +745,17 @@ an `Http2Session` object. If no listener is registered for this event, an #### Event: 'socketError' -The `'socketError`' event is emitted when an `'error'` event is emitted by +The `'socketError'` event is emitted when an `'error'` event is emitted by a `Socket` associated with the server. If no listener is registered for this event, an `'error'` event is emitted on the `Socket` instance instead. +#### Event: 'unknownProtocol' + +The `'unknownProtocol'` event is emitted when a connecting client fails to +negotiate an allowed protocol (i.e. HTTP/2 or HTTP/1.1). The event handler +receives the socket for handling. If no listener is registered for this event, +the connection is terminated. See the + #### Event: 'stream' The `'stream'` event is emitted when a `'stream'` event has been emitted by @@ -864,7 +871,7 @@ server.listen(80); * `options` {Object} * `allowHTTP1` {boolean} Incoming client connections that do not support HTTP/2 will be downgraded to HTTP/1.x when set to `true`. The default value - is `false`, which rejects non-HTTP/2 client connections. + is `false`. See the [`'unknownProtocol'`][] event. * `maxDefaultDynamicTableSize` {number} (TODO: Add detail) * `maxReservedRemoteStreams` {number} (TODO: Add detail) * `maxSendHeaderBlockLength` {number} (TODO: Add detail) @@ -1111,3 +1118,4 @@ TBD [Settings Object]: #http2_settings_object [Using options.selectPadding]: #http2_using_options_selectpadding [error code]: #error_codes +[`'unknownProtocol'`]: #http2_event_unknownprotocol diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 9771bcc325..3c37b24c50 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1619,11 +1619,17 @@ function connectionListener(socket) { socket.on('timeout', socketOnTimeout); } - // TLS ALPN fallback to HTTP/1.1 - if (options.allowHTTP1 === true && - (socket.alpnProtocol === false || - socket.alpnProtocol === 'http/1.1')) { - return httpConnectionListener.call(this, socket); + if (socket.alpnProtocol === false || socket.alpnProtocol === 'http/1.1') { + if (options.allowHTTP1 === true) { + // Fallback to HTTP/1.1 + return httpConnectionListener.call(this, socket); + } else if (this.emit('unknownProtocol', socket)) { + // Let event handler deal with the socket + return; + } else { + // Reject non-HTTP/2 client + return socket.destroy(); + } } socket.on('error', socketOnError); @@ -1658,10 +1664,11 @@ function initializeOptions(options) { function initializeTLSOptions(options, servername) { options = initializeOptions(options); - options.ALPNProtocols = ['h2', 'http/1.1']; - if (servername !== undefined && options.servername === undefined) { + options.ALPNProtocols = ['h2']; + if (options.allowHTTP1 === true) + options.ALPNProtocols.push('http/1.1'); + if (servername !== undefined && options.servername === undefined) options.servername = servername; - } return options; } diff --git a/test/parallel/test-http2-https-fallback.js b/test/parallel/test-http2-https-fallback.js index 643b18aeae..66e41a3819 100644 --- a/test/parallel/test-http2-https-fallback.js +++ b/test/parallel/test-http2-https-fallback.js @@ -1,6 +1,11 @@ 'use strict'; -const { fixturesDir, mustCall, mustNotCall } = require('../common'); +const { + fixturesDir, + mustCall, + mustNotCall, + platformTimeout +} = require('../common'); const { strictEqual } = require('assert'); const { join } = require('path'); const { readFileSync } = require('fs'); @@ -8,9 +13,21 @@ const { createSecureContext } = require('tls'); const { createSecureServer, connect } = require('http2'); const { get } = require('https'); const { parse } = require('url'); +const { connect: tls } = require('tls'); const countdown = (count, done) => () => --count === 0 && done(); +function expire(callback, ttl) { + const timeout = setTimeout( + mustNotCall('Callback expired'), + platformTimeout(ttl) + ); + return function expire() { + clearTimeout(timeout); + return callback(); + }; +} + function loadKey(keyname) { return readFileSync(join(fixturesDir, 'keys', keyname)); } @@ -68,7 +85,7 @@ function onSession(session) { server.listen(0); server.on('listening', mustCall(() => { - const port = server.address().port; + const { port } = server.address(); const origin = `https://localhost:${port}`; const cleanup = countdown(2, () => server.close()); @@ -110,13 +127,17 @@ function onSession(session) { mustCall(onRequest) ); + server.on('unknownProtocol', mustCall((socket) => { + socket.destroy(); + }, 2)); + server.listen(0); server.on('listening', mustCall(() => { - const port = server.address().port; + const { port } = server.address(); const origin = `https://localhost:${port}`; - const cleanup = countdown(2, () => server.close()); + const cleanup = countdown(3, () => server.close()); // HTTP/2 client connect( @@ -128,5 +149,9 @@ function onSession(session) { // HTTP/1.1 client get(Object.assign(parse(origin), clientOptions), mustNotCall()) .on('error', mustCall(cleanup)); + + // Incompatible ALPN TLS client + tls(Object.assign({ port, ALPNProtocols: ['fake'] }, clientOptions)) + .on('error', expire(mustCall(cleanup), 200)); })); }