From 11b4e2c0db0ea9ab555c83985f3c2f1a6354fe64 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 10 Aug 2019 22:27:48 +0200 Subject: [PATCH] http2: limit number of rejected stream openings 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: https://github.com/nodejs/node/pull/29124 PR-URL: https://github.com/nodejs/node/pull/29122 Reviewed-By: Rich Trott Reviewed-By: James M Snell --- src/node_http2.cc | 13 +++++++++---- src/node_http2.h | 5 +++++ src/node_revert.h | 6 +++++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index bf4dd8569d816f..b42773c3f82a55 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -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 @@ -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; diff --git a/src/node_http2.h b/src/node_http2.h index 226cafdb2bfe35..da86ff7e088684 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -1071,6 +1071,11 @@ class Http2Session : public AsyncWrap { std::vector outgoing_buffers_; std::vector outgoing_storage_; std::vector 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); diff --git a/src/node_revert.h b/src/node_revert.h index 9b4f1a9bbf6a20..1bb381c2f108de 100644 --- a/src/node_revert.h +++ b/src/node_revert.h @@ -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,