From: Willy Tarreau <w@1wt.eu>
Date: Fri, 20 Oct 2023 15:51:12 +0000 (+0200)
Subject: BUG/MINOR: mux-h2: commit the current stream ID even on reject
X-Git-Tag: v2.9-dev8~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=250b630fb933d652533487cba9e2ae448582b550;p=thirdparty%2Fhaproxy.git

BUG/MINOR: mux-h2: commit the current stream ID even on reject

The H2 spec says that a HEADERS frame turns an idle stream to the open
state, and it may then turn to half-closed(remote) on ES, then to close,
all at once, if we respond with RST (e.g. on error). Due to the fact that
we process a complete frame at once since h2_dec_hdrs() may reassemble
CONTINUATION frames until everything is complete, the state was only
committed after the frame was completley valid (otherwise multiple passes
could result in subsequent frames being rejected as the stream ID would
be equal to the highest one).

However this is not correct because it means that a client may retry on
the same ID as a previously failed one, which technically is forbidden
(for example the client couldn't know which of them a WINDOW_UPDATE or
RST_STREAM frame is for).

In practice, due to the error paths, this would only be possible when
failing to decode HPACK while leaving the HPACK stream intact, thus
when the valid decoded HPACK stream cannot be turned into a valid HTTP
representation, e.g. when the resulting headers are too large for example.
The solution to avoid this consists in committing the stream ID on this
error path as well. h2spec continues to be happy.

Thanks to Annika Wickert and Tim Windelschmidt for reporting this issue.

This fix must be backported to all stable versions.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index ee5d0deda9..e0105e29cf 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2806,6 +2806,13 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
 		 */
 		sess_log(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);
 		goto send_rst;
 	}