Skip to content

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:
- #30553
- #27711 (comment)
- #30515

Backport-PR-URL: #30471
PR-URL: #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>
  • Loading branch information
sam-github committed Feb 4, 2020
1 parent a28e5cc commit a9849c0
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 9 deletions.
11 changes: 11 additions & 0 deletions doc/api/cli.md
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand Down
6 changes: 6 additions & 0 deletions doc/node.1
Expand Up @@ -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.
.
Expand Down
4 changes: 3 additions & 1 deletion lib/_http_client.js
Expand Up @@ -31,6 +31,7 @@ const {
debug,
freeParser,
httpSocketSetup,
isLenient,
parsers
} = require('_http_common');
const { OutgoingMessage } = require('_http_outgoing');
Expand Down Expand Up @@ -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;
Expand Down
17 changes: 15 additions & 2 deletions lib/_http_common.js
Expand Up @@ -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,
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand All @@ -243,5 +255,6 @@ module.exports = {
httpSocketSetup,
methods,
parsers,
kIncomingMessage
kIncomingMessage,
isLenient
};
4 changes: 3 additions & 1 deletion lib/_http_server.js
Expand Up @@ -34,6 +34,7 @@ const {
chunkExpression,
httpSocketSetup,
kIncomingMessage,
isLenient,
_checkInvalidHeaderChar: checkInvalidHeaderChar
} = require('_http_common');
const { OutgoingMessage } = require('_http_outgoing');
Expand Down Expand Up @@ -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.
Expand Down
13 changes: 8 additions & 5 deletions src/node_http_parser.cc
Expand Up @@ -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);
}


Expand Down Expand Up @@ -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());
}


Expand Down Expand Up @@ -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());
Expand All @@ -493,7 +495,7 @@ class Parser : public AsyncWrap, public StreamListener {
if (isReused) {
parser->AsyncReset();
}
parser->Init(type);
parser->Init(type, lenient);
}


Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions src/node_options.h
Expand Up @@ -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;
Expand Down

0 comments on commit a9849c0

Please sign in to comment.