From: Willy Tarreau <w@1wt.eu>
Date: Thu, 26 Sep 2019 06:47:15 +0000 (+0200)
Subject: BUG/MEDIUM: mux-h2: don't reject valid frames on closed streams
X-Git-Tag: v2.1-dev2~25
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4c08f12dd86bda574c15261bcd69135dd662f990;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-h2: don't reject valid frames on closed streams

Consecutive to commit 6884aa3eb0 ("BUG/MAJOR: mux-h2: Handle HEADERS frames
received after a RST_STREAM frame") some valid frames on closed streams
(RST_STREAM, PRIORITY, WINDOW_UPDATE) were now rejected. It turns out that
the previous condition was in fact intentional to catch only sensitive
frames, which was indeed a mistake since these ones needed to be decoded
to keep HPACK synchronized. But we must absolutely accept WINDOW_UPDATES
or we risk to stall some transfers. And RST/PRIO definitely are valid.

Let's adjust the condition to reflect that and update the comment to
explain the reason for this unobvious condition.

This must be backported to 2.0 and 1.9 after the commit above is brought
there.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index c1172bb060..cfbc0ad6b2 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2731,7 +2731,8 @@ static int h2_frame_check_vs_state(struct h2c *h2c, struct h2s *h2s)
 			return 0;
 		}
 
-		if (h2s->flags & H2_SF_RST_RCVD && !(h2_ft_bit(h2c->dft) & H2_FT_HDR_MASK)) {
+		if (h2s->flags & H2_SF_RST_RCVD &&
+		    !(h2_ft_bit(h2c->dft) & (H2_FT_HDR_MASK | H2_FT_RST_STREAM_BIT | H2_FT_PRIORITY_BIT | H2_FT_WINDOW_UPDATE_BIT))) {
 			/* RFC7540#5.1:closed: an endpoint that
 			 * receives any frame other than PRIORITY after
 			 * receiving a RST_STREAM MUST treat that as a
@@ -2746,6 +2747,10 @@ static int h2_frame_check_vs_state(struct h2c *h2c, struct h2s *h2s)
 			 * happen with request trailers received after sending an
 			 * RST_STREAM, or with header/trailers responses received after
 			 * sending RST_STREAM (aborted stream).
+			 *
+			 * In addition, since our CLOSED streams always carry the
+			 * RST_RCVD bit, we don't want to accidently catch valid
+			 * frames for a closed stream, i.e. RST/PRIO/WU.
 			 */
 			h2s_error(h2s, H2_ERR_STREAM_CLOSED);
 			h2c->st0 = H2_CS_FRAME_E;