http: add requestTimeout

This commits introduces a new http.Server option called requestTimeout
with a default value in milliseconds of 0.

If requestTimeout is set to a positive value, the server will start a new
timer set to expire in requestTimeout milliseconds when a new connection
is established. The timer is also set again if new requests after the
first are received on the socket (this handles pipelining and keep-alive
cases).
The timer is cancelled when:

1. the request body is completely received by the server.
2. the response is completed. This handles the case where the
application responds to the client without consuming the request body.
3. the connection is upgraded, like in the WebSocket case.

If the timer expires, then the server responds with status code 408 and
closes the connection.

CVE-2020-8251

PR-URL: https://github.com/nodejs-private/node-private/pull/208
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
Co-Authored-By: Paolo Insogna <paolo@cowtech.it>
Co-Authored-By: Robert Nagy <ronagy@icloud.com>

1 files changed, 83 insertions, 2 deletions

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 84f2696b307..21eb68d463e 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -40,6 +40,7 @@ const {
   continueExpression,
   chunkExpression,
   kIncomingMessage,
+  kRequestTimeout,
   HTTPParser,
   isLenient,
   _checkInvalidHeaderChar: checkInvalidHeaderChar,
@@ -61,6 +62,7 @@ const {
   codes
 } = require('internal/errors');
 const {
+  ERR_HTTP_REQUEST_TIMEOUT,
   ERR_HTTP_HEADERS_SENT,
   ERR_HTTP_INVALID_STATUS_CODE,
   ERR_HTTP_SOCKET_ENCODING,
@@ -77,6 +79,7 @@ const {
   DTRACE_HTTP_SERVER_RESPONSE
 } = require('internal/dtrace');
 const { observerCounts, constants } = internalBinding('performance');
+const { setTimeout, clearTimeout } = require('timers');
 const { NODE_PERFORMANCE_ENTRY_TYPE_HTTP } = constants;
 
 const kServerResponse = Symbol('ServerResponse');
@@ -148,6 +151,7 @@ const STATUS_CODES = {
   511: 'Network Authentication Required' // RFC 6585 6
 };
 
+const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0;
 const kOnExecute = HTTPParser.kOnExecute | 0;
 const kOnTimeout = HTTPParser.kOnTimeout | 0;
 
@@ -369,6 +373,7 @@ function Server(options, requestListener) {
   this.keepAliveTimeout = 5000;
   this.maxHeadersCount = null;
   this.headersTimeout = 60 * 1000; // 60 seconds
+  this.requestTimeout = 0; // 120 seconds
 }
 ObjectSetPrototypeOf(Server.prototype, net.Server.prototype);
 ObjectSetPrototypeOf(Server, net.Server);
@@ -491,6 +496,16 @@ function connectionListenerInternal(server, socket) {
   parser[kOnTimeout] =
     onParserTimeout.bind(undefined, server, socket);
 
+  // When receiving new requests on the same socket (pipelining or keep alive)
+  // make sure the requestTimeout is active.
+  parser[kOnMessageBegin] =
+    setRequestTimeout.bind(undefined, server, socket);
+
+  // This protects from DOS attack where an attacker establish the connection
+  // without sending any data on applications where server.timeout is left to
+  // the default value of zero.
+  setRequestTimeout(server, socket);
+
   socket._paused = false;
 }
 
@@ -586,6 +601,11 @@ function socketOnData(server, socket, parser, state, d) {
   onParserExecuteCommon(server, socket, parser, state, ret, d);
 }
 
+function onRequestTimeout(socket) {
+  socket[kRequestTimeout] = undefined;
+  socketOnError.call(socket, new ERR_HTTP_REQUEST_TIMEOUT());
+}
+
 function onParserExecute(server, socket, parser, state, ret) {
   // When underlying `net.Socket` instance is consumed - no
   // `data` events are emitted, and thus `socket.setTimeout` fires the
@@ -608,6 +628,10 @@ const badRequestResponse = Buffer.from(
   `HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
   `Connection: close${CRLF}${CRLF}`, 'ascii'
 );
+const requestTimeoutResponse = Buffer.from(
+  `HTTP/1.1 408 ${STATUS_CODES[408]}${CRLF}` +
+  `Connection: close${CRLF}${CRLF}`, 'ascii'
+);
 const requestHeaderFieldsTooLargeResponse = Buffer.from(
   `HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}` +
   `Connection: close${CRLF}${CRLF}`, 'ascii'
@@ -619,8 +643,20 @@ function socketOnError(e) {
 
   if (!this.server.emit('clientError', e, this)) {
     if (this.writable && this.bytesWritten === 0) {
-      const response = e.code === 'HPE_HEADER_OVERFLOW' ?
-        requestHeaderFieldsTooLargeResponse : badRequestResponse;
+      let response;
+
+      switch (e.code) {
+        case 'HPE_HEADER_OVERFLOW':
+          response = requestHeaderFieldsTooLargeResponse;
+          break;
+        case 'ERR_HTTP_REQUEST_TIMEOUT':
+          response = requestTimeoutResponse;
+          break;
+        default:
+          response = badRequestResponse;
+          break;
+      }
+
       this.write(response);
     }
     this.destroy(e);
@@ -660,11 +696,20 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
       const bodyHead = d.slice(ret, d.length);
 
       socket.readableFlowing = null;
+
+      // Clear the requestTimeout after upgrading the connection.
+      clearRequestTimeout(req);
+
       server.emit(eventName, req, socket, bodyHead);
     } else {
       // Got CONNECT method, but have no handler.
       socket.destroy();
     }
+  } else {
+    // When receiving new requests on the same socket (pipelining or keep alive)
+    // make sure the requestTimeout is active.
+    parser[kOnMessageBegin] =
+      setRequestTimeout.bind(undefined, server, socket);
   }
 
   if (socket._paused && socket.parser) {
@@ -692,6 +737,32 @@ function clearIncoming(req) {
   }
 }
 
+function setRequestTimeout(server, socket) {
+  // Set the request timeout handler.
+  if (
+    !socket[kRequestTimeout] &&
+    server.requestTimeout && server.requestTimeout > 0
+  ) {
+    debug('requestTimeout timer set');
+    socket[kRequestTimeout] =
+      setTimeout(onRequestTimeout, server.requestTimeout, socket).unref();
+  }
+}
+
+function clearRequestTimeout(req) {
+  if (!req) {
+    req = this;
+  }
+
+  if (!req[kRequestTimeout]) {
+    return;
+  }
+
+  debug('requestTimeout timer cleared');
+  clearTimeout(req[kRequestTimeout]);
+  req[kRequestTimeout] = undefined;
+}
+
 function resOnFinish(req, res, socket, state, server) {
   // Usually the first incoming element should be our request.  it may
   // be that in the case abortIncoming() was called that the incoming
@@ -706,6 +777,14 @@ function resOnFinish(req, res, socket, state, server) {
   if (!req._consuming && !req._readableState.resumeScheduled)
     req._dump();
 
+  // Make sure the requestTimeout is cleared before finishing.
+  // This might occur if the application has sent a response
+  // without consuming the request body, which would have alredy
+  // cleared the timer.
+  // clearRequestTimeout can be executed even if the timer is not active,
+  // so this is safe.
+  clearRequestTimeout(req);
+
   res.detachSocket(socket);
   clearIncoming(req);
   process.nextTick(emitCloseNT, res);
@@ -802,6 +881,8 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
       res.end();
     }
   } else {
+    req.on('end', clearRequestTimeout);
+
     server.emit('request', req, res);
   }
   return 0;  // No special treatment.