From f2129cbfeff3b206fb5d00bbae06049a4a21383e Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 9 Aug 2019 22:04:31 -0700 Subject: [PATCH] http2: configure logging of HTTP/2 flood attacks through runtime. (#34) Signed-off-by: Matt Klein --- docs/root/intro/version_history.rst | 2 +- source/common/http/conn_manager_impl.cc | 13 +++++++++ test/common/http/BUILD | 1 + test/common/http/conn_manager_impl_test.cc | 34 +++++++++++++++++++++- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 7149dfcc0716..b188266557da 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -28,7 +28,7 @@ Version history 1.11.1 (August 13, 2019) ======================== -* http: added mitigation of client initiated atacks that result in flooding of the downstream HTTP/2 connections. +* http: added mitigation of client initiated attacks that result in flooding of the downstream HTTP/2 connections. Those attacks can be logged at the "warning" level when the runtime feature `http.connection_manager.log_flood_exception` is enabled. The runtime setting defaults to disabled to avoid log spam when under attack. * http: added :ref:`inbound_empty_frames_flood ` counter stat to the HTTP/2 codec stats, for tracking number of connections terminated for exceeding the limit on consecutive inbound frames with an empty payload and no end stream flag. The limit is configured by setting the :ref:`max_consecutive_inbound_frames_with_empty_payload config setting `. Runtime feature `envoy.reloadable_features.http2_protocol_options.max_consecutive_inbound_frames_with_empty_payload` overrides :ref:`max_consecutive_inbound_frames_with_empty_payload setting `. Large override value (i.e. 2147483647) effectively disables mitigation of inbound frames with empty payload. * http: added :ref:`inbound_priority_frames_flood ` counter stat to the HTTP/2 codec stats, for tracking number of connections terminated for exceeding the limit on inbound PRIORITY frames. The limit is configured by setting the :ref:`max_inbound_priority_frames_per_stream config setting `. diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 2d907859b8ce..8ee08cf2b988 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -289,6 +289,19 @@ Network::FilterStatus ConnectionManagerImpl::onData(Buffer::Instance& data, bool try { codec_->dispatch(data); } catch (const FrameFloodException& e) { + // TODO(mattklein123): This is an emergency substitute for the lack of connection level + // logging in the HCM. In a public follow up change we will add full support for connection + // level logging in the HCM, similar to what we have in tcp_proxy. This will allow abuse + // indicators to be stored in the connection level stream info, and then matched, sampled, + // etc. when logged. + const envoy::type::FractionalPercent default_value; // 0 + if (runtime_.snapshot().featureEnabled("http.connection_manager.log_flood_exception", + default_value)) { + ENVOY_CONN_LOG(warn, "downstream HTTP flood from IP '{}': {}", + read_callbacks_->connection(), + read_callbacks_->connection().remoteAddress()->asString(), e.what()); + } + handleCodecException(e.what()); return Network::FilterStatus::StopIteration; } catch (const CodecProtocolException& e) { diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 5cfeaff671fa..56eefc2b25a6 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -211,6 +211,7 @@ envoy_cc_test( "//test/mocks/ssl:ssl_mocks", "//test/mocks/tracing:tracing_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:logging_lib", "//test/test_common:test_time_lib", ], ) diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 811c5e98d473..2ae4d2483eed 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -39,6 +39,7 @@ #include "test/mocks/tracing/mocks.h" #include "test/mocks/upstream/cluster_info.h" #include "test/mocks/upstream/mocks.h" +#include "test/test_common/logging.h" #include "test/test_common/printers.h" #include "test/test_common/test_time.h" @@ -55,6 +56,7 @@ using testing::HasSubstr; using testing::InSequence; using testing::Invoke; using testing::InvokeWithoutArgs; +using testing::Matcher; using testing::NiceMock; using testing::Ref; using testing::Return; @@ -2335,7 +2337,37 @@ TEST_F(HttpConnectionManagerImplTest, FrameFloodError) { // Kick off the incoming data. Buffer::OwnedImpl fake_input("1234"); - conn_manager_->onData(fake_input, false); + EXPECT_LOG_NOT_CONTAINS("warning", "downstream HTTP flood", + conn_manager_->onData(fake_input, false)); +} + +// Verify that FrameFloodException causes connection to be closed abortively as well as logged +// if runtime indicates to do so. +TEST_F(HttpConnectionManagerImplTest, FrameFloodErrorWithLog) { + InSequence s; + setup(false, ""); + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void { + conn_manager_->newStream(response_encoder_); + throw FrameFloodException("too many outbound frames."); + })); + + EXPECT_CALL(runtime_.snapshot_, featureEnabled("http.connection_manager.log_flood_exception", + Matcher(_))) + .WillOnce(Return(true)); + + EXPECT_CALL(response_encoder_.stream_, removeCallbacks(_)); + EXPECT_CALL(filter_factory_, createFilterChain(_)).Times(0); + + // FrameFloodException should result in reset of the streams followed by abortive close. + EXPECT_CALL(filter_callbacks_.connection_, + close(Network::ConnectionCloseType::FlushWriteAndDelay)); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + EXPECT_LOG_CONTAINS("warning", + "downstream HTTP flood from IP '0.0.0.0:0': too many outbound frames.", + conn_manager_->onData(fake_input, false)); } TEST_F(HttpConnectionManagerImplTest, IdleTimeoutNoCodec) {