V10.17.0 proposal #29150
Closed
V10.17.0 proposal #29150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Multiple general improvements to http2 internals for readability and efficiency [This backport applied to v10.x cleanly.] Backport-PR-URL: #29123 PR-URL: #23984 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
[This backport applied to v10.x cleanly.] Backport-PR-URL: #29123 PR-URL: #25822 Reviewed-By: Gireesh Punathil <[email protected]>
Backport-PR-URL: #29123 PR-URL: #26990 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Backport-PR-URL: #29123 PR-URL: #27295 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Masashi Hirano <[email protected]>
Backport-PR-URL: #29123 PR-URL: #28448 Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This includes mitigations for CVE-2019-9512/CVE-2019-9515. Backport-PR-URL: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
DRY up the `debug()` calls, and in particular, avoid building template strings before we know whether we need to. Backport-PR-URL: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #26201 Backport-PR-URL: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
This is intended to mitigate CVE-2019-9518. Backport-PR-URL: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: #27914 Backport-PR-URL: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
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:
nodejs-github-bot
added
lib / src
|
Is there any possibility that e0e776331a, which was included in the |
To clarify, there is very likely not going to be a semver-minor release today (see #29148), and we don’t typically mix in other commits into security updates for LTS branches. I’ve added the |
|
Thanks @addaleax. |
|
@michaelsbradleyjr Also, if you want to have your own part in making that happen, feel free to do the backport yourself, https://github.com/nodejs/node/blob/master/doc/guides/backporting-to-release-lines.md should help with that and if you have any questions, feel free to ask them (in IRC or in #28799) 🙂 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
2019-08-15, Version 10.17.0 'Dubnium' (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:
Commits
74507fae34] - deps: update nghttp2 to 1.39.2 (Anna Henningsen) #29122a397c881ec] - deps: update nghttp2 to 1.39.1 (gengjiawen) #28448fedfa12a33] - deps: update nghttp2 to 1.38.0 (gengjiawen) #27295ab0f2ace36] - (SEMVER-MINOR) deps: update nghttp2 to 1.37.0 (gengjiawen) #269900acbe05ee2] - http2: allow security revert for Ping/Settings Flood (Anna Henningsen) #29122c152449012] - http2: pause input processing if sending output (Anna Henningsen) #291220ce699c7b1] - http2: stop reading from socket if writes are in progress (Anna Henningsen) #2912217357d37a9] - http2: consider 0-length non-end DATA frames an error (Anna Henningsen) #29122460f896c63] - http2: shrink defaultvector::reserve()allocations (Anna Henningsen) #29122f4242e24f9] - http2: handle 0-length headers better (Anna Henningsen) #29122477461a51f] - http2: limit number of invalid incoming frames (Anna Henningsen) #2912205dada46ee] - http2: limit number of rejected stream openings (Anna Henningsen) #291227f11465572] - http2: do not create ArrayBuffers when no DATA received (Anna Henningsen) #291222eb914ff5f] - http2: only call into JS when necessary for session events (Anna Henningsen) #2912276a7ada15d] - http2: improve JS-side debug logging (Anna Henningsen) #2912200f153da13] - http2: improve http2 code a bit (James M Snell) #23984a0a14c809f] - src: pass along errors from http2 object creation (Anna Henningsen) #25822d85e4006ab] - test: apply test-http2-max-session-memory-leak from v12.x (Anna Henningsen) #29122