Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: Proactively disconnect connections flooded in drain timeout #13420

Merged
merged 4 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions source/common/http/http2/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ void ConnectionImpl::goAway() {
auto status = sendPendingFrames();
// See comment in the `encodeHeadersBase()` method about this RELEASE_ASSERT.
RELEASE_ASSERT(status.ok(), "sendPendingFrames() failure in non dispatching context");
checkProtocolConstraintViolation();
}

void ConnectionImpl::shutdownNotice() {
Expand Down
1 change: 1 addition & 0 deletions source/common/http/http2/codec_impl_legacy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ void ConnectionImpl::goAway() {
ASSERT(rc == 0);

sendPendingFrames();
checkProtocolConstraintViolation();
}

void ConnectionImpl::shutdownNotice() {
Expand Down
40 changes: 40 additions & 0 deletions test/common/http/http2/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2299,6 +2299,46 @@ TEST_P(Http2CodecImplTest, EmptyDataFloodOverride) {
EXPECT_TRUE(status.ok());
}

// Verify that codec detects flood of outbound frames caused by goAway() method
TEST_P(Http2CodecImplTest, GoAwayCausesOutboundFlood) {
initialize();

TestRequestHeaderMapImpl request_headers;
HttpTestUtility::addDefaultHeaders(request_headers);
EXPECT_CALL(request_decoder_, decodeHeaders_(_, false));
request_encoder_->encodeHeaders(request_headers, false);

int frame_count = 0;
Buffer::OwnedImpl buffer;
ON_CALL(server_connection_, write(_, _))
.WillByDefault(Invoke([&buffer, &frame_count](Buffer::Instance& frame, bool) {
++frame_count;
buffer.move(frame);
}));

auto* violation_callback =
new NiceMock<Event::MockSchedulableCallback>(&server_connection_.dispatcher_);

TestResponseHeaderMapImpl response_headers{{":status", "200"}};
response_encoder_->encodeHeaders(response_headers, false);
// Account for the single HEADERS frame above
for (uint32_t i = 0; i < CommonUtility::OptionsLimits::DEFAULT_MAX_OUTBOUND_FRAMES - 1; ++i) {
Buffer::OwnedImpl data("0");
EXPECT_NO_THROW(response_encoder_->encodeData(data, false));
}

EXPECT_FALSE(violation_callback->enabled_);

server_->goAway();

EXPECT_TRUE(violation_callback->enabled_);
EXPECT_CALL(server_connection_, close(Envoy::Network::ConnectionCloseType::NoFlush));
violation_callback->invokeCallback();

EXPECT_EQ(frame_count, CommonUtility::OptionsLimits::DEFAULT_MAX_OUTBOUND_FRAMES + 1);
EXPECT_EQ(1, server_stats_store_.counter("http2.outbound_flood").value());
}

// Verify that codec detects flood of outbound frames caused by shutdownNotice() method
TEST_P(Http2CodecImplTest, ShudowNoticeCausesOutboundFlood) {
initialize();
Expand Down
26 changes: 26 additions & 0 deletions test/integration/http2_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2011,6 +2011,32 @@ TEST_P(Http2FloodMitigationTest, RST_STREAM) {
test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value());
}

// Verify detection of frame flood when sending second GOAWAY frame on drain timeout
TEST_P(Http2FloodMitigationTest, GoAwayOverflowOnDrainTimeout) {
config_helper_.addConfigModifier(
[](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
hcm) {
auto* drain_time_out = hcm.mutable_drain_timeout();
std::chrono::milliseconds timeout(1000);
auto seconds = std::chrono::duration_cast<std::chrono::seconds>(timeout);
drain_time_out->set_seconds(seconds.count());

auto* http_protocol_options = hcm.mutable_common_http_protocol_options();
auto* idle_time_out = http_protocol_options->mutable_idle_timeout();
idle_time_out->set_seconds(seconds.count());
});
// pre-fill two away from overflow
prefillOutboundDownstreamQueue(AllFrameFloodLimit - 2);

// connection idle timeout will send first GOAWAY frame and start drain timer
// drain timeout will send second GOAWAY frame which should trigger flood protection
// Wait for connection to be flooded with outbound GOAWAY frame and disconnected.
tcp_client_->waitForDisconnect();

// Verify that the flood check was triggered
EXPECT_EQ(1, test_server_->counter("http2.outbound_flood")->value());
}

// Verify detection of overflowing outbound frame queue with the GOAWAY frames sent after the
// downstream idle connection timeout disconnects the connection.
// The test verifies protocol constraint violation handling in the
Expand Down