diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 85efa2c11102..10b1afd7a71e 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -454,6 +454,22 @@ void ConnectionImpl::completeLastHeader() { ASSERT(current_header_value_.empty()); } +uint32_t ConnectionImpl::getHeadersSize() { + return current_header_field_.size() + current_header_value_.size() + + headersOrTrailers().byteSize(); +} + +void ConnectionImpl::checkMaxHeadersSize() { + const uint32_t total = getHeadersSize(); + if (total > (max_headers_kb_ * 1024)) { + const absl::string_view header_type = + processing_trailers_ ? Http1HeaderTypes::get().Trailers : Http1HeaderTypes::get().Headers; + error_code_ = Http::Code::RequestHeaderFieldsTooLarge; + sendProtocolError(Http1ResponseCodeDetails::get().HeadersTooLarge); + throw CodecProtocolException(absl::StrCat(header_type, " size exceeds limit")); + } +} + bool ConnectionImpl::maybeDirectDispatch(Buffer::Instance& data) { if (!handling_upgrade_) { // Only direct dispatch for Upgrade requests. @@ -523,12 +539,15 @@ void ConnectionImpl::onHeaderField(const char* data, size_t length) { } processing_trailers_ = true; header_parsing_state_ = HeaderParsingState::Field; + allocTrailers(); } if (header_parsing_state_ == HeaderParsingState::Value) { completeLastHeader(); } current_header_field_.append(data, length); + + checkMaxHeadersSize(); } void ConnectionImpl::onHeaderValue(const char* data, size_t length) { @@ -537,10 +556,6 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) { return; } - if (processing_trailers_) { - maybeAllocTrailers(); - } - // Work around a bug in http_parser where trailing whitespace is not trimmed // as the spec requires: https://tools.ietf.org/html/rfc7230#section-3.2.4 const absl::string_view header_value = StringUtil::trim(absl::string_view(data, length)); @@ -557,15 +572,7 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) { header_parsing_state_ = HeaderParsingState::Value; current_header_value_.append(header_value.data(), header_value.length()); - const uint32_t total = - current_header_field_.size() + current_header_value_.size() + headersOrTrailers().byteSize(); - if (total > (max_headers_kb_ * 1024)) { - const absl::string_view header_type = - processing_trailers_ ? Http1HeaderTypes::get().Trailers : Http1HeaderTypes::get().Headers; - error_code_ = Http::Code::RequestHeaderFieldsTooLarge; - sendProtocolError(Http1ResponseCodeDetails::get().HeadersTooLarge); - throw CodecProtocolException(absl::StrCat(header_type, " size exceeds limit")); - } + checkMaxHeadersSize(); } int ConnectionImpl::onHeadersCompleteBase() { @@ -708,6 +715,14 @@ ServerConnectionImpl::ServerConnectionImpl( Runtime::runtimeFeatureEnabled("envoy.reloadable_features.http1_flood_protection")), headers_with_underscores_action_(headers_with_underscores_action) {} +uint32_t ServerConnectionImpl::getHeadersSize() { + // Add in the the size of the request URL if processing request headers. + const uint32_t url_size = (!processing_trailers_ && active_request_.has_value()) + ? active_request_.value().request_url_.size() + : 0; + return url_size + ConnectionImpl::getHeadersSize(); +} + void ServerConnectionImpl::onEncodeComplete() { if (active_request_.value().remote_complete_) { // Only do this if remote is complete. If we are replying before the request is complete the @@ -842,6 +857,8 @@ void ServerConnectionImpl::onMessageBegin() { void ServerConnectionImpl::onUrl(const char* data, size_t length) { if (active_request_.has_value()) { active_request_.value().request_url_.append(data, length); + + checkMaxHeadersSize(); } } diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index cdc651c19766..468f9a5d15c0 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -214,6 +214,20 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable(headers_or_trailers_)); + ASSERT(!processing_trailers_); headers_or_trailers_.emplace(std::make_unique()); } - void maybeAllocTrailers() override { + void allocTrailers() override { ASSERT(processing_trailers_); if (!absl::holds_alternative(headers_or_trailers_)) { headers_or_trailers_.emplace(std::make_unique()); @@ -520,9 +539,10 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl { } void allocHeaders() override { ASSERT(nullptr == absl::get(headers_or_trailers_)); + ASSERT(!processing_trailers_); headers_or_trailers_.emplace(std::make_unique()); } - void maybeAllocTrailers() override { + void allocTrailers() override { ASSERT(processing_trailers_); if (!absl::holds_alternative(headers_or_trailers_)) { headers_or_trailers_.emplace( @@ -541,9 +561,9 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl { bool ignore_message_complete_for_100_continue_{}; // TODO(mattklein123): This should be a member of PendingResponse but this change needs dedicated // thought as some of the reset and no header code paths make this difficult. Headers are - // populated on message begin. Trailers are populated on the first parsed trailer field (if - // trailers are enabled). The variant is reset to null headers on message complete for assertion - // purposes. + // populated on message begin. Trailers are populated when the switch to trailer processing is + // detected while parsing the first trailer field (if trailers are enabled). The variant is reset + // to null headers on message complete for assertion purposes. absl::variant headers_or_trailers_; // The default limit of 80 KiB is the vanilla http_parser behaviour. diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 0c26f621f9e8..d501e8f99736 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -216,8 +216,10 @@ void Http1ServerConnectionImplTest::testTrailersExceedLimit(std::string trailer_ "Transfer-Encoding: chunked\r\n\r\n" "4\r\n" "body\r\n0\r\n"); + codec_->dispatch(buffer); - buffer = Buffer::OwnedImpl(trailer_string + "\r\n\r\n"); + buffer = Buffer::OwnedImpl(trailer_string); + if (enable_trailers) { EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "trailers size exceeds limit"); @@ -1964,26 +1966,57 @@ TEST_F(Http1ClientConnectionImplTest, LowWatermarkDuringClose) { TEST_F(Http1ServerConnectionImplTest, LargeTrailersRejected) { // Default limit of 60 KiB - std::string long_string = "big: " + std::string(60 * 1024, 'q') + "\r\n"; + std::string long_string = "big: " + std::string(60 * 1024, 'q') + "\r\n\r\n\r\n"; + testTrailersExceedLimit(long_string, true); +} + +TEST_F(Http1ServerConnectionImplTest, LargeTrailerFieldRejected) { + // Construct partial headers with a long field name that exceeds the default limit of 60KiB. + std::string long_string = "bigfield" + std::string(60 * 1024, 'q'); testTrailersExceedLimit(long_string, true); } // Tests that the default limit for the number of request headers is 100. TEST_F(Http1ServerConnectionImplTest, ManyTrailersRejected) { // Send a request with 101 headers. - testTrailersExceedLimit(createHeaderFragment(101), true); + testTrailersExceedLimit(createHeaderFragment(101) + "\r\n\r\n", true); } TEST_F(Http1ServerConnectionImplTest, LargeTrailersRejectedIgnored) { // Default limit of 60 KiB - std::string long_string = "big: " + std::string(60 * 1024, 'q') + "\r\n"; + std::string long_string = "big: " + std::string(60 * 1024, 'q') + "\r\n\r\n\r\n"; + testTrailersExceedLimit(long_string, false); +} + +TEST_F(Http1ServerConnectionImplTest, LargeTrailerFieldRejectedIgnored) { + // Default limit of 60 KiB + std::string long_string = "bigfield" + std::string(60 * 1024, 'q') + ": value\r\n\r\n\r\n"; testTrailersExceedLimit(long_string, false); } // Tests that the default limit for the number of request headers is 100. TEST_F(Http1ServerConnectionImplTest, ManyTrailersIgnored) { // Send a request with 101 headers. - testTrailersExceedLimit(createHeaderFragment(101), false); + testTrailersExceedLimit(createHeaderFragment(101) + "\r\n\r\n", false); +} + +TEST_F(Http1ServerConnectionImplTest, LargeRequestUrlRejected) { + initialize(); + + std::string exception_reason; + NiceMock decoder; + Http::ResponseEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](ResponseEncoder& encoder, bool) -> RequestDecoder& { + response_encoder = &encoder; + return decoder; + })); + + // Default limit of 60 KiB + std::string long_url = "/" + std::string(60 * 1024, 'q'); + Buffer::OwnedImpl buffer("GET " + long_url + " HTTP/1.1\r\n"); + EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); + EXPECT_EQ("http1.headers_too_large", response_encoder->getStream().responseDetails()); } TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersRejected) { @@ -2069,8 +2102,24 @@ TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersAccepted) { testRequestHeadersAccepted(createHeaderFragment(150)); } -// Tests that response headers of 80 kB fails. -TEST_F(Http1ClientConnectionImplTest, LargeResponseHeadersRejected) { +// Tests that incomplete response headers of 80 kB header value fails. +TEST_F(Http1ClientConnectionImplTest, ResponseHeadersWithLargeValueRejected) { + initialize(); + + NiceMock response_decoder; + Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder); + TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}}; + request_encoder.encodeHeaders(headers, true); + + Buffer::OwnedImpl buffer("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n"); + codec_->dispatch(buffer); + std::string long_header = "big: " + std::string(80 * 1024, 'q'); + buffer = Buffer::OwnedImpl(long_header); + EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); +} + +// Tests that incomplete response headers with a 80 kB header field fails. +TEST_F(Http1ClientConnectionImplTest, ResponseHeadersWithLargeFieldRejected) { initialize(); NiceMock response_decoder; @@ -2080,7 +2129,7 @@ TEST_F(Http1ClientConnectionImplTest, LargeResponseHeadersRejected) { Buffer::OwnedImpl buffer("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n"); codec_->dispatch(buffer); - std::string long_header = "big: " + std::string(80 * 1024, 'q') + "\r\n"; + std::string long_header = "big: " + std::string(80 * 1024, 'q'); buffer = Buffer::OwnedImpl(long_header); EXPECT_THROW_WITH_MESSAGE(codec_->dispatch(buffer), EnvoyException, "headers size exceeds limit"); } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index f485f3083c64..d8bbaa062704 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -959,6 +959,44 @@ void HttpIntegrationTest::testTwoRequests(bool network_backup) { EXPECT_EQ(1024U, response->body().size()); } +void HttpIntegrationTest::testLargeRequestUrl(uint32_t url_size, uint32_t max_headers_size) { + // `size` parameter dictates the size of each header that will be added to the request and `count` + // parameter is the number of headers to be added. The actual request byte size will exceed `size` + // due to the keys and other headers. The actual request header count will exceed `count` by four + // due to default headers. + + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { hcm.mutable_max_request_headers_kb()->set_value(max_headers_size); }); + max_request_headers_kb_ = max_headers_size; + + Http::TestRequestHeaderMapImpl big_headers{{":method", "GET"}, + {":path", "/" + std::string(url_size * 1024, 'a')}, + {":scheme", "http"}, + {":authority", "host"}}; + + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + if (url_size >= max_headers_size) { + // header size includes keys too, so expect rejection when equal + auto encoder_decoder = codec_client_->startRequest(big_headers); + auto response = std::move(encoder_decoder.second); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("431", response->headers().Status()->value().getStringView()); + } else { + response->waitForReset(); + codec_client_->close(); + } + } else { + auto response = sendRequestAndWaitForResponse(big_headers, 0, default_response_headers_, 0); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); + } +} + void HttpIntegrationTest::testLargeRequestHeaders(uint32_t size, uint32_t count, uint32_t max_size, uint32_t max_count) { // `size` parameter dictates the size of each header that will be added to the request and `count` diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 99f2c8850521..edf76387fe0b 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -195,6 +195,7 @@ class HttpIntegrationTest : public BaseIntegrationTest { void testLargeHeaders(Http::TestRequestHeaderMapImpl request_headers, Http::TestRequestTrailerMapImpl request_trailers, uint32_t size, uint32_t max_size); + void testLargeRequestUrl(uint32_t url_size, uint32_t max_headers_size); void testLargeRequestHeaders(uint32_t size, uint32_t count, uint32_t max_size = 60, uint32_t max_count = 100); void testLargeRequestTrailers(uint32_t size, uint32_t max_size = 60); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index d051884e2629..aea5c80e56ff 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1047,6 +1047,16 @@ name: decode-headers-only EXPECT_EQ(0, upstream_request_->body().length()); } +TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlRejected) { + // Send one 95 kB URL with limit 60 kB headers. + testLargeRequestUrl(95, 60); +} + +TEST_P(DownstreamProtocolIntegrationTest, LargeRequestUrlAccepted) { + // Send one 95 kB URL with limit 96 kB headers. + testLargeRequestUrl(95, 96); +} + TEST_P(DownstreamProtocolIntegrationTest, LargeRequestHeadersRejected) { // Send one 95 kB header with limit 60 kB and 100 headers. testLargeRequestHeaders(95, 1, 60, 100);