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 <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Author: mcollina

doc/api/http.md

@@ -941,6 +941,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

doc/api/https.md

@@ -43,6 +43,12 @@ This method is identical to [`server.listen()`][] from [`net.Server`][].
 
 See [`http.Server#maxHeadersCount`][].
 
+### server.headersTimeout
+
+-{number} **Default:**`40000`
+
+See [`http.Server#headersTimeout`][].
+
 ### server.setTimeout([msecs][, callback])
 <!-- YAML
 added: v0.11.2
@@ -360,6 +366,7 @@ headers: max-age=0; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18=" p
 [`http.Agent`]: http.html#http_class_http_agent
 [`http.Server#keepAliveTimeout`]: http.html#http_server_keepalivetimeout
 [`http.Server#maxHeadersCount`]: http.html#http_server_maxheaderscount
+[`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

lib/_http_server.js

@@ -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
@@ -303,6 +303,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);
 
@@ -341,6 +342,9 @@ function connectionListenerInternal(server, socket){
 varparser=parsers.alloc();
 parser.reinitialize(HTTPParser.REQUEST);
 parser.socket=socket;
+
+// We are starting to wait for our headers.
+parser.parsingHeadersStart=nowDate();
 socket.parser=parser;
 
 // Propagate headers limit from server instance to parser
@@ -478,7 +482,20 @@ function socketOnData(server, socket, parser, state, d){
 
 functiononParserExecute(server,socket,parser,state,ret){
 socket._unrefTimer();
+conststart=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){
+constserverTimeout=server.emit('timeout',socket);
+
+if(!serverTimeout)
+socket.destroy();
+return;
+}
+
 onParserExecuteCommon(server,socket,parser,state,ret,undefined);
 }
 
@@ -589,6 +606,9 @@ function resOnFinish(req, res, socket, state, server){
 functionparserOnIncoming(server,socket,state,req,keepAlive){
 resetSocketTimeout(server,socket,state);
 
+// Set to zero to communicate that we have finished parsing.
+socket.parser.parsingHeadersStart=0;
+
 if(req.upgrade){
 req.upgrade=req.method==='CONNECT'||
 server.listenerCount('upgrade')>0;

lib/https.js

@@ -75,6 +75,7 @@ function Server(opts, requestListener){
 this.timeout=2*60*1000;
 this.keepAliveTimeout=5000;
 this.maxHeadersCount=null;
+this.headersTimeout=40*1000;// 40 seconds
 }
 inherits(Server,tls.Server);
 

lib/internal/http.js

@@ -2,19 +2,29 @@
 
 const{ setUnrefTimeout }=require('internal/timers');
 
-vardateCache;
+varnowCache;
+varutcCache;
+
+functionnowDate(){
+if(!nowCache)cache();
+returnnowCache;
+}
+
 functionutcDate(){
-if(!dateCache){
-constd=newDate();
-dateCache=d.toUTCString();
+if(!utcCache)cache();
+returnutcCache;
+}
 
-setUnrefTimeout(resetCache,1000-d.getMilliseconds());
-}
-returndateCache;
+functioncache(){
+constd=newDate();
+nowCache=d.valueOf();
+utcCache=d.toUTCString();
+setUnrefTimeout(resetCache,1000-d.getMilliseconds());
 }
 
 functionresetCache(){
-dateCache=undefined;
+nowCache=undefined;
+utcCache=undefined;
 }
 
 functionondrain(){
@@ -24,5 +34,6 @@ function ondrain(){
 module.exports={
 outHeadersKey: Symbol('outHeadersKey'),
  ondrain,
+ nowDate,
  utcDate
 };

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

@@ -52,10 +52,10 @@ process.on('exit', function(){
 triggerAsyncId: 'tcp:2'},
 {type: 'Timeout',
 id: 'timeout:2',
-triggerAsyncId: 'httpparser:4'},
+triggerAsyncId: 'tcp:2'},
 {type: 'TIMERWRAP',
 id: 'timer:2',
-triggerAsyncId: 'httpparser:4'},
+triggerAsyncId: 'tcp:2'},
 {type: 'SHUTDOWNWRAP',
 id: 'shutdown:1',
 triggerAsyncId: 'tcp:2'}]

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

@@ -0,0 +1,50 @@
+'use strict';
+
+constcommon=require('../common');
+constassert=require('assert');
+const{ createServer }=require('http');
+const{ connect }=require('net');
+const{ finished }=require('stream');
+
+// This test validates that the 'timeout' event fires
+// after server.headersTimeout.
+
+constheaders=
+'GET / HTTP/1.1\r\n'+
+'Host: localhost\r\n'+
+'Agent: node\r\n';
+
+constserver=createServer(common.mustNotCall());
+letsendCharEvery=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(()=>{
+constclient=connect(server.address().port);
+client.write(headers);
+client.write('X-CRASH: ');
+
+constinterval=setInterval(()=>{
+client.write('a');
+},sendCharEvery);
+
+client.resume();
+
+finished(client,common.mustCall((err)=>{
+clearInterval(interval);
+server.close();
+}));
+}));

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

@@ -0,0 +1,63 @@
+'use strict';
+
+constcommon=require('../common');
+const{ readKey }=require('../common/fixtures');
+
+if(!common.hasCrypto)
+common.skip('missing crypto');
+
+constassert=require('assert');
+const{ createServer }=require('https');
+const{ connect }=require('tls');
+const{ finished }=require('stream');
+
+// This test validates that the 'timeout' event fires
+// after server.headersTimeout.
+
+constheaders=
+'GET / HTTP/1.1\r\n'+
+'Host: localhost\r\n'+
+'Agent: node\r\n';
+
+constserver=createServer({
+key: readKey('agent1-key.pem'),
+cert: readKey('agent1-cert.pem'),
+ca: readKey('ca1-cert.pem'),
+},common.mustNotCall());
+
+letsendCharEvery=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(()=>{
+constclient=connect({
+port: server.address().port,
+rejectUnauthorized: false
+});
+client.write(headers);
+client.write('X-CRASH: ');
+
+constinterval=setInterval(()=>{
+client.write('a');
+},sendCharEvery);
+
+client.resume();
+
+finished(client,common.mustCall((err)=>{
+clearInterval(interval);
+server.close();
+}));
+}));