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
Backport 496736f

Original commit message:

    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
zsw007 committed Feb 11, 2020
1 parent 52c955e commit 11fdae4
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 7 deletions.
12 changes: 12 additions & 0 deletions doc/api/cli.md
Expand Up @@ -131,6 +131,17 @@ Useful when activating the inspector by sending the `SIGUSR1` signal.
Default host is 127.0.0.1.


### `--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.


### `--no-deprecation`
<!-- YAML
added: v0.8.0
Expand Down Expand Up @@ -476,6 +487,7 @@ Node.js options that are allowed are:
- `--enable-fips`
- `--force-fips`
- `--icu-data-dir`
- `--insecure-http-parser`
- `--inspect-brk`
- `--inspect-port`
- `--inspect`
Expand Down
7 changes: 7 additions & 0 deletions doc/node.1
Expand Up @@ -110,6 +110,13 @@ Activate inspector on host:port and break at start of user script.
.BR \-\-inspect-port \fI=[host:]port\fR
Set the host:port to be used when the inspector is activated.

.TP
.BR \-\-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.

.TP
.BR \-\-max\-http\-header-size \fI=size\fR
Specify the maximum size of HTTP headers in bytes. Defaults to 8KB.
Expand Down
7 changes: 5 additions & 2 deletions lib/_http_client.js
Expand Up @@ -31,7 +31,8 @@ const {
debug,
freeParser,
httpSocketSetup,
parsers
parsers,
isLenient
} = require('_http_common');
const { OutgoingMessage } = require('_http_outgoing');
const Agent = require('_http_agent');
Expand Down Expand Up @@ -622,7 +623,9 @@ 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());
if (process.domain) {
process.domain.add(parser);
}
Expand Down
13 changes: 12 additions & 1 deletion lib/_http_common.js
Expand Up @@ -353,6 +353,16 @@ function checkInvalidHeaderChar(val) {
return false;
}

let warnedLenient = false;

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

module.exports = {
_checkInvalidHeaderChar: checkInvalidHeaderChar,
_checkIsHttpToken: checkIsHttpToken,
Expand All @@ -364,5 +374,6 @@ module.exports = {
httpSocketSetup,
methods,
parsers,
kIncomingMessage
kIncomingMessage,
isLenient
};
9 changes: 7 additions & 2 deletions lib/_http_server.js
Expand Up @@ -34,7 +34,8 @@ const {
chunkExpression,
httpSocketSetup,
kIncomingMessage,
_checkInvalidHeaderChar: checkInvalidHeaderChar
_checkInvalidHeaderChar: checkInvalidHeaderChar,
isLenient
} = require('_http_common');
const { OutgoingMessage } = require('_http_outgoing');
const { outHeadersKey, ondrain, nowDate } = require('internal/http');
Expand Down Expand Up @@ -333,7 +334,11 @@ 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: 13 additions & 0 deletions src/node.cc
Expand Up @@ -240,6 +240,9 @@ std::string icu_data_dir; // NOLINT(runtime/string)

uint64_t max_http_header_size = 8 * 1024;

// Set in node.cc by ParseArgs when --insecure-http-parser is used.
bool insecure_http_parser = false;

// used by C++ modules as well
bool no_deprecation = false;

Expand Down Expand Up @@ -3265,6 +3268,11 @@ void SetupProcessObject(Environment* env,
preload_modules.clear();
}

// --insecure-http-parser
if (insecure_http_parser) {
READONLY_PROPERTY(process, "insecureHTTPParser", True(env->isolate()));
}

// --no-deprecation
if (no_deprecation) {
READONLY_PROPERTY(process, "noDeprecation", True(env->isolate()));
Expand Down Expand Up @@ -3539,6 +3547,8 @@ static void PrintHelp() {
" --inspect-port=[host:]port\n"
" set host:port for inspector\n"
#endif
" --insecure-http-parser use an insecure HTTP parser that\n"
" accepts invalid HTTP headers\n"
" --no-deprecation silence deprecation warnings\n"
" --max-http-header-size Specify the maximum size of HTTP\n"
" headers in bytes. Defaults to 8KB.\n"
Expand Down Expand Up @@ -3686,6 +3696,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env,
static const char* whitelist[] = {
// Node options, sorted in `node --help` order for ease of comparison.
"--require", "-r",
"--insecure-http-parser",
"--inspect",
"--inspect-brk",
"--inspect-port",
Expand Down Expand Up @@ -3832,6 +3843,8 @@ static void ParseArgs(int* argc,
syntax_check_only = true;
} else if (strcmp(arg, "--interactive") == 0 || strcmp(arg, "-i") == 0) {
force_repl = true;
} else if (strcmp(arg, "--insecure-http-parser") == 0) {
insecure_http_parser = true;
} else if (strcmp(arg, "--no-deprecation") == 0) {
no_deprecation = true;
} else if (strcmp(arg, "--napi-modules") == 0) {
Expand Down
6 changes: 4 additions & 2 deletions src/node_http_parser.cc
Expand Up @@ -476,6 +476,7 @@ class Parser : public AsyncWrap {

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


Expand Down Expand Up @@ -738,8 +739,9 @@ class Parser : public AsyncWrap {
}


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

0 comments on commit 11fdae4

Please sign in to comment.