Skip to content

Commit

Permalink
http2: remove security revert flags
Browse files Browse the repository at this point in the history
As the comment in `node_revert.h` indicates, the master branch should
not provide security revert flags.

Refs: #29122

PR-URL: #29141
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and Trott committed Aug 18, 2019
1 parent 27b7656 commit 4e782c9
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 20 deletions.
18 changes: 4 additions & 14 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,6 @@ Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
buffer[IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS]);
}

if (IsReverted(SECURITY_REVERT_CVE_2019_9512))
nghttp2_option_set_max_outbound_ack(options_, 10000);

// The padding strategy sets the mechanism by which we determine how much
// additional frame padding to apply to DATA and HEADERS frames. Currently
// this is set on a per-session basis, but eventually we may switch to
Expand Down Expand Up @@ -919,10 +916,8 @@ 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)) {
if (session->rejected_stream_count_++ > 100)
return NGHTTP2_ERR_CALLBACK_FAILURE;
}
// Too many concurrent streams being opened
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
NGHTTP2_ENHANCE_YOUR_CALM);
Expand Down Expand Up @@ -1013,10 +1008,8 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
Http2Session* session = static_cast<Http2Session*>(user_data);

Debug(session, "invalid frame received, code: %d", lib_error_code);
if (session->invalid_frame_count_++ > 1000 &&
!IsReverted(SECURITY_REVERT_CVE_2019_9514)) {
if (session->invalid_frame_count_++ > 1000)
return 1;
}

// If the error is fatal or if error code is ERR_STREAM_CLOSED... emit error
if (nghttp2_is_fatal(lib_error_code) ||
Expand Down Expand Up @@ -1383,8 +1376,7 @@ int Http2Session::HandleDataFrame(const nghttp2_frame* frame) {

if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
stream->EmitRead(UV_EOF);
} else if (frame->hd.length == 0 &&
!IsReverted(SECURITY_REVERT_CVE_2019_9518)) {
} else if (frame->hd.length == 0) {
return 1; // Consider 0-length frame without END_STREAM an error.
}
return 0;
Expand Down Expand Up @@ -2269,9 +2261,7 @@ bool Http2Stream::AddHeader(nghttp2_rcbuf* name,
if (this->statistics_.first_header == 0)
this->statistics_.first_header = uv_hrtime();
size_t name_len = nghttp2_rcbuf_get_buf(name).len;
if (name_len == 0 && !IsReverted(SECURITY_REVERT_CVE_2019_9516)) {
return true; // Ignore headers with empty names.
}
if (name_len == 0) return true; // Ignore headers with empty names.
size_t value_len = nghttp2_rcbuf_get_buf(value).len;
size_t length = name_len + value_len + 32;
// A header can only be added if we have not exceeded the maximum number
Expand Down
6 changes: 0 additions & 6 deletions src/node_revert.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,7 @@
namespace node {

#define SECURITY_REVERSIONS(XX) \
XX(CVE_2019_9512, "CVE-2019-9512", "HTTP/2 Ping/Settings Flood") \
XX(CVE_2019_9514, "CVE-2019-9514", "HTTP/2 Reset Flood") \
XX(CVE_2019_9516, "CVE-2019-9516", "HTTP/2 0-Length Headers Leak") \
XX(CVE_2019_9518, "CVE-2019-9518", "HTTP/2 Empty DATA Frame Flooding") \
// 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 4e782c9

Please sign in to comment.