Skip to content

Commit

Permalink
http: add config for max response header size
Browse files Browse the repository at this point in the history
Signed-off-by: Greg Greenway <[email protected]>
  • Loading branch information
ggreenway committed Sep 19, 2024
1 parent 52d8f34 commit 14525f8
Show file tree
Hide file tree
Showing 17 changed files with 168 additions and 15 deletions.
11 changes: 10 additions & 1 deletion api/envoy/config/core/v3/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,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 @@ -260,6 +260,15 @@ message HttpProtocolOptions {
// a 431 response for HTTP/1.x and cause a stream reset for HTTP/2.
google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}];

// The maximum combined size of headers.
// If unconfigured, the default is 60 KiB, except for HTTP/1 response headers which have a default
// of 80KiB.
// Requests that exceed this limit will receive a 431 response.
// If both this setting and :ref:`max_request_headers_kb
// <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.max_request_headers_kb>`
// are set, this setting is used.
google.protobuf.UInt32Value max_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,9 @@ 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.
// If both this setting and :ref:`max_headers_kb
// <envoy_v3_api_field_config.core.v3.HttpProtocolOptions.max_headers_kb>` are set,
// that setting is used and this one is ignored.
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 @@ -264,6 +264,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 headers
<envoy_v3_api_field_config.core.v3.HttpProtocolOptions.max_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 @@ -277,21 +277,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_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 @@ -369,9 +369,11 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
config.stream_error_on_invalid_http_message(),
xff_num_trusted_hops_ == 0 && use_remote_address_)),
max_request_headers_kb_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(
config, max_request_headers_kb,
context.serverFactoryContext().runtime().snapshot().getInteger(
Http::MaxRequestHeadersSizeOverrideKey, Http::DEFAULT_MAX_REQUEST_HEADERS_KB))),
config.common_http_protocol_options(), max_headers_kb,
PROTOBUF_GET_WRAPPED_OR_DEFAULT(
config, max_request_headers_kb,
context.serverFactoryContext().runtime().snapshot().getInteger(
Http::MaxRequestHeadersSizeOverrideKey, Http::DEFAULT_MAX_REQUEST_HEADERS_KB)))),
max_request_headers_count_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(
config.common_http_protocol_options(), max_headers_count,
context.serverFactoryContext().runtime().snapshot().getInteger(
Expand Down Expand Up @@ -432,6 +434,14 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
return;
}

if (config.has_max_request_headers_kb() &&
config.common_http_protocol_options().has_max_headers_kb() &&
config.max_request_headers_kb().value() !=
config.common_http_protocol_options().max_headers_kb().value()) {
ENVOY_LOG(warn, "Both `max_request_headers_kb` and `max_headers_kb` are configured. Ignoring "
"`max_request_headers_kb`.");
}

auto options_or_error = Http2::Utility::initializeAndValidateOptions(
config.http2_protocol_options(), config.has_stream_error_on_invalid_http_message(),
config.stream_error_on_invalid_http_message());
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 @@ -826,6 +826,56 @@ TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbConfigured) {
EXPECT_EQ(16, config.maxRequestHeadersKb());
}

TEST_F(HttpConnectionManagerConfigTest, MaxHeadersKbConfigured) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
common_http_protocol_options:
max_headers_kb: 16
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_);
ASSERT_TRUE(creation_status_.ok());
EXPECT_EQ(16, config.maxRequestHeadersKb());
}

TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbAndMaxHeadersKbConfigured) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
common_http_protocol_options:
max_headers_kb: 16
max_request_headers_kb: 32
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";

EXPECT_LOG_CONTAINS("warn",
"Both `max_request_headers_kb` and `max_headers_kb` are configured. Ignoring "
"`max_request_headers_kb`.",
{
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_);

ASSERT_TRUE(creation_status_.ok());
EXPECT_EQ(16, config.maxRequestHeadersKb());
});
}

TEST_F(HttpConnectionManagerConfigTest, MaxRequestHeadersKbMaxConfigurable) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
Expand Down
27 changes: 27 additions & 0 deletions test/integration/multiplexed_upstream_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,33 @@ TEST_P(MultiplexedUpstreamIntegrationTest, LargeResponseHeadersRejected) {
EXPECT_EQ("503", response->headers().getStatusValue());
}

// Tests configuration of max response headers size.
TEST_P(MultiplexedUpstreamIntegrationTest, LargeResponseHeadersAccepted) {
constexpr uint32_t limit_kb = 150;
config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
ConfigHelper::HttpProtocolOptions protocol_options;
auto* http_protocol_options = protocol_options.mutable_common_http_protocol_options();
http_protocol_options->mutable_max_headers_kb()->set_value(limit_kb);
ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0),
protocol_options);
});
Http::TestResponseHeaderMapImpl large_headers(default_response_headers_);
large_headers.addCopy("large", std::string((limit_kb - 1) * 1024, 'a'));
auto response = codec_client_->makeHeaderOnlyRequest(default_request_headers_);

initialize();
codec_client_ = makeHttpConnection(lookupPort("http"));
auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024);
waitForNextUpstreamRequest();

upstream_request_->encodeHeaders(response_headers, false);
upstream_request_->encodeData(512, true);
ASSERT_TRUE(response->waitForEndStream());

EXPECT_TRUE(upstream_request_->complete());
EXPECT_TRUE(response->complete());
}

TEST_P(MultiplexedUpstreamIntegrationTest, NoInitialStreams) {
// Set the fake upstream to start with 0 streams available.
upstreamConfig().http2_options_.mutable_max_concurrent_streams()->set_value(0);
Expand Down
1 change: 1 addition & 0 deletions test/mocks/upstream/cluster_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class MockClusterInfo : public ClusterInfo {
(const));
MOCK_METHOD(bool, maintenanceMode, (), (const));
MOCK_METHOD(uint32_t, maxResponseHeadersCount, (), (const));
MOCK_METHOD(absl::optional<uint16_t>, maxResponseHeadersKb, (), (const));
MOCK_METHOD(uint64_t, maxRequestsPerConnection, (), (const));
MOCK_METHOD(const std::string&, name, (), (const));
MOCK_METHOD(const std::string&, observabilityName, (), (const));
Expand Down

0 comments on commit 14525f8

Please sign in to comment.