Skip to content

Commit

Permalink
http3: respecting header number limits (envoyproxy#15970)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
alyssawilk authored and Gokul Nair committed May 6, 2021
1 parent 7366497 commit fcf28ca
Show file tree
Hide file tree
Showing 18 changed files with 97 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.

2 changes: 1 addition & 1 deletion source/common/http/codec_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 4 additions & 2 deletions source/common/quic/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
5 changes: 3 additions & 2 deletions source/common/quic/codec_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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;
Expand Down
21 changes: 15 additions & 6 deletions source/common/quic/envoy_quic_client_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}
}

Expand Down
3 changes: 2 additions & 1 deletion source/common/quic/envoy_quic_client_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down
8 changes: 7 additions & 1 deletion source/common/quic/envoy_quic_server_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
17 changes: 0 additions & 17 deletions source/common/quic/envoy_quic_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
32 changes: 30 additions & 2 deletions source/common/quic/envoy_quic_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions source/common/quic/quic_filter_manager_connection_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion test/common/quic/envoy_quic_client_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion test/common/quic/envoy_quic_server_session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
7 changes: 4 additions & 3 deletions test/common/quic/envoy_quic_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/integration/fake_upstream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/integration/http_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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&
Expand Down
6 changes: 3 additions & 3 deletions test/integration/multiplexed_upstream_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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"));
Expand Down
Loading

0 comments on commit fcf28ca

Please sign in to comment.