From e8a6ed6e350f415b639881291c629957343b5a9c Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Mon, 18 Oct 2021 16:42:48 -0400 Subject: [PATCH] [core] thwart h2c smuggling when Upgrade enabled Existing behavior: mod_proxy *does not* forward Upgrade header unless explicitly enabled in lighttpd.conf (default: not enabled) (proxy.header += ("upgrade" => "enable")) mod_cgi previously used to forward Upgrade request header, but would remove Upgrade response header if cgi.upgrade was not explicitly enabled (cgi.upgrade = "enable") This patch thwarts h2c smuggling when lighttpd.conf has also been explicitly configured to pass "Upgrade" request header x-ref: "h2c Smuggling: Request Smuggling Via HTTP/2 Cleartext (h2c)" https://labs.bishopfox.com/tech-blog/h2c-smuggling-request-smuggling-via-http/2-cleartext-h2c --- src/connections.c | 2 +- src/h2.c | 1 + src/mod_cgi.c | 11 +++++++---- src/mod_proxy.c | 2 +- src/request.c | 7 +++++++ 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/connections.c b/src/connections.c index 9b725732..4a675ee4 100644 --- a/src/connections.c +++ b/src/connections.c @@ -779,7 +779,7 @@ static int connection_handle_read_state(connection * const con) { connection_set_state(r, CON_STATE_REQUEST_END); if (light_btst(r->rqst_htags, HTTP_HEADER_UPGRADE) - && !con->is_ssl_sock && r->conf.h2proto && 0 == r->http_status + && 0 == r->http_status && h2_check_con_upgrade_h2c(r)) { /*(Upgrade: h2c over cleartext does not have SNI; no COMP_HTTP_HOST)*/ r->conditional_is_valid = (1 << COMP_SERVER_SOCKET) diff --git a/src/h2.c b/src/h2.c index 4a9d714a..a6398204 100644 --- a/src/h2.c +++ b/src/h2.c @@ -2810,6 +2810,7 @@ h2_check_con_upgrade_h2c (request_st * const r) buffer * const b = r->tmp_buf; buffer_clear(b); if (r->conf.h2proto > 1/*(must be enabled with server.h2c feature)*/ + && !r->con->is_ssl_sock /*(disallow h2c over TLS socket)*/ && http_header_str_contains_token(BUF_PTR_LEN(http_connection), CONST_STR_LEN("HTTP2-Settings")) diff --git a/src/mod_cgi.c b/src/mod_cgi.c index 8aa728dc..d956fa08 100644 --- a/src/mod_cgi.c +++ b/src/mod_cgi.c @@ -980,10 +980,13 @@ URIHANDLER_FUNC(cgi_is_handled) { hctx->plugin_data = p; hctx->cgi_handler = &ds->value; memcpy(&hctx->conf, &p->conf, sizeof(plugin_config)); - hctx->conf.upgrade = - hctx->conf.upgrade - && r->http_version == HTTP_VERSION_1_1 - && light_btst(r->rqst_htags, HTTP_HEADER_UPGRADE); + if (!light_btst(r->rqst_htags, HTTP_HEADER_UPGRADE)) + hctx->conf.upgrade = 0; + else if (!hctx->conf.upgrade || r->http_version != HTTP_VERSION_1_1) { + hctx->conf.upgrade = 0; + http_header_request_unset(r, HTTP_HEADER_UPGRADE, + CONST_STR_LEN("Upgrade")); + } hctx->opts.max_per_read = !(r->conf.stream_response_body /*(if not streaming response body)*/ & (FDEVENT_STREAM_RESPONSE|FDEVENT_STREAM_RESPONSE_BUFMIN)) diff --git a/src/mod_proxy.c b/src/mod_proxy.c index 92cb26a0..83e7c14a 100644 --- a/src/mod_proxy.c +++ b/src/mod_proxy.c @@ -930,7 +930,7 @@ static handler_t proxy_create_env(gw_handler_ctx *gwhctx) { te = &ds->value; /*("trailers")*/ break; case HTTP_HEADER_UPGRADE: - if (hctx->conf.header.force_http10 || r->http_version == HTTP_VERSION_1_0) continue; + if (hctx->conf.header.force_http10 || r->http_version != HTTP_VERSION_1_1) continue; if (!hctx->conf.header.upgrade) continue; if (!buffer_is_blank(&ds->value)) upgrade = &ds->value; break; diff --git a/src/request.c b/src/request.c index ed2e5310..4f8a4fba 100644 --- a/src/request.c +++ b/src/request.c @@ -1167,6 +1167,13 @@ http_request_parse (request_st * const restrict r, const int scheme_port) return http_request_header_line_invalid(r, 400, "HTTP/1.1 but Host missing -> 400"); } + if (HTTP_VERSION_1_1 != r->http_version + && (r->rqst_htags + & (light_bshift(HTTP_HEADER_UPGRADE) + |light_bshift(HTTP_HEADER_HTTP2_SETTINGS)))) { + return http_request_header_line_invalid(r, 400, "invalid hop-by-hop header w/o HTTP/1.1 -> 400"); + } + if (0 == r->reqbody_length) { /* POST requires Content-Length (or Transfer-Encoding) * (-1 == r->reqbody_length when Transfer-Encoding: chunked)*/