BUG/MINOR: mux-h2: only convert GET upgrade requests to Extended CONNECT

An H1 request carrying "Connection: upgrade" sent to an H2 server is
converted to an Extended CONNECT (RFC8441) whatever its method, while
RFC8441 only defines GET requests, and an Extended CONNECT received by
haproxy on the H2 side is always converted back to a GET. So through an
H2 hop (H1 -> haproxy -> h2 -> haproxy -> H1 server), a bodyless DELETE,
OPTIONS, POST or PUT request with an upgrade reaches the server as a GET
(bodies are rejected however). There's no direct implication to this but
it could at least be confusing, and maybe cause trouble in certain cases
(idempotent methods, HEAD etc).

Let's reject the conversion for methods other than GET, like it is
already done for upgrade requests with a payload since commit 001fb1a54
("BUG/MEDIUM: mux-h1/mux-h2: Reject upgrades with payload on H2 side
only"). This way we don't block them in H1 where they are supported
(and used as shown in the commit above).

The protocol_upgrade reg-test was extended to check that a bodyless
DELETE upgrade request is rejected instead of reaching the server
as a GET.

This could be backported to all stable versions.

diff --git a/reg-tests/http-messaging/protocol_upgrade.vtc b/reg-tests/http-messaging/protocol_upgrade.vtc
index 28cd665994e54ad3f33aeaeb912e9d6a57ca6a68..0936fd38f117cfcdef57102c6e1171aff2780724 100644 (file)
--- a/reg-tests/http-messaging/protocol_upgrade.vtc
+++ b/reg-tests/http-messaging/protocol_upgrade.vtc
@@ -215,6 +215,21 @@ client c1_h1_h2 -connect ${hap_frt_h1_h2_sock} {
        expect resp.http.upgrade == "custom_protocol"
 } -run
 
+## connect to h1 translation frontend with a method other than GET: it
+## cannot be converted to an Extended CONNECT, which would be turned into
+## a GET by the h2 frontend, so it must be rejected without reaching srv_h1.
+client c1_h1_h2_delete -connect ${hap_frt_h1_h2_sock} {
+       txreq \
+         -req "DELETE" \
+         -url "/" \
+         -hdr "host: 127.0.0.1" \
+         -hdr "connection: upgrade" \
+         -hdr "upgrade: custom_protocol"
+
+       rxresp
+       expect resp.status == 502
+} -run
+
 # connect to h2 server frontend
 client c2_h2 -connect ${hap_frt_h2_sock} {
        txpri

diff --git a/src/mux_h2.c b/src/mux_h2.c
index ee5ce7a909d0cd1a51ae88729dbacb16fe02c103..1bab63c2aac055ab1ca2173a255ac65f97555426 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -7137,6 +7137,15 @@ static size_t h2s_snd_bhdrs(struct h2s *h2s, struct htx *htx)
                                        goto fail;
                                }
 
+                               /* RFC8441 only maps GET + Upgrade requests. An Extended
+                                * CONNECT is turned into a GET on the other side, so
+                                * other methods cannot be converted.
+                                */
+                               if (sl->info.req.meth != HTTP_METH_GET) {
+                                       TRACE_STATE("cannot convert upgrade for non-GET request", H2_EV_TX_FRAME|H2_EV_TX_HDR, h2c->conn, h2s);
+                                       goto fail;
+                               }
+
                                do {
                                        if (isteqi(iststop(connection_ist, ','),
                                                   ist("upgrade"))) {