Fix PRIORITY flood bypass

changelogs/current/minor_behavior_changes/http2__priority-flood-protection.rst

@@ -0,0 +1,5 @@
+Fixed a vulnerability where HTTP/2 PRIORITY and WINDOW_UPDATE frame flood protection could be bypassed
+by rapidly opening and closing streams. The protection now scales with the number of active streams
+rather than cumulative opened streams, and frame usage is retired upon stream closure. This behavioral
+change can be toggled on or off via the guard
+``envoy.reloadable_features.http2_flood_protection_active_streams``.

source/common/http/http2/codec_impl.cc

@@ -997,7 +997,10 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stat
                                     "envoy.reloadable_features.http2_max_cookies_size_in_kb", 0) *
                                     1024
                               : 0),
-      protocol_constraints_(stats, http2_options), random_(random_generator),
+      protocol_constraints_(stats, http2_options,
+                            Runtime::runtimeFeatureEnabled(
+                                "envoy.reloadable_features.http2_flood_protection_active_streams")),
+      dispatching_(false), raised_goaway_(false), random_(random_generator),
       last_received_data_time_(connection_.dispatcher().timeSource().monotonicTime()) {
   if (http2_options.has_use_oghttp2_codec()) {
     use_oghttp2_library_ = http2_options.use_oghttp2_codec().value();
@@ -1607,6 +1610,7 @@ Status ConnectionImpl::onStreamClose(StreamImpl* stream, uint32_t error_code) {
       return okStatus();
     }
 
+    protocol_constraints_.decrementActiveStreamCount();
     stream->destroy();
     current_stream_id_.reset();
     // TODO(antoniovicente) Test coverage for onCloseStream before deferred reset handling happens.

source/common/http/http2/protocol_constraints.cc

@@ -8,7 +8,8 @@ namespace Http {
 namespace Http2 {
 
 ProtocolConstraints::ProtocolConstraints(
-    CodecStats& stats, const envoy::config::core::v3::Http2ProtocolOptions& http2_options)
+    CodecStats& stats, const envoy::config::core::v3::Http2ProtocolOptions& http2_options,
+    bool use_active_streams_for_limits)
     : stats_(stats), 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()),
@@ -18,7 +19,8 @@ ProtocolConstraints::ProtocolConstraints(
       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()) {}
+          http2_options.max_inbound_window_update_frames_per_data_frame_sent().value()),
+      use_active_streams_for_limits_(use_active_streams_for_limits) {}
 
 ProtocolConstraints::ReleasorProc
 ProtocolConstraints::incrementOutboundFrameCount(bool is_outbound_flood_monitored_control_frame) {
@@ -101,13 +103,14 @@ Status ProtocolConstraints::checkInboundFrameLimits() {
   }
 
   if (inbound_priority_frames_ >
-      static_cast<uint64_t>(max_inbound_priority_frames_per_stream_) * (1 + opened_streams_)) {
+      static_cast<uint64_t>(max_inbound_priority_frames_per_stream_) *
+          (1 + (use_active_streams_for_limits_ ? active_streams_ : opened_streams_))) {
     stats_.inbound_priority_frames_flood_.inc();
     return bufferFloodError("Too many PRIORITY frames");
   }
 
   if (inbound_window_update_frames_ >
-      5 + 2 * (opened_streams_ +
+      5 + 2 * ((use_active_streams_for_limits_ ? active_streams_ : opened_streams_) +
                max_inbound_window_update_frames_per_data_frame_sent_ * outbound_data_frames_)) {
     stats_.inbound_window_update_frames_flood_.inc();
     return bufferFloodError("Too many WINDOW_UPDATE frames");
@@ -124,7 +127,8 @@ void ProtocolConstraints::dumpState(std::ostream& os, int indent_level) const {
      << DUMP_MEMBER(max_outbound_control_frames_)
      << DUMP_MEMBER(consecutive_inbound_frames_with_empty_payload_)
      << DUMP_MEMBER(max_consecutive_inbound_frames_with_empty_payload_)
-     << DUMP_MEMBER(opened_streams_) << DUMP_MEMBER(inbound_priority_frames_)
+     << DUMP_MEMBER(opened_streams_) << DUMP_MEMBER(active_streams_)
+     << DUMP_MEMBER(inbound_priority_frames_)
      << DUMP_MEMBER(max_inbound_priority_frames_per_stream_)
      << DUMP_MEMBER(inbound_window_update_frames_) << DUMP_MEMBER(outbound_data_frames_)
      << DUMP_MEMBER(max_inbound_window_update_frames_per_data_frame_sent_) << '\n';

source/common/http/http2/protocol_constraints.h

@@ -45,7 +45,8 @@ class ProtocolConstraints : public ScopeTrackedObject {
   using ReleasorProc = std::function<void()>;
 
   explicit ProtocolConstraints(CodecStats& stats,
-                               const envoy::config::core::v3::Http2ProtocolOptions& http2_options);
+                               const envoy::config::core::v3::Http2ProtocolOptions& http2_options,
+                               bool use_active_streams_for_limits);
 
   // Return ok status if no protocol constraints were violated.
   // Return error status of the first detected violation. Subsequent violations of constraints
@@ -68,7 +69,28 @@ class ProtocolConstraints : public ScopeTrackedObject {
   Status trackInboundFrame(uint8_t type, bool end_stream, bool is_empty);
   // Increment the number of DATA frames sent to the peer.
   void incrementOutboundDataFrameCount() { ++outbound_data_frames_; }
-  void incrementOpenedStreamCount() { ++opened_streams_; }
+  void incrementOpenedStreamCount() {
+    ++opened_streams_;
+    ++active_streams_;
+  }
+  void decrementActiveStreamCount() {
+    ASSERT(active_streams_ > 0);
+    if (active_streams_ > 0) {
+      --active_streams_;
+      if (use_active_streams_for_limits_) {
+        if (inbound_priority_frames_ > max_inbound_priority_frames_per_stream_) {
+          inbound_priority_frames_ -= max_inbound_priority_frames_per_stream_;
+        } else {
+          inbound_priority_frames_ = 0;
+        }
+        if (inbound_window_update_frames_ > 2) {
+          inbound_window_update_frames_ -= 2;
+        } else {
+          inbound_window_update_frames_ = 0;
+        }
+      }
+    }
+  }
 
   Status checkOutboundFrameLimits();
 
@@ -115,6 +137,8 @@ class ProtocolConstraints : public ScopeTrackedObject {
   // For upstream connections this is incremented when the first HEADERS frame with the new
   // stream ID is sent to the upstream server.
   uint32_t opened_streams_ = 0;
+  // This counter keeps track of the number of currently active streams.
+  uint32_t active_streams_ = 0;
   // This counter keeps track of the number of inbound PRIORITY frames. If this counter exceeds
   // the value calculated using this formula:
   //
@@ -139,6 +163,8 @@ class ProtocolConstraints : public ScopeTrackedObject {
   // 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_;
+
+  const bool use_active_streams_for_limits_;
 };
 
 } // namespace Http2

source/common/runtime/runtime_features.cc

@@ -69,6 +69,7 @@ RUNTIME_GUARD(envoy_reloadable_features_hide_transport_failure_reason_in_respons
 RUNTIME_GUARD(envoy_reloadable_features_http1_close_connection_on_zombie_stream_complete);
 RUNTIME_GUARD(envoy_reloadable_features_http2_discard_host_header);
 RUNTIME_GUARD(envoy_reloadable_features_http2_fix_goaway_loadshed_point);
+RUNTIME_GUARD(envoy_reloadable_features_http2_flood_protection_active_streams);
 RUNTIME_GUARD(envoy_reloadable_features_http2_include_cookies_in_limits);
 RUNTIME_GUARD(envoy_reloadable_features_http_async_client_retry_respect_buffer_limits);
 RUNTIME_GUARD(envoy_reloadable_features_http_inspector_use_balsa_parser);

test/common/http/http2/codec_impl_test.cc

@@ -1570,7 +1570,8 @@ TEST_P(Http2CodecImplTest, DumpsStreamlessConnectionWithoutAllocatingMemory) {
                "outbound_control_frames_: 0, max_outbound_control_frames_: 1000, "
                "consecutive_inbound_frames_with_empty_payload_: 0, "
                "max_consecutive_inbound_frames_with_empty_payload_: 1, opened_streams_: 0, "
-               "inbound_priority_frames_: 0, max_inbound_priority_frames_per_stream_: 100, "
+               "active_streams_: 0, inbound_priority_frames_: 0, "
+               "max_inbound_priority_frames_per_stream_: 100, "
                "inbound_window_update_frames_: 1, outbound_data_frames_: 0, "
                "max_inbound_window_update_frames_per_data_frame_sent_: 10\n"
                "  Number of active streams: 0, current_stream_id_: null Dumping 0 Active Streams:\n"

test/common/http/http2/protocol_constraints_test.cc

@@ -26,14 +26,14 @@ class ProtocolConstraintsTest : public ::testing::Test {
 };
 
 TEST_F(ProtocolConstraintsTest, DefaultStatusOk) {
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   EXPECT_TRUE(constraints.status().ok());
 }
 
 TEST_F(ProtocolConstraintsTest, OutboundControlFrameFlood) {
   options_.mutable_max_outbound_frames()->set_value(20);
   options_.mutable_max_outbound_control_frames()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   constraints.incrementOutboundFrameCount(true);
   constraints.incrementOutboundFrameCount(true);
   EXPECT_TRUE(constraints.checkOutboundFrameLimits().ok());
@@ -51,7 +51,7 @@ TEST_F(ProtocolConstraintsTest, OutboundControlFrameFlood) {
 TEST_F(ProtocolConstraintsTest, OutboundFrameFlood) {
   options_.mutable_max_outbound_frames()->set_value(5);
   options_.mutable_max_outbound_control_frames()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   constraints.incrementOutboundFrameCount(false);
   constraints.incrementOutboundFrameCount(false);
   constraints.incrementOutboundFrameCount(false);
@@ -73,7 +73,7 @@ TEST_F(ProtocolConstraintsTest, OutboundFrameFlood) {
 TEST_F(ProtocolConstraintsTest, OutboundFrameFloodStatusIsIdempotent) {
   options_.mutable_max_outbound_frames()->set_value(5);
   options_.mutable_max_outbound_control_frames()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   // First trigger control frame flood
   constraints.incrementOutboundFrameCount(true);
   constraints.incrementOutboundFrameCount(true);
@@ -94,7 +94,7 @@ TEST_F(ProtocolConstraintsTest, OutboundFrameFloodStatusIsIdempotent) {
 
 TEST_F(ProtocolConstraintsTest, InboundZeroLenData) {
   options_.mutable_max_consecutive_inbound_frames_with_empty_payload()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   const uint8_t type = NGHTTP2_DATA;
   const bool end_stream = false;
   const bool is_empty = true;
@@ -112,7 +112,7 @@ TEST_F(ProtocolConstraintsTest, OutboundAndInboundFrameFloodStatusIsIdempotent)
   options_.mutable_max_outbound_frames()->set_value(5);
   options_.mutable_max_outbound_control_frames()->set_value(2);
   options_.mutable_max_consecutive_inbound_frames_with_empty_payload()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   // First trigger inbound frame flood
   const uint8_t type = NGHTTP2_DATA;
   const bool end_stream = false;
@@ -133,7 +133,7 @@ TEST_F(ProtocolConstraintsTest, OutboundAndInboundFrameFloodStatusIsIdempotent)
 
 TEST_F(ProtocolConstraintsTest, InboundZeroLenDataWithPadding) {
   options_.mutable_max_consecutive_inbound_frames_with_empty_payload()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   const uint8_t type = NGHTTP2_DATA;
   const bool end_stream = false;
   const bool is_empty = true;
@@ -147,7 +147,7 @@ TEST_F(ProtocolConstraintsTest, InboundZeroLenDataWithPadding) {
 
 TEST_F(ProtocolConstraintsTest, InboundZeroLenDataEndStreamResetCounter) {
   options_.mutable_max_consecutive_inbound_frames_with_empty_payload()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   const uint8_t type = NGHTTP2_DATA;
   const bool is_empty = true;
   bool end_stream = false;
@@ -166,7 +166,7 @@ TEST_F(ProtocolConstraintsTest, InboundZeroLenDataEndStreamResetCounter) {
 
 TEST_F(ProtocolConstraintsTest, Priority) {
   options_.mutable_max_inbound_priority_frames_per_stream()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   // Create one stream
   constraints.incrementOpenedStreamCount();
 
@@ -185,7 +185,7 @@ TEST_F(ProtocolConstraintsTest, Priority) {
 
 TEST_F(ProtocolConstraintsTest, WindowUpdate) {
   options_.mutable_max_inbound_window_update_frames_per_data_frame_sent()->set_value(2);
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
   // Create one stream
   constraints.incrementOpenedStreamCount();
   // Send 2 DATA frames
@@ -208,23 +208,106 @@ TEST_F(ProtocolConstraintsTest, WindowUpdate) {
   EXPECT_EQ(1, stats_store_.counter("http2.inbound_window_update_frames_flood").value());
 }
 
+TEST_F(ProtocolConstraintsTest, WindowUpdateActiveStreamsDecrement) {
+  options_.mutable_max_inbound_window_update_frames_per_data_frame_sent()->set_value(0);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
+  // Create two streams
+  constraints.incrementOpenedStreamCount();
+  constraints.incrementOpenedStreamCount();
+
+  // Formula: 5 + 2 * (active_streams + 0) = 5 + 2 * 2 = 9
+  const uint8_t type = OGHTTP2_WINDOW_UPDATE_FRAME_TYPE;
+  const bool end_stream = false;
+  const bool is_empty = false;
+  for (uint32_t i = 0; i < 9; ++i) {
+    EXPECT_TRUE(constraints.trackInboundFrame(type, end_stream, is_empty).ok());
+  }
+  EXPECT_TRUE(constraints.status().ok());
+
+  // Close one stream. active_streams becomes 1.
+  // inbound_window_update_frames should be decremented by 2: 9 - 2 = 7.
+  // New limit: 5 + 2 * (1 + 0) = 7.
+  constraints.decrementActiveStreamCount();
+  EXPECT_TRUE(constraints.status().ok());
+
+  // One more WINDOW_UPDATE should fail. 7 + 1 = 8 > 7.
+  EXPECT_TRUE(isBufferFloodError(constraints.trackInboundFrame(type, end_stream, is_empty)));
+  EXPECT_TRUE(isBufferFloodError(constraints.status()));
+}
+
+TEST_F(ProtocolConstraintsTest, WindowUpdateOpenedStreamsNoDecrement) {
+  options_.mutable_max_inbound_window_update_frames_per_data_frame_sent()->set_value(0);
+  ProtocolConstraints constraints(http2CodecStats(), options_, false);
+  // Create two streams
+  constraints.incrementOpenedStreamCount();
+  constraints.incrementOpenedStreamCount();
+
+  // Formula: 5 + 2 * (opened_streams + 0) = 5 + 2 * 2 = 9
+  const uint8_t type = OGHTTP2_WINDOW_UPDATE_FRAME_TYPE;
+  const bool end_stream = false;
+  const bool is_empty = false;
+  for (uint32_t i = 0; i < 9; ++i) {
+    EXPECT_TRUE(constraints.trackInboundFrame(type, end_stream, is_empty).ok());
+  }
+  EXPECT_TRUE(constraints.status().ok());
+
+  // Close one stream. active_streams becomes 1.
+  // inbound_window_update_frames should NOT be decremented.
+  // Limit still uses opened_streams (2): 5 + 2 * (2 + 0) = 9.
+  constraints.decrementActiveStreamCount();
+  EXPECT_TRUE(constraints.status().ok());
+
+  // One more WINDOW_UPDATE should fail. 9 + 1 = 10 > 9.
+  EXPECT_TRUE(isBufferFloodError(constraints.trackInboundFrame(type, end_stream, is_empty)));
+  EXPECT_TRUE(isBufferFloodError(constraints.status()));
+}
+
+TEST_F(ProtocolConstraintsTest, PriorityActiveStreamsDecrement) {
+  options_.mutable_max_inbound_priority_frames_per_stream()->set_value(10);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
+  // Create two streams
+  constraints.incrementOpenedStreamCount();
+  constraints.incrementOpenedStreamCount();
+
+  // Formula: max * (1 + active_streams) = 10 * (1 + 2) = 30
+  const uint8_t type = OGHTTP2_PRIORITY_FRAME_TYPE;
+  const bool end_stream = false;
+  const bool is_empty = false;
+  for (uint32_t i = 0; i < 30; ++i) {
+    EXPECT_TRUE(constraints.trackInboundFrame(type, end_stream, is_empty).ok());
+  }
+  EXPECT_TRUE(constraints.status().ok());
+
+  // Close one stream. active_streams becomes 1.
+  // inbound_priority_frames should be decremented by max (10): 30 - 10 = 20.
+  // New limit: 10 * (1 + 1) = 20.
+  constraints.decrementActiveStreamCount();
+  EXPECT_TRUE(constraints.status().ok());
+
+  // One more PRIORITY frame should fail. 20 + 1 = 21 > 20.
+  EXPECT_TRUE(isBufferFloodError(constraints.trackInboundFrame(type, end_stream, is_empty)));
+  EXPECT_TRUE(isBufferFloodError(constraints.status()));
+}
+
 TEST_F(ProtocolConstraintsTest, DumpsStateWithoutAllocatingMemory) {
   std::array<char, 1024> buffer;
   OutputBufferStream ostream{buffer.data(), buffer.size()};
-  ProtocolConstraints constraints(http2CodecStats(), options_);
+  ProtocolConstraints constraints(http2CodecStats(), options_, true);
 
   Memory::TestUtil::MemoryTest memory_test;
   constraints.dumpState(ostream, 0);
   EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 0);
   EXPECT_THAT(ostream.contents(), HasSubstr("ProtocolConstraints "));
   EXPECT_THAT(
       ostream.contents(),
-      HasSubstr(" outbound_frames_: 0, max_outbound_frames_: 0, outbound_control_frames_: 0, "
-                "max_outbound_control_frames_: 0, consecutive_inbound_frames_with_empty_payload_: "
-                "0, max_consecutive_inbound_frames_with_empty_payload_: 0, opened_streams_: 0, "
-                "inbound_priority_frames_: 0, max_inbound_priority_frames_per_stream_: 0, "
-                "inbound_window_update_frames_: 0, outbound_data_frames_: 0, "
-                "max_inbound_window_update_frames_per_data_frame_sent_: 0"));
+      HasSubstr(
+          " outbound_frames_: 0, max_outbound_frames_: 0, outbound_control_frames_: 0, "
+          "max_outbound_control_frames_: 0, consecutive_inbound_frames_with_empty_payload_: 0, "
+          "max_consecutive_inbound_frames_with_empty_payload_: 0, opened_streams_: 0, "
+          "active_streams_: 0, inbound_priority_frames_: 0, "
+          "max_inbound_priority_frames_per_stream_: "
+          "0, inbound_window_update_frames_: 0, outbound_data_frames_: 0, "
+          "max_inbound_window_update_frames_per_data_frame_sent_: 0"));
 }
 
 } // namespace Http2