Skip to content

Commit

Permalink
[http1] Include request URL in request header size computation, and r…
Browse files Browse the repository at this point in the history 
…eject partial headers that exceed configured limits (#145)

Signed-off-by: antonio <avd@google.com>
  • Loading branch information
@antoniovicente @tonya11en
antoniovicente authored and tonya11en committed Jun 30, 2020
1 parent 542f84c commit 7ca28ff
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 28 deletions.
44 changes: 30 additions & 14 deletions source/common/http/http1/codec_impl.cc
View file
Expand Up @@ -503,6 +503,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.
Expand Down Expand Up @@ -581,12 +597,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) {
Expand All @@ -595,12 +614,7 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) {
return;
}

if (processing_trailers_) {
maybeAllocTrailers();
}

absl::string_view header_value{data, length};

if (strict_header_validation_) {
if (!Http::HeaderUtility::headerValueIsValid(header_value)) {
ENVOY_CONN_LOG(debug, "invalid header value: {}", connection_, header_value);
Expand All @@ -620,15 +634,7 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) {
}
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() {
Expand Down Expand Up @@ -786,6 +792,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
Expand Down Expand Up @@ -918,6 +932,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();
}
}

Expand Down
30 changes: 24 additions & 6 deletions source/common/http/http1/codec_impl.h
View file
Expand Up @@ -220,6 +220,20 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
bool resetStreamCalled() { return reset_stream_called_; }
void onMessageBeginBase();

/**
* Get memory used to represent HTTP headers or trailers currently being parsed.
* Computed by adding the partial header field and value that is currently being parsed and the
* estimated header size for previous header lines provided by HeaderMap::byteSize().
*/
virtual uint32_t getHeadersSize();

/**
* Called from onUrl, onHeaderField and onHeaderValue to verify that the headers do not exceed the
* configured max header size limit. Throws a CodecProtocolException if headers exceed the size
* limit.
*/
void checkMaxHeadersSize();

Network::Connection& connection_;
CodecStats& stats_;
http_parser parser_;
Expand All @@ -246,7 +260,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
virtual HeaderMap& headersOrTrailers() PURE;
virtual RequestOrResponseHeaderMap& requestOrResponseHeaders() PURE;
virtual void allocHeaders() PURE;
virtual void maybeAllocTrailers() PURE;
virtual void allocTrailers() PURE;

/**
* Called in order to complete an in progress header decode.
Expand Down Expand Up @@ -426,6 +440,8 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
absl::optional<ActiveRequest>& activeRequest() { return active_request_; }
// ConnectionImpl
void onMessageComplete() override;
// Add the size of the request_url to the reported header size when processing request headers.
uint32_t getHeadersSize() override;

private:
/**
Expand Down Expand Up @@ -462,9 +478,10 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
}
void allocHeaders() override {
ASSERT(nullptr == absl::get<RequestHeaderMapPtr>(headers_or_trailers_));
ASSERT(!processing_trailers_);
headers_or_trailers_.emplace<RequestHeaderMapPtr>(RequestHeaderMapImpl::create());
}
void maybeAllocTrailers() override {
void allocTrailers() override {
ASSERT(processing_trailers_);
if (!absl::holds_alternative<RequestTrailerMapPtr>(headers_or_trailers_)) {
headers_or_trailers_.emplace<RequestTrailerMapPtr>(RequestTrailerMapImpl::create());
Expand Down Expand Up @@ -547,9 +564,10 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
}
void allocHeaders() override {
ASSERT(nullptr == absl::get<ResponseHeaderMapPtr>(headers_or_trailers_));
ASSERT(!processing_trailers_);
headers_or_trailers_.emplace<ResponseHeaderMapPtr>(ResponseHeaderMapImpl::create());
}
void maybeAllocTrailers() override {
void allocTrailers() override {
ASSERT(processing_trailers_);
if (!absl::holds_alternative<ResponseTrailerMapPtr>(headers_or_trailers_)) {
headers_or_trailers_.emplace<ResponseTrailerMapPtr>(ResponseTrailerMapImpl::create());
Expand All @@ -567,9 +585,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<ResponseHeaderMapPtr, ResponseTrailerMapPtr> headers_or_trailers_;

// The default limit of 80 KiB is the vanilla http_parser behaviour.
Expand Down
69 changes: 61 additions & 8 deletions test/common/http/http1/codec_impl_test.cc
View file
Expand Up @@ -238,7 +238,7 @@ void Http1ServerConnectionImplTest::testTrailersExceedLimit(std::string trailer_
"body\r\n0\r\n");
auto status = codec_->dispatch(buffer);
EXPECT_TRUE(status.ok());
buffer = Buffer::OwnedImpl(trailer_string + "\r\n\r\n");
buffer = Buffer::OwnedImpl(trailer_string);
if (enable_trailers) {
EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _));
status = codec_->dispatch(buffer);
Expand Down Expand Up @@ -2520,26 +2520,60 @@ 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<MockRequestDecoder> 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");

auto status = codec_->dispatch(buffer);
EXPECT_TRUE(isCodecProtocolError(status));
EXPECT_EQ(status.message(), "headers size exceeds limit");
EXPECT_EQ("http1.headers_too_large", response_encoder->getStream().responseDetails());
}

TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersRejected) {
Expand Down Expand Up @@ -2631,8 +2665,27 @@ 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<MockResponseDecoder> 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");
auto status = codec_->dispatch(buffer);
EXPECT_TRUE(status.ok());
std::string long_header = "big: " + std::string(80 * 1024, 'q');
buffer = Buffer::OwnedImpl(long_header);
status = codec_->dispatch(buffer);
EXPECT_TRUE(isCodecProtocolError(status));
EXPECT_EQ(status.message(), "headers size exceeds limit");
}

// Tests that incomplete response headers with a 80 kB header field fails.
TEST_F(Http1ClientConnectionImplTest, ResponseHeadersWithLargeFieldRejected) {
initialize();

NiceMock<MockRequestDecoder> decoder;
Expand All @@ -2644,7 +2697,7 @@ TEST_F(Http1ClientConnectionImplTest, LargeResponseHeadersRejected) {
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(80 * 1024, 'q') + "\r\n";
std::string long_header = "big: " + std::string(80 * 1024, 'q');
buffer = Buffer::OwnedImpl(long_header);
status = codec_->dispatch(buffer);
EXPECT_TRUE(isCodecProtocolError(status));
Expand Down
38 changes: 38 additions & 0 deletions test/integration/http_integration.cc
View file
Expand Up @@ -958,6 +958,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`
Expand Down
1 change: 1 addition & 0 deletions test/integration/http_integration.h
View file
Expand Up @@ -196,6 +196,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);
Expand Down
10 changes: 10 additions & 0 deletions test/integration/protocol_integration_test.cc
View file
Expand Up @@ -1352,6 +1352,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);
Expand Down

0 comments on commit 7ca28ff

Please sign in to comment.