Skip to content
This repository has been archived by the owner on Oct 16, 2021. It is now read-only.

Commit

Permalink
http: opt-in insecure HTTP header parsing
Browse files Browse the repository at this point in the history
Allow insecure HTTP header parsing. Make clear it is insecure.

See:
- nodejs/node#30553
- nodejs/node#27711 (comment)
- nodejs/node#30515

PR-URL: nodejs/node#30567
Backport-PR-URL: nodejs/node#30473
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>
  • Loading branch information
sam-github committed Feb 4, 2020
1 parent d41314e commit 496736f
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 4 deletions.
11 changes: 11 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,16 @@ added: v9.0.0
Specify the `module` of a custom [experimental ECMAScript Module][] loader.
`module` may be either a path to a file, or an ECMAScript Module name.

### `--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: v11.6.0
Expand Down Expand Up @@ -1043,6 +1053,7 @@ Node.js options that are allowed are:
* `--http-server-default-timeout`
* `--icu-data-dir`
* `--input-type`
* `--insecure-http-parser`
* `--inspect-brk`
* `--inspect-port`, `--debug-port`
* `--inspect-publish-uid`
Expand Down
6 changes: 6 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,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.
.
Expand Down
4 changes: 3 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const {
freeParser,
parsers,
HTTPParser,
isLenient,
prepareError,
} = require('_http_common');
const { OutgoingMessage } = require('_http_outgoing');
Expand Down Expand Up @@ -654,7 +655,8 @@ function tickOnSocket(req, socket) {
req.socket = socket;
req.connection = socket;
parser.initialize(HTTPParser.RESPONSE,
new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req));
new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
isLenient());
parser.socket = socket;
parser.outgoing = req;
req.parser = parser;
Expand Down
12 changes: 12 additions & 0 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const { getOptionValue } = require('internal/options');
const { methods, HTTPParser } =
getOptionValue('--http-parser') === 'legacy' ?
internalBinding('http_parser') : internalBinding('http_parser_llhttp');
const insecureHTTPParser = getOptionValue('--insecure-http-parser');

const FreeList = require('internal/freelist');
const incoming = require('_http_incoming');
Expand Down Expand Up @@ -238,6 +239,16 @@ function prepareError(err, parser, rawPacket) {
err.message = `Parse Error: ${err.reason}`;
}

let warnedLenient = false;

function isLenient() {
if (insecureHTTPParser && !warnedLenient) {
warnedLenient = true;
process.emitWarning('Using insecure HTTP parsing');
}
return insecureHTTPParser;
}

module.exports = {
_checkInvalidHeaderChar: checkInvalidHeaderChar,
_checkIsHttpToken: checkIsHttpToken,
Expand All @@ -250,5 +261,6 @@ module.exports = {
parsers,
kIncomingMessage,
HTTPParser,
isLenient,
prepareError,
};
4 changes: 3 additions & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const {
chunkExpression,
kIncomingMessage,
HTTPParser,
isLenient,
_checkInvalidHeaderChar: checkInvalidHeaderChar,
prepareError,
} = require('_http_common');
Expand Down Expand Up @@ -385,7 +386,8 @@ function connectionListenerInternal(server, socket) {
// https://github.com/nodejs/node/pull/21313
parser.initialize(
HTTPParser.REQUEST,
new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket)
new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
isLenient(),
);
parser.socket = socket;

Expand Down
7 changes: 5 additions & 2 deletions src/node_http_parser_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ class Parser : public AsyncWrap, public StreamListener {

static void Initialize(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
bool lenient = args[2]->IsTrue();

CHECK(args[0]->IsInt32());
CHECK(args[1]->IsObject());
Expand All @@ -528,7 +529,7 @@ class Parser : public AsyncWrap, public StreamListener {

parser->set_provider_type(provider);
parser->AsyncReset(args[1].As<Object>());
parser->Init(type);
parser->Init(type, lenient);
}

template <bool should_pause>
Expand Down Expand Up @@ -812,12 +813,14 @@ class Parser : public AsyncWrap, public StreamListener {
}


void Init(parser_type_t type) {
void Init(parser_type_t type, bool lenient) {
#ifdef NODE_EXPERIMENTAL_HTTP
llhttp_init(&parser_, type, &settings);
llhttp_set_lenient(&parser_, lenient);
header_nread_ = 0;
#else /* !NODE_EXPERIMENTAL_HTTP */
http_parser_init(&parser_, type);
parser_.lenient_http_headers = lenient;
#endif /* NODE_EXPERIMENTAL_HTTP */
url_.Reset();
status_message_.Reset();
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"(default: 120000)",
&EnvironmentOptions::http_server_default_timeout,
kAllowedInEnvironment);
AddOption("--insecure-http-parser",
"Use an insecure HTTP parser that accepts invalid HTTP headers",
&EnvironmentOptions::insecure_http_parser,
kAllowedInEnvironment);
AddOption("--input-type",
"set module type for string input",
&EnvironmentOptions::module_type,
Expand Down
2 changes: 2 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ class EnvironmentOptions : public Options {
bool print_eval = false;
bool force_repl = false;

bool insecure_http_parser = false;

bool tls_min_v1_0 = false;
bool tls_min_v1_1 = false;
bool tls_min_v1_2 = false;
Expand Down

0 comments on commit 496736f

Please sign in to comment.