[v10.x] http2: mitigate reported DoS attacks by addaleax · Pull Request #29123

[v10.x] http2: mitigate reported DoS attacks by addaleax · Pull Request #29123 · nodejs/node

and others added 18 commits

August 13, 2019 17:21

Multiple general improvements to http2 internals for
readability and efficiency

[This backport applied to v10.x cleanly.]

PR-URL: nodejs#23984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

[This backport applied to v10.x cleanly.]

PR-URL: nodejs#25822
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

PR-URL: nodejs#26990
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

PR-URL: nodejs#27295
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>

PR-URL: nodejs#28448
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

DRY up the `debug()` calls, and in particular, avoid building template
strings before we know whether we need to.

For some JS events, it only makes sense to call into JS when there
are listeners for the event in question.

The overhead is noticeable if a lot of these events are emitted during
the lifetime of a session. To reduce this overhead, keep track of
whether any/how many JS listeners are present, and if there are none,
skip calls into JS altogether.

This is part of performance improvements to mitigate CVE-2019-9513.

Lazily allocate `ArrayBuffer`s for the contents of DATA frames.
Creating `ArrayBuffer`s is, sadly, not a cheap operation with V8.

This is part of performance improvements to mitigate CVE-2019-9513.

Together with the previous commit, these changes improve throughput
in the adversarial case by about 100 %, and there is little more
that we can do besides artificially limiting the rate of incoming
metadata frames (i.e. after this patch, CPU usage is virtually
exclusively in libnghttp2).

[This backport also applies changes from 83e1b97 and required
some manual work due to the lack of `AllocatedBuffer` on v10.x.]

Refs: nodejs#26201

Limit the number of streams that are rejected upon creation. Since
each such rejection is associated with an `NGHTTP2_ENHANCE_YOUR_CALM`
error that should tell the peer to not open any more streams,
continuing to open streams should be read as a sign of a misbehaving
peer. The limit is currently set to 100 but could be changed or made
configurable.

This is intended to mitigate CVE-2019-9514.

Limit the number of invalid input frames, as they may be pointing towards a
misbehaving peer. The limit is currently set to 1000 but could be changed or
made configurable.

This is intended to mitigate CVE-2019-9514.

[This commit differs from the v12.x one due to the lack of
libuv/libuv@ee24ce900e5714c950b248da2b.
See the comment in the test for more details.]

Ignore headers with 0-length names and track memory for headers
the way we track it for other HTTP/2 session memory too.

This is intended to mitigate CVE-2019-9516.

Allocating memory upfront comes with overhead, and in particular,
`std::vector` implementations do not necessarily return memory
to the system when one might expect that (e.g. after shrinking the
vector).

If a write to the underlying socket finishes asynchronously, that
means that we cannot write any more data at that point without waiting
for it to finish. If this happens, we should also not be producing any
more input.

This is part of mitigating CVE-2019-9511/CVE-2019-9517.

If we are waiting for the ability to send more output, we should not
process more input. This commit a) makes us send output earlier,
during processing of input, if we accumulate a lot and b) allows
interrupting the call into nghttp2 that processes input data
and resuming it at a later time, if we do find ourselves in a position
where we are waiting to be able to send more output.

This is part of mitigating CVE-2019-9511/CVE-2019-9517.

nghttp2 has updated its limit for outstanding Ping/Settings ACKs
to 1000. This commit allows reverting to the old default of 10000.

The associated CVEs are CVE-2019-9512/CVE-2019-9515.

addaleax added security

Issues and PRs related to security.

http2

Issues or PRs related to the http2 subsystem.

fast-track

PRs that do not need to wait for 72 hours to land.

v10.x labels

Aug 14, 2019

addaleax removed the lib / src

Issues and PRs related to general changes in the lib or src directory.

label

Aug 14, 2019

addaleax added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Aug 14, 2019