Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: remove security revert flags #29141

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -945,10 +942,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 @@ -1039,10 +1034,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 @@ -1412,8 +1405,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 @@ -2298,9 +2290,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