Skip to content

Commit 05b4570

Browse files
committed
MEDIUM: mux-h1: implement basic glitches support
We now count glitches for each parsing error, including those that have been accepted via accept-unsafe-violations-*. Front and back are considered and the connection gets killed on error once if the threshold is reached or passed and the CPU usage is beyond the configured limit (0 by default). This was tested with: curl -ivH "host : blah" 0:4445{,,,,,,,,,} which sends 10 requests to a configuration having a threshold of 5. The global keywords are named similarly to H2 and quic: tune.h1.be.glitches-threshold xxxx tune.h1.fe.glitches-threshold xxxx The glitches count of each connection is also reported when non-null in the connection dumps (e.g. "show fd").
1 parent 0901f60 commit 05b4570

2 files changed

Lines changed: 103 additions & 3 deletions

File tree

doc/configuration.txt

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,6 +1877,8 @@ The following keywords are supported in the "global" section :
18771877
- tune.events.max-events-at-once
18781878
- tune.fail-alloc
18791879
- tune.fd.edge-triggered
1880+
- tune.h1.be.glitches-threshold
1881+
- tune.h1.fe.glitches-threshold
18801882
- tune.h1.zero-copy-fwd-recv
18811883
- tune.h1.zero-copy-fwd-send
18821884
- tune.h2.be.glitches-threshold
@@ -4193,9 +4195,40 @@ tune.glitches.kill.cpu-usage <number>
41934195
will automatically get killed. A rule of thumb would be to set this value to
41944196
twice the usually observed CPU usage, or the commonly observed CPU usage plus
41954197
half the idle one (i.e. if CPU commonly reaches 60%, setting 80 here can make
4196-
sense). This parameter has no effect without tune.h2.fe.glitches-threshold or
4197-
tune.quic.fe.sec.glitches-threshold. See also the global parameters
4198-
"tune.h2.fe.glitches-threshold" and "tune.quic.fe.sec.glitches-threshold".
4198+
sense). This parameter has no effect without tune.h2.fe.glitches-threshold,
4199+
tune.quic.fe.sec.glitches-threshold or tune.h1.fe.glitches-threshold. See
4200+
also the global parameters "tune.h2.fe.glitches-threshold",
4201+
"tune.h1.fe.glitches-threshold" and "tune.quic.fe.sec.glitches-threshold".
4202+
4203+
tune.h1.be.glitches-threshold <number>
4204+
Sets the threshold for the number of glitches on a HTTP/1 backend connection,
4205+
after which that connection will automatically be killed. This allows to
4206+
automatically kill misbehaving connections without having to write explicit
4207+
rules for them. The default value is zero, indicating that no threshold is
4208+
set so that no event will cause a connection to be closed. Typical events
4209+
include improperly formatted headers that had been nevertheless accepted by
4210+
"accept-unsafe-violations-in-http-response". Any non-zero value here should
4211+
probably be in the hundreds or thousands to be effective without affecting
4212+
slightly bogus servers. It is also possible to only kill connections when the
4213+
CPU usage crosses a certain level, by using "tune.glitches.kill.cpu-usage".
4214+
4215+
See also: tune.h1.fe.glitches-threshold, bc_glitches, and
4216+
tune.glitches.kill.cpu-usage
4217+
4218+
tune.h1.fe.glitches-threshold <number>
4219+
Sets the threshold for the number of glitches on a HTTP/1 frontend connection
4220+
after which that connection will automatically be killed. This allows to
4221+
automatically kill misbehaving connections without having to write explicit
4222+
rules for them. The default value is zero, indicating that no threshold is
4223+
set so that no event will cause a connection to be closed. Typical events
4224+
include improperly formatted headers that had been nevertheless accepted by
4225+
"accept-unsafe-violations-in-http-request". Any non-zero value here should
4226+
probably be in the hundreds or thousands to be effective without affecting
4227+
slightly bogus clients. It is also possible to only kill connections when the
4228+
CPU usage crosses a certain level, by using "tune.glitches.kill.cpu-usage".
4229+
4230+
See also: tune.h1.be.glitches-threshold, fc_glitches, and
4231+
tune.glitches.kill.cpu-usage
41994232

42004233
tune.h1.zero-copy-fwd-recv { on | off }
42014234
Enables ('on') of disabled ('off') the zero-copy receives of data for the H1

src/mux_h1.c

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ struct h1c {
5858
struct h1_counters *px_counters; /* h1 counters attached to proxy */
5959
struct buffer_wait buf_wait; /* Wait list for buffer allocation */
6060
struct wait_event wait_event; /* To be used if we're waiting for I/Os */
61+
int glitches; /* Number of glitches on this connection */
6162
};
6263

6364
/* H1 stream descriptor */
@@ -95,6 +96,9 @@ struct h1_hdr_entry {
9596
static struct h1_hdrs_map hdrs_map = { .name = NULL, .map = EB_ROOT };
9697
static int accept_payload_with_any_method = 0;
9798

99+
static int h1_be_glitches_threshold = 0; /* backend's max glitches: unlimited */
100+
static int h1_fe_glitches_threshold = 0; /* frontend's max glitches: unlimited */
101+
98102
/* trace source and events */
99103
static void h1_trace(enum trace_level level, uint64_t mask,
100104
const struct trace_source *src,
@@ -504,6 +508,31 @@ static void h1_trace_fill_ctx(struct trace_ctx *ctx, const struct trace_source *
504508
}
505509
}
506510

511+
/* report one or more glitches on the connection. That is any unexpected event
512+
* that may occasionally happen but if repeated a bit too much, might indicate
513+
* a misbehaving or completely bogus peer. It normally returns zero, unless the
514+
* glitch limit was reached, in which case an error is also reported on the
515+
* connection.
516+
*/
517+
#define h1_report_glitch(h1c, inc, ...) ({ \
518+
COUNT_GLITCH(__VA_ARGS__); \
519+
_h1_report_glitch(h1c, inc); \
520+
})
521+
522+
static inline int _h1_report_glitch(struct h1c *h1c, int increment)
523+
{
524+
int thres = (h1c->flags & H1C_F_IS_BACK) ?
525+
h1_be_glitches_threshold : h1_fe_glitches_threshold;
526+
527+
h1c->glitches += increment;
528+
if (thres && h1c->glitches >= thres &&
529+
(th_ctx->idle_pct <= global.tune.glitch_kill_maxidle)) {
530+
h1c->flags |= H1C_F_ERROR;
531+
return 1;
532+
}
533+
return 0;
534+
}
535+
507536

508537
/*****************************************************/
509538
/* functions below are for dynamic buffer management */
@@ -1265,6 +1294,7 @@ static int h1_init(struct connection *conn, struct proxy *proxy, struct session
12651294
h1c->task = NULL;
12661295
h1c->req_count = 0;
12671296
h1c->term_evts_log = 0;
1297+
h1c->glitches = 0;
12681298

12691299
LIST_INIT(&h1c->buf_wait.list);
12701300
h1c->wait_event.tasklet = tasklet_new();
@@ -1962,6 +1992,7 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
19621992
h1s->h1c->errcode = h1m->err_code;
19631993
TRACE_ERROR("parsing error, reject H1 message", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
19641994
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
1995+
h1_report_glitch(h1s->h1c, 1, "parsing error");
19651996
}
19661997
else if (ret == -2) {
19671998
TRACE_STATE("RX path congested, waiting for more space", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_BLK, h1s->h1c->conn, h1s);
@@ -1987,6 +2018,7 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
19872018
h1s->h1c->errcode = 413;
19882019
TRACE_ERROR("HTTP/1.0 GET/HEAD/DELETE request with a payload forbidden", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
19892020
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
2021+
h1_report_glitch(h1s->h1c, 1, "HTTP/1.0 GET/HEAD/DELETE with payload");
19902022
ret = 0;
19912023
goto end;
19922024
}
@@ -2003,6 +2035,7 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
20032035
h1s->h1c->errcode = 422;
20042036
TRACE_ERROR("Unknown transfer-encoding", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
20052037
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
2038+
h1_report_glitch(h1s->h1c, 1, "unknown transfer-encoding");
20062039
ret = 0;
20072040
goto end;
20082041
}
@@ -2022,12 +2055,14 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
20222055
TRACE_ERROR("missing/invalid websocket key, reject H1 message",
20232056
H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
20242057

2058+
h1_report_glitch(h1s->h1c, 1, "rejecting missing/invalid websocket key");
20252059
ret = 0;
20262060
goto end;
20272061
} else {
20282062
TRACE_ERROR("missing/invalid websocket key, but accepting this "
20292063
"violation according to configuration",
20302064
H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
2065+
h1_report_glitch(h1s->h1c, 1, "accepting missing/invalid websocket key");
20312066
}
20322067
}
20332068
}
@@ -2039,6 +2074,7 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
20392074
*/
20402075
TRACE_STATE("Ignored parsing error", H1_EV_RX_DATA|H1_EV_RX_HDRS, h1s->h1c->conn, h1s);
20412076
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
2077+
h1_report_glitch(h1s->h1c, 1, "ignored parsing error");
20422078
}
20432079

20442080
if (!(h1m->flags & H1_MF_RESP)) {
@@ -2078,6 +2114,7 @@ static size_t h1_handle_data(struct h1s *h1s, struct h1m *h1m, struct htx **htx,
20782114
h1s->flags |= H1S_F_PARSING_ERROR;
20792115
TRACE_ERROR("parsing error, reject H1 message", H1_EV_RX_DATA|H1_EV_RX_BODY|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
20802116
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
2117+
h1_report_glitch(h1s->h1c, 1, "parsing error");
20812118
}
20822119
goto end;
20832120
}
@@ -2114,6 +2151,7 @@ static size_t h1_handle_trailers(struct h1s *h1s, struct h1m *h1m, struct htx *h
21142151
h1s->flags |= H1S_F_PARSING_ERROR;
21152152
TRACE_ERROR("parsing error, reject H1 message", H1_EV_RX_DATA|H1_EV_RX_TLRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
21162153
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
2154+
h1_report_glitch(h1s->h1c, 1, "parsing error");
21172155
}
21182156
else if (ret == -2) {
21192157
TRACE_STATE("RX path congested, waiting for more space", H1_EV_RX_DATA|H1_EV_RX_TLRS|H1_EV_H1S_BLK, h1s->h1c->conn, h1s);
@@ -4093,6 +4131,7 @@ static int h1_process(struct h1c * h1c)
40934131
if (b_data(&h1c->ibuf) && /* Input data to be processed */
40944132
((h1c->state < H1_CS_RUNNING) || (h1c->state == H1_CS_DRAINING)) && /* IDLE, EMBRYONIC, UPGRADING or DRAINING */
40954133
!(h1c->flags & (H1C_F_IN_SALLOC|H1C_F_ABRT_PENDING))) { /* No allocation failure on the stream rxbuf and no ERROR on the H1C */
4134+
int prev_glitches = h1c->glitches;
40964135
struct h1s *h1s = h1c->h1s;
40974136
struct buffer *buf;
40984137
size_t count;
@@ -4161,6 +4200,8 @@ static int h1_process(struct h1c * h1c)
41614200
h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
41624201
}
41634202
}
4203+
if (h1c->glitches != prev_glitches && !(h1c->flags & H1C_F_IS_BACK))
4204+
session_add_glitch_ctr(h1c->conn->owner, h1c->glitches - prev_glitches);
41644205
}
41654206

41664207
no_parsing:
@@ -5416,6 +5457,8 @@ static int h1_ctl(struct connection *conn, enum mux_ctl_type mux_ctl, void *outp
54165457
if (!(h1c->wait_event.events & SUB_RETRY_RECV))
54175458
h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
54185459
return 0;
5460+
case MUX_CTL_GET_GLITCHES:
5461+
return h1c->glitches;
54195462
case MUX_CTL_GET_NBSTRM:
54205463
return h1_used_streams(conn);
54215464
case MUX_CTL_GET_MAXSTRM:
@@ -5484,6 +5527,7 @@ static int h1_dump_h1c_info(struct buffer *msg, struct h1c *h1c, const char *pfx
54845527
(unsigned int)b_head_ofs(&h1c->obuf), (unsigned int)b_size(&h1c->obuf),
54855528
tevt_evts2str(h1c->term_evts_log));
54865529

5530+
chunk_appendf(msg, " .glitches=%d", h1c->glitches);
54875531
chunk_appendf(msg, " .task=%p", h1c->task);
54885532
if (h1c->task) {
54895533
chunk_appendf(msg, " .exp=%s",
@@ -5871,6 +5915,27 @@ static int cfg_parse_h1_headers_case_adjust_file(char **args, int section_type,
58715915
return 0;
58725916
}
58735917

5918+
/* config parser for global "tune.h1.{fe,be}.glitches-threshold" */
5919+
static int cfg_parse_h1_glitches_threshold(char **args, int section_type, struct proxy *curpx,
5920+
const struct proxy *defpx, const char *file, int line,
5921+
char **err)
5922+
{
5923+
int *vptr;
5924+
5925+
if (too_many_args(1, args, err, NULL))
5926+
return -1;
5927+
5928+
/* backend/frontend */
5929+
vptr = (args[0][8] == 'b') ? &h1_be_glitches_threshold : &h1_fe_glitches_threshold;
5930+
5931+
*vptr = atoi(args[1]);
5932+
if (*vptr < 0) {
5933+
memprintf(err, "'%s' expects a positive numeric value.", args[0]);
5934+
return -1;
5935+
}
5936+
return 0;
5937+
}
5938+
58745939
/* config parser for global "tune.h1.zero-copy-fwd-recv" */
58755940
static int cfg_parse_h1_zero_copy_fwd_rcv(char **args, int section_type, struct proxy *curpx,
58765941
const struct proxy *defpx, const char *file, int line,
@@ -5914,6 +5979,8 @@ static struct cfg_kw_list cfg_kws = {{ }, {
59145979
{ CFG_GLOBAL, "h1-accept-payload-with-any-method", cfg_parse_h1_accept_payload_with_any_method },
59155980
{ CFG_GLOBAL, "h1-case-adjust", cfg_parse_h1_header_case_adjust },
59165981
{ CFG_GLOBAL, "h1-case-adjust-file", cfg_parse_h1_headers_case_adjust_file },
5982+
{ CFG_GLOBAL, "tune.h1.be.glitches-threshold", cfg_parse_h1_glitches_threshold },
5983+
{ CFG_GLOBAL, "tune.h1.fe.glitches-threshold", cfg_parse_h1_glitches_threshold },
59175984
{ CFG_GLOBAL, "tune.h1.zero-copy-fwd-recv", cfg_parse_h1_zero_copy_fwd_rcv },
59185985
{ CFG_GLOBAL, "tune.h1.zero-copy-fwd-send", cfg_parse_h1_zero_copy_fwd_snd },
59195986
{ 0, NULL, NULL },

0 commit comments

Comments
 (0)