HTTP: Strip or reject leading or trailing HWS in field values

All versions of HTTP forbid field (header and trailer) values from
having leading or trailing horizontal whitespace (HWS, 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 default is to always strip leading and trailing
whitespace, regardless of the HTTP version.  Standards-conforming
behavior can be obtained with the new reject_leading_trailing_whitespace
directive.  If one only wants to effect how client requests are
processed, one can use reject_leading_trailing_whitespace_client.  If
one only wants to effect processing of upstream responses, one can use
reject_leading_trailing_whitespace_upstream.

Fixes: #187
Fixes: #598
This commit is contained in:
Demi Marie Obenour 2025-04-07 17:30:13 -04:00
parent 73002aa4c9
commit df2f71d720
4 changed files with 110 additions and 0 deletions

View file

@ -266,6 +266,29 @@ 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("reject_leading_trailing_whitespace_client"),
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_client),
NULL },
{ ngx_string("reject_leading_trailing_whitespace_upstream"),
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_upstream),
NULL },
{ ngx_string("merge_slashes"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,
ngx_conf_set_flag_slot,
@ -3547,6 +3570,9 @@ 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->reject_leading_trailing_whitespace_client = NGX_CONF_UNSET;
cscf->reject_leading_trailing_whitespace_upstream = NGX_CONF_UNSET;
cscf->file_name = cf->conf_file->file.name.data;
cscf->line = cf->conf_file->line;
@ -3595,6 +3621,22 @@ 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);
if (conf->reject_leading_trailing_whitespace_client == NGX_CONF_UNSET) {
conf->reject_leading_trailing_whitespace_client = (
conf->reject_leading_trailing_whitespace != NGX_CONF_UNSET ?
conf->reject_leading_trailing_whitespace :
(prev->reject_leading_trailing_whitespace_client != NGX_CONF_UNSET ?
prev->reject_leading_trailing_whitespace_client : 0));
}
if (conf->reject_leading_trailing_whitespace_upstream == NGX_CONF_UNSET) {
conf->reject_leading_trailing_whitespace_upstream = (
conf->reject_leading_trailing_whitespace != NGX_CONF_UNSET ?
conf->reject_leading_trailing_whitespace :
(prev->reject_leading_trailing_whitespace_upstream != NGX_CONF_UNSET ?
prev->reject_leading_trailing_whitespace_upstream : 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);

View file

@ -212,6 +212,9 @@ typedef struct {
unsigned allow_connect:1;
ngx_http_core_loc_conf_t **named_locations;
ngx_flag_t reject_leading_trailing_whitespace;
ngx_flag_t reject_leading_trailing_whitespace_client;
ngx_flag_t reject_leading_trailing_whitespace_upstream;
} ngx_http_core_srv_conf_t;

View file

@ -1027,6 +1027,7 @@ ngx_http_parse_header_line(ngx_http_request_t *r, ngx_buf_t *b,
case sw_space_before_value:
switch (ch) {
case ' ':
case '\t':
break;
case CR:
r->header_start = p;
@ -1051,6 +1052,7 @@ ngx_http_parse_header_line(ngx_http_request_t *r, ngx_buf_t *b,
case sw_value:
switch (ch) {
case ' ':
case '\t':
r->header_end = p;
state = sw_space_after_value;
break;
@ -1071,6 +1073,7 @@ ngx_http_parse_header_line(ngx_http_request_t *r, ngx_buf_t *b,
case sw_space_after_value:
switch (ch) {
case ' ':
case '\t':
break;
case CR:
state = sw_almost_done;

View file

@ -17,6 +17,7 @@ ngx_http_v23_validate_header(ngx_http_request_t *r,
ngx_str_t *name, ngx_str_t *value, ngx_int_t is_client)
{
u_char ch;
ngx_str_t tmp;
ngx_uint_t i;
ngx_http_core_srv_conf_t *cscf;
@ -61,6 +62,11 @@ ngx_http_v23_validate_header(ngx_http_request_t *r,
}
}
/* Keep subsequent code from having to special-case empty strings. */
if (value->len == 0) {
return NGX_OK;
}
for (i = 0; i != value->len; i++) {
ch = value->data[i];
@ -75,6 +81,62 @@ ngx_http_v23_validate_header(ngx_http_request_t *r,
}
}
if (!ngx_isspace(value->data[0])
&& !ngx_isspace(value->data[value->len - 1])) {
/* Fast path: nothing to strip. */
return NGX_OK;
}
if (is_client ? cscf->reject_leading_trailing_whitespace_client
: cscf->reject_leading_trailing_whitespace_upstream) {
ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
"%s sent header \"%V\" with "
"leading or trailing space",
is_client ? "client" : "upstream", name);
return NGX_ERROR;
}
tmp = *value;
/*
* Strip trailing whitespace. Do this first so that
* if the string is all whitespace, tmp.data is not a
* past-the-end pointer, which cannot be safely passed
* to memmove(). After the loop, the string is either
* empty or ends with a non-whitespace character.
*/
while (tmp.len && ngx_isspace(tmp.data[tmp.len - 1])) {
tmp.len--;
}
/* Strip leading whitespace */
if (tmp.len && ngx_isspace(tmp.data[0])) {
/*
* Last loop guaranteed that 'tmp' does not end with whitespace, and
* this check guarantees it is not empty and starts with whitespace.
* Therefore, 'tmp' must end with a non-whitespace character, and must
* be of length at least 2. This means that it is safe to keep going
* until a non-whitespace character is found.
*/
do {
tmp.len--;
tmp.data++;
} while (ngx_isspace(tmp.data[0]));
/* Move remaining string to start of buffer. */
memmove(value->data, tmp.data, tmp.len);
}
/*
* NUL-pad the data, so that if it was NUL-terminated before, it stil is.
* At least one byte will have been stripped, so value->data + tmp.len
* is not a past-the-end pointer.
*/
memset(value->data + tmp.len, '\0', value->len - tmp.len);
/* Fix up length and return. */
value->len = tmp.len;
return NGX_OK;
}