Remove envoy.reloadable_features.upstream_http2_flood_checks and legacy code paths

api/envoy/config/core/v3/protocol.proto

@@ -408,17 +408,13 @@ 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,
   // preventing high memory utilization when receiving continuous stream of these frames. Exceeding
   // 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}];
 

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

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<uint64_t>(hd->type), static_cast<uint64_t>(hd->flags),
-                   static_cast<uint64_t>(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<uint64_t>(hd->type), static_cast<uint64_t>(hd->flags),
+                 static_cast<uint64_t>(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());

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<int> checkHeaderNameForUnderscores(absl::string_view header_name) override;
   StreamResetReason getMessagingErrorResetReason() const override {

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.

test/integration/http2_flood_integration_test.cc

@@ -60,7 +60,7 @@ class Http2FloodMitigationTest
   }
 
 protected:
-  bool initializeUpstreamFloodTest();
+  void initializeUpstreamFloodTest();
   std::vector<char> 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,20 +1313,15 @@ 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,
               "cluster.cluster_0.http2.inbound_priority_frames_flood");
 }
 
 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<std::chrono::seconds>(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_);

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