MEDIUM: mux-h1: Return an error on h2 upgrade attempts if not allowed

If h1 to h2 upgrades are not allowed, a 405-method-not-allowed error is now
returned from the H1 multiplexer itself instead of dealing with "PRI *
HTTP/2.0\r\n\r\n" request as a normal request.

Before this kind of request was caught by the HTTP analyzers and a
400-bad-request was returned. This was added before the multiplexers era to
protect backend apps against unexpected H1 to H2 uprade on server side.

Now, it is possible to handle the error in the H1 multiplexer. One benefit
is to be able to increment the glichtes counters. However, the error is
still handled in HTTP analyzers to be sure to detect unwanted upgrades that
can be hidden in H2 or H3 requests.

There is a special case. TCP > H1 > H2 upgrades. In that case, a H1 stream
exist. So we must report an error to the upper layer too.

A reg-test script was added to validate the feature. In addition,
tcp_to_http_upgrade.vtc was updated accordingly.

reg-tests/connection/tcp_to_http_upgrade.vtc

@@ -145,7 +145,7 @@ client c3 -connect ${h1_li1h1_sock} {
 client c_err1 -connect ${h1_err1h1_sock} {
     send "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
     rxresp
-    expect resp.status == 400
+    expect resp.status == 405
 } -run
 
 
@@ -153,7 +153,7 @@ client c_err1 -connect ${h1_err1h1_sock} {
 client c_err2 -connect ${h1_err2h1_sock} {
     send "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
     rxresp
-    expect resp.status == 400
+    expect resp.status == 405
 } -run
 
 

reg-tests/http-messaging/h1_to_h2_upgrade.vtc

@@ -0,0 +1,73 @@
+varnishtest "Tests of H1->H2 upgrades"
+
+feature ignore_unknown_macro
+
+server s1 {
+	rxreq
+	txresp \
+	  -status 200 \
+} -start
+
+haproxy h1 -conf {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+        # WT: limit false-positives causing "HTTP header incomplete" due to
+        # idle server connections being randomly used and randomly expiring
+        # under us.
+        tune.idle-pool.shared off
+
+    defaults
+	#log stdout format raw daemon
+	mode http
+	option http-buffer-request
+	timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+	timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+	timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen feh1
+	bind "fd@${feh1}"
+	bind "fd@${feh2}" proto h1
+	server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_feh1_sock} {
+	txpri
+	stream 0 {
+		txsettings
+		rxsettings
+		txsettings -ack
+		rxsettings
+		expect settings.ack == true
+	} -run
+
+	# first request is valid
+	stream 1 {
+		txreq \
+		  -req "GET" \
+		  -scheme "https" \
+		  -url "/"
+		rxresp
+		expect resp.status == 200
+	} -run
+
+	# Second request. hide something similar to a H2 preface (PRI method). it is invalid
+	stream 3 {
+		txreq \
+		  -req "PRI" \
+		  -scheme "https" \
+		  -url "*" \
+		  -body "SM\r\n\r\n"
+		rxresp
+		expect resp.status == 400
+	} -run
+
+} -run
+
+client c2 -connect ${h1_feh2_sock} {
+        send "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
+        rxresp
+        expect resp.status == 405
+} -run

src/mux_h1.c

@@ -4179,13 +4179,15 @@ static int h1_process(struct h1c * h1c)
 		if (h1c->flags & H1C_F_IS_BACK)
 			goto release;
 
-		/* First of all handle H1 to H2 upgrade (no need to create the H1 stream) */
-		if (h1c->state != H1_CS_DRAINING &&                /* Not draining message */
-		    !(h1c->flags & H1C_F_WAIT_NEXT_REQ) &&         /* First request */
-		    !(h1c->px->options2 & PR_O2_NO_H2_UPGRADE) &&  /* H2 upgrade supported by the proxy */
-		    !(conn->mux->flags & MX_FL_NO_UPG)) {          /* the current mux supports upgrades */
-			/* Try to match H2 preface before parsing the request headers. */
-			if (b_isteq(&h1c->ibuf, 0, b_data(&h1c->ibuf), ist(H2_CONN_PREFACE)) > 0) {
+		/* First of all handle H2 preface (except for DRAINING mode).
+		 * If H1 to H2 upgrade is allowed, no need to create the H1
+		 * stream. If not allowed, a 405 must be returned.
+		 */
+		if (h1c->state != H1_CS_DRAINING &&
+		    b_isteq(&h1c->ibuf, 0, b_data(&h1c->ibuf), ist(H2_CONN_PREFACE)) > 0) {
+			if (!(h1c->flags & H1C_F_WAIT_NEXT_REQ) &&         /* First request */
+			    !(h1c->px->options2 & PR_O2_NO_H2_UPGRADE) &&  /* H2 upgrade supported by the proxy */
+			    !(conn->mux->flags & MX_FL_NO_UPG)) {          /* the current mux supports upgrades */
 				h1c->flags |= H1C_F_UPG_H2C;
 				if (h1c->state == H1_CS_UPGRADING) {
 					BUG_ON(!h1s);
@@ -4194,6 +4196,19 @@ static int h1_process(struct h1c * h1c)
 				TRACE_STATE("release h1c to perform H2 upgrade ", H1_EV_RX_DATA|H1_EV_H1C_WAKE);
 				goto release;
 			}
+			else {
+				if (h1c->state == H1_CS_UPGRADING) {
+					/* In case of TCP > H1 > H2 upgrade, h1s exist, so report an error to the SD */
+					BUG_ON(!h1s);
+					h1s->flags |= H1S_F_PARSING_ERROR;
+					se_fl_set(h1s->sd, SE_FL_ERROR); /* Set EOS here to release the SC */
+				}
+				h1c->errcode = 405;
+				TRACE_ERROR("H2 update not allowed", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
+				h1_report_glitch(h1c, 1, "H2 update not allowed");
+				h1_handle_parsing_error(h1c);
+				goto no_parsing;
+			}
 		}
 
 		/* Create the H1 stream if not already there */