Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

V8.16.1 proposal #29152

Merged
merged 19 commits into from
Aug 15, 2019
Merged

V8.16.1 proposal #29152

merged 19 commits into from
Aug 15, 2019

Conversation

BethGriggs
Copy link
Member

2019-08-15, Version 8.16.1 'Carbon' (LTS), @BethGriggs

Notable changes

This is a security release.

Node.js, as well as many other implementations of HTTP/2, have been found
vulnerable to Denial of Service attacks.
See https://github.com/Netflix/security-bulletins/blob/master/advisories/third-party/2019-002.md
for more information.

Vulnerabilities fixed:

  • CVE-2019-9511 “Data Dribble”: The attacker requests a large amount of data from a specified resource over multiple streams. They manipulate window size and stream priority to force the server to queue the data in 1-byte chunks. Depending on how efficiently this data is queued, this can consume excess CPU, memory, or both, potentially leading to a denial of service.
  • CVE-2019-9512 “Ping Flood”: The attacker sends continual pings to an HTTP/2 peer, causing the peer to build an internal queue of responses. Depending on how efficiently this data is queued, this can consume excess CPU, memory, or both, potentially leading to a denial of service.
  • CVE-2019-9513 “Resource Loop”: The attacker creates multiple request streams and continually shuffles the priority of the streams in a way that causes substantial churn to the priority tree. This can consume excess CPU, potentially leading to a denial of service.
  • CVE-2019-9514 “Reset Flood”: The attacker opens a number of streams and sends an invalid request over each stream that should solicit a stream of RST_STREAM frames from the peer. Depending on how the peer queues the RST_STREAM frames, this can consume excess memory, CPU, or both, potentially leading to a denial of service.
  • CVE-2019-9515 “Settings Flood”: The attacker sends a stream of SETTINGS frames to the peer. Since the RFC requires that the peer reply with one acknowledgement per SETTINGS frame, an empty SETTINGS frame is almost equivalent in behavior to a ping. Depending on how efficiently this data is queued, this can consume excess CPU, memory, or both, potentially leading to a denial of service.
  • CVE-2019-9516 “0-Length Headers Leak”: The attacker sends a stream of headers with a 0-length header name and 0-length header value, optionally Huffman encoded into 1-byte or greater headers. Some implementations allocate memory for these headers and keep the allocation alive until the session dies. This can consume excess memory, potentially leading to a denial of service.
  • CVE-2019-9517 “Internal Data Buffering”: The attacker opens the HTTP/2 window so the peer can send without constraint; however, they leave the TCP window closed so the peer cannot actually write (many of) the bytes on the wire. The attacker then sends a stream of requests for a large response object. Depending on how the servers queue the responses, this can consume excess memory, CPU, or both, potentially leading to a denial of service.
  • CVE-2019-9518 “Empty Frames Flood”: The attacker sends a stream of frames with an empty payload and without the end-of-stream flag. These frames can be DATA, HEADERS, CONTINUATION and/or PUSH_PROMISE. The peer spends time processing each frame disproportionate to attack bandwidth. This can consume excess CPU, potentially leading to a denial of service. (Discovered by Piotr Sikora of Google)

Commits

  • [6d427378c0] - deps: update nghttp2 to 1.39.2 (Anna Henningsen) #29122
  • [33d4d916d5] - deps: update nghttp2 to 1.39.1 (gengjiawen) #28448
  • [17fad97113] - deps: update nghttp2 to 1.38.0 (gengjiawen) #27295
  • [0b44733695] - deps: update nghttp2 to 1.37.0 (gengjiawen) #26990
  • [5afc77b044] - deps: update nghttp2 to 1.34.0 (James M Snell) #23284
  • [073108c855] - http2: allow security revert for Ping/Settings Flood (Anna Henningsen) #29122
  • [6d687f7af8] - http2: pause input processing if sending output (Anna Henningsen) #29122
  • [854dba649e] - http2: stop reading from socket if writes are in progress (Anna Henningsen) #29122
  • [a3191689dd] - http2: consider 0-length non-end DATA frames an error (Anna Henningsen) #29122
  • [156f2f35df] - http2: shrink default vector::reserve() allocations (Anna Henningsen) #29122
  • [10f05b65c4] - http2: handle 0-length headers better (Anna Henningsen) #29122
  • [ac28a628a5] - http2: limit number of invalid incoming frames (Anna Henningsen) #29122
  • [11b4e2c0db] - http2: limit number of rejected stream openings (Anna Henningsen) #29122
  • [7de642b6f9] - http2: do not create ArrayBuffers when no DATA received (Anna Henningsen) #29122
  • [dd60d3561a] - http2: only call into JS when necessary for session events (Anna Henningsen) #29122
  • [00f6846b73] - http2: improve JS-side debug logging (Anna Henningsen) #29122
  • [b095e35f1f] - http2: improve http2 code a bit (James M Snell) #23984
  • [cc282239c1] - test: apply test-http2-max-session-memory-leak from v12.x (Anna Henningsen) #29122

jasnell and others added 18 commits August 15, 2019 17:31
@jasnell @BethGriggs
http2: improve http2 code a bit
b095e35 
Multiple general improvements to http2 internals for
readability and efficiency

[This backport applied to v10.x cleanly but had several
merge conflicts on v8.x.]

PR-URL: #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>
@jasnell @BethGriggs
deps: update nghttp2 to 1.34.0
5afc77b 
Key new feature: RFC 8441 `:protocol` support

PR-URL: #23284
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@gengjiawen @BethGriggs
deps: update nghttp2 to 1.37.0
0b44733 
PR-URL: #26990
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@gengjiawen @BethGriggs
deps: update nghttp2 to 1.38.0
17fad97 
PR-URL: #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>
@gengjiawen @BethGriggs
deps: update nghttp2 to 1.39.1
33d4d91 
PR-URL: #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>
@addaleax @BethGriggs
deps: update nghttp2 to 1.39.2
6d42737 
This includes mitigations for CVE-2019-9512/CVE-2019-9515.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: improve JS-side debug logging
00f6846 
DRY up the `debug()` calls, and in particular, avoid building template
strings before we know whether we need to.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: only call into JS when necessary for session events
dd60d35 
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.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: do not create ArrayBuffers when no DATA received
7de642b 
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.
More work was necessary for v8.x, including copying utilities
for `util.h` from more recent Node.js versions.]

Refs: #26201

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: limit number of rejected stream openings
11b4e2c 
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.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: limit number of invalid incoming frames
ac28a62 
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.]

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: handle 0-length headers better
10f05b6 
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.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: shrink default vector::reserve() allocations
156f2f3 
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).

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: consider 0-length non-end DATA frames an error
a319168 
This is intended to mitigate CVE-2019-9518.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: stop reading from socket if writes are in progress
854dba6 
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.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: pause input processing if sending output
6d687f7 
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.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
http2: allow security revert for Ping/Settings Flood
073108c 
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.

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax @BethGriggs
test: apply test-http2-max-session-memory-leak from v12.x
cc28223 
Refs: #27914

Backport-PR-URL: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v8.x labels Aug 15, 2019
richardlau
richardlau reviewed Aug 15, 2019
View reviewed changes
src/node_version.h Outdated Show resolved Hide resolved
src/node_version.h Outdated Show resolved Hide resolved
@BethGriggs
2019-08-15, Version 8.16.1 'Carbon' (LTS)
5744b46 
This is a security release.

Notable changes:

Node.js, as well as many other implementations of HTTP/2, have been
found vulnerable to Denial of Service attacks.
See https://github.com/Netflix/security-bulletins/blob/master/advisories/third-party/2019-002.md
for more information.

Vulnerabilities fixed:

* CVE-2019-9511 “Data Dribble”: The attacker requests a large amount of
  data from a specified resource over multiple streams. They manipulate
  window size and stream priority to force the server to queue the data
  in 1-byte chunks. Depending on how efficiently this data is queued,
  this can consume excess CPU, memory, or both, potentially leading to a
  denial of service.
* CVE-2019-9512 “Ping Flood”: The attacker sends continual pings to an
  HTTP/2 peer, causing the peer to build an internal queue of responses.
  Depending on how efficiently this data is queued, this can consume
  excess CPU, memory, or both, potentially leading to a denial of
  service.
* CVE-2019-9513 “Resource Loop”: The attacker creates multiple request
  streams and continually shuffles the priority of the streams in a way
  that causes substantial churn to the priority tree. This can consume
  excess CPU, potentially leading to a denial of service.
* CVE-2019-9514 “Reset Flood”: The attacker opens a number of streams
  and sends an invalid request over each stream that should solicit a
  stream of RST_STREAM frames from the peer. Depending on how the peer
  queues the RST_STREAM frames, this can consume excess memory, CPU,or
  both, potentially leading to a denial of service.
* CVE-2019-9515 “Settings Flood”: The attacker sends a stream of
  SETTINGS frames to the peer. Since the RFC requires that the peer
  reply with one acknowledgement per SETTINGS frame, an empty SETTINGS
  frame is almost equivalent in behavior to a ping. Depending on how
  efficiently this data is queued, this can consume excess CPU, memory,
  or both, potentially leading to a denial of service.
* CVE-2019-9516 “0-Length Headers Leak”: The attacker sends a stream of
  headers with a 0-length header name and 0-length header value,
  optionally Huffman encoded into 1-byte or greater headers. Some
  implementations allocate memory for these headers and keep the
  allocation alive until the session dies. This can consume excess
  memory, potentially leading to a denial of service.
* CVE-2019-9517 “Internal Data Buffering”: The attacker opens the HTTP/2
  window so the peer can send without constraint; however, they leave
  the TCP window closed so the peer cannot actually write (many of) the
  bytes on the wire. The attacker then sends a stream of requests for a
  large response object. Depending on how the servers queue the
  responses, this can consume excess memory, CPU, or both, potentially
  leading to a denial of service.
* CVE-2019-9518 “Empty Frames Flood”: The attacker sends a stream of
  frames with an empty payload and without the end-of-stream flag. These
  frames can be DATA, HEADERS, CONTINUATION and/or PUSH_PROMISE. The
  peer spends time processing each frame disproportionate to attack
  bandwidth. This can consume excess CPU, potentially leading to a
  denial of service. (Discovered by Piotr Sikora of Google)

PR-URL: #29152
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/24999/

mhdawson
mhdawson approved these changes Aug 15, 2019
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25005/

@BethGriggs
Copy link
Member Author

I'm fairly confident the AIX failures are failures we were previously hitting in other release lines, and the relevant fixes haven't been backported yet (#29124 (comment)).

All releases apart from the PI builds are complete (refs: nodejs/build#1889)

@addaleax
Copy link
Member

Are the Pi issues blocking the release as a whole? :/

@BethGriggs
Copy link
Member Author

Going by https://github.com/nodejs/node/blob/master/doc/releases.md#9-produce-release-builds, I think we can release without the Pi builds, and follow up with those ASAP after the release

@BethGriggs BethGriggs merged commit 5744b46 into v8.x Aug 15, 2019
BethGriggs added a commit that referenced this pull request Aug 15, 2019
@BethGriggs
Working on v8.16.2
7bef222 
PR-URL: #29152
BethGriggs added a commit that referenced this pull request Aug 15, 2019
@BethGriggs
2019-08-15, Version 8.16.1 'Carbon' (LTS)
affa23b 
This is a security release.

Notable changes:

Node.js, as well as many other implementations of HTTP/2, have been
found vulnerable to Denial of Service attacks.
See https://github.com/Netflix/security-bulletins/blob/master/advisories/third-party/2019-002.md
for more information.

Vulnerabilities fixed:

* CVE-2019-9511 “Data Dribble”: The attacker requests a large amount of
  data from a specified resource over multiple streams. They manipulate
  window size and stream priority to force the server to queue the data
  in 1-byte chunks. Depending on how efficiently this data is queued,
  this can consume excess CPU, memory, or both, potentially leading to a
  denial of service.
* CVE-2019-9512 “Ping Flood”: The attacker sends continual pings to an
  HTTP/2 peer, causing the peer to build an internal queue of responses.
  Depending on how efficiently this data is queued, this can consume
  excess CPU, memory, or both, potentially leading to a denial of
  service.
* CVE-2019-9513 “Resource Loop”: The attacker creates multiple request
  streams and continually shuffles the priority of the streams in a way
  that causes substantial churn to the priority tree. This can consume
  excess CPU, potentially leading to a denial of service.
* CVE-2019-9514 “Reset Flood”: The attacker opens a number of streams
  and sends an invalid request over each stream that should solicit a
  stream of RST_STREAM frames from the peer. Depending on how the peer
  queues the RST_STREAM frames, this can consume excess memory, CPU,or
  both, potentially leading to a denial of service.
* CVE-2019-9515 “Settings Flood”: The attacker sends a stream of
  SETTINGS frames to the peer. Since the RFC requires that the peer
  reply with one acknowledgement per SETTINGS frame, an empty SETTINGS
  frame is almost equivalent in behavior to a ping. Depending on how
  efficiently this data is queued, this can consume excess CPU, memory,
  or both, potentially leading to a denial of service.
* CVE-2019-9516 “0-Length Headers Leak”: The attacker sends a stream of
  headers with a 0-length header name and 0-length header value,
  optionally Huffman encoded into 1-byte or greater headers. Some
  implementations allocate memory for these headers and keep the
  allocation alive until the session dies. This can consume excess
  memory, potentially leading to a denial of service.
* CVE-2019-9517 “Internal Data Buffering”: The attacker opens the HTTP/2
  window so the peer can send without constraint; however, they leave
  the TCP window closed so the peer cannot actually write (many of) the
  bytes on the wire. The attacker then sends a stream of requests for a
  large response object. Depending on how the servers queue the
  responses, this can consume excess memory, CPU, or both, potentially
  leading to a denial of service.
* CVE-2019-9518 “Empty Frames Flood”: The attacker sends a stream of
  frames with an empty payload and without the end-of-stream flag. These
  frames can be DATA, HEADERS, CONTINUATION and/or PUSH_PROMISE. The
  peer spends time processing each frame disproportionate to attack
  bandwidth. This can consume excess CPU, potentially leading to a
  denial of service. (Discovered by Piotr Sikora of Google)

PR-URL: #29152
BethGriggs added a commit to BethGriggs/nodejs.org that referenced this pull request Aug 15, 2019
@BethGriggs
Blog: v8.16.1 release post
561fe78 
Refs: nodejs/node#29152
BethGriggs added a commit to BethGriggs/nodejs.org that referenced this pull request Aug 15, 2019
@BethGriggs
Blog: v10.16.3 release post
a3048d5 
Refs: nodejs/node#29152
This was referenced Aug 15, 2019
Merged
Merged
BethGriggs added a commit to nodejs/nodejs.org that referenced this pull request Aug 15, 2019
@BethGriggs
Blog: v10.16.3 release post (#2413)
f7e2b28 
Refs: nodejs/node#29152
BethGriggs added a commit to nodejs/nodejs.org that referenced this pull request Aug 15, 2019
@BethGriggs
Blog: v8.16.1 release post (#2414)
5312c60 
Refs: nodejs/node#29152
@BethGriggs BethGriggs deleted the v8.16.1-proposal branch August 16, 2019 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@richardlau richardlau richardlau approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@sam-github sam-github sam-github approved these changes

Assignees
No one assigned
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@BethGriggs @nodejs-github-bot @addaleax @sam-github @cjihrig @richardlau @mhdawson @jasnell @gengjiawen