BUG/MINOR: h2: only accept :protocol with extended CONNECT

As reported by Huangbin Zhan in github issue #3355, we're too lax on
the :protocol pseudo header. It is currently accepted with regular
CONNECT as well as non-CONNECT methods while it only ought to be
accepted with extended CONNECT (i.e. CONNECT after the connection
negotiated the RFC8441 extension). Let's refine the check in H2 by
leveraging the new flag H2_MSGF_EXT_CONN_OK that is passed by the
caller when the connection supports the extension. This is sufficient
to sort the various cases.

The proto upgrade regtest was updated to verify that CONNECT with
:protocol without nego and another method with nego and :protocol
both fail.

Thanks to Huangbin Zhan (@zhanhb) for the report and helpful reproducer.

This needs to be backported to all versions. It relies on these patches
first:

  REGTESTS: http-messaging: always send RFC8441 client settings to use ext connect
  BUG/MINOR: mux-h2: condition the processing of 8441 extension to global setting
  MINOR: mux-h2: add a new message flag to indicate ext connect support

diff --git a/reg-tests/http-messaging/protocol_upgrade.vtc b/reg-tests/http-messaging/protocol_upgrade.vtc
index b4b7da3ad1a7199761de1cf97650d7cb021163a3..3cd0f4cb0b78a6b47cf9a6acc21a5244903f42f6 100644 (file)
--- a/reg-tests/http-messaging/protocol_upgrade.vtc
+++ b/reg-tests/http-messaging/protocol_upgrade.vtc
@@ -72,7 +72,7 @@ server srv_h2 {
                txresp \
                  -status 200
        } -run
-} -repeat 2 -start
+} -repeat 4 -start
 
 # http2 server without support for RFC8441
 server srv_h2_no_ws {
@@ -242,6 +242,56 @@ client c2_h2 -connect ${hap_frt_h2_sock} {
        } -run
 } -run
 
+# connect to h2 server frontend without extension: must fail
+client c2_h2_rej1 -connect ${hap_frt_h2_sock} {
+       txpri
+       stream 0 {
+               # no extension sent
+               txsettings
+               rxsettings
+               txsettings -ack
+               rxsettings
+               expect settings.ack == true
+       } -run
+
+       stream 1 {
+               txreq \
+                 -req "CONNECT" \
+                 -scheme "http" \
+                 -url "/" \
+                 -hdr ":authority" "127.0.0.1" \
+                 -hdr ":protocol" "custom_protocol" \
+                 -nostrend
+
+               rxrst
+       } -run
+} -run
+
+# connect to h2 server with ext but :proto with bad method: must fail
+client c2_h2_rej2 -connect ${hap_frt_h2_sock} {
+       txpri
+       stream 0 {
+               # manually send RFC8441 SETTINGS_ENABLE_CONNECT_PROTOCOL
+               sendhex "00 00 06 04   00    00 00 00 00    00 08   00 00 00 01"
+               rxsettings
+               txsettings -ack
+               rxsettings
+               expect settings.ack == true
+       } -run
+
+       stream 1 {
+               txreq \
+                 -req "GET" \
+                 -scheme "http" \
+                 -url "/" \
+                 -hdr ":authority" "127.0.0.1" \
+                 -hdr ":protocol" "custom_protocol" \
+                 -nostrend
+
+               rxrst
+       } -run
+} -run
+
 # connect to h2 translation frontend
 client c3_h2_h1 -connect ${hap_frt_h2_h1_sock} {
        txpri

diff --git a/src/h2.c b/src/h2.c
index 7a2c9d884af93be267c983c71652c298d777f159..f0302211a6b9974a8c38318da6e13bf2421f8d1e 100644 (file)
--- a/src/h2.c
+++ b/src/h2.c
@@ -92,7 +92,7 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
        size_t i;
 
        if ((fields & H2_PHDR_FND_METH) && isteq(phdr[H2_PHDR_IDX_METH], ist("CONNECT"))) {
-               if (fields & H2_PHDR_FND_PROT) {
+               if ((fields & H2_PHDR_FND_PROT) && (*msgf & H2_MSGF_EXT_CONN_OK)) {
                        /* rfc 8441 Extended Connect Protocol
                         * #4 :scheme and :path must be present, as well as
                         * :authority like all h2 requests
@@ -130,6 +130,11 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
                         * MUST be omitted ; ":authority" contains the host and port
                         * to connect to.
                         */
+                       if (fields & H2_PHDR_FND_PROT) {
+                               /* protocol not allowed without RFC8441 support */
+                               goto fail;
+                       }
+
                        if (fields & H2_PHDR_FND_SCHM) {
                                /* scheme not allowed */
                                goto fail;
@@ -148,11 +153,11 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
 
                *msgf |= H2_MSGF_BODY_TUNNEL;
        }
-       else if ((fields & (H2_PHDR_FND_METH|H2_PHDR_FND_SCHM|H2_PHDR_FND_PATH)) !=
+       else if ((fields & (H2_PHDR_FND_METH|H2_PHDR_FND_SCHM|H2_PHDR_FND_PATH|H2_PHDR_FND_PROT)) !=
                 (H2_PHDR_FND_METH|H2_PHDR_FND_SCHM|H2_PHDR_FND_PATH)) {
                /* RFC 7540 #8.1.2.3 : all requests MUST include exactly one
                 * valid value for the ":method", ":scheme" and ":path" phdr
-                * unless it is a CONNECT request.
+                * and no ":protocol" phdr unless it is a CONNECT request..
                 */
                if (!(fields & H2_PHDR_FND_METH)) {
                        /* missing method */
@@ -163,7 +168,7 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
                        goto fail;
                }
                else {
-                       /* missing path */
+                       /* missing path or extra protocol */
                        goto fail;
                }
        }