diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 0149bff398f0..adb9da1f1d37 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -423,6 +423,20 @@ void ConnectionImpl::completeLastHeader() { ASSERT(current_header_value_.empty()); } +uint32_t ConnectionImpl::getHeadersSize() { + return current_header_field_.size() + current_header_value_.size() + + (current_header_map_->byteSize() ? *current_header_map_->byteSize() : 0); +} + +void ConnectionImpl::checkMaxHeadersSize() { + const uint32_t total = getHeadersSize(); + if (total > (max_headers_kb_ * 1024)) { + error_code_ = Http::Code::RequestHeaderFieldsTooLarge; + sendProtocolError(); + throw CodecProtocolException("headers size exceeds limit"); + } +} + bool ConnectionImpl::maybeDirectDispatch(Buffer::Instance& data) { if (!handling_upgrade_) { // Only direct dispatch for Upgrade requests. @@ -494,6 +508,8 @@ void ConnectionImpl::onHeaderField(const char* data, size_t length) { } current_header_field_.append(data, length); + + checkMaxHeadersSize(); } void ConnectionImpl::onHeaderValue(const char* data, size_t length) { @@ -524,16 +540,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()); - // Verify that the cached value in byte size exists. - ASSERT(current_header_map_->byteSize().has_value()); - const uint32_t total = current_header_field_.size() + current_header_value_.size() + - current_header_map_->byteSize().value(); - if (total > (max_headers_kb_ * 1024)) { - - error_code_ = Http::Code::RequestHeaderFieldsTooLarge; - sendProtocolError(); - throw CodecProtocolException("headers size exceeds limit"); - } + checkMaxHeadersSize(); } int ConnectionImpl::onHeadersCompleteBase() { @@ -634,6 +641,12 @@ 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 = active_request_ ? active_request_->request_url_.size() : 0; + return url_size + ConnectionImpl::getHeadersSize(); +} + void ServerConnectionImpl::onEncodeComplete() { ASSERT(active_request_); if (active_request_->remote_complete_) { @@ -741,6 +754,8 @@ void ServerConnectionImpl::onMessageBegin() { void ServerConnectionImpl::onUrl(const char* data, size_t length) { if (active_request_) { active_request_->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 e16ec8401286..8efd80a4f803 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -209,6 +209,20 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable(codec_.get()) ->onUnderlyingConnectionBelowWriteBufferLowWatermark(); } + +TEST_F(Http1ServerConnectionImplTest, LargeRequestUrlRejected) { + initialize(); + + std::string exception_reason; + NiceMock decoder; + Http::StreamEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](StreamEncoder& encoder, bool) -> StreamDecoder& { + 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"); +} + TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersRejected) { // Default limit of 60 KiB std::string long_string = "big: " + std::string(60 * 1024, 'q') + "\r\n"; @@ -1635,8 +1655,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::StreamEncoder& request_encoder = codec_->newStream(response_decoder); + TestHeaderMapImpl 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; @@ -1646,7 +1682,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 = "bigfield" + 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 310542194c8a..6a8a52004b19 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -884,6 +884,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::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { hcm.mutable_max_request_headers_kb()->set_value(max_headers_size); }); + max_request_headers_kb_ = max_headers_size; + + Http::TestHeaderMapImpl 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 8bbae101304b..7882bd3e85d5 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -190,6 +190,7 @@ class HttpIntegrationTest : public BaseIntegrationTest { void testTwoRequests(bool force_network_backup = false); void testLargeHeaders(Http::TestHeaderMapImpl request_headers, Http::TestHeaderMapImpl 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 2302e34a3c4d..d94fe99d5057 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1006,6 +1006,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);