From 9e910adf645a000ed9ecde4e9f6ddeac2b1c9e30 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 4 Jun 2026 18:47:38 -0400 Subject: [PATCH] Upstream: prevent HTTP response smuggling Enforce that downstream connections stay synchronized with what the client expects, even if other parts of NGINX are buggy or the backend sends a 1xx response other than 101 or 103. This ensures that the connection continues to carry HTTP unless both the client and server are expecting it not to. If the client is expecting it but the server is not or visa versa, or if NGINX cannot handle the connection no longer being upgraded, return 502 Bad Gateway or 500 Internal Server Error. This also ensures that 1xx responses (other than 101 Switching Protocols) are only sent via the early hints path, rather than via the normal response path. 1xx responses are always followed by another response. The early hints path ensures this, while the normal response path does not. Catch attempts by an upstream server to send a 1xx HTTP/1.0 response, as 1xx responses were not defined by HTTP/1.0. Finally, catch attempts to call ngx_http_send_response() with a 1xx status, or with a 2xx status in response to a CONNECT request. --- src/http/modules/ngx_http_proxy_module.c | 4 +- src/http/modules/ngx_http_rewrite_module.c | 2 +- src/http/ngx_http_core_module.c | 9 ++ src/http/ngx_http_upstream.c | 107 ++++++++++++++++++++- src/http/ngx_http_upstream.h | 1 + 5 files changed, 119 insertions(+), 4 deletions(-) diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c index 7e08df702..18629bbad 100644 --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -1810,9 +1810,9 @@ ngx_http_proxy_process_status_line(ngx_http_request_t *r) if (ctx->status.http_version < NGX_HTTP_VERSION_11) { - if (ctx->status.code == NGX_HTTP_EARLY_HINTS) { + if (ctx->status.code < NGX_HTTP_OK) { ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream sent HTTP/1.0 response with early hints"); + "upstream sent 1xx HTTP/1.0 response"); return NGX_HTTP_UPSTREAM_INVALID_HEADER; } diff --git a/src/http/modules/ngx_http_rewrite_module.c b/src/http/modules/ngx_http_rewrite_module.c index ff3b68716..13a533268 100644 --- a/src/http/modules/ngx_http_rewrite_module.c +++ b/src/http/modules/ngx_http_rewrite_module.c @@ -482,7 +482,7 @@ ngx_http_rewrite_return(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) } else { - if (ret->status > 999) { + if (ret->status > 999 || ret->status < NGX_HTTP_OK) { ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid return code \"%V\"", &value[1]); return NGX_CONF_ERROR; diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c index 4e1c67282..f1f76a72e 100644 --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -1804,6 +1804,15 @@ ngx_http_send_response(ngx_http_request_t *r, ngx_uint_t status, return rc; } + /* + * Can't respond 1xx, as that would mean a subsequent response is + * coming or the protocol is switching. Can't respond 2xx to a + * CONNECT request, as that would mean a tunnel has opened. + */ + if (status < (r->method == NGX_HTTP_CONNECT ? 300 : NGX_HTTP_OK)) { + return NGX_HTTP_INTERNAL_SERVER_ERROR; + } + r->headers_out.status = status; if (ngx_http_complex_value(r, cv, &val) != NGX_OK) { diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c index bb8964add..3bfd94000 100644 --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -27,6 +27,8 @@ static ngx_int_t ngx_http_upstream_cache_last_modified(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_upstream_cache_etag(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); +static ngx_int_t ngx_http_upstream_process_upgrade(ngx_http_request_t *r, + ngx_table_elt_t *h, ngx_uint_t offset); #endif static void ngx_http_upstream_init_request(ngx_http_request_t *r); @@ -330,6 +332,10 @@ static ngx_http_upstream_header_t ngx_http_upstream_headers_in[] = { ngx_http_upstream_copy_header_line, offsetof(ngx_http_headers_out_t, content_encoding), 0 }, + { ngx_string("Upgrade"), + ngx_http_upstream_process_upgrade, 0, + ngx_http_upstream_copy_header_line, 0, 0 }, + { ngx_null_string, NULL, 0, NULL, 0, 0 } }; @@ -2605,6 +2611,96 @@ done: /* rc == NGX_OK */ + /* + * Check that we don't send a 0xx response to the client. + * Also check that we don't send a 1xx response to a client that can't + * handle it, and that the connection was upgraded if it was supposed + * to be. + */ + if (r->method == NGX_HTTP_CONNECT + && u->headers_in.status_n >= NGX_HTTP_OK + && u->headers_in.status_n <= 299) + { + if (!u->upgrade) { + /* + * Something (probably an upstream server) successfully handled + * a CONNECT request, but this isn't supported. + */ + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "Successful response to CONNECT request, " + "but no upgrade. Check that your upstream " + "servers reject CONNECT requests."); + ngx_http_upstream_finalize_request(r, u, + NGX_HTTP_INTERNAL_SERVER_ERROR); + return; + } + + if (r->http_version != NGX_HTTP_VERSION_11) { + /* Rejected in ngx_http_request.c */ + ngx_log_error(NGX_LOG_ALERT, c->log, 0, + "Attempt to upgrade non-HTTP/1.1 connection"); + ngx_http_upstream_finalize_request(r, u, + NGX_HTTP_INTERNAL_SERVER_ERROR); + return; + } + } else if (u->upgrade) { + /* Someone upgraded the connection when they should not have. */ + if (u->headers_in.status_n != NGX_HTTP_SWITCHING_PROTOCOLS) { + ngx_log_error(NGX_LOG_ALERT, c->log, 0, + "Upgrade flag sent, but status is %ui (not 101)", + u->headers_in.status_n); + ngx_http_upstream_finalize_request(r, u, + NGX_HTTP_INTERNAL_SERVER_ERROR); + return; + } + + if (r->http_version != NGX_HTTP_VERSION_11) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "Upstream sent 101 response to non-HTTP/1.1 request"); + ngx_http_upstream_finalize_request(r, u, NGX_HTTP_BAD_GATEWAY); + return; + } + + if (r->method == NGX_HTTP_HEAD) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "Upstream sent 101 response to HEAD request"); + ngx_http_upstream_finalize_request(r, u, NGX_HTTP_BAD_GATEWAY); + return; + } + + if (!r->headers_in.upgrade) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "Upstream sent 101 response, but client didn't " + "send Upgrade header"); + ngx_http_upstream_finalize_request(r, u, NGX_HTTP_BAD_GATEWAY); + return; + } + + if (!u->headers_in.upgrade) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "Upstream sent 101 without setting Upgrade header"); + ngx_http_upstream_finalize_request(r, u, NGX_HTTP_BAD_GATEWAY); + return; + } + } else if (u->headers_in.status_n < NGX_HTTP_OK) { + /* + * This means that the upstream protocol (probably FastCGI, uWSGI, + * or SCGI) cannot handle 1xx responses of the type the upstream server + * provided. It could also mean that 0xx responses were not rejected. + * Log an error and return 502 Bad Gateway. + * + * 1xx responses from upstream HTTP servers don't reach here. Their + * protocol handlers return NGX_HTTP_UPSTREAM_EARLY_HINTS from their + * process_headers() function when they get a 1xx response. That + * causes a 1xx response to be sent to the client. + */ + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "Upstream sent response with status %ui that upstream " + "protocol does not support", u->headers_in.status_n); + ngx_http_upstream_finalize_request(r, u, NGX_HTTP_BAD_GATEWAY); + return; + } + u->state->header_time = ngx_current_msec - u->start_time; if (u->headers_in.status_n >= NGX_HTTP_SPECIAL_RESPONSE) { @@ -2644,7 +2740,7 @@ ngx_http_upstream_process_early_hints(ngx_http_request_t *r, ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http upstream early hints"); - if (u->conf->pass_early_hints) { + if (u->conf->pass_early_hints && r->http_version >= NGX_HTTP_VERSION_11) { u->early_hints_length += u->buffer.pos - u->buffer.start; @@ -5458,6 +5554,15 @@ ngx_http_upstream_process_connection(ngx_http_request_t *r, ngx_table_elt_t *h, } +static ngx_int_t +ngx_http_upstream_process_upgrade(ngx_http_request_t *r, + ngx_table_elt_t *h, ngx_uint_t offset) +{ + r->upstream->headers_in.upgrade = 1; + return NGX_OK; +} + + static ngx_int_t ngx_http_upstream_process_transfer_encoding(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset) diff --git a/src/http/ngx_http_upstream.h b/src/http/ngx_http_upstream.h index 4560bbe9a..392e11cea 100644 --- a/src/http/ngx_http_upstream.h +++ b/src/http/ngx_http_upstream.h @@ -311,6 +311,7 @@ typedef struct { unsigned chunked:1; unsigned no_cache:1; unsigned expired:1; + unsigned upgrade:1; } ngx_http_upstream_headers_in_t;