From a627d15da26d5493114955b55f831a1033685ead Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 19 Jan 2023 23:22:03 +0100 Subject: [PATCH] BUG/MINOR: mux-h2: make sure to produce a log on invalid requests As reported by Dominik Froehlich in github issue #1968, some H2 request parsing errors do not result in a log being emitted. This is annoying for debugging because while an RST_STREAM is correctly emitted to the client, there's no way without enabling traces to find it on the haproxy side. After some testing with various abnormal requests, a few places were found where logs were missing and could be added. In this case, we simply use sess_log() so some sample fetch functions might not be available since the stream is not created. But at least there will be a BADREQ in the logs. A good eaxmple of this consists in sending forbidden headers or header syntax (e.g. presence of LF in value). Some quick tests can be done this way: - protocol error (LF in value): curl -iv --http2-prior-knowledge -H "$(printf 'a:b\na')" http://0:8001/ - too large header block after decoding: curl -v --http2-prior-knowledge -H "a:$(perl -e "print('a'x10000)")" -H "a:$(perl -e "print('a'x10000)")" http://localhost:8001/ This should be backported where needed, most likely 2.7 and 2.6 at least for a start, and progressively to other versions. (cherry picked from commit f43f36da5be06da0f08efdb496e36a0edb35206a) Signed-off-by: Willy Tarreau (cherry picked from commit 5341eee2ffcbc24ea6b903f73e9781d1b074e40e) Signed-off-by: Christopher Faulet --- src/mux_h2.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mux_h2.c b/src/mux_h2.c index e4a64b21a..436440a4a 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -2798,8 +2798,10 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s) if (h2s->st != H2_SS_CLOSED) { error = h2c_decode_headers(h2c, &h2s->rxbuf, &h2s->flags, &body_len, NULL); /* unrecoverable error ? */ - if (h2c->st0 >= H2_CS_ERROR) + if (h2c->st0 >= H2_CS_ERROR) { + sess_log(h2c->conn->owner); goto out; + } if (error == 0) { /* Demux not blocked because of the stream, it is an incomplete frame */ @@ -2812,6 +2814,7 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s) /* Failed to decode this frame (e.g. too large request) * but the HPACK decompressor is still synchronized. */ + sess_log(h2c->conn->owner); h2s_error(h2s, H2_ERR_INTERNAL_ERROR); h2c->st0 = H2_CS_FRAME_E; goto out; @@ -2822,6 +2825,7 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s) * the data and send another RST. */ error = h2c_decode_headers(h2c, &rxbuf, &flags, &body_len, NULL); + sess_log(h2c->conn->owner); h2s = (struct h2s*)h2_error_stream; goto send_rst; } @@ -2839,8 +2843,10 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s) error = h2c_decode_headers(h2c, &rxbuf, &flags, &body_len, NULL); /* unrecoverable error ? */ - if (h2c->st0 >= H2_CS_ERROR) + if (h2c->st0 >= H2_CS_ERROR) { + sess_log(h2c->conn->owner); goto out; + } if (error <= 0) { if (error == 0) { @@ -2853,6 +2859,7 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s) /* Failed to decode this stream (e.g. too large request) * but the HPACK decompressor is still synchronized. */ + sess_log(h2c->conn->owner); h2s = (struct h2s*)h2_error_stream; goto send_rst; }