Skip to content

Commit f2129cb

Browse files
mattklein123PiotrSikora
authored andcommitted
http2: configure logging of HTTP/2 flood attacks through runtime. (#34)
Signed-off-by: Matt Klein <mklein@lyft.com>
1 parent d3d5dcd commit f2129cb

File tree

4 files changed

+48
-2
lines changed

4 files changed

+48
-2
lines changed

docs/root/intro/version_history.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Version history
2828

2929
1.11.1 (August 13, 2019)
3030
========================
31-
* http: added mitigation of client initiated atacks that result in flooding of the downstream HTTP/2 connections.
31+
* 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.
3232
* http: added :ref:`inbound_empty_frames_flood <config_http_conn_man_stats_per_codec>` 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 <envoy_api_field_core.Http2ProtocolOptions.max_consecutive_inbound_frames_with_empty_payload>`.
3333
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 <envoy_api_field_core.Http2ProtocolOptions.max_consecutive_inbound_frames_with_empty_payload>`. Large override value (i.e. 2147483647) effectively disables mitigation of inbound frames with empty payload.
3434
* http: added :ref:`inbound_priority_frames_flood <config_http_conn_man_stats_per_codec>` 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 <envoy_api_field_core.Http2ProtocolOptions.max_inbound_priority_frames_per_stream>`.

source/common/http/conn_manager_impl.cc

+13
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,19 @@ Network::FilterStatus ConnectionManagerImpl::onData(Buffer::Instance& data, bool
289289
try {
290290
codec_->dispatch(data);
291291
} catch (const FrameFloodException& e) {
292+
// TODO(mattklein123): This is an emergency substitute for the lack of connection level
293+
// logging in the HCM. In a public follow up change we will add full support for connection
294+
// level logging in the HCM, similar to what we have in tcp_proxy. This will allow abuse
295+
// indicators to be stored in the connection level stream info, and then matched, sampled,
296+
// etc. when logged.
297+
const envoy::type::FractionalPercent default_value; // 0
298+
if (runtime_.snapshot().featureEnabled("http.connection_manager.log_flood_exception",
299+
default_value)) {
300+
ENVOY_CONN_LOG(warn, "downstream HTTP flood from IP '{}': {}",
301+
read_callbacks_->connection(),
302+
read_callbacks_->connection().remoteAddress()->asString(), e.what());
303+
}
304+
292305
handleCodecException(e.what());
293306
return Network::FilterStatus::StopIteration;
294307
} catch (const CodecProtocolException& e) {

test/common/http/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ envoy_cc_test(
211211
"//test/mocks/ssl:ssl_mocks",
212212
"//test/mocks/tracing:tracing_mocks",
213213
"//test/mocks/upstream:upstream_mocks",
214+
"//test/test_common:logging_lib",
214215
"//test/test_common:test_time_lib",
215216
],
216217
)

test/common/http/conn_manager_impl_test.cc

+33-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include "test/mocks/tracing/mocks.h"
4040
#include "test/mocks/upstream/cluster_info.h"
4141
#include "test/mocks/upstream/mocks.h"
42+
#include "test/test_common/logging.h"
4243
#include "test/test_common/printers.h"
4344
#include "test/test_common/test_time.h"
4445

@@ -55,6 +56,7 @@ using testing::HasSubstr;
5556
using testing::InSequence;
5657
using testing::Invoke;
5758
using testing::InvokeWithoutArgs;
59+
using testing::Matcher;
5860
using testing::NiceMock;
5961
using testing::Ref;
6062
using testing::Return;
@@ -2335,7 +2337,37 @@ TEST_F(HttpConnectionManagerImplTest, FrameFloodError) {
23352337

23362338
// Kick off the incoming data.
23372339
Buffer::OwnedImpl fake_input("1234");
2338-
conn_manager_->onData(fake_input, false);
2340+
EXPECT_LOG_NOT_CONTAINS("warning", "downstream HTTP flood",
2341+
conn_manager_->onData(fake_input, false));
2342+
}
2343+
2344+
// Verify that FrameFloodException causes connection to be closed abortively as well as logged
2345+
// if runtime indicates to do so.
2346+
TEST_F(HttpConnectionManagerImplTest, FrameFloodErrorWithLog) {
2347+
InSequence s;
2348+
setup(false, "");
2349+
2350+
EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void {
2351+
conn_manager_->newStream(response_encoder_);
2352+
throw FrameFloodException("too many outbound frames.");
2353+
}));
2354+
2355+
EXPECT_CALL(runtime_.snapshot_, featureEnabled("http.connection_manager.log_flood_exception",
2356+
Matcher<const envoy::type::FractionalPercent&>(_)))
2357+
.WillOnce(Return(true));
2358+
2359+
EXPECT_CALL(response_encoder_.stream_, removeCallbacks(_));
2360+
EXPECT_CALL(filter_factory_, createFilterChain(_)).Times(0);
2361+
2362+
// FrameFloodException should result in reset of the streams followed by abortive close.
2363+
EXPECT_CALL(filter_callbacks_.connection_,
2364+
close(Network::ConnectionCloseType::FlushWriteAndDelay));
2365+
2366+
// Kick off the incoming data.
2367+
Buffer::OwnedImpl fake_input("1234");
2368+
EXPECT_LOG_CONTAINS("warning",
2369+
"downstream HTTP flood from IP '0.0.0.0:0': too many outbound frames.",
2370+
conn_manager_->onData(fake_input, false));
23392371
}
23402372

23412373
TEST_F(HttpConnectionManagerImplTest, IdleTimeoutNoCodec) {

0 commit comments

Comments
 (0)