http3: respecting header number limits (#15970)

Respecting upstream and downstream caps on number of headers for HTTP/3 Risk Level: n/a (http/3) Testing: turned up integration tests Docs Changes: n/a Release Notes: n/a #14829 among others. Signed-off-by: Alyssa Wilk <[email protected]>
envoyproxy · May 3, 2021 · 6a2e112 · 6a2e112
1 parent c86cfb5
commit 6a2e112
Show file tree

Hide file tree

Showing 18 changed files with 97 additions and 50 deletions.
diff --git a/docs/root/configuration/http/http_conn_man/response_code_details.rst b/docs/root/configuration/http/http_conn_man/response_code_details.rst
@@ -112,3 +112,6 @@ All http3 details are rooted at *http3.*
     http3.invalid_header_field, One of the HTTP/3 headers was invalid
     http3.headers_too_large, The size of headers (or trailers) exceeded the configured limits
     http3.unexpected_underscore, Envoy was configured to drop or reject requests with header keys beginning with underscores.
+    http3.too_many_headers, Either incoming request or response headers contained too many headers.
+    http3.too_many_trailers, Either incoming request or response trailers contained too many entries.
+
diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc
@@ -193,7 +193,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne
     auto& quic_session = dynamic_cast<Quic::EnvoyQuicClientSession&>(*connection_);
     codec_ = std::make_unique<Quic::QuicHttpClientConnectionImpl>(
         quic_session, *this, host->cluster().http3CodecStats(), host->cluster().http3Options(),
-        Http::DEFAULT_MAX_REQUEST_HEADERS_KB);
+        Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount());
     // Initialize the session after max request header size is changed in above http client
     // connection creation.
     quic_session.Initialize();

diff --git a/source/common/quic/codec_impl.cc b/source/common/quic/codec_impl.cc
@@ -22,14 +22,15 @@ QuicHttpServerConnectionImpl::QuicHttpServerConnectionImpl(
     EnvoyQuicServerSession& quic_session, Http::ServerConnectionCallbacks& callbacks,
     Http::Http3::CodecStats& stats,
     const envoy::config::core::v3::Http3ProtocolOptions& http3_options,
-    const uint32_t max_request_headers_kb,
+    const uint32_t max_request_headers_kb, const uint32_t max_request_headers_count,
     envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
         headers_with_underscores_action)
     : QuicHttpConnectionImplBase(quic_session, stats), quic_server_session_(quic_session) {
   quic_session.setCodecStats(stats);
   quic_session.setHttp3Options(http3_options);
   quic_session.setHeadersWithUnderscoreAction(headers_with_underscores_action);
   quic_session.setHttpConnectionCallbacks(callbacks);
+  quic_session.setMaxIncomingHeadersCount(max_request_headers_count);
   quic_session.set_max_inbound_header_list_size(max_request_headers_kb * 1024u);
 }
 
@@ -69,11 +70,12 @@ QuicHttpClientConnectionImpl::QuicHttpClientConnectionImpl(
     EnvoyQuicClientSession& session, Http::ConnectionCallbacks& callbacks,
     Http::Http3::CodecStats& stats,
     const envoy::config::core::v3::Http3ProtocolOptions& http3_options,
-    const uint32_t max_request_headers_kb)
+    const uint32_t max_request_headers_kb, const uint32_t max_response_headers_count)
     : QuicHttpConnectionImplBase(session, stats), quic_client_session_(session) {
   session.setCodecStats(stats);
   session.setHttp3Options(http3_options);
   session.setHttpConnectionCallbacks(callbacks);
+  session.setMaxIncomingHeadersCount(max_response_headers_count);
   session.set_max_inbound_header_list_size(max_request_headers_kb * 1024);
 }
 

diff --git a/source/common/quic/codec_impl.h b/source/common/quic/codec_impl.h
@@ -43,7 +43,7 @@ class QuicHttpServerConnectionImpl : public QuicHttpConnectionImplBase,
       EnvoyQuicServerSession& quic_session, Http::ServerConnectionCallbacks& callbacks,
       Http::Http3::CodecStats& stats,
       const envoy::config::core::v3::Http3ProtocolOptions& http3_options,
-      const uint32_t max_request_headers_kb,
+      const uint32_t max_request_headers_kb, const uint32_t max_request_headers_count,
       envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
           headers_with_underscores_action);
 
@@ -63,7 +63,8 @@ class QuicHttpClientConnectionImpl : public QuicHttpConnectionImplBase,
   QuicHttpClientConnectionImpl(EnvoyQuicClientSession& session,
                                Http::ConnectionCallbacks& callbacks, Http::Http3::CodecStats& stats,
                                const envoy::config::core::v3::Http3ProtocolOptions& http3_options,
-                               const uint32_t max_request_headers_kb);
+                               const uint32_t max_request_headers_kb,
+                               const uint32_t max_response_headers_count);
 
   // Http::ClientConnection
   Http::RequestEncoder& newStream(Http::ResponseDecoder& response_decoder) override;

diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc
@@ -134,7 +134,8 @@ void EnvoyQuicClientStream::OnInitialHeadersComplete(bool fin, size_t frame_len,
   }
   quic::QuicSpdyStream::OnInitialHeadersComplete(fin, frame_len, header_list);
   if (!headers_decompressed() || header_list.empty()) {
-    onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value());
+    onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value(),
+                  quic::QUIC_BAD_APPLICATION_PAYLOAD);
     return;
   }
 
@@ -143,16 +144,18 @@ void EnvoyQuicClientStream::OnInitialHeadersComplete(bool fin, size_t frame_len,
     end_stream_decoded_ = true;
   }
   std::unique_ptr<Http::ResponseHeaderMapImpl> headers =
-      quicHeadersToEnvoyHeaders<Http::ResponseHeaderMapImpl>(header_list, *this);
+      quicHeadersToEnvoyHeaders<Http::ResponseHeaderMapImpl>(
+          header_list, *this, filterManagerConnection()->maxIncomingHeadersCount(), details_);
   if (headers == nullptr) {
-    onStreamError(close_connection_upon_invalid_header_);
+    onStreamError(close_connection_upon_invalid_header_, quic::QUIC_STREAM_EXCESSIVE_LOAD);
     return;
   }
   const absl::optional<uint64_t> optional_status =
       Http::Utility::getResponseStatusNoThrow(*headers);
   if (!optional_status.has_value()) {
     details_ = Http3ResponseCodeDetailValues::invalid_http_header;
-    onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value());
+    onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value(),
+                  quic::QUIC_BAD_APPLICATION_PAYLOAD);
     return;
   }
   const uint64_t status = optional_status.value();
@@ -239,6 +242,11 @@ void EnvoyQuicClientStream::maybeDecodeTrailers() {
   if (sequencer()->IsClosed() && !FinishedReadingTrailers()) {
     // Only decode trailers after finishing decoding body.
     end_stream_decoded_ = true;
+    if (received_trailers().size() > filterManagerConnection()->maxIncomingHeadersCount()) {
+      details_ = Http3ResponseCodeDetailValues::too_many_trailers;
+      onStreamError(close_connection_upon_invalid_header_, quic::QUIC_STREAM_EXCESSIVE_LOAD);
+      return;
+    }
     response_decoder_->decodeTrailers(
         spdyHeaderBlockToEnvoyHeaders<Http::ResponseTrailerMapImpl>(received_trailers()));
     MarkTrailersConsumed();
@@ -299,7 +307,8 @@ QuicFilterManagerConnectionImpl* EnvoyQuicClientStream::filterManagerConnection(
   return dynamic_cast<QuicFilterManagerConnectionImpl*>(session());
 }
 
-void EnvoyQuicClientStream::onStreamError(absl::optional<bool> should_close_connection) {
+void EnvoyQuicClientStream::onStreamError(absl::optional<bool> should_close_connection,
+                                          quic::QuicRstStreamErrorCode rst_code) {
   if (details_.empty()) {
     details_ = Http3ResponseCodeDetailValues::invalid_http_header;
   }
@@ -313,7 +322,7 @@ void EnvoyQuicClientStream::onStreamError(absl::optional<bool> should_close_conn
   if (close_connection_upon_invalid_header) {
     stream_delegate()->OnStreamError(quic::QUIC_HTTP_FRAME_ERROR, "Invalid headers");
   } else {
-    Reset(quic::QUIC_BAD_APPLICATION_PAYLOAD);
+    Reset(rst_code);
   }
 }
 

diff --git a/source/common/quic/envoy_quic_client_stream.h b/source/common/quic/envoy_quic_client_stream.h
@@ -78,7 +78,8 @@ class EnvoyQuicClientStream : public quic::QuicSpdyClientStream,
 
   // Either reset the stream or close the connection according to
   // should_close_connection and configured http3 options.
-  void onStreamError(absl::optional<bool> should_close_connection);
+  void onStreamError(absl::optional<bool> should_close_connection,
+                     quic::QuicRstStreamErrorCode rst_code);
 
   Http::ResponseDecoder* response_decoder_{nullptr};
 

diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc
@@ -160,7 +160,8 @@ void EnvoyQuicServerStream::OnInitialHeadersComplete(bool fin, size_t frame_len,
     end_stream_decoded_ = true;
   }
   std::unique_ptr<Http::RequestHeaderMapImpl> headers =
-      quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(header_list, *this);
+      quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(
+          header_list, *this, filterManagerConnection()->maxIncomingHeadersCount(), details_);
   if (headers == nullptr) {
     onStreamError(close_connection_upon_invalid_header_);
     return;
@@ -242,6 +243,11 @@ void EnvoyQuicServerStream::maybeDecodeTrailers() {
   if (sequencer()->IsClosed() && !FinishedReadingTrailers()) {
     // Only decode trailers after finishing decoding body.
     end_stream_decoded_ = true;
+    if (received_trailers().size() > filterManagerConnection()->maxIncomingHeadersCount()) {
+      details_ = Http3ResponseCodeDetailValues::too_many_trailers;
+      onStreamError(close_connection_upon_invalid_header_);
+      return;
+    }
     request_decoder_->decodeTrailers(
         spdyHeaderBlockToEnvoyHeaders<Http::RequestTrailerMapImpl>(received_trailers()));
     MarkTrailersConsumed();

diff --git a/source/common/quic/envoy_quic_stream.h b/source/common/quic/envoy_quic_stream.h
@@ -13,23 +13,6 @@
 namespace Envoy {
 namespace Quic {
 
-// Changes or additions to details should be reflected in
-// docs/root/configuration/http/http_conn_man/response_code_details_details.rst
-class Http3ResponseCodeDetailValues {
-public:
-  // Invalid HTTP header field was received and stream is going to be
-  // closed.
-  static constexpr absl::string_view invalid_http_header = "http3.invalid_header_field";
-  // The size of headers (or trailers) exceeded the configured limits.
-  static constexpr absl::string_view headers_too_large = "http3.headers_too_large";
-  // Envoy was configured to drop requests with header keys beginning with underscores.
-  static constexpr absl::string_view invalid_underscore = "http3.unexpected_underscore";
-  // The peer refused the stream.
-  static constexpr absl::string_view remote_refused = "http3.remote_refuse";
-  // The peer reset the stream.
-  static constexpr absl::string_view remote_reset = "http3.remote_reset";
-};
-
 // Base class for EnvoyQuicServer|ClientStream.
 class EnvoyQuicStream : public virtual Http::StreamEncoder,
                         public Http::Stream,

diff --git a/source/common/quic/envoy_quic_utils.h b/source/common/quic/envoy_quic_utils.h
@@ -32,6 +32,27 @@
 namespace Envoy {
 namespace Quic {
 
+// Changes or additions to details should be reflected in
+// docs/root/configuration/http/http_conn_man/response_code_details.rst
+class Http3ResponseCodeDetailValues {
+public:
+  // Invalid HTTP header field was received and stream is going to be
+  // closed.
+  static constexpr absl::string_view invalid_http_header = "http3.invalid_header_field";
+  // The size of headers (or trailers) exceeded the configured limits.
+  static constexpr absl::string_view headers_too_large = "http3.headers_too_large";
+  // Envoy was configured to drop requests with header keys beginning with underscores.
+  static constexpr absl::string_view invalid_underscore = "http3.unexpected_underscore";
+  // The peer refused the stream.
+  static constexpr absl::string_view remote_refused = "http3.remote_refuse";
+  // The peer reset the stream.
+  static constexpr absl::string_view remote_reset = "http3.remote_reset";
+  // Too many trailers were sent.
+  static constexpr absl::string_view too_many_trailers = "http3.too_many_trailers";
+  // Too many headers were sent.
+  static constexpr absl::string_view too_many_headers = "http3.too_many_headers";
+};
+
 // TODO(danzh): this is called on each write. Consider to return an address instance on the stack if
 // the heap allocation is too expensive.
 Network::Address::InstanceConstSharedPtr
@@ -48,14 +69,21 @@ class HeaderValidator {
 
 // The returned header map has all keys in lower case.
 template <class T>
-std::unique_ptr<T> quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list,
-                                             HeaderValidator& validator) {
+std::unique_ptr<T>
+quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, HeaderValidator& validator,
+                          uint32_t max_headers_allowed, absl::string_view& details) {
   auto headers = T::create();
   for (const auto& entry : header_list) {
+    if (max_headers_allowed == 0) {
+      details = Http3ResponseCodeDetailValues::too_many_headers;
+      return nullptr;
+    }
+    max_headers_allowed--;
     Http::HeaderUtility::HeaderValidationResult result =
         validator.validateHeader(entry.first, entry.second);
     switch (result) {
     case Http::HeaderUtility::HeaderValidationResult::REJECT:
+      // The validator sets the details to Http3ResponseCodeDetailValues::invalid_underscore
       return nullptr;
     case Http::HeaderUtility::HeaderValidationResult::DROP:
       continue;

diff --git a/source/common/quic/quic_filter_manager_connection_impl.h b/source/common/quic/quic_filter_manager_connection_impl.h
@@ -142,6 +142,12 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase,
     codec_stats_ = std::reference_wrapper<Http::Http3::CodecStats>(stats);
   }
 
+  uint32_t maxIncomingHeadersCount() { return max_headers_count_; }
+
+  void setMaxIncomingHeadersCount(uint32_t max_headers_count) {
+    max_headers_count_ = max_headers_count;
+  }
+
 protected:
   // Propagate connection close to network_connection_callbacks_.
   void onConnectionCloseEvent(const quic::QuicConnectionCloseFrame& frame,
@@ -179,6 +185,7 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase,
   StreamInfo::StreamInfoImpl stream_info_;
   std::string transport_failure_reason_;
   uint32_t bytes_to_send_{0};
+  uint32_t max_headers_count_{std::numeric_limits<uint32_t>::max()};
   // Keeps the buffer state of the connection, and react upon the changes of how many bytes are
   // buffered cross all streams' send buffer. The state is evaluated and may be changed upon each
   // stream write. QUICHE doesn't buffer data in connection, all the data is buffered in stream's

diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc
@@ -606,7 +606,7 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection,
     return std::make_unique<Quic::QuicHttpServerConnectionImpl>(
         dynamic_cast<Quic::EnvoyQuicServerSession&>(connection), callbacks,
         Http::Http3::CodecStats::atomicGet(http3_codec_stats_, context_.scope()), http3_options_,
-        maxRequestHeadersKb(), headersWithUnderscoresAction());
+        maxRequestHeadersKb(), maxRequestHeadersCount(), headersWithUnderscoresAction());
 #else
     // Should be blocked by configuration checking at an earlier point.
     NOT_REACHED_GCOVR_EXCL_LINE;

diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc
@@ -118,7 +118,7 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<bool> {
         stats_({ALL_HTTP3_CODEC_STATS(POOL_COUNTER_PREFIX(scope_, "http3."),
                                       POOL_GAUGE_PREFIX(scope_, "http3."))}),
         http_connection_(envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_,
-                         64 * 1024) {
+                         64 * 1024, 100) {
     EXPECT_EQ(time_system_.systemTime(), envoy_quic_session_.streamInfo().startTime());
     EXPECT_EQ(EMPTY_STRING, envoy_quic_session_.nextProtocol());
     EXPECT_EQ(Http::Protocol::Http3, http_connection_.protocol());

diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc
@@ -204,7 +204,7 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam<bool> {
     EXPECT_CALL(*read_filter_, onNewConnection()).WillOnce(Invoke([this]() {
       // Create ServerConnection instance and setup callbacks for it.
       http_connection_ = std::make_unique<QuicHttpServerConnectionImpl>(
-          envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_, 64 * 1024,
+          envoy_quic_session_, http_connection_callbacks_, stats_, http3_options_, 64 * 1024, 100,
           envoy::config::core::v3::HttpProtocolOptions::ALLOW);
       EXPECT_EQ(Http::Protocol::Http3, http_connection_->protocol());
       // Stop iteration to avoid calling getRead/WriteBuffer().

diff --git a/test/common/quic/envoy_quic_utils_test.cc b/test/common/quic/envoy_quic_utils_test.cc
@@ -87,8 +87,9 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) {
         }
         return Http::HeaderUtility::HeaderValidationResult::ACCEPT;
       });
+  absl::string_view details;
   auto envoy_headers2 =
-      quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(quic_headers, validator);
+      quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(quic_headers, validator, 100, details);
   EXPECT_EQ(*envoy_headers, *envoy_headers2);
 
   quic::QuicHeaderList quic_headers2;
@@ -105,8 +106,8 @@ TEST(EnvoyQuicUtilsTest, HeadersConversion) {
         }
         return Http::HeaderUtility::HeaderValidationResult::ACCEPT;
       });
-  EXPECT_EQ(nullptr,
-            quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(quic_headers2, validator));
+  EXPECT_EQ(nullptr, quicHeadersToEnvoyHeaders<Http::RequestHeaderMapImpl>(quic_headers2, validator,
+                                                                           100, details));
 }
 
 } // namespace Quic

diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc
@@ -346,7 +346,8 @@ FakeHttpConnection::FakeHttpConnection(
     Http::Http3::CodecStats& stats = fake_upstream.http3CodecStats();
     codec_ = std::make_unique<Quic::QuicHttpServerConnectionImpl>(
         dynamic_cast<Quic::EnvoyQuicServerSession&>(shared_connection_.connection()), *this, stats,
-        fake_upstream.http3Options(), max_request_headers_kb, headers_with_underscores_action);
+        fake_upstream.http3Options(), max_request_headers_kb, max_request_headers_count,
+        headers_with_underscores_action);
 #else
     ASSERT(false, "running a QUIC integration test without compiling QUIC");
 #endif

diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc
@@ -1251,7 +1251,7 @@ void HttpIntegrationTest::testManyRequestHeaders(std::chrono::milliseconds time)
   // This test uses an Http::HeaderMapImpl instead of an Http::TestHeaderMapImpl to avoid
   // time-consuming asserts when using a large number of headers.
   setMaxRequestHeadersKb(96);
-  setMaxRequestHeadersCount(10005);
+  setMaxRequestHeadersCount(10010);
 
   config_helper_.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&

diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc
@@ -322,7 +322,6 @@ TEST_P(Http2UpstreamIntegrationTest, ManyLargeSimultaneousRequestWithRandomBacku
 }
 
 TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) {
-  EXCLUDE_UPSTREAM_HTTP3;                     // Times out waiting for reset.
   config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream.
   const uint32_t num_requests = 20;
   std::vector<Http::RequestEncoder*> encoders;
@@ -368,6 +367,9 @@ TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) {
     ASSERT_TRUE(upstream_requests[i]->waitForEndStream(*dispatcher_));
     upstream_requests[i]->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, false);
     upstream_requests[i]->encodeData(100, false);
+    // Make sure at least the headers go through, to ensure stream reset rather
+    // than disconnect.
+    responses[i]->waitForHeaders();
   }
   // Close the connection.
   ASSERT_TRUE(fake_upstream_connection_->close());
@@ -441,8 +443,6 @@ name: router
 // Tests the default limit for the number of response headers is 100. Results in a stream reset if
 // exceeds.
 TEST_P(Http2UpstreamIntegrationTest, TestManyResponseHeadersRejected) {
-  EXCLUDE_UPSTREAM_HTTP3; // no 503.
-
   // Default limit for response headers is 100.
   initialize();
   codec_client_ = makeHttpConnection(lookupPort("http"));