mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-01-19 12:10:46 +00:00
BUG/MINOR: mux-h2: also count streams for refused ones
There are a few places where we can reject an incoming stream based on
technical errors such as decoded headers that are too large for the
internal buffers, or memory allocation errors. In this case we send
an RST_STREAM to abort the request, but the total stream counter was
not incremented. That's not really a problem, until one starts to try
to enforce a total stream limit using tune.h2.fe.max-total-streams,
and which will not count such faulty streams. Typically a client that
learns too large cookies and tries to replay them in a way that
overflows the maximum buffer size would be rejected and depending on
how they're implemented, they might retry forever.
This patch removes the stream count increment from h2s_new() and moves
it instead to the calling functions, so that it translates the decision
to process a new stream instead of a successfully decoded stream. The
result is that such a bogus client will now be blocked after reaching
the total stream limit.
This can be validated this way:
global
tune.h2.fe.max-total-streams 128
expose-experimental-directives
trace h2 sink stdout
trace h2 level developer
trace h2 verbosity complete
trace h2 start now
frontend h
bind :8080
mode http
redirect location /
Sending this will fill frames with 15972 bytes of cookie headers that
expand to 16500 for storage+index once decoded, causing "message too large"
events:
(dev/h2/mkhdr.sh -t p;dev/h2/mkhdr.sh -t s;
for sid in {0..1000}; do
dev/h2/mkhdr.sh -t h -i $((sid*2+1)) -f es,eh \
-R "828684410f7777772e6578616d706c652e636f6d \
$(for i in {1..66}; do
echo -n 60 7F 73 433d $(for j in {1..24}; do
echo -n 2e313233343536373839; done);
done) ";
done) | nc 0 8080
Now it properly stops after sending 128 streams.
This may be backported wherever commit 983ac4397
("MINOR: mux-h2:
support limiting the total number of H2 streams per connection") is
present, since without it, that commit is less effective.
This commit is contained in:
parent
f3a19e70e8
commit
7021a8c4d8
41
src/mux_h2.c
41
src/mux_h2.c
@ -1558,7 +1558,6 @@ static struct h2s *h2s_new(struct h2c *h2c, int id)
|
|||||||
|
|
||||||
eb32_insert(&h2c->streams_by_id, &h2s->by_id);
|
eb32_insert(&h2c->streams_by_id, &h2s->by_id);
|
||||||
h2c->nb_streams++;
|
h2c->nb_streams++;
|
||||||
h2c->stream_cnt++;
|
|
||||||
|
|
||||||
HA_ATOMIC_INC(&h2c->px_counters->open_streams);
|
HA_ATOMIC_INC(&h2c->px_counters->open_streams);
|
||||||
HA_ATOMIC_INC(&h2c->px_counters->total_streams);
|
HA_ATOMIC_INC(&h2c->px_counters->total_streams);
|
||||||
@ -1693,6 +1692,9 @@ static struct h2s *h2c_bck_stream_new(struct h2c *h2c, struct stconn *sc, struct
|
|||||||
h2s->sess = sess;
|
h2s->sess = sess;
|
||||||
h2c->nb_sc++;
|
h2c->nb_sc++;
|
||||||
|
|
||||||
|
/* on the backend we can afford to only count total streams upon success */
|
||||||
|
h2c->stream_cnt++;
|
||||||
|
|
||||||
out:
|
out:
|
||||||
if (likely(h2s))
|
if (likely(h2s))
|
||||||
TRACE_LEAVE(H2_EV_H2S_NEW, h2c->conn, h2s);
|
TRACE_LEAVE(H2_EV_H2S_NEW, h2c->conn, h2s);
|
||||||
@ -2812,11 +2814,11 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
|
|||||||
|
|
||||||
/* unrecoverable error ? */
|
/* unrecoverable error ? */
|
||||||
if (h2c->st0 >= H2_CS_ERROR) {
|
if (h2c->st0 >= H2_CS_ERROR) {
|
||||||
|
/* This is mainly used for completeness, but a stream error is
|
||||||
|
* currently not set by h2c_dec_hdrs().
|
||||||
|
*/
|
||||||
TRACE_USER("Unrecoverable error decoding H2 request", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, 0, &rxbuf);
|
TRACE_USER("Unrecoverable error decoding H2 request", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, 0, &rxbuf);
|
||||||
sess_log(h2c->conn->owner);
|
goto strm_err;
|
||||||
session_inc_http_req_ctr(h2c->conn->owner);
|
|
||||||
session_inc_http_err_ctr(h2c->conn->owner);
|
|
||||||
goto out;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (error <= 0) {
|
if (error <= 0) {
|
||||||
@ -2830,20 +2832,8 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
|
|||||||
/* Failed to decode this stream (e.g. too large request)
|
/* Failed to decode this stream (e.g. too large request)
|
||||||
* but the HPACK decompressor is still synchronized.
|
* but the HPACK decompressor is still synchronized.
|
||||||
*/
|
*/
|
||||||
sess_log(h2c->conn->owner);
|
|
||||||
session_inc_http_req_ctr(h2c->conn->owner);
|
|
||||||
session_inc_http_err_ctr(h2c->conn->owner);
|
|
||||||
|
|
||||||
h2s = (struct h2s*)h2_error_stream;
|
|
||||||
|
|
||||||
/* This stream ID is now opened anyway until we send the RST on
|
|
||||||
* it, it must not be reused.
|
|
||||||
*/
|
|
||||||
if (h2c->dsi > h2c->max_id)
|
|
||||||
h2c->max_id = h2c->dsi;
|
|
||||||
|
|
||||||
TRACE_USER("rcvd unparsable H2 request", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, h2s, &rxbuf);
|
TRACE_USER("rcvd unparsable H2 request", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, h2s, &rxbuf);
|
||||||
goto send_rst;
|
goto strm_err;
|
||||||
}
|
}
|
||||||
|
|
||||||
TRACE_USER("rcvd H2 request ", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW, h2c->conn, 0, &rxbuf);
|
TRACE_USER("rcvd H2 request ", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW, h2c->conn, 0, &rxbuf);
|
||||||
@ -2853,6 +2843,7 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
|
|||||||
*/
|
*/
|
||||||
if (h2c->dsi > h2c->max_id)
|
if (h2c->dsi > h2c->max_id)
|
||||||
h2c->max_id = h2c->dsi;
|
h2c->max_id = h2c->dsi;
|
||||||
|
h2c->stream_cnt++;
|
||||||
|
|
||||||
/* Note: we don't emit any other logs below because if we return
|
/* Note: we don't emit any other logs below because if we return
|
||||||
* positively from h2c_frt_stream_new(), the stream will report the error,
|
* positively from h2c_frt_stream_new(), the stream will report the error,
|
||||||
@ -2891,6 +2882,20 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
|
|||||||
h2s = NULL;
|
h2s = NULL;
|
||||||
goto leave;
|
goto leave;
|
||||||
|
|
||||||
|
strm_err:
|
||||||
|
sess_log(h2c->conn->owner);
|
||||||
|
session_inc_http_req_ctr(h2c->conn->owner);
|
||||||
|
session_inc_http_err_ctr(h2c->conn->owner);
|
||||||
|
|
||||||
|
h2s = (struct h2s*)h2_error_stream;
|
||||||
|
|
||||||
|
/* This stream ID is now opened anyway until we send the RST on
|
||||||
|
* it, it must not be reused.
|
||||||
|
*/
|
||||||
|
if (h2c->dsi > h2c->max_id)
|
||||||
|
h2c->max_id = h2c->dsi;
|
||||||
|
h2c->stream_cnt++;
|
||||||
|
|
||||||
send_rst:
|
send_rst:
|
||||||
/* make the demux send an RST for the current stream. We may only
|
/* make the demux send an RST for the current stream. We may only
|
||||||
* do this if we're certain that the HEADERS frame was properly
|
* do this if we're certain that the HEADERS frame was properly
|
||||||
|
Loading…
Reference in New Issue
Block a user