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.  Stripping leading and/or
trailing whitespace would require either allocating a new buffer or
changing the pointers in the existing buffer, and I am not familiar
enough with NGINX to know if subsequent code expects a buffer that was
allocated in a particualar way.  If header values were ever passed to
ngx_pfree(), munging them to skip leading whitespace would mean that a
request with leading whitespace would cause ngx_pfree() to be called
with an invalid pointer, which would be a security vulnerability.
Rejecting the request doesn't introduce any new error paths that clients
cannot already trigger, and it doesn't risk violating any invariants
that existing code might assume.  Also, Varnish Cache rejects HTTP/2
requests with leading and/or trailing whitespace in field values, so
there is precedent for doing so.
This commit is contained in:
Demi Marie Obenour 2025-03-25 14:58:32 -04:00
parent 3a45410074
commit c68c7adcdb
4 changed files with 33 additions and 6 deletions

View File

@ -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,
@ -178,6 +177,8 @@ ngx_uint_t ngx_http_degraded(ngx_http_request_t *);
#if (NGX_HTTP_V2 || NGX_HTTP_V3)
ngx_int_t ngx_http_v23_validate_header(ngx_http_request_t *r, ngx_str_t *name,
ngx_str_t *value);
ngx_int_t ngx_http_huff_decode(u_char *state, u_char *src, size_t len,
u_char **dst, ngx_uint_t last, ngx_log_t *log);
size_t ngx_http_huff_encode(u_char *src, size_t len, u_char *dst,

View File

@ -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);

View File

@ -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;

View File

@ -1136,16 +1136,30 @@ ngx_http_v23_validate_header(ngx_http_request_t *r, ngx_str_t *name,
r->invalid_header = 1;
}
for (i = 0; i != value->len; i++) {
ch = value->data[i];
if (ch == '\0' || ch == LF || ch == CR) {
if (value->len > 0) {
if (cscf->reject_leading_trailing_whitespace
&& (value->data[0] == ' ' || value->data[0] == '\t'
|| value->data[value->len - 1] == ' '
|| value->data[value->len - 1] == '\t')) {
ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
"client sent header \"%V\" with "
"invalid value", name, value);
" leading or trailing space",
name);
return NGX_ERROR;
}
for (i = 0; i != value->len; i++) {
ch = value->data[i];
if (ch == '\0' || ch == LF || ch == CR) {
ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
"client sent header \"%V\" with "
"invalid value", name, value);
return NGX_ERROR;
}
}
}
return NGX_OK;