Skip to content

Commit

Permalink
http2: refactor flood checks into a separate class. (#13098)
Browse files Browse the repository at this point in the history 
http2: refactor flood checks into a separate class.

Signed-off-by: Yan Avlasov <yavlasov@google.com>
  • Loading branch information
@yanavlasov
yanavlasov authored Sep 18, 2020
1 parent fe17c1c commit 68967b7
Show file tree
Hide file tree
Showing 14 changed files with 578 additions and 354 deletions.
4 changes: 2 additions & 2 deletions source/common/http/conn_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,15 @@ Network::FilterStatus ConnectionManagerImpl::onData(Buffer::Instance& data, bool

const Status status = codec_->dispatch(data);

ASSERT(!isPrematureResponseError(status));
if (isBufferFloodError(status)) {
if (isBufferFloodError(status) || isInboundFramesWithEmptyPayloadError(status)) {
handleCodecError(status.message());
return Network::FilterStatus::StopIteration;
} else if (isCodecProtocolError(status)) {
stats_.named_.downstream_cx_protocol_error_.inc();
handleCodecError(status.message());
return Network::FilterStatus::StopIteration;
}
ASSERT(status.ok());

// Processing incoming data may release outbound data so check for closure here as well.
checkForDeferredClose();
Expand Down
15 changes: 15 additions & 0 deletions source/common/http/http2/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ CODEC_LIB_DEPS = [
":codec_stats_lib",
":metadata_decoder_lib",
":metadata_encoder_lib",
":protocol_constraints_lib",
"//include/envoy/event:deferred_deletable",
"//include/envoy/event:dispatcher_interface",
"//include/envoy/http:codec_interface",
Expand Down Expand Up @@ -131,3 +132,17 @@ envoy_cc_library(
"//source/common/common:minimal_logger_lib",
],
)

envoy_cc_library(
name = "protocol_constraints_lib",
srcs = ["protocol_constraints.cc"],
hdrs = ["protocol_constraints.h"],
deps = [
":codec_stats_lib",
"//bazel/foreign_cc:nghttp2",
"//include/envoy/network:connection_interface",
"//source/common/common:assert_lib",
"//source/common/http:status_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
)
136 changes: 23 additions & 113 deletions source/common/http/http2/codec_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ Status ConnectionImpl::StreamImpl::onDataSourceSend(const uint8_t* framehd, size
// https://nghttp2.org/documentation/types.html#c.nghttp2_send_data_callback
static const uint64_t FRAME_HEADER_SIZE = 9;

parent_.outbound_data_frames_++;
parent_.protocol_constraints_.incrementOutboundDataFrameCount();

Buffer::OwnedImpl output;
auto status = parent_.addOutboundFrameFragment(output, framehd, FRAME_HEADER_SIZE);
Expand Down Expand Up @@ -552,16 +552,7 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stat
per_stream_buffer_limit_(http2_options.initial_stream_window_size().value()),
stream_error_on_invalid_http_messaging_(
http2_options.override_stream_error_on_invalid_http_message().value()),
max_outbound_frames_(http2_options.max_outbound_frames().value()),
frame_buffer_releasor_([this]() { releaseOutboundFrame(); }),
max_outbound_control_frames_(http2_options.max_outbound_control_frames().value()),
control_frame_buffer_releasor_([this]() { releaseOutboundControlFrame(); }),
max_consecutive_inbound_frames_with_empty_payload_(
http2_options.max_consecutive_inbound_frames_with_empty_payload().value()),
max_inbound_priority_frames_per_stream_(
http2_options.max_inbound_priority_frames_per_stream().value()),
max_inbound_window_update_frames_per_data_frame_sent_(
http2_options.max_inbound_window_update_frames_per_data_frame_sent().value()),
protocol_constraints_(stats, http2_options),
skip_encoding_empty_trailers_(Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.http2_skip_encoding_empty_trailers")),
dispatching_(false), raised_goaway_(false), pending_deferred_reset_(false) {}
Expand Down Expand Up @@ -857,39 +848,21 @@ int ConnectionImpl::onBeforeFrameSend(const nghttp2_frame* frame) {
return 0;
}

Status ConnectionImpl::incrementOutboundFrameCount(bool is_outbound_flood_monitored_control_frame) {
++outbound_frames_;
if (is_outbound_flood_monitored_control_frame) {
++outbound_control_frames_;
}
return checkOutboundQueueLimits();
}

Status ConnectionImpl::addOutboundFrameFragment(Buffer::OwnedImpl& output, const uint8_t* data,
size_t length) {
// Reset the outbound frame type (set in the onBeforeFrameSend callback) since the
// onBeforeFrameSend callback is not called for DATA frames.
bool is_outbound_flood_monitored_control_frame = false;
std::swap(is_outbound_flood_monitored_control_frame, is_outbound_flood_monitored_control_frame_);
RETURN_IF_ERROR(incrementOutboundFrameCount(is_outbound_flood_monitored_control_frame));
auto releasor =
protocol_constraints_.incrementOutboundFrameCount(is_outbound_flood_monitored_control_frame);
RETURN_IF_ERROR(checkOutboundFrameLimits());

output.add(data, length);
output.addDrainTracker(is_outbound_flood_monitored_control_frame ? control_frame_buffer_releasor_
: frame_buffer_releasor_);
output.addDrainTracker(releasor);
return okStatus();
}

void ConnectionImpl::releaseOutboundFrame() {
ASSERT(outbound_frames_ >= 1);
--outbound_frames_;
}

void ConnectionImpl::releaseOutboundControlFrame() {
ASSERT(outbound_control_frames_ >= 1);
--outbound_control_frames_;
releaseOutboundFrame();
}

StatusOr<ssize_t> ConnectionImpl::onSend(const uint8_t* data, size_t length) {
ENVOY_CONN_LOG(trace, "send data: bytes={}", connection_, length);
Buffer::OwnedImpl buffer;
Expand Down Expand Up @@ -1037,10 +1010,9 @@ Status ConnectionImpl::sendPendingFrames() {
return nghttp2_callback_status_;
}

// The frame flood error should set the nghttp2_callback_status_ error, and return at the
// statement above.
ASSERT(outbound_frames_ <= max_outbound_frames_ &&
outbound_control_frames_ <= max_outbound_control_frames_);
// Protocol constrain violations should set the nghttp2_callback_status_ error, and return at
// the statement above.
ASSERT(protocol_constraints_.status().ok());

return codecProtocolError(nghttp2_strerror(rc));
}
Expand Down Expand Up @@ -1420,86 +1392,24 @@ Status ServerConnectionImpl::trackInboundFrames(const nghttp2_frame_hd* hd,
ENVOY_CONN_LOG(trace, "track inbound frame type={} flags={} length={} padding_length={}",
connection_, static_cast<uint64_t>(hd->type), static_cast<uint64_t>(hd->flags),
static_cast<uint64_t>(hd->length), padding_length);
switch (hd->type) {
case NGHTTP2_HEADERS:
case NGHTTP2_CONTINUATION:
// Track new streams.
if (hd->flags & NGHTTP2_FLAG_END_HEADERS) {
inbound_streams_++;
}
FALLTHRU;
case NGHTTP2_DATA:
// Track frames with an empty payload and no end stream flag.
if (hd->length - padding_length == 0 && !(hd->flags & NGHTTP2_FLAG_END_STREAM)) {
ENVOY_CONN_LOG(trace, "frame with an empty payload and no end stream flag.", connection_);
consecutive_inbound_frames_with_empty_payload_++;
} else {
consecutive_inbound_frames_with_empty_payload_ = 0;
}
break;
case NGHTTP2_PRIORITY:
inbound_priority_frames_++;
break;
case NGHTTP2_WINDOW_UPDATE:
inbound_window_update_frames_++;
break;
default:
break;
}

return checkInboundFrameLimits(hd->stream_id);
}

Status ServerConnectionImpl::checkInboundFrameLimits(int32_t stream_id) {
ASSERT(dispatching_downstream_data_);
ConnectionImpl::StreamImpl* stream = getStream(stream_id);

if (consecutive_inbound_frames_with_empty_payload_ >
max_consecutive_inbound_frames_with_empty_payload_) {
ENVOY_CONN_LOG(trace,
"error reading frame: Too many consecutive frames with an empty payload "
"received in this HTTP/2 session.",
connection_);
if (stream) {
stream->setDetails(Http2ResponseCodeDetails::get().inbound_empty_frame_flood);
auto result = protocol_constraints_.trackInboundFrames(hd, padding_length);
if (!result.ok()) {
ENVOY_CONN_LOG(trace, "error reading frame: {} received in this HTTP/2 session.", connection_,
result.message());
if (isInboundFramesWithEmptyPayloadError(result)) {
ConnectionImpl::StreamImpl* stream = getStream(hd->stream_id);
if (stream) {
stream->setDetails(Http2ResponseCodeDetails::get().inbound_empty_frame_flood);
}
}
stats_.inbound_empty_frames_flood_.inc();
return bufferFloodError("Too many consecutive frames with an empty payload");
}

if (inbound_priority_frames_ >
static_cast<uint64_t>(max_inbound_priority_frames_per_stream_) * (1 + inbound_streams_)) {
ENVOY_CONN_LOG(trace,
"error reading frame: Too many PRIORITY frames received in this HTTP/2 session.",
connection_);
stats_.inbound_priority_frames_flood_.inc();
return bufferFloodError("Too many PRIORITY frames");
}

if (inbound_window_update_frames_ >
1 + 2 * (inbound_streams_ +
max_inbound_window_update_frames_per_data_frame_sent_ * outbound_data_frames_)) {
ENVOY_CONN_LOG(
trace,
"error reading frame: Too many WINDOW_UPDATE frames received in this HTTP/2 session.",
connection_);
stats_.inbound_window_update_frames_flood_.inc();
return bufferFloodError("Too many WINDOW_UPDATE frames");
}

return okStatus();
return result;
}

Status ServerConnectionImpl::checkOutboundQueueLimits() {
if (outbound_frames_ > max_outbound_frames_ && dispatching_downstream_data_) {
stats_.outbound_flood_.inc();
return bufferFloodError("Too many frames in the outbound queue.");
}
if (outbound_control_frames_ > max_outbound_control_frames_ && dispatching_downstream_data_) {
stats_.outbound_control_flood_.inc();
return bufferFloodError("Too many control frames in the outbound queue.");
}
return okStatus();
Status ServerConnectionImpl::checkOutboundFrameLimits() {
return dispatching_downstream_data_ ? protocol_constraints_.checkOutboundFrameLimits()
: okStatus();
}

Http::Status ServerConnectionImpl::dispatch(Buffer::Instance& data) {
Expand All @@ -1518,7 +1428,7 @@ Http::Status ServerConnectionImpl::innerDispatch(Buffer::Instance& data) {
Cleanup cleanup([this]() { dispatching_downstream_data_ = false; });

// Make sure downstream outbound queue was not flooded by the upstream frames.
RETURN_IF_ERROR(checkOutboundQueueLimits());
RETURN_IF_ERROR(checkOutboundFrameLimits());
return ConnectionImpl::innerDispatch(data);
}

Expand Down
67 changes: 6 additions & 61 deletions source/common/http/http2/codec_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "common/http/http2/codec_stats.h"
#include "common/http/http2/metadata_decoder.h"
#include "common/http/http2/metadata_encoder.h"
#include "common/http/http2/protocol_constraints.h"
#include "common/http/status.h"
#include "common/http/utility.h"

Expand Down Expand Up @@ -451,57 +452,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
// Set if the type of frame that is about to be sent is PING or SETTINGS with the ACK flag set, or
// RST_STREAM.
bool is_outbound_flood_monitored_control_frame_ = 0;
// This counter keeps track of the number of outbound frames of all types (these that were
// buffered in the underlying connection but not yet written into the socket). If this counter
// exceeds the `max_outbound_frames_' value the connection is terminated.
uint32_t outbound_frames_ = 0;
// Maximum number of outbound frames. Initialized from corresponding http2_protocol_options.
// Default value is 10000.
const uint32_t max_outbound_frames_;
const std::function<void()> frame_buffer_releasor_;
// This counter keeps track of the number of outbound frames of types PING, SETTINGS and
// RST_STREAM (these that were buffered in the underlying connection but not yet written into the
// socket). If this counter exceeds the `max_outbound_control_frames_' value the connection is
// terminated.
uint32_t outbound_control_frames_ = 0;
// Maximum number of outbound frames of types PING, SETTINGS and RST_STREAM. Initialized from
// corresponding http2_protocol_options. Default value is 1000.
const uint32_t max_outbound_control_frames_;
const std::function<void()> control_frame_buffer_releasor_;
// This counter keeps track of the number of consecutive inbound frames of types HEADERS,
// CONTINUATION and DATA with an empty payload and no end stream flag. If this counter exceeds
// the `max_consecutive_inbound_frames_with_empty_payload_` value the connection is terminated.
uint32_t consecutive_inbound_frames_with_empty_payload_ = 0;
// Maximum number of consecutive inbound frames of types HEADERS, CONTINUATION and DATA without
// a payload. Initialized from corresponding http2_protocol_options. Default value is 1.
const uint32_t max_consecutive_inbound_frames_with_empty_payload_;

// This counter keeps track of the number of inbound streams.
uint32_t inbound_streams_ = 0;
// This counter keeps track of the number of inbound PRIORITY frames. If this counter exceeds
// the value calculated using this formula:
//
// max_inbound_priority_frames_per_stream_ * (1 + inbound_streams_)
//
// the connection is terminated.
uint64_t inbound_priority_frames_ = 0;
// Maximum number of inbound PRIORITY frames per stream. Initialized from corresponding
// http2_protocol_options. Default value is 100.
const uint32_t max_inbound_priority_frames_per_stream_;

// This counter keeps track of the number of inbound WINDOW_UPDATE frames. If this counter exceeds
// the value calculated using this formula:
//
// 1 + 2 * (inbound_streams_ +
// max_inbound_window_update_frames_per_data_frame_sent_ * outbound_data_frames_)
//
// the connection is terminated.
uint64_t inbound_window_update_frames_ = 0;
// This counter keeps track of the number of outbound DATA frames.
uint64_t outbound_data_frames_ = 0;
// Maximum number of inbound WINDOW_UPDATE frames per outbound DATA frame sent. Initialized
// from corresponding http2_protocol_options. Default value is 10.
const uint32_t max_inbound_window_update_frames_per_data_frame_sent_;
ProtocolConstraints protocol_constraints_;

// For the flood mitigation to work the onSend callback must be called once for each outbound
// frame. This is what the nghttp2 library is doing, however this is not documented. The
Expand Down Expand Up @@ -535,12 +486,8 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
// Adds buffer fragment for a new outbound frame to the supplied Buffer::OwnedImpl.
// Returns Ok Status on success or error if outbound queue limits were exceeded.
Status addOutboundFrameFragment(Buffer::OwnedImpl& output, const uint8_t* data, size_t length);
virtual Status checkOutboundQueueLimits() PURE;
Status incrementOutboundFrameCount(bool is_outbound_flood_monitored_control_frame);
virtual Status checkOutboundFrameLimits() PURE;
virtual Status trackInboundFrames(const nghttp2_frame_hd* hd, uint32_t padding_length) PURE;
virtual Status checkInboundFrameLimits(int32_t stream_id) PURE;
void releaseOutboundFrame();
void releaseOutboundControlFrame();

bool dispatching_ : 1;
bool raised_goaway_ : 1;
Expand Down Expand Up @@ -576,9 +523,8 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
// mitigation on the downstream connections, however there is currently no mechanism for
// handling these types of errors.
// TODO(yanavlasov): add flood mitigation for upstream connections as well.
Status checkOutboundQueueLimits() override { return okStatus(); }
Status checkOutboundFrameLimits() override { return okStatus(); }
Status trackInboundFrames(const nghttp2_frame_hd*, uint32_t) override { return okStatus(); }
Status checkInboundFrameLimits(int32_t) override { return okStatus(); }

Http::ConnectionCallbacks& callbacks_;
};
Expand All @@ -601,16 +547,15 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
ConnectionCallbacks& callbacks() override { return callbacks_; }
Status onBeginHeaders(const nghttp2_frame* frame) override;
int onHeader(const nghttp2_frame* frame, HeaderString&& name, HeaderString&& value) override;
Status checkOutboundQueueLimits() override;
Status checkOutboundFrameLimits() override;
Status trackInboundFrames(const nghttp2_frame_hd* hd, uint32_t padding_length) override;
Status checkInboundFrameLimits(int32_t stream_id) override;
absl::optional<int> checkHeaderNameForUnderscores(absl::string_view header_name) override;

// Http::Connection
// The reason for overriding the dispatch method is to do flood mitigation only when
// processing data from downstream client. Doing flood mitigation when processing upstream
// responses makes clean-up tricky, which needs to be improved (see comments for the
// ClientConnectionImpl::checkOutboundQueueLimits method). The dispatch method on the
// ClientConnectionImpl::checkProtocolConstraintsStatus method). The dispatch method on the
// ServerConnectionImpl objects is called only when processing data from the downstream client in
// the ConnectionManagerImpl::onData method.
Http::Status dispatch(Buffer::Instance& data) override;
Expand Down
Loading

0 comments on commit 68967b7

Please sign in to comment.