From bb0a2b9de8781fe42dcab755fef551ce6829a306 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Tue, 25 Mar 2025 14:58:32 -0400 Subject: [PATCH] HTTP: Allow rejecting leading and trailing whitespace in HTTP2+ fields All versions of HTTP forbid field (header and trailer) values from having leading or trailing horizontal whitespace (0x20 and 0x09). In HTTP/1.0 and HTTP/1.1, leading and trailing whitespace must be stripped from the field value before further processing. In HTTP/2 and HTTP/3, leading and trailing whitespace must cause the entire message to be considered malformed. Willy Tarreau (lead developer of HAProxy) has indicated that there are clients that actually do send leading and/or trailing whitespace in HTTP/2 and/or HTTP/3 cookie headers, which is why HAProxy accepts them. Therefore, the fix is disabled by default and must be enabled with the reject_leading_trailing_whitespace directive. --- src/http/ngx_http.h | 1 - src/http/ngx_http_core_module.c | 11 +++++++++++ src/http/ngx_http_core_module.h | 1 + src/http/ngx_http_parse.c | 11 +++++++++-- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/http/ngx_http.h b/src/http/ngx_http.h index 1bfb7aa7f..883d2ec72 100644 --- a/src/http/ngx_http.h +++ b/src/http/ngx_http.h @@ -97,7 +97,6 @@ int ngx_http_ssl_servername(ngx_ssl_conn_t *ssl_conn, int *ad, void *arg); int ngx_http_ssl_certificate(ngx_ssl_conn_t *ssl_conn, void *arg); #endif - ngx_int_t ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b); ngx_int_t ngx_http_parse_uri(ngx_http_request_t *r); ngx_int_t ngx_http_parse_complex_uri(ngx_http_request_t *r, diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c index 92c3eae8a..495d476ac 100644 --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -259,6 +259,13 @@ static ngx_command_t ngx_http_core_commands[] = { offsetof(ngx_http_core_srv_conf_t, ignore_invalid_headers), NULL }, + { ngx_string("reject_leading_trailing_whitespace"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_conf_set_flag_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_core_srv_conf_t, reject_leading_trailing_whitespace), + NULL }, + { ngx_string("merge_slashes"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, ngx_conf_set_flag_slot, @@ -3476,6 +3483,7 @@ ngx_http_core_create_srv_conf(ngx_conf_t *cf) cscf->ignore_invalid_headers = NGX_CONF_UNSET; cscf->merge_slashes = NGX_CONF_UNSET; cscf->underscores_in_headers = NGX_CONF_UNSET; + cscf->reject_leading_trailing_whitespace = NGX_CONF_UNSET; cscf->file_name = cf->conf_file->file.name.data; cscf->line = cf->conf_file->line; @@ -3522,6 +3530,9 @@ ngx_http_core_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_value(conf->underscores_in_headers, prev->underscores_in_headers, 0); + ngx_conf_merge_value(conf->reject_leading_trailing_whitespace, + prev->reject_leading_trailing_whitespace, 0); + if (conf->server_names.nelts == 0) { /* the array has 4 empty preallocated elements, so push cannot fail */ sn = ngx_array_push(&conf->server_names); diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h index e7e266bf8..8e7e9eb8b 100644 --- a/src/http/ngx_http_core_module.h +++ b/src/http/ngx_http_core_module.h @@ -208,6 +208,7 @@ typedef struct { #endif ngx_http_core_loc_conf_t **named_locations; + ngx_flag_t reject_leading_trailing_whitespace; } ngx_http_core_srv_conf_t; diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c index f6d71e2f7..6d2991c38 100644 --- a/src/http/ngx_http_parse.c +++ b/src/http/ngx_http_parse.c @@ -1136,8 +1136,7 @@ ngx_http_v23_fixup_header(ngx_http_request_t *r, ngx_str_t *name, if (!ngx_http_field_value_char(value->data[i])) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, "client sent header \"%V\" with " - "invalid value: \"%V\"", - name, value); + "invalid value", name); return NGX_ERROR; } @@ -1181,6 +1180,14 @@ ngx_http_v23_fixup_header(ngx_http_request_t *r, ngx_str_t *name, return NGX_OK; } + if (cscf->reject_leading_trailing_whitespace) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent header \"%V\" with " + "leading or trailing space", + name); + return NGX_ERROR; + } + /* * Strip trailing whitespace. Do this first so that * if the string is all whitespace, tmp.data is not a