Skip to content

Commit

Permalink
http2: limit number of rejected stream openings
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax authored and BethGriggs committed Aug 15, 2019
1 parent 7de642b commit 11b4e2c
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
13 changes: 9 additions & 4 deletions src/node_http2.cc
Expand Up @@ -4,6 +4,8 @@
#include "node_http2.h"
#include "node_http2_state.h"
#include "node_perf.h"
#include "node_revert.h"
#include "util-inl.h"

#include <algorithm>

Expand Down Expand Up @@ -970,15 +972,18 @@ inline int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
if (LIKELY(session->CanAddStream())) {
new Http2Stream(session, id, frame->headers.cat);
} else {
if (session->rejected_stream_count_++ > 100 &&
!IsReverted(SECURITY_REVERT_CVE_2019_9514)) {
return NGHTTP2_ERR_CALLBACK_FAILURE;
}
// Too many concurrent streams being opened
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
NGHTTP2_ENHANCE_YOUR_CALM);
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
} else {
// If the stream has already been destroyed, ignore.
if (stream->IsDestroyed())
return 0;

session->rejected_stream_count_ = 0;
} else if (!stream->IsDestroyed()) {
stream->StartHeaders(frame->headers.cat);
}
return 0;
Expand Down
5 changes: 5 additions & 0 deletions src/node_http2.h
Expand Up @@ -1071,6 +1071,11 @@ class Http2Session : public AsyncWrap {
std::vector<nghttp2_stream_write> outgoing_buffers_;
std::vector<uint8_t> outgoing_storage_;
std::vector<int32_t> pending_rst_streams_;
// Count streams that have been rejected while being opened. Exceeding a fixed
// limit will result in the session being destroyed, as an indication of a
// misbehaving peer. This counter is reset once new streams are being
// accepted again.
int32_t rejected_stream_count_ = 0;

void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length);
void ClearOutgoing(int status);
Expand Down
6 changes: 5 additions & 1 deletion src/node_revert.h
Expand Up @@ -16,7 +16,11 @@
namespace node {

#define SECURITY_REVERSIONS(XX) \
XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting")
XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting") \
XX(CVE_2019_9514, "CVE-2019-9514", "HTTP/2 Reset Flood") \
// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
// TODO(addaleax): Remove all of the above before Node.js 13 as the comment
// at the start of the file indicates.

enum reversion {
#define V(code, ...) SECURITY_REVERT_##code,
Expand Down

0 comments on commit 11b4e2c

Please sign in to comment.