You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2019/06/03 16:49:06 UTC
[trafficserver] branch 7.1.x updated: Allow number of settings per
H2 session to be configurable
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/7.1.x by this push:
new 6f291c1 Allow number of settings per H2 session to be configurable
6f291c1 is described below
commit 6f291c115002693ebe267ac058adfb863c4085b6
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Thu Apr 18 15:27:19 2019 +0900
Allow number of settings per H2 session to be configurable
(cherry picked from commit d91a8ca17bb17b08ce7f040b868e3c286a7bb0a6)
Conflicts:
doc/admin-guide/files/records.config.en.rst
mgmt/RecordsConfig.cc
proxy/http2/HTTP2.cc
proxy/http2/HTTP2.h
proxy/http2/Http2ConnectionState.cc
proxy/http2/Http2ConnectionState.h
---
doc/admin-guide/files/records.config.en.rst | 14 ++++++++++
mgmt/RecordsConfig.cc | 4 +++
proxy/http2/HTTP2.cc | 4 +++
proxy/http2/HTTP2.h | 2 ++
proxy/http2/Http2ConnectionState.cc | 40 +++++++++++++++++++++++++++++
proxy/http2/Http2ConnectionState.h | 8 ++++++
6 files changed, 72 insertions(+)
diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index 8fff065..a1cfec0 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -3383,6 +3383,20 @@ HTTP/2 Configuration
HTTP/2 connection to avoid duplicate pushes on the same connection. If the
maximum number is reached, new entries are not remembered.
+.. ts:cv:: CONFIG proxy.config.http2.max_settings_per_frame INT 7
+ :reloadable:
+
+ Specifies how many settings in an HTTP/2 SETTINGS frame |TS| accepts.
+ Clients exceeded this limit will be immediately disconnected with an error
+ code of ENHANCE_YOUR_CALM.
+
+.. ts:cv:: CONFIG proxy.config.http2.max_settings_per_minute INT 14
+ :reloadable:
+
+ Specifies how many settings in HTTP/2 SETTINGS frames |TS| accept for a minute.
+ Clients exceeded this limit will be immediately disconnected with an error
+ code of ENHANCE_YOUR_CALM.
+
Plug-in Configuration
=====================
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index 7ee5fcc..30613ff 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1438,6 +1438,10 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.http2.push_diary_size", RECD_INT, "256", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
+ {RECT_CONFIG, "proxy.config.http2.max_settings_per_frame", RECD_INT, "7", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
+ ,
+ {RECT_CONFIG, "proxy.config.http2.max_settings_per_minute", RECD_INT, "14", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
+ ,
//# Add LOCAL Records Here
{RECT_LOCAL, "proxy.local.incoming_ip_to_bind", RECD_STRING, nullptr, RECU_NULL, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index 7d949e9..568638a 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -729,6 +729,8 @@ uint32_t Http2::accept_no_activity_timeout = 120;
uint32_t Http2::no_activity_timeout_in = 120;
uint32_t Http2::active_timeout_in = 0;
uint32_t Http2::push_diary_size = 256;
+uint32_t Http2::max_settings_per_frame = 7;
+uint32_t Http2::max_settings_per_minute = 14;
void
Http2::init()
@@ -745,6 +747,8 @@ Http2::init()
REC_EstablishStaticConfigInt32U(no_activity_timeout_in, "proxy.config.http2.no_activity_timeout_in");
REC_EstablishStaticConfigInt32U(active_timeout_in, "proxy.config.http2.active_timeout_in");
REC_EstablishStaticConfigInt32U(push_diary_size, "proxy.config.http2.push_diary_size");
+ REC_EstablishStaticConfigInt32U(max_settings_per_frame, "proxy.config.http2.max_settings_per_frame");
+ REC_EstablishStaticConfigInt32U(max_settings_per_minute, "proxy.config.http2.max_settings_per_minute");
// If any settings is broken, ATS should not start
ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, max_concurrent_streams_in}));
diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
index 9d94e4f..4b1da0b 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -376,6 +376,8 @@ public:
static uint32_t no_activity_timeout_in;
static uint32_t active_timeout_in;
static uint32_t push_diary_size;
+ static uint32_t max_settings_per_frame;
+ static uint32_t max_settings_per_minute;
static void init();
};
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 7f81cf4..009e80f 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -522,7 +522,14 @@ rcv_settings_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
"recv settings header wrong length");
}
+ uint32_t n_settings = 0;
while (nbytes < frame.header().length) {
+ if (n_settings >= Http2::max_settings_per_frame) {
+ Http2StreamDebug(cstate.ua_session, stream_id, "Observed too many settings in a frame");
+ return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM,
+ "recv settings too many settings in a frame");
+ }
+
unsigned read_bytes = read_rcv_buffer(buf, sizeof(buf), nbytes, frame);
if (!http2_parse_settings_parameter(make_iovec(buf, read_bytes), param)) {
@@ -551,6 +558,17 @@ rcv_settings_frame(Http2ConnectionState &cstate, const Http2Frame &frame)
}
cstate.client_settings.set((Http2SettingsIdentifier)param.id, param.value);
+ ++n_settings;
+ }
+
+ // Update settigs count per minute
+ cstate.increment_received_settings_count(n_settings);
+ // Close this conection if its settings count received exceeds a limit
+ if (cstate.get_received_settings_count() > Http2::max_settings_per_minute) {
+ Http2StreamDebug(cstate.ua_session, stream_id, "Observed too frequent setting changes: %u settings within a last minute",
+ cstate.get_received_settings_count());
+ return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION, Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM,
+ "recv settings too frequent setting changes");
}
// [RFC 7540] 6.5. Once all values have been applied, the recipient MUST
@@ -1748,6 +1766,28 @@ Http2ConnectionState::send_window_update_frame(Http2StreamId id, uint32_t size)
this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT, &window_update);
}
+void
+Http2ConnectionState::increment_received_settings_count(uint32_t count)
+{
+ ink_hrtime hrtime_sec = Thread::get_hrtime() / HRTIME_SECOND;
+ uint8_t counter_index = ((hrtime_sec % 60) >= 30);
+
+ if ((hrtime_sec - 60) > this->settings_count_last_update) {
+ this->settings_count[0] = 0;
+ this->settings_count[1] = 0;
+ } else if (counter_index != ((this->settings_count_last_update % 60) >= 30)) {
+ this->settings_count[counter_index] = 0;
+ }
+ this->settings_count[counter_index] += count;
+ this->settings_count_last_update = hrtime_sec;
+}
+
+uint32_t
+Http2ConnectionState::get_received_settings_count()
+{
+ return this->settings_count[0] + this->settings_count[1];
+}
+
// Return min_concurrent_streams_in when current client streams number is larger than max_active_streams_in.
// Main purpose of this is preventing DDoS Attacks.
unsigned
diff --git a/proxy/http2/Http2ConnectionState.h b/proxy/http2/Http2ConnectionState.h
index e773f32..a802ed0 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -269,6 +269,9 @@ public:
}
}
+ void increment_received_settings_count(uint32_t count);
+ uint32_t get_received_settings_count();
+
private:
Http2ConnectionState(const Http2ConnectionState &); // noncopyable
Http2ConnectionState &operator=(const Http2ConnectionState &); // noncopyable
@@ -295,6 +298,11 @@ private:
// Counter for current active streams and streams in the process of shutting down
uint32_t total_client_streams_count;
+ // Counter for settings received within last 60 seconds
+ // Each item holds a count for 30 seconds.
+ uint16_t settings_count[2] = {0};
+ ink_hrtime settings_count_last_update = 0;
+
// NOTE: Id of stream which MUST receive CONTINUATION frame.
// - [RFC 7540] 6.2 HEADERS
// "A HEADERS frame without the END_HEADERS flag set MUST be followed by a