From: Willy Tarreau <w@1wt.eu>
Date: Sun, 23 Dec 2018 17:30:44 +0000 (+0100)
Subject: MINOR: mux-h2: fail stream creation more cleanly using RST_STREAM
X-Git-Tag: v2.0-dev1~323
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96a10c24cf57bf8514a9665d63b2133a20ec664d;p=thirdparty%2Fhaproxy.git

MINOR: mux-h2: fail stream creation more cleanly using RST_STREAM

The H2 demux only checks for too many streams in h2c_frt_stream_new(),
then refuses to create a new stream and causes the connection to be
aborted by sending a GOAWAY frame. This will also happen if any error
happens during the stream creation (e.g. memory allocation).

RFC7540#5.1.2 says that attempts to create streams in excess should
instead be dealt with using an RST_STREAM frame conveying either the
PROTOCOL_ERROR or REFUSED_STREAM reason (the latter being usable only
if it is guaranteed that the stream was not processed). In theory it
should not happen for well behaving clients, though it may if we
configure a low enough h2.max_concurrent_streams limit. This error
however may definitely happen on memory shortage.

Previously it was not possible to use RST_STREAM due to the fact that
the HPACK decompressor would be desynchronized. But now we first decode
and only then try to allocate the stream, so the decompressor remains
synchronized regardless of policy or resources issues.

With this patch we enforce stream termination with RST_STREAM and
REFUSED_STREAM if this protocol violation happens, as well as if there
is a temporary condition like a memory allocation issue. It will allow
a client to recover cleanly.

This could possibly be backported to 1.9. Note that this requires that
these five previous patches are merged as well :

    MINOR: h2: add a bit-based frame type representation
    MEDIUM: mux-h2: remove padlen during headers phase
    MEDIUM: mux-h2: decode HEADERS frames before allocating the stream
    MINOR: mux-h2: make h2c_send_rst_stream() use the dummy stream's error code
    MINOR: mux-h2: add a new dummy stream for the REFUSED_STREAM error code
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index b319d47b93..2fce076dca 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1879,8 +1879,8 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
 	 */
 	h2s = h2c_frt_stream_new(h2c, h2c->dsi);
 	if (!h2s) {
-		error = H2_ERR_INTERNAL_ERROR;
-		goto conn_err;
+		h2s = (struct h2s*)h2_refused_stream;
+		goto send_rst;
 	}
 
 	h2s->st = H2_SS_OPEN;
@@ -1921,6 +1921,15 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
  out:
 	h2_release_buf(h2c, &rxbuf);
 	return NULL;
+
+ send_rst:
+	/* 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
+	 * decompressed so that the HPACK decoder is still kept up to date.
+	 */
+	h2_release_buf(h2c, &rxbuf);
+	h2c->st0 = H2_CS_FRAME_E;
+	return h2s;
 }
 
 /* processes a HEADERS frame. Returns h2s on success or NULL on missing data.