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.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index defb3c71a674abc6ec65e20f2ac451aac7609ff7..9490c5514efff323b1dd9b04b7de85b81eda96c6 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -1363,8 +1363,10 @@ The following keywords are supported in the "global" section :
    - tune.fd.edge-triggered
    - tune.h1.zero-copy-fwd-recv
    - tune.h1.zero-copy-fwd-send
+   - 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
@@ -3135,6 +3137,18 @@ tune.h1.zero-copy-fwd-send { on | off }
 
   See also: tune.disable-zero-copy-forwarding, tune.h1.zero-copy-fwd-recv
 
+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
@@ -3160,6 +3174,18 @@ tune.h2.be.max-concurrent-streams <number>
   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 6f5a6a112f7098d19e3033976cc01534dd7cdaa5..2a2a15d4ffdad34702134780bc0f65b4755558ce 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -452,6 +452,8 @@ static int h2_settings_header_table_size      =  4096; /* initial value */
 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 */
@@ -646,15 +648,6 @@ static inline int h2c_max_concurrent_streams(const struct h2c *h2c)
        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)
 {
@@ -1353,6 +1346,25 @@ static void __maybe_unused h2s_alert(struct h2s *h2s)
        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)
 {
@@ -2365,7 +2377,11 @@ static int h2c_handle_settings(struct h2c *h2c)
                         * 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;
@@ -5414,8 +5430,12 @@ next_frame:
         * 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;
 
@@ -7555,6 +7575,27 @@ static int h2_takeover(struct connection *conn, int orig_tid)
 /* 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,
@@ -7713,8 +7754,10 @@ INITCALL1(STG_REGISTER, register_mux_proto, &mux_proto_h2);
 
 /* 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      },