Skip to content

Commit

Permalink
http,https: protect against slow headers attack
Browse files Browse the repository at this point in the history
CVE-2018-12122

An attacker can send a char/s within headers and exahust the resources
(file descriptors) of a system even with a tight max header length
protection. This PR destroys a socket if it has not received the headers
in 40s.

PR-URL: nodejs-private/node-private#151
Ref: nodejs-private/node-private#144
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
mcollina authored and rvagg committed Nov 27, 2018
1 parent 93dba83 commit 696f063
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 12 deletions.
20 changes: 20 additions & 0 deletions doc/api/http.md
Expand Up @@ -882,6 +882,26 @@ added: v0.7.0

Limits maximum incoming headers count. If set to 0 - no limit will be applied.

### server.headersTimeout
<!-- YAML
added: REPLACEME
-->

* {number} **Default:** `40000`

Limit the amount of time the parser will wait to receive the complete HTTP
headers.

In case of inactivity, the rules defined in [server.timeout][] apply. However,
that inactivity based timeout would still allow the connection to be kept open
if the headers are being sent very slowly (by default, up to a byte per 2
minutes). In order to prevent this, whenever header data arrives an additional
check is made that more than `server.headersTimeout` milliseconds has not
passed since the connection was established. If the check fails, a `'timeout'`
event is emitted on the server object, and (by default) the socket is destroyed.
See [server.timeout][] for more information on how timeout behaviour can be
customised.

### server.setTimeout([msecs][, callback])
<!-- YAML
added: v0.9.12
Expand Down
7 changes: 7 additions & 0 deletions doc/api/https.md
Expand Up @@ -36,6 +36,12 @@ See [`server.close()`][`http.close()`] from the HTTP module for details.
Starts the HTTPS server listening for encrypted connections.
This method is identical to [`server.listen()`][] from [`net.Server`][].

### server.headersTimeout

- {number} **Default:** `40000`

See [`http.Server#headersTimeout`][].

### server.setTimeout([msecs][, callback])
<!-- YAML
added: v0.11.2
Expand Down Expand Up @@ -253,6 +259,7 @@ const req = https.request(options, (res) => {
[`URL`]: url.html#url_the_whatwg_url_api
[`http.Agent`]: http.html#http_class_http_agent
[`http.Server#keepAliveTimeout`]: http.html#http_server_keepalivetimeout
[`http.Server#headersTimeout`]: http.html#http_server_headerstimeout
[`http.Server#setTimeout()`]: http.html#http_server_settimeout_msecs_callback
[`http.Server#timeout`]: http.html#http_server_timeout
[`http.Server`]: http.html#http_class_http_server
Expand Down
22 changes: 21 additions & 1 deletion lib/_http_server.js
Expand Up @@ -37,7 +37,7 @@ const {
_checkInvalidHeaderChar: checkInvalidHeaderChar
} = require('_http_common');
const { OutgoingMessage } = require('_http_outgoing');
const { outHeadersKey, ondrain } = require('internal/http');
const { outHeadersKey, ondrain, nowDate } = require('internal/http');
const {
defaultTriggerAsyncIdScope,
getOrSetAsyncId
Expand Down Expand Up @@ -296,6 +296,7 @@ function Server(options, requestListener) {
this.keepAliveTimeout = 5000;
this._pendingResponseData = 0;
this.maxHeadersCount = null;
this.headersTimeout = 40 * 1000; // 40 seconds
}
util.inherits(Server, net.Server);

Expand Down Expand Up @@ -334,6 +335,9 @@ function connectionListenerInternal(server, socket) {
var parser = parsers.alloc();
parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]);
parser.socket = socket;

// We are starting to wait for our headers.
parser.parsingHeadersStart = nowDate();
socket.parser = parser;
parser.incoming = null;

Expand Down Expand Up @@ -475,7 +479,20 @@ function socketOnData(server, socket, parser, state, d) {

function onParserExecute(server, socket, parser, state, ret) {
socket._unrefTimer();
const start = parser.parsingHeadersStart;
debug('SERVER socketOnParserExecute %d', ret);

// If we have not parsed the headers, destroy the socket
// after server.headersTimeout to protect from DoS attacks.
// start === 0 means that we have parsed headers.
if (start !== 0 && nowDate() - start > server.headersTimeout) {
const serverTimeout = server.emit('timeout', socket);

if (!serverTimeout)
socket.destroy();
return;
}

onParserExecuteCommon(server, socket, parser, state, ret, undefined);
}

Expand Down Expand Up @@ -579,6 +596,9 @@ function resOnFinish(req, res, socket, state, server) {
function parserOnIncoming(server, socket, state, req, keepAlive) {
resetSocketTimeout(server, socket, state);

// Set to zero to communicate that we have finished parsing.
socket.parser.parsingHeadersStart = 0;

state.incoming.push(req);

// If the writable end isn't consuming, then stop reading
Expand Down
2 changes: 2 additions & 0 deletions lib/https.js
Expand Up @@ -70,6 +70,8 @@ function Server(opts, requestListener) {

this.timeout = 2 * 60 * 1000;
this.keepAliveTimeout = 5000;

this.headersTimeout = 40 * 1000; // 40 seconds
}
inherits(Server, tls.Server);
exports.Server = Server;
Expand Down
33 changes: 23 additions & 10 deletions lib/internal/http.js
Expand Up @@ -2,18 +2,30 @@

const timers = require('timers');

var dateCache;
var nowCache;
var utcCache;

function nowDate() {
if (!nowCache) cache();
return nowCache;
}

function utcDate() {
if (!dateCache) {
const d = new Date();
dateCache = d.toUTCString();
timers.enroll(utcDate, 1000 - d.getMilliseconds());
timers._unrefActive(utcDate);
}
return dateCache;
if (!utcCache) cache();
return utcCache;
}
utcDate._onTimeout = function() {
dateCache = undefined;

function cache() {
const d = new Date();
nowCache = d.valueOf();
utcCache = d.toUTCString();
timers.enroll(cache, 1000 - d.getMilliseconds());
timers._unrefActive(cache);
}

cache._onTimeout = function() {
nowCache = undefined;
utcCache = undefined;
};

function ondrain() {
Expand All @@ -23,5 +35,6 @@ function ondrain() {
module.exports = {
outHeadersKey: Symbol('outHeadersKey'),
ondrain,
nowDate,
utcDate
};
2 changes: 1 addition & 1 deletion test/async-hooks/test-graph.http.js
Expand Up @@ -46,7 +46,7 @@ process.on('exit', function() {
triggerAsyncId: 'tcp:2' },
{ type: 'Timeout',
id: 'timeout:2',
triggerAsyncId: 'httpparser:2' },
triggerAsyncId: 'tcp:2' },
{ type: 'SHUTDOWNWRAP',
id: 'shutdown:1',
triggerAsyncId: 'tcp:2' } ]
Expand Down
56 changes: 56 additions & 0 deletions test/parallel/test-http-slow-headers.js
@@ -0,0 +1,56 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const { createServer } = require('http');
const { connect } = require('net');

// This test validates that the 'timeout' event fires
// after server.headersTimeout.

const headers =
'GET / HTTP/1.1\r\n' +
'Host: localhost\r\n' +
'Agent: node\r\n';

const server = createServer(common.mustNotCall());
let sendCharEvery = 1000;

// 40 seconds is the default
assert.strictEqual(server.headersTimeout, 40 * 1000);

// Pass a REAL env variable to shortening up the default
// value which is 40s otherwise this is useful for manual
// testing
if (!process.env.REAL) {
sendCharEvery = common.platformTimeout(10);
server.headersTimeout = 2 * sendCharEvery;
}

server.once('timeout', common.mustCall((socket) => {
socket.destroy();
}));

server.listen(0, common.mustCall(() => {
const client = connect(server.address().port);
client.write(headers);
client.write('X-CRASH: ');

const interval = setInterval(() => {
client.write('a');
}, sendCharEvery);

client.resume();

const onClose = common.mustCall(() => {
client.removeListener('close', onClose);
client.removeListener('error', onClose);
client.removeListener('end', onClose);
clearInterval(interval);
server.close();
});

client.on('error', onClose);
client.on('close', onClose);
client.on('end', onClose);
}));
69 changes: 69 additions & 0 deletions test/parallel/test-https-slow-headers.js
@@ -0,0 +1,69 @@
'use strict';

const common = require('../common');
const { readKey } = require('../common/fixtures');

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

const assert = require('assert');
const { createServer } = require('https');
const { connect } = require('tls');

// This test validates that the 'timeout' event fires
// after server.headersTimeout.

const headers =
'GET / HTTP/1.1\r\n' +
'Host: localhost\r\n' +
'Agent: node\r\n';

const server = createServer({
key: readKey('agent1-key.pem'),
cert: readKey('agent1-cert.pem'),
ca: readKey('ca1-cert.pem'),
}, common.mustNotCall());

let sendCharEvery = 1000;

// 40 seconds is the default
assert.strictEqual(server.headersTimeout, 40 * 1000);

// pass a REAL env variable to shortening up the default
// value which is 40s otherwise
// this is useful for manual testing
if (!process.env.REAL) {
sendCharEvery = common.platformTimeout(10);
server.headersTimeout = 2 * sendCharEvery;
}

server.once('timeout', common.mustCall((socket) => {
socket.destroy();
}));

server.listen(0, common.mustCall(() => {
const client = connect({
port: server.address().port,
rejectUnauthorized: false
});
client.write(headers);
client.write('X-CRASH: ');

const interval = setInterval(() => {
client.write('a');
}, sendCharEvery);

client.resume();

const onClose = common.mustCall(() => {
client.removeListener('close', onClose);
client.removeListener('error', onClose);
client.removeListener('end', onClose);
clearInterval(interval);
server.close();
});

client.on('error', onClose);
client.on('close', onClose);
client.on('end', onClose);
}));

0 comments on commit 696f063

Please sign in to comment.