cherry picked fixes up to release/v1.34@4ee84e4

bazel/external/quiche.patch

@@ -0,0 +1,226 @@
+commit 6c726cca04b68c5593fbded31fadb001771844ef
+Author: danzh <[email protected]>
+Date:   Wed Jul 9 08:28:21 2025 -0700
+
+    Patch a fix from https://github.com/villainb-dg for the issue of leaking connection flow control window when a HTTP2 stream receives a DATA frame after being closed (reset, etc).
+
+    When a stream receives a DATA frame while it is already closed, the data is counted against the connection flow control window, but is never marked as consumed.
+    Fix: Mark the data as consumed when the received DATA frame is on a reset or invalid stream.
+
+    Reported QUICHE issue https://github.com/google/quiche/issues/91
+    Reported Envoy issue https://github.com/envoyproxy/envoy/issues/40085
+    Proposed external fix: https://github.com/google/quiche/pull/92
+
+    PiperOrigin-RevId: 781064590
+
+diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc
+index c25b2917b..9b1afb22b 100644
+--- a/quiche/http2/adapter/oghttp2_session.cc
++++ b/quiche/http2/adapter/oghttp2_session.cc
+@@ -1152,11 +1152,15 @@ void OgHttp2Session::OnDataFrameHeader(spdy::SpdyStreamId stream_id,
+
+ void OgHttp2Session::OnStreamFrameData(spdy::SpdyStreamId stream_id,
+                                        const char* data, size_t len) {
+-  // Count the data against flow control, even if the stream is unknown.
++  // Count the data against flow control, even if the stream is unknown, so that
++  // the connection flow control window is in sync with peer's.
+   MarkDataBuffered(stream_id, len);
+
+   auto iter = stream_map_.find(stream_id);
+   if (iter == stream_map_.end()) {
++    // Mark the data consumed immediately as we are dropping them. This will
++    // allow the connection flow control window to shift.
++    Consume(stream_id, len);
+     return;
+   }
+   // Validate against the content-length if it exists.
+@@ -1171,6 +1175,9 @@ void OgHttp2Session::OnStreamFrameData(spdy::SpdyStreamId stream_id,
+   if (streams_reset_.contains(stream_id)) {
+     // If the stream was unknown due to a protocol error, the visitor was
+     // informed in OnDataFrameHeader().
++    // Mark the data consumed immediately as we are dropping them. This will
++    // allow the connection flow control window to shift.
++    Consume(stream_id, len);
+     return;
+   }
+
+diff --git a/quiche/http2/adapter/oghttp2_session_test.cc b/quiche/http2/adapter/oghttp2_session_test.cc
+index 34a387bec..4afcf1a3d 100644
+--- a/quiche/http2/adapter/oghttp2_session_test.cc
++++ b/quiche/http2/adapter/oghttp2_session_test.cc
+@@ -1217,6 +1217,175 @@ TEST(OgHttp2SessionTest, ServerClosesStreamDuringOnEndStream) {
+   EXPECT_EQ(result, frames.size());
+ }
+
++TEST(OgHttp2SessionTest, ResetStreamRaceWithIncomingData) {
++  TestVisitor visitor;
++  OgHttp2Session::Options options;
++  options.perspective = Perspective::kServer;
++  OgHttp2Session session(visitor, options);
++
++  const std::string frames = TestFrameSequence()
++                                 .ClientPreface()
++                                 .Headers(1,
++                                          {{":method", "POST"},
++                                           {":scheme", "https"},
++                                           {":authority", "example.com"},
++                                           {":path", "/"}},
++                                          /*fin=*/false)
++                                 .Data(1, "Request body", false)
++                                 .Serialize();
++  testing::InSequence s;
++
++  // Client preface (empty SETTINGS)
++  EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
++  EXPECT_CALL(visitor, OnSettingsStart());
++  EXPECT_CALL(visitor, OnSettingsEnd());
++  // Stream 1
++  EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 0x4));
++  EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST"));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/"));
++  EXPECT_CALL(visitor, OnEndHeadersForStream(1));
++
++  EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0x0));
++  EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
++  EXPECT_CALL(visitor, OnDataForStream(1, "Request body"))
++      .WillOnce(testing::InvokeWithoutArgs([&session]() {
++        session.Consume(1, 12);
++        return true;
++      }));
++
++  session.ProcessBytes(frames);
++
++  EXPECT_TRUE(session.want_write());
++  EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
++  EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
++  EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
++  EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
++  int result1 = session.Send();
++  EXPECT_EQ(0, result1);
++  absl::string_view serialized1 = visitor.data();
++  EXPECT_THAT(serialized1,
++              EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS}));
++  EXPECT_FALSE(session.want_write());
++
++  EXPECT_LT(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize);
++
++  // Reset the stream and receive more data on this stream.
++  session.EnqueueFrame(std::make_unique<spdy::SpdyRstStreamIR>(
++      1, spdy::ERROR_CODE_PROTOCOL_ERROR));
++  const std::string more_frames =
++      TestFrameSequence()
++          .Data(1, std::string(16 * 1024, 'x'), false)
++          .Data(1, std::string(16 * 1024, 'y'), false)
++          .Serialize();
++  // These bytes are counted against the connection flow control window but
++  // should be dropped right away and considerred as consumed.
++  EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, _)).Times(0);
++  EXPECT_CALL(visitor, OnBeginDataForStream(1, _)).Times(0);
++  EXPECT_CALL(visitor, OnDataForStream(1, _)).Times(0);
++
++  session.ProcessBytes(more_frames);
++  EXPECT_TRUE(session.want_write());
++
++  EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0));
++  EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, 1, _, 0x0, 1));
++  EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
++  EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, _, 0x0));
++  EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, _, 0x0, 0));
++  int result2 = session.Send();
++  EXPECT_EQ(0, result2);
++  absl::string_view serialized2 = visitor.data();
++  serialized2.remove_prefix(serialized1.size());
++  EXPECT_THAT(serialized2, EqualsFrames({SpdyFrameType::RST_STREAM,
++                                         SpdyFrameType::WINDOW_UPDATE}));
++  EXPECT_EQ(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize);
++}
++
++TEST(OgHttp2SessionTest, ResetAndCloseStreamRaceWithIncomingData) {
++  TestVisitor visitor;
++  OgHttp2Session::Options options;
++  options.perspective = Perspective::kServer;
++  OgHttp2Session session(visitor, options);
++
++  const std::string frames = TestFrameSequence()
++                                 .ClientPreface()
++                                 .Headers(1,
++                                          {{":method", "POST"},
++                                           {":scheme", "https"},
++                                           {":authority", "example.com"},
++                                           {":path", "/"}},
++                                          /*fin=*/false)
++                                 .Data(1, "Request body", false)
++                                 .Serialize();
++  testing::InSequence s;
++
++  // Client preface (empty SETTINGS)
++  EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
++  EXPECT_CALL(visitor, OnSettingsStart());
++  EXPECT_CALL(visitor, OnSettingsEnd());
++  // Stream 1
++  EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 0x4));
++  EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST"));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
++  EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/"));
++  EXPECT_CALL(visitor, OnEndHeadersForStream(1));
++
++  EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0x0));
++  EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
++  EXPECT_CALL(visitor, OnDataForStream(1, "Request body"))
++      .WillOnce(testing::InvokeWithoutArgs([&session]() {
++        session.Consume(1, 12);
++        return true;
++      }));
++
++  session.ProcessBytes(frames);
++
++  EXPECT_TRUE(session.want_write());
++  EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
++  EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
++  EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
++  EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
++  int result1 = session.Send();
++  EXPECT_EQ(0, result1);
++  absl::string_view serialized1 = visitor.data();
++  EXPECT_THAT(serialized1,
++              EqualsFrames({SpdyFrameType::SETTINGS, SpdyFrameType::SETTINGS}));
++  EXPECT_FALSE(session.want_write());
++
++  EXPECT_LT(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize);
++
++  // Reset the stream and receive more data on this stream.
++  session.EnqueueFrame(std::make_unique<spdy::SpdyRstStreamIR>(
++      1, spdy::ERROR_CODE_PROTOCOL_ERROR));
++  EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0));
++  EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, 1, _, 0x0, 1));
++  EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
++  EXPECT_EQ(0, session.Send());
++
++  const std::string more_frames =
++      TestFrameSequence()
++          .Data(1, std::string(16 * 1024, 'x'), false)
++          .Data(1, std::string(16 * 1024, 'y'), false)
++          .Serialize();
++  // These bytes are counted against the connection flow control window but
++  // should be dropped right away and considered as consumed.
++  EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, _)).Times(2);
++  EXPECT_CALL(visitor, OnBeginDataForStream(1, _)).Times(0);
++  EXPECT_CALL(visitor, OnDataForStream(1, _)).Times(0);
++
++  session.ProcessBytes(more_frames);
++  EXPECT_TRUE(session.want_write());
++
++  EXPECT_CALL(visitor, OnBeforeFrameSent(WINDOW_UPDATE, 0, _, 0x0));
++  EXPECT_CALL(visitor, OnFrameSent(WINDOW_UPDATE, 0, _, 0x0, 0));
++  EXPECT_EQ(0, session.Send());
++  EXPECT_EQ(session.GetReceiveWindowSize(), kInitialFlowControlWindowSize);
++}
++
+ }  // namespace test
+ }  // namespace adapter
+ }  // namespace http2

changelogs/current.yaml

@@ -8,19 +8,16 @@ minor_behavior_changes:
 
 bug_fixes:
 # *Changes expected to improve the state of the world and are unlikely to have negative effects*
-- area: conn_pool
+- area: tls
   change: |
-    Fixed an issue that could lead to too many connections when using
-    :ref:`AutoHttpConfig <envoy_v3_api_msg_extensions.upstreams.http.v3.HttpProtocolOptions.AutoHttpConfig>` if the
-    established connection is ``http/2`` and Envoy predicted it would have lower concurrent capacity.
-- area: conn_pool
+    Fixed an issue with incorrectly cached connection properties on TLS connections.
+    If TLS connection data was queried before it was available, an empty value was being incorrectly cached, preventing later calls from
+    getting the correct value. This could be triggered with a ``tcp_proxy`` access log configured to emit a log upon connection
+    establishment if the log contains fields of the the TLS peer certificate. Then a later use of the data, such as the network RBAC
+    filter validating a peer certificate SAN, may incorrectly fail due to the empty cached value.
+- area: http2
   change: |
-    Fixed an issue that could lead to insufficient connections for current pending requests. If a connection starts draining while it
-    has negative unused capacity (which happens if an HTTP/2 ``SETTINGS`` frame reduces allowed concurrency to below the current number
-    of requests), that connection's unused capacity will be included in total pool capacity even though it is unusable because it is
-    draining. This can result in not enough connections being established for current pending requests. This is most problematic for
-    long-lived requests (such as streaming gRPC requests or long-poll requests) because a connection could be in the draining state
-    for a long time.
+    Fixed an issue where http/2 connections using the default codec of ``oghttp2`` could get stuck due to a window buffer leak.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

source/common/quic/envoy_quic_server_stream.cc

@@ -44,6 +44,10 @@ EnvoyQuicServerStream::EnvoyQuicServerStream(
   stats_gatherer_ = new QuicStatsGatherer(&filterManagerConnection()->dispatcher().timeSource());
   set_ack_listener(stats_gatherer_);
   RegisterMetadataVisitor(this);
+  if (Runtime::runtimeFeatureEnabled("envoy.restart_features.validate_http3_pseudo_headers") &&
+      session->allow_extended_connect()) {
+    header_validator().SetAllowExtendedConnect();
+  }
 }
 
 void EnvoyQuicServerStream::encode1xxHeaders(const Http::ResponseHeaderMap& headers) {

source/common/quic/envoy_quic_stream.h

@@ -143,6 +143,29 @@ class EnvoyQuicStream : public virtual Http::StreamEncoder,
     return Http::HeaderUtility::HeaderValidationResult::ACCEPT;
   }
 
+  void startHeaderBlock() override {
+    if (!Runtime::runtimeFeatureEnabled("envoy.restart_features.validate_http3_pseudo_headers")) {
+      return;
+    }
+    header_validator_.StartHeaderBlock();
+  }
+
+  bool finishHeaderBlock(bool is_trailing_headers) override {
+    if (!Runtime::runtimeFeatureEnabled("envoy.restart_features.validate_http3_pseudo_headers")) {
+      return true;
+    }
+    if (is_trailing_headers) {
+      return header_validator_.FinishHeaderBlock(quic_session_.perspective() ==
+                                                         quic::Perspective::IS_CLIENT
+                                                     ? http2::adapter::HeaderType::RESPONSE_TRAILER
+                                                     : http2::adapter::HeaderType::REQUEST_TRAILER);
+    }
+    return header_validator_.FinishHeaderBlock(quic_session_.perspective() ==
+                                                       quic::Perspective::IS_CLIENT
+                                                   ? http2::adapter::HeaderType::RESPONSE
+                                                   : http2::adapter::HeaderType::REQUEST);
+  }
+
   absl::string_view responseDetails() override { return details_; }
 
   const StreamInfo::BytesMeterSharedPtr& bytesMeter() override { return bytes_meter_; }
@@ -191,6 +214,8 @@ class EnvoyQuicStream : public virtual Http::StreamEncoder,
     return received_metadata_bytes_ > 1 << 20;
   }
 
+  http2::adapter::HeaderValidator& header_validator() { return header_validator_; }
+
 #ifdef ENVOY_ENABLE_HTTP_DATAGRAMS
   // Setting |http_datagram_handler_| enables HTTP Datagram support.
   std::unique_ptr<HttpDatagramHandler> http_datagram_handler_;

source/common/quic/envoy_quic_utils.h

@@ -57,8 +57,12 @@ quic::QuicSocketAddress envoyIpAddressToQuicSocketAddress(const Network::Address
 class HeaderValidator {
 public:
   virtual ~HeaderValidator() = default;
+  virtual void startHeaderBlock() = 0;
   virtual Http::HeaderUtility::HeaderValidationResult
   validateHeader(absl::string_view name, absl::string_view header_value) = 0;
+  // Returns true if all required pseudo-headers and no extra pseudo-headers are
+  // present for the given header type.
+  virtual bool finishHeaderBlock(bool is_trailing_headers) = 0;
 };
 
 // The returned header map has all keys in lower case.
@@ -67,6 +71,7 @@ std::unique_ptr<T>
 quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, HeaderValidator& validator,
                           uint32_t max_headers_kb, uint32_t max_headers_allowed,
                           absl::string_view& details, quic::QuicRstStreamErrorCode& rst) {
+  validator.startHeaderBlock();
   auto headers = T::create(max_headers_kb, max_headers_allowed);
   for (const auto& entry : header_list) {
     if (max_headers_allowed == 0) {
@@ -96,6 +101,9 @@ quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, HeaderValidat
       }
     }
   }
+  if (!validator.finishHeaderBlock(/*is_trailing_headers=*/false)) {
+    return nullptr;
+  }
   return headers;
 }
 
@@ -111,6 +119,7 @@ http2HeaderBlockToEnvoyTrailers(const quiche::HttpHeaderBlock& header_block,
     rst = quic::QUIC_STREAM_EXCESSIVE_LOAD;
     return nullptr;
   }
+  validator.startHeaderBlock();
   for (auto entry : header_block) {
     // TODO(danzh): Avoid temporary strings and addCopy() with string_view.
     std::string key(entry.first);
@@ -136,6 +145,9 @@ http2HeaderBlockToEnvoyTrailers(const quiche::HttpHeaderBlock& header_block,
       }
     }
   }
+  if (!validator.finishHeaderBlock(/*is_trailing_headers=*/true)) {
+    return nullptr;
+  }
   return headers;
 }
 

source/common/runtime/runtime_features.cc

@@ -117,6 +117,7 @@ RUNTIME_GUARD(envoy_restart_features_skip_backing_cluster_check_for_sds);
 RUNTIME_GUARD(envoy_restart_features_use_eds_cache_for_ads);
 RUNTIME_GUARD(envoy_restart_features_use_fast_protobuf_hash);
 RUNTIME_GUARD(envoy_reloadable_features_enable_intermediate_ca);
+RUNTIME_GUARD(envoy_restart_features_validate_http3_pseudo_headers);
 
 // Begin false flags. Most of them should come with a TODO to flip true.
 

source/common/tls/connection_info_impl_base.cc

@@ -17,6 +17,16 @@ namespace Extensions {
 namespace TransportSockets {
 namespace Tls {
 
+namespace {
+// There must be an version of this function for each type possible in variant `CachedValue`.
+bool shouldRecalculateCachedEntry(const std::string& str) { return str.empty(); }
+bool shouldRecalculateCachedEntry(const std::vector<std::string>& vec) { return vec.empty(); }
+bool shouldRecalculateCachedEntry(const Ssl::ParsedX509NamePtr& ptr) { return ptr == nullptr; }
+bool shouldRecalculateCachedEntry(const bssl::UniquePtr<GENERAL_NAMES>& ptr) {
+  return ptr == nullptr;
+}
+} // namespace
+
 template <typename ValueType>
 const ValueType&
 ConnectionInfoImplBase::getCachedValueOrCreate(CachedValueTag tag,
@@ -26,6 +36,16 @@ ConnectionInfoImplBase::getCachedValueOrCreate(CachedValueTag tag,
     const ValueType* val = absl::get_if<ValueType>(&it->second);
     ASSERT(val != nullptr, "Incorrect type in variant");
     if (val != nullptr) {
+
+      // Some values are retrieved too early, for example if properties of a peer certificate are
+      // retrieved before the handshake is complete, an empty value is cached. The value must be
+      // in the cache, so that we can return a valid reference, but in those cases if another caller
+      // later retrieves the same value, we must recalculate the value.
+      if (shouldRecalculateCachedEntry(*val)) {
+        it->second = create(ssl());
+        val = &absl::get<ValueType>(it->second);
+      }
+
       return *val;
     }
   }