http3: respecting header number limits (envoyproxy#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 envoyproxy#14829 among others. Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Gokul Nair <gnair@twitter.com>
gokulnair · May 6, 2021 · 6ace0e0 · 6ace0e0
1 parent c452d32
commit 6ace0e0
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 7 deletions.
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/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc
@@ -1322,10 +1322,14 @@ TEST_P(DownstreamProtocolIntegrationTest, LargeCookieParsingConcatenated) {
     // header size to avoid QUIC_HEADERS_TOO_LARGE stream error.
     config_helper_.addConfigModifier(
         [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
-                hcm) -> void { hcm.mutable_max_request_headers_kb()->set_value(96); });
+                hcm) -> void {
+          hcm.mutable_max_request_headers_kb()->set_value(96);
+          hcm.mutable_common_http_protocol_options()->mutable_max_headers_count()->set_value(8000);
+        });
   }
   if (upstreamProtocol() == FakeHttpConnection::Type::HTTP3) {
     setMaxRequestHeadersKb(96);
+    setMaxRequestHeadersCount(8000);
   }
   initialize();
 
@@ -1598,9 +1602,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ManyRequestHeadersAccepted) {
 }
 
 TEST_P(DownstreamProtocolIntegrationTest, ManyRequestTrailersRejected) {
-  // QUICHE doesn't limit number of headers.
-  EXCLUDE_DOWNSTREAM_HTTP3
-  // The default configured header (and trailer) count limit is 100.
+  // Default header (and trailer) count limit is 100.
   config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1());
   config_helper_.addConfigModifier(setEnableUpstreamTrailersHttp1());
   Http::TestRequestTrailerMapImpl request_trailers;