trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bc...@apache.org
Subject [trafficserver] branch 8.0.x updated: Allow number of settings per H2 session to be configurable
Date Tue, 28 May 2019 23:34:28 GMT
This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch 8.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/8.0.x by this push:
     new 5d238f8  Allow number of settings per H2 session to be configurable
5d238f8 is described below

commit 5d238f8c32695990142bfcc8a9dac7e2dc870af0
Author: Masakazu Kitajo <maskit@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.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 2cfd2a6..b2ca394 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -3518,6 +3518,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 19a25ff..5f781f8 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -1324,6 +1324,10 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.http2.zombie_debug_timeout_in", RECD_INT, "0", 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 2817fef..cd7a20d 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -732,6 +732,8 @@ 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::zombie_timeout_in          = 0;
+uint32_t Http2::max_settings_per_frame     = 7;
+uint32_t Http2::max_settings_per_minute    = 14;
 
 void
 Http2::init()
@@ -749,6 +751,8 @@ Http2::init()
   REC_EstablishStaticConfigInt32U(active_timeout_in, "proxy.config.http2.active_timeout_in");
   REC_EstablishStaticConfigInt32U(push_diary_size, "proxy.config.http2.push_diary_size");
   REC_EstablishStaticConfigInt32U(zombie_timeout_in, "proxy.config.http2.zombie_debug_timeout_in");
+  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 b344743..72a626c 100644
--- a/proxy/http2/HTTP2.h
+++ b/proxy/http2/HTTP2.h
@@ -377,6 +377,8 @@ public:
   static uint32_t active_timeout_in;
   static uint32_t push_diary_size;
   static uint32_t zombie_timeout_in;
+  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 37c35e2..243e038 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -540,7 +540,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)) {
@@ -569,6 +576,17 @@ rcv_settings_frame(Http2ConnectionState &cstate, const Http2Frame
&frame)
     }
 
     cstate.client_settings.set(static_cast<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
@@ -1815,6 +1833,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 c2eea39..7e6a840 100644
--- a/proxy/http2/Http2ConnectionState.h
+++ b/proxy/http2/Http2ConnectionState.h
@@ -293,6 +293,9 @@ public:
     }
   }
 
+  void increment_received_settings_count(uint32_t count);
+  uint32_t get_received_settings_count();
+
 private:
   unsigned _adjust_concurrent_stream();
 
@@ -316,6 +319,11 @@ private:
   // Counter for current active streams and streams in the process of shutting down
   std::atomic<uint32_t> total_client_streams_count = 0;
 
+  // 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


Mime
View raw message