Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: add config for max response header size #36231

Merged
merged 12 commits into from
Sep 25, 2024
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,
ggreenway marked this conversation as resolved.
Show resolved Hide resolved
// 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),
alyssawilk marked this conversation as resolved.
Show resolved Hide resolved
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