diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 89f488130969a..70d6f3ad39afd 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -408,8 +408,6 @@ message Http2ProtocolOptions { // be written into the socket). Exceeding this limit triggers flood mitigation and connection is // terminated. The ``http2.outbound_flood`` stat tracks the number of terminated connections due // to flood mitigation. The default limit is 10000. - // NOTE: flood and abuse mitigation for upstream connections is presently enabled by the - // `envoy.reloadable_features.upstream_http2_flood_checks` flag. google.protobuf.UInt32Value max_outbound_frames = 7 [(validate.rules).uint32 = {gte: 1}]; // Limit the number of pending outbound downstream frames of types PING, SETTINGS and RST_STREAM, @@ -417,8 +415,6 @@ message Http2ProtocolOptions { // this limit triggers flood mitigation and connection is terminated. The // ``http2.outbound_control_flood`` stat tracks the number of terminated connections due to flood // mitigation. The default limit is 1000. - // NOTE: flood and abuse mitigation for upstream connections is presently enabled by the - // `envoy.reloadable_features.upstream_http2_flood_checks` flag. google.protobuf.UInt32Value max_outbound_control_frames = 8 [(validate.rules).uint32 = {gte: 1}]; // Limit the number of consecutive inbound frames of types HEADERS, CONTINUATION and DATA with an @@ -427,8 +423,6 @@ message Http2ProtocolOptions { // stat tracks the number of connections terminated due to flood mitigation. // Setting this to 0 will terminate connection upon receiving first frame with an empty payload // and no end stream flag. The default limit is 1. - // NOTE: flood and abuse mitigation for upstream connections is presently enabled by the - // `envoy.reloadable_features.upstream_http2_flood_checks` flag. google.protobuf.UInt32Value max_consecutive_inbound_frames_with_empty_payload = 9; // Limit the number of inbound PRIORITY frames allowed per each opened stream. If the number @@ -442,8 +436,6 @@ message Http2ProtocolOptions { // `opened_streams` is incremented when Envoy send the HEADERS frame for a new stream. The // ``http2.inbound_priority_frames_flood`` stat tracks // the number of connections terminated due to flood mitigation. The default limit is 100. - // NOTE: flood and abuse mitigation for upstream connections is presently enabled by the - // `envoy.reloadable_features.upstream_http2_flood_checks` flag. google.protobuf.UInt32Value max_inbound_priority_frames_per_stream = 10; // Limit the number of inbound WINDOW_UPDATE frames allowed per DATA frame sent. If the number @@ -460,8 +452,6 @@ message Http2ProtocolOptions { // flood mitigation. The default max_inbound_window_update_frames_per_data_frame_sent value is 10. // Setting this to 1 should be enough to support HTTP/2 implementations with basic flow control, // but more complex implementations that try to estimate available bandwidth require at least 2. - // NOTE: flood and abuse mitigation for upstream connections is presently enabled by the - // `envoy.reloadable_features.upstream_http2_flood_checks` flag. google.protobuf.UInt32Value max_inbound_window_update_frames_per_data_frame_sent = 11 [(validate.rules).uint32 = {gte: 1}]; diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 2bf18985b28ad..64866f1d8b634 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -46,6 +46,7 @@ Removed Config or Runtime * http: removed ``envoy.reloadable_features.return_502_for_upstream_protocol_errors``. Envoy will always return 502 code upon encountering upstream protocol error. * http: removed ``envoy.reloadable_features.treat_host_like_authority`` and legacy code paths. * http: removed ``envoy.reloadable_features.treat_upstream_connect_timeout_as_connect_failure`` and legacy code paths. +* http: removed ``envoy.reloadable_features.upstream_http2_flood_checks`` and legacy code paths. * upstream: removed ``envoy.reloadable_features.upstream_host_weight_change_causes_rebuild`` and legacy code paths. New Features diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index d663aca876d5f..261e644f7ab29 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -1210,7 +1210,8 @@ void ConnectionImpl::addOutboundFrameFragment(Buffer::OwnedImpl& output, const u // 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_); - auto releasor = trackOutboundFrames(is_outbound_flood_monitored_control_frame); + auto releasor = + protocol_constraints_.incrementOutboundFrameCount(is_outbound_flood_monitored_control_frame); output.add(data, length); output.addDrainTracker(releasor); } @@ -1864,8 +1865,7 @@ ClientConnectionImpl::ClientConnectionImpl( Nghttp2SessionFactory& http2_session_factory) : ConnectionImpl(connection, stats, random_generator, http2_options, max_response_headers_kb, max_response_headers_count), - callbacks_(callbacks), enable_upstream_http2_flood_checks_(Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.upstream_http2_flood_checks")) { + callbacks_(callbacks) { ClientHttp2Options client_http2_options(http2_options); if (use_new_codec_wrapper_) { adapter_ = http2_session_factory.create(http2_callbacks_.callbacks(), base(), @@ -1932,36 +1932,24 @@ int ClientConnectionImpl::onHeader(const nghttp2_frame* frame, HeaderString&& na Status ClientConnectionImpl::trackInboundFrames(const nghttp2_frame_hd* hd, uint32_t padding_length) { Status result; - if (enable_upstream_http2_flood_checks_) { - ENVOY_CONN_LOG(trace, "track inbound frame type={} flags={} length={} padding_length={}", - connection_, static_cast(hd->type), static_cast(hd->flags), - static_cast(hd->length), padding_length); - - 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); - } + ENVOY_CONN_LOG(trace, "track inbound frame type={} flags={} length={} padding_length={}", + connection_, static_cast(hd->type), static_cast(hd->flags), + static_cast(hd->length), padding_length); + + 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); } } } return result; } -// TODO(yanavlasov): move to the base class once the runtime flag is removed. -ProtocolConstraints::ReleasorProc -ClientConnectionImpl::trackOutboundFrames(bool is_outbound_flood_monitored_control_frame) { - if (enable_upstream_http2_flood_checks_) { - return protocol_constraints_.incrementOutboundFrameCount( - is_outbound_flood_monitored_control_frame); - } - return ProtocolConstraints::ReleasorProc([]() {}); -} - StreamResetReason ClientConnectionImpl::getMessagingErrorResetReason() const { connection_.streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamProtocolError); @@ -2053,12 +2041,6 @@ Status ServerConnectionImpl::trackInboundFrames(const nghttp2_frame_hd* hd, return result; } -ProtocolConstraints::ReleasorProc -ServerConnectionImpl::trackOutboundFrames(bool is_outbound_flood_monitored_control_frame) { - return protocol_constraints_.incrementOutboundFrameCount( - is_outbound_flood_monitored_control_frame); -} - Http::Status ServerConnectionImpl::dispatch(Buffer::Instance& data) { // Make sure downstream outbound queue was not flooded by the upstream frames. RETURN_IF_ERROR(protocol_constraints_.checkOutboundFrameLimits()); diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index bf750fc543e5b..4f775f2cb7b4b 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -601,8 +601,6 @@ class ConnectionImpl : public virtual Connection, // Adds buffer fragment for a new outbound frame to the supplied Buffer::OwnedImpl. void addOutboundFrameFragment(Buffer::OwnedImpl& output, const uint8_t* data, size_t length); - virtual ProtocolConstraints::ReleasorProc - trackOutboundFrames(bool is_outbound_flood_monitored_control_frame) PURE; virtual Status trackInboundFrames(const nghttp2_frame_hd* hd, uint32_t padding_length) PURE; void onKeepaliveResponse(); void onKeepaliveResponseTimeout(); @@ -649,19 +647,11 @@ class ClientConnectionImpl : public ClientConnection, 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; - - // Tracking of frames for flood and abuse mitigation for upstream connections is presently enabled - // by the `envoy.reloadable_features.upstream_http2_flood_checks` flag. - // TODO(yanavlasov): move to the base class once the runtime flag is removed. - ProtocolConstraints::ReleasorProc trackOutboundFrames(bool) override; Status trackInboundFrames(const nghttp2_frame_hd*, uint32_t) override; - void dumpStreams(std::ostream& os, int indent_level) const override; StreamResetReason getMessagingErrorResetReason() const override; Http::ConnectionCallbacks& callbacks_; std::chrono::milliseconds idle_session_requires_ping_interval_; - // Latched value of "envoy.reloadable_features.upstream_http2_flood_checks" runtime feature. - bool enable_upstream_http2_flood_checks_; }; /** @@ -682,8 +672,6 @@ 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; - ProtocolConstraints::ReleasorProc - trackOutboundFrames(bool is_outbound_flood_monitored_control_frame) override; Status trackInboundFrames(const nghttp2_frame_hd* hd, uint32_t padding_length) override; absl::optional checkHeaderNameForUnderscores(absl::string_view header_name) override; StreamResetReason getMessagingErrorResetReason() const override { diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 8194a069014b5..fa8fa98a0f79f 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -86,7 +86,6 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.use_observable_cluster_name", "envoy.reloadable_features.validate_connect", "envoy.reloadable_features.vhds_heartbeats", - "envoy.reloadable_features.upstream_http2_flood_checks", "envoy.restart_features.explicit_wildcard_resource", "envoy.restart_features.use_apple_api_for_dns_lookups", // Misplaced flags: please do not add flags to this section. diff --git a/test/integration/http2_flood_integration_test.cc b/test/integration/http2_flood_integration_test.cc index f24d631531773..848ad5b9964f4 100644 --- a/test/integration/http2_flood_integration_test.cc +++ b/test/integration/http2_flood_integration_test.cc @@ -60,7 +60,7 @@ class Http2FloodMitigationTest } protected: - bool initializeUpstreamFloodTest(); + void initializeUpstreamFloodTest(); std::vector serializeFrames(const Http2Frame& frame, uint32_t num_frames); void floodServer(const Http2Frame& frame, const std::string& flood_stat, uint32_t num_frames); void floodServer(absl::string_view host, absl::string_view path, @@ -81,13 +81,12 @@ INSTANTIATE_TEST_SUITE_P( testing::ValuesIn({false, true})), testParamsToString); -bool Http2FloodMitigationTest::initializeUpstreamFloodTest() { +void Http2FloodMitigationTest::initializeUpstreamFloodTest() { setDownstreamProtocol(Http::CodecType::HTTP2); setUpstreamProtocol(Http::CodecType::HTTP2); // set lower upstream outbound frame limits to make tests run faster config_helper_.setUpstreamOutboundFramesLimits(AllFrameFloodLimit, ControlFrameFloodLimit); initialize(); - return true; } void Http2FloodMitigationTest::setNetworkConnectionBufferSize() { @@ -1139,28 +1138,19 @@ TEST_P(Http2FloodMitigationTest, ZerolenHeaderAllowed) { } TEST_P(Http2FloodMitigationTest, UpstreamPingFlood) { - if (!initializeUpstreamFloodTest()) { - return; - } - + initializeUpstreamFloodTest(); floodClient(Http2Frame::makePingFrame(), ControlFrameFloodLimit + 1, "cluster.cluster_0.http2.outbound_control_flood"); } TEST_P(Http2FloodMitigationTest, UpstreamSettings) { - if (!initializeUpstreamFloodTest()) { - return; - } - + initializeUpstreamFloodTest(); floodClient(Http2Frame::makeEmptySettingsFrame(), ControlFrameFloodLimit + 1, "cluster.cluster_0.http2.outbound_control_flood"); } TEST_P(Http2FloodMitigationTest, UpstreamWindowUpdate) { - if (!initializeUpstreamFloodTest()) { - return; - } - + initializeUpstreamFloodTest(); constexpr uint32_t max_allowed = 5 + 2 * (1 + Http2::Utility::OptionsLimits:: DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT * @@ -1181,9 +1171,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamEmptyHeaders) { ->set_value(0); ConfigHelper::setProtocolOptions(*cluster, protocol_options); }); - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); @@ -1202,10 +1190,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamEmptyHeaders) { // Verify that the HTTP/2 connection is terminated upon receiving invalid HEADERS frame. TEST_P(Http2FloodMitigationTest, UpstreamZerolenHeader) { - if (!initializeUpstreamFloodTest()) { - return; - } - + initializeUpstreamFloodTest(); // Send client request which will send an upstream request. codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); @@ -1238,9 +1223,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamZerolenHeaderAllowed) { ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0), protocol_options); }); - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); // Send client request which will send an upstream request. codec_client_ = makeHttpConnection(lookupPort("http")); @@ -1280,9 +1263,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamZerolenHeaderAllowed) { } TEST_P(Http2FloodMitigationTest, UpstreamEmptyData) { - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); // Send client request which will send an upstream request. codec_client_ = makeHttpConnection(lookupPort("http")); @@ -1309,9 +1290,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamEmptyData) { } TEST_P(Http2FloodMitigationTest, UpstreamEmptyHeadersContinuation) { - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); @@ -1334,10 +1313,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamEmptyHeadersContinuation) { } TEST_P(Http2FloodMitigationTest, UpstreamPriorityNoOpenStreams) { - if (!initializeUpstreamFloodTest()) { - return; - } - + initializeUpstreamFloodTest(); floodClient(Http2Frame::makePriorityFrame(Http2Frame::makeClientStreamId(1), Http2Frame::makeClientStreamId(2)), Http2::Utility::OptionsLimits::DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM * 2 + 1, @@ -1345,9 +1321,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamPriorityNoOpenStreams) { } TEST_P(Http2FloodMitigationTest, UpstreamPriorityOneOpenStream) { - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); waitForNextUpstreamRequest(); @@ -1375,9 +1349,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamPriorityOneOpenStream) { // Verify that protocol constraint tracker correctly applies limits to the CLOSED streams as well. TEST_P(Http2FloodMitigationTest, UpstreamPriorityOneClosedStream) { - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); codec_client_ = makeHttpConnection(lookupPort("http")); auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_); @@ -1416,9 +1388,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamRstStreamOnStreamIdleTimeout) { auto seconds = std::chrono::duration_cast(timeout); stream_idle_timeout->set_seconds(seconds.count()); }); - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); // pre-fill upstream connection 1 away from overflow auto response = prefillOutboundUpstreamQueue(ControlFrameFloodLimit); @@ -1435,9 +1405,7 @@ TEST_P(Http2FloodMitigationTest, UpstreamRstStreamOnStreamIdleTimeout) { // Verify that the server can detect flooding by the RST_STREAM sent to upstream when downstream // disconnects. TEST_P(Http2FloodMitigationTest, UpstreamRstStreamOnDownstreamRemoteClose) { - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); // pre-fill 1 away from overflow auto response = prefillOutboundUpstreamQueue(ControlFrameFloodLimit); @@ -1464,9 +1432,7 @@ TEST_P(Http2FloodMitigationTest, RequestMetadata) { [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { hcm.mutable_http2_protocol_options()->set_allow_metadata(true); }); - if (!initializeUpstreamFloodTest()) { - return; - } + initializeUpstreamFloodTest(); codec_client_ = makeHttpConnection(lookupPort("http")); auto encoder_decoder = codec_client_->startRequest(default_request_headers_); diff --git a/tools/code_format/check_format.py b/tools/code_format/check_format.py index 09744dc0e2831..ac6f2d3e9f898 100755 --- a/tools/code_format/check_format.py +++ b/tools/code_format/check_format.py @@ -267,7 +267,6 @@ UNSORTED_FLAGS = { "envoy.reloadable_features.activate_timers_next_event_loop", "envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits", - "envoy.reloadable_features.upstream_http2_flood_checks", "envoy.reloadable_features.sanitize_http_header_referer", } # yapf: enable