BUG/MINOR: h1: do not forward h2c upgrade header token

haproxy supports tunnel establishment through HTTP Upgrade mechanism.
Since the following commit, extended CONNECT is also supported for
HTTP/2 both on frontend and backend side.

  commit 9bf957335e2c385b74901481f7a89c9565dfce53
  MEDIUM: mux_h2: generate Extended CONNECT from htx upgrade

As specified by HTTP/2 rfc, "h2c" can be used by an HTTP/1.1 client to
request an upgrade to HTTP/2. In haproxy, this is not supported so it
silently ignores this. However, Connection and Upgrade headers are
forwarded as-is on the backend side.

If using HTTP/1 on the backend side and the server supports this upgrade
mechanism, haproxy won't be able to parse the HTTP response. If using
HTTP/2, mux backend tries to incorrectly convert the request to an
Extended CONNECT with h2c protocol, which may also prevent the response
to be transmitted.

To fix this, flag HTTP/1 request with "h2c" or "h2" token in an upgrade
header. On converting the header list to HTX, the upgrade header is
skipped if any of this token is present and the H1_MF_CONN_UPG flag is
removed.

This issue can easily be reproduced using curl --http2 argument to
connect to an HTTP/1 frontend.

This must be backported up to 2.4 after a period of observation.

diff --git a/include/haproxy/h1.h b/include/haproxy/h1.h
index 283bbad5b1f104057fa6ad0456c90324a0fe9b13..b4e66908e03f7e43c187d66eebeb7e75c322cd11 100644 (file)
--- a/include/haproxy/h1.h
+++ b/include/haproxy/h1.h
@@ -98,6 +98,7 @@ enum h1m_state {
 #define H1_MF_UPG_WEBSOCKET     0x00008000 // Set for a Websocket upgrade handshake
 #define H1_MF_TE_CHUNKED        0x00010000 // T-E "chunked"
 #define H1_MF_TE_OTHER          0x00020000 // T-E other than supported ones found (only "chunked" is supported for now)
+#define H1_MF_UPG_H2C           0x00040000 // "h2c" or "h2" used as upgrade token
 
 /* Mask to use to reset H1M flags when we restart headers parsing.
  *

diff --git a/reg-tests/http-messaging/protocol_upgrade.vtc b/reg-tests/http-messaging/protocol_upgrade.vtc
index ebb6328c3014d6de75a75c32c96878529f41b2b3..be291f18c50a3dbaa633f0a03d734f380f2b66f7 100644 (file)
--- a/reg-tests/http-messaging/protocol_upgrade.vtc
+++ b/reg-tests/http-messaging/protocol_upgrade.vtc
@@ -85,6 +85,15 @@ server srv_h2_no_ws2 {
        } -run
 } -start
 
+# http/1.1 server
+server srv_h1_h2c {
+       rxreq
+       expect req.method == "GET"
+       expect req.http.upgrade != "h2c"
+       txresp \
+         -status 200
+} -start
+
 haproxy hap -conf {
        defaults
        mode http
@@ -121,6 +130,11 @@ haproxy hap -conf {
        listen frt_h2_h1
        bind "fd@${frt_h2_h1}" proto h2
        server frt_h1 ${hap_frt_h1_addr}:${hap_frt_h1_port}
+
+       # h1 frontend to handle "h2c" token
+       listen frt_h1_h2c
+       bind "fd@${frt_h1_h2c}"
+       server srv_h1_h2c ${srv_h1_h2c_addr}:${srv_h1_h2c_port}
 } -start
 
 ## connect to h1 translation frontend
@@ -226,3 +240,16 @@ client c6 -connect ${hap_frt_h1_no_ws2_sock} {
        rxresp
        expect resp.status == 502
 } -run
+
+# connect as http/1 with invalid "h2c" protocol
+client c7_h2c -connect ${hap_frt_h1_h2c_sock} {
+       txreq \
+         -req "GET" \
+         -url "/" \
+         -hdr "host: 127.0.0.1" \
+         -hdr "connection: upgrade" \
+         -hdr "upgrade: h2c"
+
+       rxresp
+       expect resp.status == 200
+} -run

diff --git a/src/h1.c b/src/h1.c
index 2cc15040fbbc8927ea97e9130c2c3b7f681ede22..e1a7028fbc5c7267403f2798ffe037dd19cd46ea 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -365,13 +365,14 @@ void h1_parse_connection_header(struct h1m *h1m, struct ist *value)
 
 /* Parse the Upgrade: header of an HTTP/1 request.
  * If "websocket" is found, set H1_MF_UPG_WEBSOCKET flag
+ * If "h2c" or "h2" found, set H1_MF_UPG_H2C flag.
  */
 void h1_parse_upgrade_header(struct h1m *h1m, struct ist value)
 {
        char *e, *n;
        struct ist word;
 
-       h1m->flags &= ~H1_MF_UPG_WEBSOCKET;
+       h1m->flags &= ~(H1_MF_UPG_WEBSOCKET|H1_MF_UPG_H2C);
 
        word.ptr = value.ptr - 1; // -1 for next loop's pre-increment
        e = istend(value);
@@ -390,6 +391,8 @@ void h1_parse_upgrade_header(struct h1m *h1m, struct ist value)
 
                if (isteqi(word, ist("websocket")))
                        h1m->flags |= H1_MF_UPG_WEBSOCKET;
+               else if (isteqi(word, ist("h2c")) || isteqi(word, ist("h2")))
+                       h1m->flags |= H1_MF_UPG_H2C;
 
                word.ptr = n;
        }

diff --git a/src/h1_htx.c b/src/h1_htx.c
index 562c0f2b6fdfb1931d00deb4b022dc62d2505030..472169aec55077fef9c9e5bf64377001b3e0040c 100644 (file)
--- a/src/h1_htx.c
+++ b/src/h1_htx.c
@@ -180,7 +180,13 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
 
 
        flags |= h1m_htx_sl_flags(h1m);
-       if ((flags & (HTX_SL_F_CONN_UPG|HTX_SL_F_BODYLESS)) == HTX_SL_F_CONN_UPG) {
+
+       /* Remove Upgrade header in problematic cases :
+        * - body present
+        * - "h2c" or "h2" token specified as token
+        */
+       if (((flags & (HTX_SL_F_CONN_UPG|HTX_SL_F_BODYLESS)) == HTX_SL_F_CONN_UPG) ||
+           ((h1m->flags & (H1_MF_CONN_UPG|H1_MF_UPG_H2C)) == (H1_MF_CONN_UPG|H1_MF_UPG_H2C))) {
                int i;
 
                for (i = 0; hdrs[i].n.len; i++) {
@@ -190,6 +196,7 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
                h1m->flags &=~ H1_MF_CONN_UPG;
                flags &= ~HTX_SL_F_CONN_UPG;
        }
+
        sl = htx_add_stline(htx, HTX_BLK_REQ_SL, flags, meth, uri, vsn);
        if (!sl || !htx_add_all_headers(htx, hdrs))
                goto error;