From 0df043608f53cdb367586714e3ba0a7a3e2b0e89 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 18 Oct 2021 09:43:29 +0200 Subject: [PATCH] BUG/MEDIUM: mux-h2: reject upgrade if no RFC8441 support The RFC8441 was not respected by haproxy in regards with server support for Extended CONNECT. The Extended CONNECT method was used to convert an Upgrade header stream even if no SETTINGS_ENABLE_CONNECT_PROTOCOL was received, which is forbidden by the RFC8441. In this case, the behavior of the http/2 server is unspecified. Fix this by flagging the connection on receiption of the RFC8441 settings SETTINGS_ENABLE_CONNECT_PROTOCOL. Extended CONNECT is thus only be used if the flag is present. In the other case, the stream is immediatly closed as there is no way to handle it in http/2. It results in a http/1.1 502 or http/2 RESET_STREAM to the client side. The protocol-upgrade regtest has been extended to test that haproxy does not emit Extended CONNECT on servers without RFC8441 support. It must be backported up to 2.4. --- reg-tests/http-messaging/protocol_upgrade.vtc | 75 ++++++++++++++++++- src/mux_h2.c | 11 ++- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/reg-tests/http-messaging/protocol_upgrade.vtc b/reg-tests/http-messaging/protocol_upgrade.vtc index 7d122aa084..3458e772a0 100644 --- a/reg-tests/http-messaging/protocol_upgrade.vtc +++ b/reg-tests/http-messaging/protocol_upgrade.vtc @@ -28,7 +28,8 @@ server srv_h2 { rxpri stream 0 { - txsettings + # 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 @@ -48,6 +49,42 @@ server srv_h2 { } -run } -repeat 2 -start +# http2 server without support for RFC8441 +server srv_h2_no_ws { + rxpri + + stream 0 { + txsettings + rxsettings + txsettings -ack + rxsettings + expect settings.ack == true + } -run + + stream 1 { + rxrst + } -run +} -start + +# http2 server without support for RFC8441 : settings announced with value 0 +server srv_h2_no_ws2 { + rxpri + + stream 0 { + # manually send RFC8441 SETTINGS_ENABLE_CONNECT_PROTOCOL with a value of 0 + sendhex "00 00 06 04 00 00 00 00 00 00 08 00 00 00 00" + txsettings + rxsettings + txsettings -ack + rxsettings + expect settings.ack == true + } -run + + stream 1 { + rxrst + } -run +} -start + haproxy hap -conf { defaults mode http @@ -70,6 +107,16 @@ haproxy hap -conf { bind "fd@${frt_h1}" server srv_h2 ${srv_h2_addr}:${srv_h2_port} proto h2 + # h1 frontend connected to srv_h2_no_ws + listen frt_h1_no_ws + bind "fd@${frt_h1_no_ws}" + server srv_h2_no_ws ${srv_h2_no_ws_addr}:${srv_h2_no_ws_port} proto h2 + + # h1 frontend connected to srv_h2_no_ws2 + listen frt_h1_no_ws2 + bind "fd@${frt_h1_no_ws2}" + server srv_h2_no_ws2 ${srv_h2_no_ws2_addr}:${srv_h2_no_ws2_port} proto h2 + # h2 frontend connected to h1 frontend listen frt_h2_h1 bind "fd@${frt_h2_h1}" proto h2 @@ -153,3 +200,29 @@ client c4_h1 -connect ${hap_frt_h1_sock} { expect resp.http.connection == "upgrade" expect resp.http.upgrade == "custom_protocol" } -run + +# connect via h1 server frontend to h2 server without RFC8441 support +client c5 -connect ${hap_frt_h1_no_ws_sock} { + txreq \ + -req "GET" \ + -url "/" \ + -hdr "host: 127.0.0.1" \ + -hdr "connection: upgrade" \ + -hdr "upgrade: custom_protocol" + + rxresp + expect resp.status == 502 +} -run + +# connect via h1 server frontend to h2 server without RFC8441 support +client c6 -connect ${hap_frt_h1_no_ws2_sock} { + txreq \ + -req "GET" \ + -url "/" \ + -hdr "host: 127.0.0.1" \ + -hdr "connection: upgrade" \ + -hdr "upgrade: custom_protocol" + + rxresp + expect resp.status == 502 +} -run diff --git a/src/mux_h2.c b/src/mux_h2.c index 0418b583ab..90373310ee 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -72,6 +72,8 @@ static const struct h2s *h2_idle_stream; #define H2_CF_RCVD_SHUT 0x00020000 // a recv() attempt already failed on a shutdown #define H2_CF_END_REACHED 0x00040000 // pending data too short with RCVD_SHUT present +#define H2_CF_RCVD_RFC8441 0x00100000 // settings from RFC8441 has been received indicating support for Extended CONNECT + /* H2 connection state, in h2c->st0 */ enum h2_cs { H2_CS_PREFACE, // init done, waiting for connection preface @@ -2239,8 +2241,8 @@ static int h2c_handle_settings(struct h2c *h2c) } break; case H2_SETTINGS_ENABLE_CONNECT_PROTOCOL: - /* nothing to do here as this settings is automatically - * transmits to the client */ + if (arg == 1) + h2c->flags |= H2_CF_RCVD_RFC8441; break; } } @@ -5319,6 +5321,11 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s, struct htx *htx) do { if (isteqi(iststop(connection_ist, ','), ist("upgrade"))) { + if (!(h2c->flags & H2_CF_RCVD_RFC8441)) { + TRACE_STATE("reject upgrade because of no RFC8441 support", H2_EV_TX_FRAME|H2_EV_TX_HDR, h2c->conn, h2s); + goto fail; + } + TRACE_STATE("convert upgrade to extended connect method", H2_EV_TX_FRAME|H2_EV_TX_HDR, h2c->conn, h2s); h2s->flags |= (H2_SF_BODY_TUNNEL|H2_SF_EXT_CONNECT_SENT); sl->info.req.meth = HTTP_METH_CONNECT;