From: Willy Tarreau <w@1wt.eu>
Date: Thu, 19 Jan 2023 22:22:03 +0000 (+0100)
Subject: BUG/MINOR: mux-h2: make sure to produce a log on invalid requests
X-Git-Tag: v2.8-dev2~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f43f36da5be06da0f08efdb496e36a0edb35206a;p=thirdparty%2Fhaproxy.git

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.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index a781cb0167..2a5d9e41a8 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2584,8 +2584,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 */
@@ -2598,6 +2600,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;
@@ -2608,6 +2611,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;
 	}
@@ -2625,8 +2629,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) {
@@ -2639,6 +2645,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;
 	}