Skip to content

Commit

Permalink
http: add config for max response header size (#36231)
Browse files Browse the repository at this point in the history
Signed-off-by: Greg Greenway <[email protected]>
  • Loading branch information
ggreenway authored Sep 25, 2024
1 parent cec0e26 commit 07a8c4a
Show file tree
Hide file tree
Showing 22 changed files with 257 additions and 30 deletions.
25 changes: 21 additions & 4 deletions api/envoy/config/core/v3/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ message AlternateProtocolsCacheOptions {
repeated string canonical_suffixes = 5;
}

// [#next-free-field: 7]
// [#next-free-field: 8]
message HttpProtocolOptions {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.core.HttpProtocolOptions";
Expand Down Expand Up @@ -259,11 +259,28 @@ message HttpProtocolOptions {
// <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.drain_timeout>`.
google.protobuf.Duration max_connection_duration = 3;

// The maximum number of headers. If unconfigured, the default
// maximum number of request headers allowed is 100. Requests that exceed this limit will receive
// a 431 response for HTTP/1.x and cause a stream reset for HTTP/2.
// The maximum number of headers (request headers if configured on HttpConnectionManager,
// response headers when configured on a cluster).
// If unconfigured, the default maximum number of headers allowed is 100.
// Downstream requests that exceed this limit will receive a 431 response for HTTP/1.x and cause a stream
// reset for HTTP/2.
// Upstream responses that exceed this limit will result in a 503 response.
google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}];

// The maximum size of response headers.
// If unconfigured, the default is 60 KiB, except for HTTP/1 response headers which have a default
// of 80KiB.
// Responses that exceed this limit will result in a 503 response.
// In Envoy, this setting is only valid when configured on an upstream cluster, not on the
// :ref:`HTTP Connection Manager
// <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.common_http_protocol_options>`.
//
// Note: currently some protocol codecs impose limits on the maximum size of a single header:
// HTTP/2 (when using nghttp2) limits a single header to around 100kb.
// HTTP/3 limits a single header to around 1024kb.
google.protobuf.UInt32Value max_response_headers_kb = 7
[(validate.rules).uint32 = {lte: 8192 gt: 0}];

// Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be
// reset independent of any other timeouts. If not specified, this value is not set.
google.protobuf.Duration max_stream_duration = 4;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,10 @@ message HttpConnectionManager {
// The maximum request headers size for incoming connections.
// If unconfigured, the default max request headers allowed is 60 KiB.
// Requests that exceed this limit will receive a 431 response.
//
// Note: currently some protocol codecs impose limits on the maximum size of a single header:
// HTTP/2 (when using nghttp2) limits a single header to around 100kb.
// HTTP/3 limits a single header to around 1024kb.
google.protobuf.UInt32Value max_request_headers_kb = 29
[(validate.rules).uint32 = {lte: 8192 gt: 0}];

Expand Down
4 changes: 4 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,10 @@ new_features:
change: |
Added full feature absl::FormatTime() support to the DateFormatter. This allows the timepoint formatters (like
``%START_TIME%``) to use ``%E#S``, ``%E*S``, ``%E#f`` and ``%E*f`` to format the subsecond part of the timepoint.
- area: http
change: |
Added configuration setting for the :ref:`maximum size of response headers
<envoy_v3_api_field_config.core.v3.HttpProtocolOptions.max_response_headers_kb>` in responses.
- area: http_11_proxy
change: |
Added the option to configure the transport socket via locality or endpoint metadata.
Expand Down
5 changes: 5 additions & 0 deletions envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,11 @@ class ClusterInfo : public Http::FilterChainFactory {
*/
virtual uint32_t maxResponseHeadersCount() const PURE;

/**
* @return uint32_t the maximum total size of response headers in KB.
*/
virtual absl::optional<uint16_t> maxResponseHeadersKb() const PURE;

/**
* @return the human readable name of the cluster.
*/
Expand Down
8 changes: 5 additions & 3 deletions source/common/http/codec_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,21 +282,23 @@ CodecClientProd::CodecClientProd(CodecType type, Network::ClientConnectionPtr&&
}
codec_ = std::make_unique<Http1::ClientConnectionImpl>(
*connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(),
host->cluster().maxResponseHeadersCount(), proxied);
host->cluster().maxResponseHeadersKb(), host->cluster().maxResponseHeadersCount(), proxied);
break;
}
case CodecType::HTTP2:
codec_ = std::make_unique<Http2::ClientConnectionImpl>(
*connection_, *this, host->cluster().http2CodecStats(), random_generator,
host->cluster().http2Options(), Http::DEFAULT_MAX_REQUEST_HEADERS_KB,
host->cluster().http2Options(),
host->cluster().maxResponseHeadersKb().value_or(Http::DEFAULT_MAX_REQUEST_HEADERS_KB),
host->cluster().maxResponseHeadersCount(), Http2::ProdNghttp2SessionFactory::get());
break;
case CodecType::HTTP3: {
#ifdef ENVOY_ENABLE_QUIC
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, host->cluster().maxResponseHeadersCount());
host->cluster().maxResponseHeadersKb().value_or(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();
Expand Down
4 changes: 3 additions & 1 deletion source/common/http/http1/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1428,9 +1428,11 @@ void ServerConnectionImpl::ActiveRequest::dumpState(std::ostream& os, int indent

ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, CodecStats& stats,
ConnectionCallbacks&, const Http1Settings& settings,
absl::optional<uint16_t> max_response_headers_kb,
const uint32_t max_response_headers_count,
bool passing_through_proxy)
: ConnectionImpl(connection, stats, settings, MessageType::Response, MAX_RESPONSE_HEADERS_KB,
: ConnectionImpl(connection, stats, settings, MessageType::Response,
max_response_headers_kb.value_or(MAX_RESPONSE_HEADERS_KB),
max_response_headers_count),
owned_output_buffer_(connection.dispatcher().getWatermarkFactory().createBuffer(
[&]() -> void { this->onBelowLowWatermark(); },
Expand Down
1 change: 1 addition & 0 deletions source/common/http/http1/codec_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,7 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
public:
ClientConnectionImpl(Network::Connection& connection, CodecStats& stats,
ConnectionCallbacks& callbacks, const Http1Settings& settings,
absl::optional<uint16_t> max_response_headers_kb,
const uint32_t max_response_headers_count,
bool passing_through_proxy = false);
// Http::ClientConnection
Expand Down
6 changes: 6 additions & 0 deletions source/common/protobuf/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@
#define PROTOBUF_GET_WRAPPED_OR_DEFAULT(message, field_name, default_value) \
((message).has_##field_name() ? (message).field_name().value() : (default_value))

// Obtain the value of a wrapped field (e.g. google.protobuf.UInt32Value) if set. Otherwise, return
// absl::nullopt.
#define PROTOBUF_GET_OPTIONAL_WRAPPED(message, field_name) \
((message).has_##field_name() ? absl::make_optional((message).field_name().value()) \
: absl::nullopt)

// Obtain the value of a wrapped field (e.g. google.protobuf.UInt32Value) if set. Otherwise, throw
// a EnvoyException.

Expand Down
2 changes: 2 additions & 0 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,8 @@ ClusterInfoImpl::ClusterInfoImpl(
http_protocol_options_->common_http_protocol_options_, max_headers_count,
runtime_.snapshot().getInteger(Http::MaxResponseHeadersCountOverrideKey,
Http::DEFAULT_MAX_HEADERS_COUNT))),
max_response_headers_kb_(PROTOBUF_GET_OPTIONAL_WRAPPED(
http_protocol_options_->common_http_protocol_options_, max_response_headers_kb)),
type_(config.type()),
drain_connections_on_host_removal_(config.ignore_health_on_host_removal()),
connection_pool_per_downstream_connection_(
Expand Down
4 changes: 4 additions & 0 deletions source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,9 @@ class ClusterInfoImpl : public ClusterInfo,
bool maintenanceMode() const override;
uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; }
uint32_t maxResponseHeadersCount() const override { return max_response_headers_count_; }
absl::optional<uint16_t> maxResponseHeadersKb() const override {
return max_response_headers_kb_;
}
const std::string& name() const override { return name_; }
const std::string& observabilityName() const override {
if (observability_name_ != nullptr) {
Expand Down Expand Up @@ -1126,6 +1129,7 @@ class ClusterInfoImpl : public ClusterInfo,
// overhead via alignment
const uint32_t per_connection_buffer_limit_bytes_;
const uint32_t max_response_headers_count_;
const absl::optional<uint16_t> max_response_headers_kb_;
const envoy::config::cluster::v3::Cluster::DiscoveryType type_;
const bool drain_connections_on_host_removal_ : 1;
const bool connection_pool_per_downstream_connection_ : 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,12 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
idle_timeout_ = absl::nullopt;
}

if (config.common_http_protocol_options().has_max_response_headers_kb()) {
creation_status = absl::InvalidArgumentError(
fmt::format("Error: max_response_headers_kb cannot be set on http_connection_manager."));
return;
}

if (config.strip_any_host_port() && config.strip_matching_host_port()) {
creation_status = absl::InvalidArgumentError(fmt::format(
"Error: Only one of `strip_matching_host_port` or `strip_any_host_port` can be set."));
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/codec_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
} else {
client = std::make_unique<Http1::ClientConnectionImpl>(
client_connection, Http1::CodecStats::atomicGet(http1_stats, scope), client_callbacks,
client_http1settings, max_response_headers_count);
client_http1settings, max_request_headers_kb, max_response_headers_count);
}

if (http2) {
Expand Down
37 changes: 32 additions & 5 deletions test/common/http/http1/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2544,7 +2544,8 @@ class Http1ClientConnectionImplTest : public Http1CodecTestBase {
public:
void initialize() {
codec_ = std::make_unique<Http1::ClientConnectionImpl>(
connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_,
connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_kb_,
max_response_headers_count_,
/* passing_through_proxy=*/false);
}

Expand All @@ -2564,15 +2565,17 @@ class Http1ClientConnectionImplTest : public Http1CodecTestBase {
protected:
Stats::TestUtil::TestStore store_;
uint32_t max_response_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT};
uint32_t max_response_headers_kb_{Http::Http1::MAX_RESPONSE_HEADERS_KB};
};

void Http1ClientConnectionImplTest::testClientAllowChunkedContentLength(
[[maybe_unused]] uint32_t content_length, [[maybe_unused]] bool allow_chunked_length) {
// Response validation is not implemented in UHV yet
#ifndef ENVOY_ENABLE_UHV
codec_settings_.allow_chunked_length_ = allow_chunked_length;
codec_ = std::make_unique<Http1::ClientConnectionImpl>(
connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_);
codec_ = std::make_unique<Http1::ClientConnectionImpl>(connection_, http1CodecStats(), callbacks_,
codec_settings_, max_response_headers_kb_,
max_response_headers_count_);

NiceMock<MockResponseDecoder> response_decoder;
Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder);
Expand Down Expand Up @@ -3706,6 +3709,28 @@ TEST_P(Http1ClientConnectionImplTest, LargeResponseHeadersAccepted) {
std::string long_header = "big: " + std::string(79 * 1024, 'q') + "\r\n";
buffer = Buffer::OwnedImpl(long_header);
status = codec_->dispatch(buffer);
EXPECT_TRUE(status.ok());
}

// Tests that the size of response headers for HTTP/1 can be configured higher than the default of
// 80kB.
TEST_P(Http1ClientConnectionImplTest, LargeResponseHeadersAcceptedConfigurable) {
constexpr uint32_t size_limit_kb = 85;
max_response_headers_kb_ = size_limit_kb;
initialize();

NiceMock<MockResponseDecoder> response_decoder;
Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder);
TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}};
EXPECT_TRUE(request_encoder.encodeHeaders(headers, true).ok());

Buffer::OwnedImpl buffer("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n");
auto status = codec_->dispatch(buffer);
EXPECT_TRUE(status.ok());
std::string long_header = "big: " + std::string((size_limit_kb - 1) * 1024, 'q') + "\r\n";
buffer = Buffer::OwnedImpl(long_header);
status = codec_->dispatch(buffer);
EXPECT_TRUE(status.ok());
}

// Regression test for CVE-2019-18801. Large method headers should not trigger
Expand Down Expand Up @@ -3792,6 +3817,7 @@ TEST_P(Http1ClientConnectionImplTest, ManyResponseHeadersAccepted) {
// Response already contains one header.
buffer = Buffer::OwnedImpl(createHeaderOrTrailerFragment(150) + "\r\n");
status = codec_->dispatch(buffer);
EXPECT_TRUE(status.ok());
}

TEST_P(Http1ClientConnectionImplTest, TestResponseSplit0) {
Expand Down Expand Up @@ -3821,8 +3847,9 @@ TEST_P(Http1ClientConnectionImplTest, TestResponseSplitAllowChunkedLength100) {
TEST_P(Http1ClientConnectionImplTest, VerifyResponseHeaderTrailerMapMaxLimits) {
codec_settings_.allow_chunked_length_ = true;
codec_settings_.enable_trailers_ = true;
codec_ = std::make_unique<Http1::ClientConnectionImpl>(
connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_);
codec_ = std::make_unique<Http1::ClientConnectionImpl>(connection_, http1CodecStats(), callbacks_,
codec_settings_, max_response_headers_kb_,
max_response_headers_count_);

NiceMock<MockResponseDecoder> response_decoder;
Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder);
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/http1/http1_connection_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Http1Harness {
client_ = std::make_unique<Http1::ClientConnectionImpl>(
mock_client_connection_,
Http1::CodecStats::atomicGet(http1_stats_, *stats_store_.rootScope()),
mock_client_callbacks_, client_settings_, Http::DEFAULT_MAX_HEADERS_COUNT);
mock_client_callbacks_, client_settings_, absl::nullopt, Http::DEFAULT_MAX_HEADERS_COUNT);
Status status = client_->dispatch(payload);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,27 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeaderCountConfigurable) {
EXPECT_EQ(200, config.maxRequestHeadersCount());
}

// Check that max response header size is invalid on HCM.
TEST_F(HttpConnectionManagerConfigTest, MaxResponseHeaderKbInvalid) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
common_http_protocol_options:
max_response_headers_kb: 200
route_config:
name: local_route
http_filters:
- name: envoy.filters.http.router
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
)EOF";

HttpConnectionManagerConfig config(parseHttpConnectionManagerFromYaml(yaml_string), context_,
date_provider_, route_config_provider_manager_,
&scoped_routes_config_provider_manager_, tracer_manager_,
filter_config_provider_manager_, creation_status_);
EXPECT_FALSE(creation_status_.ok());
}

// Checking that default max_requests_per_connection is 0.
TEST_F(HttpConnectionManagerConfigTest, DefaultMaxRequestPerConnection) {
const std::string yaml_string = R"EOF(
Expand Down
Loading

0 comments on commit 07a8c4a

Please sign in to comment.