From a9849c0ff6b4459880f8f6da10e6fedb3c4df620 Mon Sep 17 00:00:00 2001
From: Sam Roberts <vieuxtech@gmail.com>
Date: Wed, 20 Nov 2019 11:48:58 -0800
Subject: [PATCH] http: opt-in insecure HTTP header parsing

Allow insecure HTTP header parsing. Make clear it is insecure.

See:
- https://github.com/nodejs/node/pull/30553
- https://github.com/nodejs/node/issues/27711#issuecomment-556265881
- https://github.com/nodejs/node/issues/30515

Backport-PR-URL: https://github.com/nodejs/node/pull/30471
PR-URL: https://github.com/nodejs/node/pull/30567
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/cli.md          | 11 +++++++++++
 doc/node.1              |  6 ++++++
 lib/_http_client.js     |  4 +++-
 lib/_http_common.js     | 17 +++++++++++++++--
 lib/_http_server.js     |  4 +++-
 src/node_http_parser.cc | 13 ++++++++-----
 src/node_options.cc     |  4 ++++
 src/node_options.h      |  2 ++
 8 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index b5f678a892c9fa..2f946c2aea329a 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -181,6 +181,16 @@ added: v9.0.0
 
 Specify the `file` of the custom [experimental ECMAScript Module][] loader.
 
+### `--insecure-http-parser`
+<!-- YAML
+added: REPLACEME
+-->
+
+Use an insecure HTTP parser that accepts invalid HTTP headers. This may allow
+interoperability with non-conformant HTTP implementations. It may also allow
+request smuggling and other HTTP attacks that rely on invalid headers being
+accepted. Avoid using this option.
+
 ### `--max-http-header-size=size`
 <!-- YAML
 added: v10.15.0
@@ -608,6 +618,7 @@ Node.js options that are allowed are:
 - `--experimental-worker`
 - `--force-fips`
 - `--icu-data-dir`
+- `--insecure-http-parser`
 - `--inspect`
 - `--inspect-brk`
 - `--inspect-port`
diff --git a/doc/node.1 b/doc/node.1
index 5457915dd6e123..71767eb88d1cd6 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -139,6 +139,12 @@ Specify the
 as a custom loader, to load
 .Fl -experimental-modules .
 .
+.It Fl -insecure-http-parser
+Use an insecure HTTP parser that accepts invalid HTTP headers. This may allow
+interoperability with non-conformant HTTP implementations. It may also allow
+request smuggling and other HTTP attacks that rely on invalid headers being
+accepted. Avoid using this option.
+.
 .It Fl -max-http-header-size Ns = Ns Ar size
 Specify the maximum size of HTTP headers in bytes. Defaults to 8KB.
 .
diff --git a/lib/_http_client.js b/lib/_http_client.js
index a5cfdcba585a4a..1705e2e9efd4f4 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -31,6 +31,7 @@ const {
   debug,
   freeParser,
   httpSocketSetup,
+  isLenient,
   parsers
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');
@@ -626,7 +627,8 @@ function tickOnSocket(req, socket) {
   var parser = parsers.alloc();
   req.socket = socket;
   req.connection = socket;
-  parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
+  parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol],
+                      isLenient());
   parser.socket = socket;
   parser.outgoing = req;
   req.parser = parser;
diff --git a/lib/_http_common.js b/lib/_http_common.js
index 8b68d0c81500a8..a6b3bf1eb09cba 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -25,6 +25,8 @@ const { methods, HTTPParser } = internalBinding('http_parser');
 
 const { FreeList } = require('internal/freelist');
 const { ondrain } = require('internal/http');
+const { getOptionValue } = require('internal/options');
+const insecureHTTPParser = getOptionValue('--insecure-http-parser');
 const incoming = require('_http_incoming');
 const {
   IncomingMessage,
@@ -149,7 +151,7 @@ function parserOnMessageComplete() {
 
 
 const parsers = new FreeList('parsers', 1000, function parsersCb() {
-  const parser = new HTTPParser(HTTPParser.REQUEST);
+  const parser = new HTTPParser(HTTPParser.REQUEST, isLenient());
 
   cleanParser(parser);
 
@@ -232,6 +234,16 @@ function cleanParser(parser) {
   parser._consumed = false;
 }
 
+let warnedLenient = false;
+
+function isLenient() {
+  if (insecureHTTPParser && !warnedLenient) {
+    warnedLenient = true;
+    process.emitWarning('Using insecure HTTP parsing');
+  }
+  return insecureHTTPParser;
+}
+
 module.exports = {
   _checkInvalidHeaderChar: checkInvalidHeaderChar,
   _checkIsHttpToken: checkIsHttpToken,
@@ -243,5 +255,6 @@ module.exports = {
   httpSocketSetup,
   methods,
   parsers,
-  kIncomingMessage
+  kIncomingMessage,
+  isLenient
 };
diff --git a/lib/_http_server.js b/lib/_http_server.js
index c9fab108bf0b1a..bb965bcf157a36 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -34,6 +34,7 @@ const {
   chunkExpression,
   httpSocketSetup,
   kIncomingMessage,
+  isLenient,
   _checkInvalidHeaderChar: checkInvalidHeaderChar
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');
@@ -342,7 +343,8 @@ function connectionListenerInternal(server, socket) {
   socket.on('timeout', socketOnTimeout);
 
   var parser = parsers.alloc();
-  parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]);
+  parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol],
+                      isLenient());
   parser.socket = socket;
 
   // We are starting to wait for our headers.
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 420e94564eebd8..cf171d8f8d4843 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -161,11 +161,12 @@ struct StringPtr {
 
 class Parser : public AsyncWrap, public StreamListener {
  public:
-  Parser(Environment* env, Local<Object> wrap, enum http_parser_type type)
+  Parser(Environment* env, Local<Object> wrap, enum http_parser_type type,
+         bool lenient)
       : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_HTTPPARSER),
         current_buffer_len_(0),
         current_buffer_data_(nullptr) {
-    Init(type);
+    Init(type, lenient);
   }
 
 
@@ -383,7 +384,7 @@ class Parser : public AsyncWrap, public StreamListener {
     http_parser_type type =
         static_cast<http_parser_type>(args[0].As<Int32>()->Value());
     CHECK(type == HTTP_REQUEST || type == HTTP_RESPONSE);
-    new Parser(env, args.This(), type);
+    new Parser(env, args.This(), type, args[1]->IsTrue());
   }
 
 
@@ -475,6 +476,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
   static void Reinitialize(const FunctionCallbackInfo<Value>& args) {
     Environment* env = Environment::GetCurrent(args);
+    bool lenient = args[2]->IsTrue();
 
     CHECK(args[0]->IsInt32());
     CHECK(args[1]->IsBoolean());
@@ -493,7 +495,7 @@ class Parser : public AsyncWrap, public StreamListener {
     if (isReused) {
       parser->AsyncReset();
     }
-    parser->Init(type);
+    parser->Init(type, lenient);
   }
 
 
@@ -722,8 +724,9 @@ class Parser : public AsyncWrap, public StreamListener {
   }
 
 
-  void Init(enum http_parser_type type) {
+  void Init(enum http_parser_type type, bool lenient) {
     http_parser_init(&parser_, type);
+    parser_.lenient_http_headers = lenient;
     url_.Reset();
     status_message_.Reset();
     num_fields_ = 0;
diff --git a/src/node_options.cc b/src/node_options.cc
index e50fd786929b05..cbd1d1d5a130b5 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -110,6 +110,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::experimental_worker,
             kAllowedInEnvironment);
   AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
+  AddOption("--insecure-http-parser",
+            "Use an insecure HTTP parser that accepts invalid HTTP headers",
+            &EnvironmentOptions::insecure_http_parser,
+            kAllowedInEnvironment);
   AddOption("--loader",
             "(with --experimental-modules) use the specified file as a "
             "custom loader",
diff --git a/src/node_options.h b/src/node_options.h
index 389fec2f2e9968..e0e207cb20349c 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -93,6 +93,8 @@ class EnvironmentOptions : public Options {
   bool print_eval = false;
   bool force_repl = false;
 
+  bool insecure_http_parser = false;
+
   std::vector<std::string> preload_modules;
 
   std::vector<std::string> user_argv;