FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http,https: protect against slow headers attack · nodejs/node@eb43bc0 · GitHub

/ node Public

Commit eb43bc0

Browse files
mcollina
authored and
rvagg
committed
http,https: protect against slow headers attack
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#150 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>
1 parent a8532d4 commit eb43bc0

8 files changed

Lines changed: 183 additions & 11 deletions

File tree

‎doc/api/http.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,26 @@ added: v0.7.0
941941

942942
Limits maximum incoming headers count. If set to 0, no limit will be applied.
943943

944+
### server.headersTimeout
945+
<!-- YAML
946+
added: REPLACEME
947+
-->
948+
949+
* {number} **Default:** `40000`
950+
951+
Limit the amount of time the parser will wait to receive the complete HTTP
952+
headers.
953+
954+
In case of inactivity, the rules defined in [server.timeout][] apply. However,
955+
that inactivity based timeout would still allow the connection to be kept open
956+
if the headers are being sent very slowly (by default, up to a byte per 2
957+
minutes). In order to prevent this, whenever header data arrives an additional
958+
check is made that more than `server.headersTimeout` milliseconds has not
959+
passed since the connection was established. If the check fails, a `'timeout'`
960+
event is emitted on the server object, and (by default) the socket is destroyed.
961+
See [server.timeout][] for more information on how timeout behaviour can be
962+
customised.
963+
944964
### server.setTimeout([msecs][, callback])
945965
<!-- YAML
946966
added: v0.9.12

‎doc/api/https.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ This method is identical to [`server.listen()`][] from [`net.Server`][].
4343

4444
See [`http.Server#maxHeadersCount`][].
4545

46+
### server.headersTimeout
47+
48+
- {number} **Default:** `40000`
49+
50+
See [`http.Server#headersTimeout`][].
51+
4652
### server.setTimeout([msecs][, callback])
4753
<!-- YAML
4854
added: v0.11.2
@@ -360,6 +366,7 @@ headers: max-age=0; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; p
360366
[`http.Agent`]: http.html#http_class_http_agent
361367
[`http.Server#keepAliveTimeout`]: http.html#http_server_keepalivetimeout
362368
[`http.Server#maxHeadersCount`]: http.html#http_server_maxheaderscount
369+
[`http.Server#headersTimeout`]: http.html#http_server_headerstimeout
363370
[`http.Server#setTimeout()`]: http.html#http_server_settimeout_msecs_callback
364371
[`http.Server#timeout`]: http.html#http_server_timeout
365372
[`http.Server`]: http.html#http_class_http_server

‎lib/_http_server.js‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const {
3737
_checkInvalidHeaderChar: checkInvalidHeaderChar
3838
} = require('_http_common');
3939
const { OutgoingMessage } = require('_http_outgoing');
40-
const { outHeadersKey, ondrain } = require('internal/http');
40+
const { outHeadersKey, ondrain, nowDate } = require('internal/http');
4141
const {
4242
defaultTriggerAsyncIdScope,
4343
getOrSetAsyncId
@@ -303,6 +303,7 @@ function Server(options, requestListener) {
303303
this.keepAliveTimeout = 5000;
304304
this._pendingResponseData = 0;
305305
this.maxHeadersCount = null;
306+
this.headersTimeout = 40 * 1000; // 40 seconds
306307
}
307308
util.inherits(Server, net.Server);
308309

@@ -341,6 +342,9 @@ function connectionListenerInternal(server, socket) {
341342
var parser = parsers.alloc();
342343
parser.reinitialize(HTTPParser.REQUEST);
343344
parser.socket = socket;
345+
346+
// We are starting to wait for our headers.
347+
parser.parsingHeadersStart = nowDate();
344348
socket.parser = parser;
345349

346350
// Propagate headers limit from server instance to parser
@@ -478,7 +482,20 @@ function socketOnData(server, socket, parser, state, d) {
478482

479483
function onParserExecute(server, socket, parser, state, ret) {
480484
socket._unrefTimer();
485+
const start = parser.parsingHeadersStart;
481486
debug('SERVER socketOnParserExecute %d', ret);
487+
488+
// If we have not parsed the headers, destroy the socket
489+
// after server.headersTimeout to protect from DoS attacks.
490+
// start === 0 means that we have parsed headers.
491+
if (start !== 0 && nowDate() - start > server.headersTimeout) {
492+
const serverTimeout = server.emit('timeout', socket);
493+
494+
if (!serverTimeout)
495+
socket.destroy();
496+
return;
497+
}
498+
482499
onParserExecuteCommon(server, socket, parser, state, ret, undefined);
483500
}
484501

@@ -589,6 +606,9 @@ function resOnFinish(req, res, socket, state, server) {
589606
function parserOnIncoming(server, socket, state, req, keepAlive) {
590607
resetSocketTimeout(server, socket, state);
591608

609+
// Set to zero to communicate that we have finished parsing.
610+
socket.parser.parsingHeadersStart = 0;
611+
592612
if (req.upgrade) {
593613
req.upgrade = req.method === 'CONNECT' ||
594614
server.listenerCount('upgrade') > 0;

‎lib/https.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ function Server(opts, requestListener) {
7575
this.timeout = 2 * 60 * 1000;
7676
this.keepAliveTimeout = 5000;
7777
this.maxHeadersCount = null;
78+
this.headersTimeout = 40 * 1000; // 40 seconds
7879
}
7980
inherits(Server, tls.Server);
8081

‎lib/internal/http.js‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,29 @@
22

33
const { setUnrefTimeout } = require('internal/timers');
44

5-
var dateCache;
5+
var nowCache;
6+
var utcCache;
7+
8+
function nowDate() {
9+
if (!nowCache) cache();
10+
return nowCache;
11+
}
12+
613
function utcDate() {
7-
if (!dateCache) {
8-
const d = new Date();
9-
dateCache = d.toUTCString();
14+
if (!utcCache) cache();
15+
return utcCache;
16+
}
1017

11-
setUnrefTimeout(resetCache, 1000 - d.getMilliseconds());
12-
}
13-
return dateCache;
18+
function cache() {
19+
const d = new Date();
20+
nowCache = d.valueOf();
21+
utcCache = d.toUTCString();
22+
setUnrefTimeout(resetCache, 1000 - d.getMilliseconds());
1423
}
1524

1625
function resetCache() {
17-
dateCache = undefined;
26+
nowCache = undefined;
27+
utcCache = undefined;
1828
}
1929

2030
function ondrain() {
@@ -24,5 +34,6 @@ function ondrain() {
2434
module.exports = {
2535
outHeadersKey: Symbol('outHeadersKey'),
2636
ondrain,
37+
nowDate,
2738
utcDate
2839
};

‎test/async-hooks/test-graph.http.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ process.on('exit', function() {
5252
triggerAsyncId: 'tcp:2' },
5353
{ type: 'Timeout',
5454
id: 'timeout:2',
55-
triggerAsyncId: 'httpparser:4' },
55+
triggerAsyncId: 'tcp:2' },
5656
{ type: 'TIMERWRAP',
5757
id: 'timer:2',
58-
triggerAsyncId: 'httpparser:4' },
58+
triggerAsyncId: 'tcp:2' },
5959
{ type: 'SHUTDOWNWRAP',
6060
id: 'shutdown:1',
6161
triggerAsyncId: 'tcp:2' } ]

‎test/parallel/test-http-slow-headers.js‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const { createServer } = require('http');
6+
const { connect } = require('net');
7+
const { finished } = require('stream');
8+
9+
// This test validates that the 'timeout' event fires
10+
// after server.headersTimeout.
11+
12+
const headers =
13+
'GET / HTTP/1.1\r\n' +
14+
'Host: localhost\r\n' +
15+
'Agent: node\r\n';
16+
17+
const server = createServer(common.mustNotCall());
18+
let sendCharEvery = 1000;
19+
20+
// 40 seconds is the default
21+
assert.strictEqual(server.headersTimeout, 40 * 1000);
22+
23+
// Pass a REAL env variable to shortening up the default
24+
// value which is 40s otherwise this is useful for manual
25+
// testing
26+
if (!process.env.REAL) {
27+
sendCharEvery = common.platformTimeout(10);
28+
server.headersTimeout = 2 * sendCharEvery;
29+
}
30+
31+
server.once('timeout', common.mustCall((socket) => {
32+
socket.destroy();
33+
}));
34+
35+
server.listen(0, common.mustCall(() => {
36+
const client = connect(server.address().port);
37+
client.write(headers);
38+
client.write('X-CRASH: ');
39+
40+
const interval = setInterval(() => {
41+
client.write('a');
42+
}, sendCharEvery);
43+
44+
client.resume();
45+
46+
finished(client, common.mustCall((err) => {
47+
clearInterval(interval);
48+
server.close();
49+
}));
50+
}));

‎test/parallel/test-https-slow-headers.js‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const { readKey } = require('../common/fixtures');
5+
6+
if (!common.hasCrypto)
7+
common.skip('missing crypto');
8+
9+
const assert = require('assert');
10+
const { createServer } = require('https');
11+
const { connect } = require('tls');
12+
const { finished } = require('stream');
13+
14+
// This test validates that the 'timeout' event fires
15+
// after server.headersTimeout.
16+
17+
const headers =
18+
'GET / HTTP/1.1\r\n' +
19+
'Host: localhost\r\n' +
20+
'Agent: node\r\n';
21+
22+
const server = createServer({
23+
key: readKey('agent1-key.pem'),
24+
cert: readKey('agent1-cert.pem'),
25+
ca: readKey('ca1-cert.pem'),
26+
}, common.mustNotCall());
27+
28+
let sendCharEvery = 1000;
29+
30+
// 40 seconds is the default
31+
assert.strictEqual(server.headersTimeout, 40 * 1000);
32+
33+
// pass a REAL env variable to shortening up the default
34+
// value which is 40s otherwise
35+
// this is useful for manual testing
36+
if (!process.env.REAL) {
37+
sendCharEvery = common.platformTimeout(10);
38+
server.headersTimeout = 2 * sendCharEvery;
39+
}
40+
41+
server.once('timeout', common.mustCall((socket) => {
42+
socket.destroy();
43+
}));
44+
45+
server.listen(0, common.mustCall(() => {
46+
const client = connect({
47+
port: server.address().port,
48+
rejectUnauthorized: false
49+
});
50+
client.write(headers);
51+
client.write('X-CRASH: ');
52+
53+
const interval = setInterval(() => {
54+
client.write('a');
55+
}, sendCharEvery);
56+
57+
client.resume();
58+
59+
finished(client, common.mustCall((err) => {
60+
clearInterval(interval);
61+
server.close();
62+
}));
63+
}));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL