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.

PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and targos committed Aug 15, 2019
1 parent 88726f2 commit 4570ed1
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
7 changes: 7 additions & 0 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#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 @@ -921,11 +922,17 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
if (UNLIKELY(!session->CanAddStream() ||
Http2Stream::New(session, id, frame->headers.cat) ==
nullptr)) {
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;
}

session->rejected_stream_count_ = 0;
} else if (!stream->IsDestroyed()) {
stream->StartHeaders(frame->headers.cat);
}
Expand Down
5 changes: 5 additions & 0 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,11 @@ class Http2Session : public AsyncWrap, public StreamListener {
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
5 changes: 4 additions & 1 deletion src/node_revert.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
**/
namespace node {

#define SECURITY_REVERSIONS(XX)
#define SECURITY_REVERSIONS(XX) \
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 4570ed1

Please sign in to comment.