MEDIUM: mux-h2: allow to set the glitches threshold to kill a connection
Till now it was still needed to write rules to eliminate bad behaving
H2 clients, while most of the time it would be desirable to just be able
to set a threshold on the level of anomalies on a connection.
This is what this patch does. By setting a glitches threshold for frontend
and backend, it allows to automatically turn a connection to the error
state when the threshold is reached so that the connection dies by itself
without having to write possibly complex rules.
One subtlety is that we still have the error state being exclusive to the
parser's state so this requires the h2c_report_glitches() function to return
a status indicating if the threshold was reached or not so that processing
can instantly stop and bypass the state update, otherwise the state could
be turned back to a valid one (e.g. after parsing CONTINUATION); we should
really contemplate the possibility to use H2_CF_ERROR for this. Fortunately
there were very few places where a glitch was reported outside of an error
path so the changes are quite minor.
Now by setting the front value to 1000, a client flooding with short
CONTINUATION frames is instantly stopped.
(cherry picked from commit 6770259083eeb4b6f8c1dfee6870c3f75b87531e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8f2ecb0d19182094a0188141f9327faf7775284c)
[wt: updated doc ctx]
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 0791ed1..0f21aa8 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -1168,8 +1168,10 @@
- tune.disable-fast-forward
- tune.fail-alloc
- tune.fd.edge-triggered
+ - tune.h2.be.glitches-threshold
- tune.h2.be.initial-window-size
- tune.h2.be.max-concurrent-streams
+ - tune.h2.fe.glitches-threshold
- tune.h2.fe.initial-window-size
- tune.h2.fe.max-concurrent-streams
- tune.h2.fe.max-total-streams
@@ -2950,6 +2952,18 @@
certain scenarios. This is still experimental, it may result in frozen
connections if bugs are still present, and is disabled by default.
+tune.h2.be.glitches-threshold <number>
+ Sets the threshold for the number of glitches on a backend connection, where
+ that connection will automatically be killed. This allows to automatically
+ kill misbehaving connections without having to write explicit rules for them.
+ The default value is zero, indicating that no threshold is set so that no
+ event will cause a connection to be closed. Beware that some H2 servers may
+ occasionally cause a few glitches over long lasting connection, so any non-
+ zero value here should probably be in the hundreds or thousands to be
+ effective without affecting slightly bogus servers.
+
+ See also: tune.h2.fe.glitches-threshold, bc_glitches
+
tune.h2.be.initial-window-size <number>
Sets the HTTP/2 initial window size for outgoing connections, which is the
number of bytes the server can respond before waiting for an acknowledgment
@@ -2975,6 +2989,18 @@
case). It is highly recommended not to increase this value; some might find
it optimal to run at low values (1..5 typically).
+tune.h2.fe.glitches-threshold <number>
+ Sets the threshold for the number of glitches on a frontend connection, where
+ that connection will automatically be killed. This allows to automatically
+ kill misbehaving connections without having to write explicit rules for them.
+ The default value is zero, indicating that no threshold is set so that no
+ event will cause a connection to be closed. Beware that some H2 clientss may
+ occasionally cause a few glitches over long lasting connection, so any non-
+ zero value here should probably be in the hundreds or thousands to be
+ effective without affecting slightly bogus clients.
+
+ See also: tune.h2.be.glitches-threshold, fc_glitches
+
tune.h2.fe.initial-window-size <number>
Sets the HTTP/2 initial window size for incoming connections, which is the
number of bytes the client can upload before waiting for an acknowledgment
diff --git a/src/mux_h2.c b/src/mux_h2.c
index b66f77d..6192c06 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -408,6 +408,8 @@
static int h2_settings_initial_window_size = 65536; /* default initial value */
static int h2_be_settings_initial_window_size = 0; /* backend's default initial value */
static int h2_fe_settings_initial_window_size = 0; /* frontend's default initial value */
+static int h2_be_glitches_threshold = 0; /* backend's max glitches: unlimited */
+static int h2_fe_glitches_threshold = 0; /* frontend's max glitches: unlimited */
static unsigned int h2_settings_max_concurrent_streams = 100; /* default value */
static unsigned int h2_be_settings_max_concurrent_streams = 0; /* backend value */
static unsigned int h2_fe_settings_max_concurrent_streams = 0; /* frontend value */
@@ -602,15 +604,6 @@
return ret;
}
-/* report one or more glitches on the connection. That is any unexpected event
- * that may occasionally happen but if repeated a bit too much, might indicate
- * a misbehaving or completely bogus peer.
- */
-static inline void h2c_report_glitch(struct h2c *h2c, int increment)
-{
- h2c->glitches += increment;
-}
-
/* update h2c timeout if needed */
static void h2c_update_timeout(struct h2c *h2c)
{
@@ -1307,6 +1300,25 @@
TRACE_LEAVE(H2_EV_H2S_WAKE, h2s->h2c->conn, h2s);
}
+/* report one or more glitches on the connection. That is any unexpected event
+ * that may occasionally happen but if repeated a bit too much, might indicate
+ * a misbehaving or completely bogus peer. It normally returns zero, unless the
+ * glitch limit was reached, in which case an error is also reported on the
+ * connection.
+ */
+static inline int h2c_report_glitch(struct h2c *h2c, int increment)
+{
+ int thres = (h2c->flags & H2_CF_IS_BACK) ?
+ h2_be_glitches_threshold : h2_fe_glitches_threshold;
+
+ h2c->glitches += increment;
+ if (thres && h2c->glitches >= thres) {
+ h2c_error(h2c, H2_ERR_ENHANCE_YOUR_CALM);
+ return 1;
+ }
+ return 0;
+}
+
/* writes the 24-bit frame size <len> at address <frame> */
static inline __maybe_unused void h2_set_frame_size(void *frame, uint32_t len)
{
@@ -2306,7 +2318,11 @@
* it's often suspicious.
*/
if (h2c->st0 != H2_CS_SETTINGS1 && arg < h2c->miw)
- h2c_report_glitch(h2c, 1);
+ if (h2c_report_glitch(h2c, 1)) {
+ error = H2_ERR_ENHANCE_YOUR_CALM;
+ TRACE_STATE("glitch limit reached on SETTINGS frame", H2_EV_RX_FRAME|H2_EV_RX_SETTINGS|H2_EV_H2C_ERR|H2_EV_PROTO_ERR, h2c->conn);
+ goto fail;
+ }
h2c->miw = arg;
break;
@@ -5206,8 +5222,12 @@
* abuser sending 1600 1-byte frames in a 16kB buffer would increment
* its counter by 100.
*/
- if (unlikely(fragments > 4) && fragments > flen / 1024 && ret != 0)
- h2c_report_glitch(h2c, (fragments + 15) / 16);
+ if (unlikely(fragments > 4) && fragments > flen / 1024 && ret != 0) {
+ if (h2c_report_glitch(h2c, (fragments + 15) / 16)) {
+ TRACE_STATE("glitch limit reached on CONTINUATION frame", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_H2C_ERR|H2_EV_PROTO_ERR, h2c->conn);
+ ret = -1;
+ }
+ }
return ret;
@@ -7144,6 +7164,27 @@
/* functions below are dedicated to the config parsers */
/*******************************************************/
+/* config parser for global "tune.h2.{fe,be}.glitches-threshold" */
+static int h2_parse_glitches_threshold(char **args, int section_type, struct proxy *curpx,
+ const struct proxy *defpx, const char *file, int line,
+ char **err)
+{
+ int *vptr;
+
+ if (too_many_args(1, args, err, NULL))
+ return -1;
+
+ /* backend/frontend */
+ vptr = (args[0][8] == 'b') ? &h2_be_glitches_threshold : &h2_fe_glitches_threshold;
+
+ *vptr = atoi(args[1]);
+ if (*vptr < 0) {
+ memprintf(err, "'%s' expects a positive numeric value.", args[0]);
+ return -1;
+ }
+ return 0;
+}
+
/* config parser for global "tune.h2.header-table-size" */
static int h2_parse_header_table_size(char **args, int section_type, struct proxy *curpx,
const struct proxy *defpx, const char *file, int line,
@@ -7279,8 +7320,10 @@
/* config keyword parsers */
static struct cfg_kw_list cfg_kws = {ILH, {
+ { CFG_GLOBAL, "tune.h2.be.glitches-threshold", h2_parse_glitches_threshold },
{ CFG_GLOBAL, "tune.h2.be.initial-window-size", h2_parse_initial_window_size },
{ CFG_GLOBAL, "tune.h2.be.max-concurrent-streams", h2_parse_max_concurrent_streams },
+ { CFG_GLOBAL, "tune.h2.fe.glitches-threshold", h2_parse_glitches_threshold },
{ CFG_GLOBAL, "tune.h2.fe.initial-window-size", h2_parse_initial_window_size },
{ CFG_GLOBAL, "tune.h2.fe.max-concurrent-streams", h2_parse_max_concurrent_streams },
{ CFG_GLOBAL, "tune.h2.fe.max-total-streams", h2_parse_max_total_streams },