Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
http: opt-in insecure HTTP header parsing
Allow insecure HTTP header parsing. Make clear it is insecure.

See:
- #30553
- #27711 (comment)
- #30515

PR-URL: #30567
Reviewed-By: Fedor Indutny <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
sam-github committed Jan 10, 2020
commit d4bcc5f6f5ebebf6fbedd542ff92eb4312e7017e
11 changes: 11 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,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 @@ -1072,6 +1082,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 @@ -655,7 +656,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 @@ -495,6 +495,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 @@ -515,7 +516,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 @@ -799,12 +800,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 @@ -396,6 +396,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 @@ -157,6 +157,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