commit b13b4a9ffb6038be1c21c20ece50d5fe367807b3 Author: Matteo Collina Date: Sat Dec 1 16:29:13 2018 +0100 http: prevent slowloris with keepalive connections Fixes: https://github.com/nodejs-private/security/issues/214 PR-URL: https://github.com/nodejs-private/node-private/pull/162 Reviewed-By: Rod Vagg Reviewed-By: Sam Roberts Reviewed-By: Michael Dawson commit e9ae4aaaad5947075bdb3d1f558281aa1a729b36 Author: Alexey Orlenko Date: Thu Jun 8 17:20:24 2017 +0300 http: fix timeout reset after keep-alive timeout Fix the logic of resetting the socket timeout of keep-alive HTTP connections and add two tests: * `test-http-server-keep-alive-timeout-slow-server` is a regression test for GH-13391. It ensures that the server-side keep-alive timeout will not fire during processing of a request. * `test-http-server-keep-alive-timeout-slow-client-headers` ensures that the regular socket timeout is restored as soon as a client starts sending a new request, not as soon as the whole message is received, so that the keep-alive timeout will not fire while, e.g., the client is sending large cookies. commit 06a208d3166493cc5bbc63b5cdbe473dbf4ad0ec Author: realwakka Date: Sun Jun 4 14:03:11 2017 +0900 test: refactor test-http-server-keep-alive-timeout Make the same reliability changes that were applied to the https test in ce5745bf92f586c58366e9f738441d69118f2c18. Refs: https://github.com/nodejs/node/pull/13312 PR-URL: https://github.com/nodejs/node/pull/13448 Reviewed-By: Rich Trott Reviewed-By: Luigi Pinca Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Alexey Orlenko commit 1c7fbdc53bbf8bf43ca32b2c6e9f513508ac90fa Author: Rich Trott Date: Tue May 30 13:06:52 2017 -0700 test: improve test-https-server-keep-alive-timeout The test is flaky under load. These changes greatly improve reliability. * Use a recurring interval to determine when the test should end rather than a timer. * Increase server timeout to 500ms to allow for events being delayed by system load Changing to an interval has the added benefit of reducing the test run time from over 2 seconds to under 1 second. Fixes: https://github.com/nodejs/node/issues/13307 PR-URL: https://github.com/nodejs/node/pull/13312 Reviewed-By: Refael Ackermann Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Alexey Orlenko commit f23b3b6bad391cc3efe4cc815954f009c05fe63a Author: Timur Shemsedinov Date: Thu Oct 29 21:53:43 2015 +0200 http: destroy sockets after keepAliveTimeout Implement server.keepAliveTimeout in addition to server.timeout to prevent temporary socket/memory leaking in keep-alive mode. PR-URL: https://github.com/nodejs/node/pull/2534 Author: Timur Shemsedinov Author: Alexey Orlenko Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Franziska Hinkelmann Reviewed-By: Refael Ackermann Index: node-v4.9.1/lib/_http_server.js =================================================================== --- node-v4.9.1.orig/lib/_http_server.js +++ node-v4.9.1/lib/_http_server.js @@ -250,6 +250,7 @@ function Server(requestListener) { }); this.timeout = 2 * 60 * 1000; + this.keepAliveTimeout = 5000; this._pendingResponseData = 0; this.headersTimeout = 40 * 1000; // 40 seconds @@ -323,6 +324,8 @@ function connectionListener(socket) { socket.destroy(); }); + socket._keepAliveTimeoutSet = false; + var parser = parsers.alloc(); parser.reinitialize(HTTPParser.REQUEST); parser.socket = socket; @@ -373,6 +376,15 @@ function connectionListener(socket) { function socketOnData(d) { assert(!socket._paused); debug('SERVER socketOnData %d', d.length); + + if (socket._keepAliveTimeoutSet) { + socket.setTimeout(0); + if (self.timeout) { + socket.setTimeout(self.timeout); + } + socket._keepAliveTimeoutSet = false; + } + var ret = parser.execute(d); onParserExecuteCommon(ret, d); @@ -399,6 +411,8 @@ function connectionListener(socket) { } function onParserExecuteCommon(ret, d) { + resetSocketTimeout(self, socket); + if (ret instanceof Error) { debug('parse error'); socket.destroy(ret); @@ -481,8 +495,14 @@ function connectionListener(socket) { } function parserOnIncoming(req, shouldKeepAlive) { + resetSocketTimeout(self, socket); + incoming.push(req); + if (self.keepAliveTimeout > 0) { + req.on('end', resetHeadersTimeoutOnReqEnd); + } + // If the writable end isn't consuming, then stop reading // so that we don't become overwhelmed by a flood of // pipelined requests that may never be resolved. @@ -534,6 +554,12 @@ function connectionListener(socket) { if (res._last) { socket.destroySoon(); + } else if (outgoing.length === 0) { + if (self.keepAliveTimeout) { + socket.setTimeout(0); + socket.setTimeout(self.keepAliveTimeout); + socket._keepAliveTimeoutSet = true; + } } else { // start sending the next message var m = outgoing.shift(); @@ -561,6 +587,14 @@ function connectionListener(socket) { } exports._connectionListener = connectionListener; +function resetSocketTimeout(server, socket) { + if (!socket._keepAliveTimeoutSet) + return; + + socket.setTimeout(server.timeout || 0); + socket._keepAliveTimeoutSet = false; +} + function onSocketResume() { // It may seem that the socket is resumed, but this is an enemy's trick to // deceive us! `resume` is emitted asynchronously, and may be called from @@ -608,3 +642,15 @@ function socketOnWrap(ev, fn) { return res; } + +function resetHeadersTimeoutOnReqEnd() { + debug('resetHeadersTimeoutOnReqEnd'); + + var parser = this.socket.parser; + // Parser can be null if the socket was destroyed + // in that case, there is nothing to do. + if (parser !== null) { + parser.parsingHeadersStart = nowDate(); + } +} + Index: node-v4.9.1/test/parallel/test-http-server-keep-alive-timeout-slow-client-headers.js =================================================================== --- /dev/null +++ node-v4.9.1/test/parallel/test-http-server-keep-alive-timeout-slow-client-headers.js @@ -0,0 +1,57 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); +const net = require('net'); + +const server = http.createServer(common.mustCall((req, res) => { + res.end(); +}, 2)); + +server.keepAliveTimeout = common.platformTimeout(100); + +server.listen(0, common.mustCall(() => { + const port = server.address().port; + const socket = net.connect({ port }, common.mustCall(() => { + request(common.mustCall(() => { + // Make a second request on the same socket, after the keep-alive timeout + // has been set on the server side. + request(common.mustCall(() => {})); + })); + })); + + server.on('timeout', common.mustCall(() => { + socket.end(); + server.close(); + })); + + function request(callback) { + socket.setEncoding('utf8'); + socket.on('data', onData); + let response = ''; + + // Simulate a client that sends headers slowly (with a period of inactivity + // that is longer than the keep-alive timeout). + socket.write('GET / HTTP/1.1\r\n' + + `Host: localhost:${port}\r\n`); + setTimeout(() => { + socket.write('Connection: keep-alive\r\n' + + '\r\n'); + }, common.platformTimeout(300)); + + function onData(chunk) { + response += chunk; + if (chunk.includes('\r\n')) { + socket.removeListener('data', onData); + onHeaders(); + } + } + + function onHeaders() { + assert.ok(response.includes('HTTP/1.1 200 OK\r\n')); + assert.ok(response.includes('Connection: keep-alive\r\n')); + callback(); + } + } +})); Index: node-v4.9.1/test/parallel/test-http-slow-headers-keepalive.js =================================================================== --- /dev/null +++ node-v4.9.1/test/parallel/test-http-slow-headers-keepalive.js @@ -0,0 +1,59 @@ +'use strict'; + +const common = require('../common'); +const http = require('http'); +const net = require('net'); + +const headers = + 'GET / HTTP/1.1\r\n' + + 'Host: localhost\r\n' + + 'Connection: keep-alive' + + 'Agent: node\r\n'; + +let sendCharEvery = 1000; + +const server = http.createServer(common.mustCall((req, res) => { + res.writeHead(200); + res.end(); +})); + +// 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, () => { + const client = net.connect(server.address().port); + client.write(headers); + // finish the first request + client.write('\r\n'); + // second request + 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); +}); + Index: node-v4.9.1/test/parallel/test-http-server-keep-alive-timeout-slow-server.js =================================================================== --- /dev/null +++ node-v4.9.1/test/parallel/test-http-server-keep-alive-timeout-slow-server.js @@ -0,0 +1,50 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const server = http.createServer(common.mustCall((req, res) => { + if (req.url === '/first') { + res.end('ok'); + return; + } + setTimeout(() => { + res.end('ok'); + }, common.platformTimeout(500)); +}, 2)); + +server.keepAliveTimeout = common.platformTimeout(200); + +const agent = new http.Agent({ + keepAlive: true, + maxSockets: 1 +}); + +function request(path, callback) { + const port = server.address().port; + const req = http.request({ agent, path, port }, common.mustCall((res) => { + assert.strictEqual(res.statusCode, 200); + + res.setEncoding('utf8'); + + let result = ''; + res.on('data', (chunk) => { + result += chunk; + }); + + res.on('end', common.mustCall(() => { + assert.strictEqual(result, 'ok'); + callback(); + })); + })); + req.end(); +} + +server.listen(0, common.mustCall(() => { + request('/first', () => { + request('/second', () => { + server.close(); + }); + }); +})); Index: node-v4.9.1/doc/api/http.md =================================================================== --- node-v4.9.1.orig/doc/api/http.md +++ node-v4.9.1/doc/api/http.md @@ -729,17 +729,33 @@ customised. added: v0.9.12 --> -* {Number} Default = 120000 (2 minutes) +* {number} Timeout in milliseconds. Defaults to 120000 (2 minutes). The number of milliseconds of inactivity before a socket is presumed to have timed out. -Note that the socket timeout logic is set up on connection, so -changing this value only affects *new* connections to the server, not -any existing connections. +A value of 0 will disable the timeout behavior on incoming connections. -Set to 0 to disable any kind of automatic timeout behavior on incoming -connections. +*Note*: The socket timeout logic is set up on connection, so changing this +value only affects new connections to the server, not any existing connections. + +### server.keepAliveTimeout + + +* {number} Timeout in milliseconds. Defaults to 5000 (5 seconds). + +The number of milliseconds of inactivity a server needs to wait for additional +incoming data, after it has finished writing the last response, before a socket +will be destroyed. If the server receives new data before the keep-alive +timeout has fired, it will reset the regular inactivity timeout, i.e., +[`server.timeout`][]. + +A value of 0 will disable the keep-alive timeout behavior on incoming connections. + +*Note*: The socket timeout logic is set up on connection, so changing this +value only affects new connections to the server, not any existing connections. ## Class: http.ServerResponse +- {number} Defaults to 5000 (5 seconds). + +See [`http.Server#keepAliveTimeout`][]. + ## https.createServer(options[, requestListener])