From 0844f630a1a48753f67d658ce6f3dd9bfb2d909e Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Tue, 25 Mar 2025 14:58:32 -0400 Subject: [PATCH 1/5] HTTP: Use common header validation function for HTTP/2 and HTTP/3 The header validation required by HTTP/2 and HTTP/3 is identical, so use a common function for both. This will make it easier to add additional validation in the future. Move the function to ngx_http_parse.c so that it can share code with the HTTP/1.x parser in the future. No functional change intended. --- src/http/ngx_http.h | 2 + src/http/ngx_http_parse.c | 62 +++++++++++++++++++++++++++++++ src/http/v2/ngx_http_v2.c | 57 +--------------------------- src/http/v3/ngx_http_v3_request.c | 55 +-------------------------- 4 files changed, 67 insertions(+), 109 deletions(-) diff --git a/src/http/ngx_http.h b/src/http/ngx_http.h index cb4a1e68a..b31cd4a1d 100644 --- a/src/http/ngx_http.h +++ b/src/http/ngx_http.h @@ -182,6 +182,8 @@ 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, ngx_uint_t lower); +ngx_int_t ngx_http_v23_validate_header(ngx_http_request_t *r, + ngx_str_t *name, ngx_str_t *value); #endif diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c index a45c04554..f1d2f9b5e 100644 --- a/src/http/ngx_http_parse.c +++ b/src/http/ngx_http_parse.c @@ -1090,6 +1090,68 @@ header_done: } +#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) +{ + u_char ch; + ngx_uint_t i; + ngx_http_core_srv_conf_t *cscf; + + r->invalid_header = 0; + + cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); + + if (name->len < 1) { + ngx_log_error(NGX_LOG_EMERG, r->connection->log, 0, + "BUG: internal zero-length header name"); + + return NGX_ERROR; + } + + for (i = (name->data[0] == ':'); i != name->len; i++) { + ch = name->data[i]; + + if ((ch >= 'a' && ch <= 'z') + || (ch == '-') + || (ch >= '0' && ch <= '9') + || (ch == '_' && cscf->underscores_in_headers)) + { + continue; + } + + if (ch <= 0x20 || ch == 0x7f || ch == ':' + || (ch >= 'A' && ch <= 'Z')) + { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent invalid header name: \"%V\"", + name); + + return NGX_ERROR; + } + + r->invalid_header = 1; + } + + 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: \"%V\"", + name, value); + + return NGX_ERROR; + } + } + + return NGX_OK; +} +#endif + + ngx_int_t ngx_http_parse_uri(ngx_http_request_t *r) { diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index 91a28b228..25b71aa4a 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -142,8 +142,6 @@ static ngx_http_v2_out_frame_t *ngx_http_v2_get_frame( static ngx_int_t ngx_http_v2_frame_handler(ngx_http_v2_connection_t *h2c, ngx_http_v2_out_frame_t *frame); -static ngx_int_t ngx_http_v2_validate_header(ngx_http_request_t *r, - ngx_http_v2_header_t *header); static ngx_int_t ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header); static ngx_int_t ngx_http_v2_parse_path(ngx_http_request_t *r, @@ -1774,7 +1772,8 @@ ngx_http_v2_state_process_header(ngx_http_v2_connection_t *h2c, u_char *pos, fc = r->connection; /* TODO Optimization: validate headers while parsing. */ - if (ngx_http_v2_validate_header(r, header) != NGX_OK) { + if (ngx_http_v23_validate_header(r, &header->name, &header->value) + != NGX_OK) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); goto error; } @@ -3232,58 +3231,6 @@ ngx_http_v2_get_closed_node(ngx_http_v2_connection_t *h2c) } -static ngx_int_t -ngx_http_v2_validate_header(ngx_http_request_t *r, ngx_http_v2_header_t *header) -{ - u_char ch; - ngx_uint_t i; - ngx_http_core_srv_conf_t *cscf; - - r->invalid_header = 0; - - cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); - - for (i = (header->name.data[0] == ':'); i != header->name.len; i++) { - ch = header->name.data[i]; - - if ((ch >= 'a' && ch <= 'z') - || (ch == '-') - || (ch >= '0' && ch <= '9') - || (ch == '_' && cscf->underscores_in_headers)) - { - continue; - } - - if (ch <= 0x20 || ch == 0x7f || ch == ':' - || (ch >= 'A' && ch <= 'Z')) - { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid header name: \"%V\"", - &header->name); - - return NGX_ERROR; - } - - r->invalid_header = 1; - } - - for (i = 0; i != header->value.len; i++) { - ch = header->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: \"%V\"", - &header->name, &header->value); - - return NGX_ERROR; - } - } - - return NGX_OK; -} - - static ngx_int_t ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header) { diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index e41ad50a8..e2ffc7543 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -17,8 +17,6 @@ static void ngx_http_v3_cleanup_request(void *data); static void ngx_http_v3_process_request(ngx_event_t *rev); static ngx_int_t ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value); -static ngx_int_t ngx_http_v3_validate_header(ngx_http_request_t *r, - ngx_str_t *name, ngx_str_t *value); static ngx_int_t ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value); static ngx_int_t ngx_http_v3_init_pseudo_headers(ngx_http_request_t *r); @@ -632,7 +630,7 @@ ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, r->v3_parse->header_limit -= len; - if (ngx_http_v3_validate_header(r, name, value) != NGX_OK) { + if (ngx_http_v23_validate_header(r, name, value) != NGX_OK) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); return NGX_ERROR; } @@ -692,57 +690,6 @@ ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, } -static ngx_int_t -ngx_http_v3_validate_header(ngx_http_request_t *r, ngx_str_t *name, - ngx_str_t *value) -{ - u_char ch; - ngx_uint_t i; - ngx_http_core_srv_conf_t *cscf; - - r->invalid_header = 0; - - cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); - - for (i = (name->data[0] == ':'); i != name->len; i++) { - ch = name->data[i]; - - if ((ch >= 'a' && ch <= 'z') - || (ch == '-') - || (ch >= '0' && ch <= '9') - || (ch == '_' && cscf->underscores_in_headers)) - { - continue; - } - - if (ch <= 0x20 || ch == 0x7f || ch == ':' - || (ch >= 'A' && ch <= 'Z')) - { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid header name: \"%V\"", name); - - return NGX_ERROR; - } - - r->invalid_header = 1; - } - - 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: \"%V\"", name, value); - - return NGX_ERROR; - } - } - - return NGX_OK; -} - - static ngx_int_t ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value) From a7ee5bffcccb2fc4a3dc82351ca7ac2413614f75 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 7 Apr 2025 17:30:13 -0400 Subject: [PATCH 2/5] Strip leading and trailing whitespace from HTTP field values Per RFC9110, HTTP field values never contain leading or trailing whitespace. Strip all such whitespace from HTTP and HTTP field values. The HTTP/1.x parser already stripped spaces but didn't strip tabs, so change the parser to strip tabs as well. In HTTP/2+, the stripping is done during validation. This requires modifying the value. There are three ways to modify the value: 1. Modify the data in-place with memmove(). 2. Move the data pointer to point to after the leading whitespace. 3. Allocate a new buffer and replace the data pointer. Both HPACK and QPACK decompression make a copy of the data, but some code might assume that the data pointer of a field value can safely be passed to ngx_pfree(). Therefore, the first option is chosen. Existing code ensures that header values are NUL-terminated, so the stripping code NUL-pads header values to ensure that the stripped strings have at least as many terminating NUL bytes as they did before being stripped. The stripping code has been tested in a standalone program to make sure that it works correctly, and it correctly strips leading and trailing whitespace from a variety of strings. This code has also been tested with real HTTP/3 requests from Cloudflare's h3i tool. Fixes: #187 Fixes: #598 --- src/http/ngx_http.h | 6 ++- src/http/ngx_http_parse.c | 71 ++++++++++++++++++++++++++++++- src/http/v2/ngx_http_v2.c | 3 +- src/http/v3/ngx_http_v3_request.c | 2 +- 4 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/http/ngx_http.h b/src/http/ngx_http.h index b31cd4a1d..3f1b0b7e3 100644 --- a/src/http/ngx_http.h +++ b/src/http/ngx_http.h @@ -182,7 +182,11 @@ 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, ngx_uint_t lower); -ngx_int_t ngx_http_v23_validate_header(ngx_http_request_t *r, +/* + * Check if a header name and/or value is valid. If the value is valid, + * strip leading and trailing space from it. + */ +ngx_int_t ngx_http_v23_fixup_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value); #endif diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c index f1d2f9b5e..261c41c16 100644 --- a/src/http/ngx_http_parse.c +++ b/src/http/ngx_http_parse.c @@ -10,6 +10,11 @@ #include +#if (NGX_HTTP_V2 || NGX_HTTP_V3) +static inline ngx_int_t ngx_isspace(u_char ch); +#endif + + static uint32_t usual[] = { 0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */ @@ -972,6 +977,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; @@ -996,6 +1002,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; @@ -1016,6 +1023,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; @@ -1091,11 +1099,21 @@ header_done: #if (NGX_HTTP_V2 || NGX_HTTP_V3) + + +static inline ngx_int_t +ngx_isspace(u_char ch) +{ + return ch == ' ' || ch == '\t'; +} + + ngx_int_t -ngx_http_v23_validate_header(ngx_http_request_t *r, ngx_str_t *name, +ngx_http_v23_fixup_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value) { u_char ch; + ngx_str_t tmp; ngx_uint_t i; ngx_http_core_srv_conf_t *cscf; @@ -1134,6 +1152,11 @@ ngx_http_v23_validate_header(ngx_http_request_t *r, ngx_str_t *name, r->invalid_header = 1; } + /* 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]; @@ -1147,6 +1170,52 @@ ngx_http_v23_validate_header(ngx_http_request_t *r, ngx_str_t *name, } } + tmp = *value; + + if (!ngx_isspace(tmp.data[0]) + && !ngx_isspace(tmp.data[tmp.len - 1])) { + /* Fast path: nothing to strip. */ + return NGX_OK; + } + + /* + * 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; } #endif diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index 25b71aa4a..11e082ca3 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1772,8 +1772,7 @@ ngx_http_v2_state_process_header(ngx_http_v2_connection_t *h2c, u_char *pos, fc = r->connection; /* TODO Optimization: validate headers while parsing. */ - if (ngx_http_v23_validate_header(r, &header->name, &header->value) - != NGX_OK) { + if (ngx_http_v23_fixup_header(r, &header->name, &header->value) != NGX_OK) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); goto error; } diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index e2ffc7543..7aade0400 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -630,7 +630,7 @@ ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, r->v3_parse->header_limit -= len; - if (ngx_http_v23_validate_header(r, name, value) != NGX_OK) { + if (ngx_http_v23_fixup_header(r, name, value) != NGX_OK) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); return NGX_ERROR; } From ed84f401cf729f2cbfdf033b2e33848a90d9d9d4 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Fri, 21 Mar 2025 19:48:30 -0400 Subject: [PATCH 3/5] HTTP/3: Do not allow invalid pseudo-header fields RFC9114 requires invalid pseudo-header fields to be rejected, and this is consistent with HTTP/2. --- src/http/v3/ngx_http_v3_request.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index 7aade0400..fff5dd909 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -635,6 +635,10 @@ ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, return NGX_ERROR; } + if (name->len && name->data[0] == ':') { + return ngx_http_v3_process_pseudo_header(r, name, value); + } + if (r->invalid_header) { cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); @@ -646,10 +650,6 @@ ngx_http_v3_process_header(ngx_http_request_t *r, ngx_str_t *name, } } - if (name->len && name->data[0] == ':') { - return ngx_http_v3_process_pseudo_header(r, name, value); - } - if (ngx_http_v3_init_pseudo_headers(r) != NGX_OK) { return NGX_ERROR; } From da7f73eb713eeaf9e7487d02d1c66d30e8247523 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sat, 22 Mar 2025 01:03:00 -0400 Subject: [PATCH 4/5] HTTP: Use common header code for v2 and v3 This makes the behavior of HTTP/2 and HTTP/3 much more similar. In particular, the HTTP/3 :authority pseudoheader is used to set the Host header, instead of the virtual server. This is arguably less correct, but it is consistent with the existing HTTP/2 behavior and unbreaks users of PHP-FPM and other FastCGI applications. In the future, NGINX could have a config option that caused :authority and Host to be treated separately in both HTTP/2 and HTTP/3. Fixes: #587 Fixes: #256 --- src/http/ngx_http.c | 300 ++++++++++++++++++++++++++++++ src/http/ngx_http.h | 2 + src/http/v2/ngx_http_v2.c | 293 +---------------------------- src/http/v3/ngx_http_v3_request.c | 237 ++--------------------- 4 files changed, 319 insertions(+), 513 deletions(-) diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c index d835f896e..7a02db7ea 100644 --- a/src/http/ngx_http.c +++ b/src/http/ngx_http.c @@ -68,6 +68,17 @@ static ngx_int_t ngx_http_add_addrs6(ngx_conf_t *cf, ngx_http_port_t *hport, ngx_http_conf_addr_t *addr); #endif +#if (NGX_HTTP_V2 || NGX_HTTP_V3) +static ngx_int_t ngx_http_v23_parse_path(ngx_http_request_t *r, + ngx_str_t *value); +static ngx_int_t ngx_http_v23_parse_method(ngx_http_request_t *r, + ngx_str_t *value); +static ngx_int_t ngx_http_v23_parse_scheme(ngx_http_request_t *r, + ngx_str_t *value); +static ngx_int_t ngx_http_v23_parse_authority(ngx_http_request_t *r, + ngx_str_t *value); +#endif + ngx_uint_t ngx_http_max_module; @@ -1227,6 +1238,295 @@ ngx_http_add_listen(ngx_conf_t *cf, ngx_http_core_srv_conf_t *cscf, return ngx_http_add_address(cf, cscf, port, lsopt); } +#if (NGX_HTTP_V2 || NGX_HTTP_V3) + +ngx_int_t +ngx_http_v23_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, + ngx_str_t *value) +{ + ngx_int_t rc; + + name->len--; + name->data++; + + switch (name->len) { + case 4: + if (ngx_memcmp(name->data, "path", sizeof("path") - 1) + == 0) + { + rc = ngx_http_v23_parse_path(r, value); + goto known; + } + + break; + + case 6: + if (ngx_memcmp(name->data, "method", sizeof("method") - 1) + == 0) + { + rc = ngx_http_v23_parse_method(r, value); + goto known; + } + + if (ngx_memcmp(name->data, "scheme", sizeof("scheme") - 1) + == 0) + { + rc = ngx_http_v23_parse_scheme(r, value); + goto known; + } + + break; + + case 9: + if (ngx_memcmp(name->data, "authority", sizeof("authority") - 1) + == 0) + { + rc = ngx_http_v23_parse_authority(r, value); + goto known; + } + + break; + } + + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent unknown pseudo-header \":%V\"", + name); + rc = NGX_DECLINED; +known: + if (rc == NGX_DECLINED) { + ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); + rc = NGX_ABORT; + } + return rc; +} + + +static ngx_int_t +ngx_http_v23_parse_path(ngx_http_request_t *r, ngx_str_t *value) +{ + if (r->unparsed_uri.len) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent duplicate :path header"); + + return NGX_DECLINED; + } + + if (value->len == 0) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent empty :path header"); + + return NGX_DECLINED; + } + + r->uri_start = value->data; + r->uri_end = value->data + value->len; + + if (ngx_http_parse_uri(r) != NGX_OK) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent invalid :path header: \"%V\"", value); + + return NGX_DECLINED; + } + + if (ngx_http_process_request_uri(r) != NGX_OK) { + /* + * request has been finalized already + * in ngx_http_process_request_uri() + */ + return NGX_ABORT; + } + + return NGX_OK; +} + + +static ngx_int_t +ngx_http_v23_parse_method(ngx_http_request_t *r, ngx_str_t *value) +{ + size_t k, len; + ngx_uint_t n; + const u_char *p, *m; + + /* + * This array takes less than 256 sequential bytes, + * and if typical CPU cache line size is 64 bytes, + * it is prefetched for 4 load operations. + */ + static const struct { + u_char len; + const u_char method[11]; + uint32_t value; + } tests[] = { + { 3, "GET", NGX_HTTP_GET }, + { 4, "POST", NGX_HTTP_POST }, + { 4, "HEAD", NGX_HTTP_HEAD }, + { 7, "OPTIONS", NGX_HTTP_OPTIONS }, + { 8, "PROPFIND", NGX_HTTP_PROPFIND }, + { 3, "PUT", NGX_HTTP_PUT }, + { 5, "MKCOL", NGX_HTTP_MKCOL }, + { 6, "DELETE", NGX_HTTP_DELETE }, + { 4, "COPY", NGX_HTTP_COPY }, + { 4, "MOVE", NGX_HTTP_MOVE }, + { 9, "PROPPATCH", NGX_HTTP_PROPPATCH }, + { 4, "LOCK", NGX_HTTP_LOCK }, + { 6, "UNLOCK", NGX_HTTP_UNLOCK }, + { 5, "PATCH", NGX_HTTP_PATCH }, + { 5, "TRACE", NGX_HTTP_TRACE }, + { 7, "CONNECT", NGX_HTTP_CONNECT } + }, *test; + + if (r->method_name.len) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent duplicate :method header"); + + return NGX_DECLINED; + } + + if (value->len == 0) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent empty :method header"); + + return NGX_DECLINED; + } + + r->method_name.len = value->len; + r->method_name.data = value->data; + + len = r->method_name.len; + n = sizeof(tests) / sizeof(tests[0]); + test = tests; + + do { + if (len == test->len) { + p = r->method_name.data; + m = test->method; + k = len; + + do { + if (*p++ != *m++) { + goto next; + } + } while (--k); + + r->method = test->value; + return NGX_OK; + } + + next: + test++; + + } while (--n); + + p = r->method_name.data; + + do { + if ((*p < 'A' || *p > 'Z') && *p != '_' && *p != '-') { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent invalid method: \"%V\"", + &r->method_name); + + return NGX_DECLINED; + } + + p++; + + } while (--len); + + return NGX_OK; +} + + +static ngx_int_t +ngx_http_v23_parse_scheme(ngx_http_request_t *r, ngx_str_t *value) +{ + u_char c, ch; + ngx_uint_t i; + + if (r->schema.len) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent duplicate :scheme header"); + + return NGX_DECLINED; + } + + if (value->len == 0) { + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent empty :scheme header"); + + return NGX_DECLINED; + } + + for (i = 0; i < value->len; i++) { + ch = value->data[i]; + + c = (u_char) (ch | 0x20); + if (c >= 'a' && c <= 'z') { + continue; + } + + if (((ch >= '0' && ch <= '9') || ch == '+' || ch == '-' || ch == '.') + && i > 0) + { + continue; + } + + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent invalid :scheme header: \"%V\"", value); + + return NGX_DECLINED; + } + + r->schema = *value; + + return NGX_OK; +} + + +static ngx_int_t +ngx_http_v23_parse_authority(ngx_http_request_t *r, ngx_str_t *value) +{ + ngx_table_elt_t *h; + ngx_http_header_t *hh; + ngx_http_core_main_conf_t *cmcf; + + static ngx_str_t host = ngx_string("host"); + + h = ngx_list_push(&r->headers_in.headers); + if (h == NULL) { + return NGX_ERROR; + } + + h->hash = ngx_hash(ngx_hash(ngx_hash('h', 'o'), 's'), 't'); + + h->key.len = host.len; + h->key.data = host.data; + + h->value.len = value->len; + h->value.data = value->data; + + h->lowcase_key = host.data; + + cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module); + + hh = ngx_hash_find(&cmcf->headers_in_hash, h->hash, + h->lowcase_key, h->key.len); + + if (hh == NULL) { + return NGX_ERROR; + } + + if (hh->handler(r, h, hh->offset) != NGX_OK) { + /* + * request has been finalized already + * in ngx_http_process_host() + */ + return NGX_ABORT; + } + + return NGX_OK; +} + +#endif static ngx_int_t ngx_http_add_addresses(ngx_conf_t *cf, ngx_http_core_srv_conf_t *cscf, diff --git a/src/http/ngx_http.h b/src/http/ngx_http.h index 3f1b0b7e3..1bfb7aa7f 100644 --- a/src/http/ngx_http.h +++ b/src/http/ngx_http.h @@ -188,6 +188,8 @@ size_t ngx_http_huff_encode(u_char *src, size_t len, u_char *dst, */ ngx_int_t ngx_http_v23_fixup_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value); +ngx_int_t ngx_http_v23_pseudo_header(ngx_http_request_t *r, + ngx_str_t *name, ngx_str_t *value); #endif diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index 11e082ca3..daf0cffdc 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -142,16 +142,6 @@ static ngx_http_v2_out_frame_t *ngx_http_v2_get_frame( static ngx_int_t ngx_http_v2_frame_handler(ngx_http_v2_connection_t *h2c, ngx_http_v2_out_frame_t *frame); -static ngx_int_t ngx_http_v2_pseudo_header(ngx_http_request_t *r, - ngx_http_v2_header_t *header); -static ngx_int_t ngx_http_v2_parse_path(ngx_http_request_t *r, - ngx_str_t *value); -static ngx_int_t ngx_http_v2_parse_method(ngx_http_request_t *r, - ngx_str_t *value); -static ngx_int_t ngx_http_v2_parse_scheme(ngx_http_request_t *r, - ngx_str_t *value); -static ngx_int_t ngx_http_v2_parse_authority(ngx_http_request_t *r, - ngx_str_t *value); static ngx_int_t ngx_http_v2_construct_request_line(ngx_http_request_t *r); static ngx_int_t ngx_http_v2_cookie(ngx_http_request_t *r, ngx_http_v2_header_t *header); @@ -1778,7 +1768,7 @@ ngx_http_v2_state_process_header(ngx_http_v2_connection_t *h2c, u_char *pos, } if (header->name.data[0] == ':') { - rc = ngx_http_v2_pseudo_header(r, header); + rc = ngx_http_v23_pseudo_header(r, &header->name, &header->value); if (rc == NGX_OK) { ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, @@ -1792,11 +1782,6 @@ ngx_http_v2_state_process_header(ngx_http_v2_connection_t *h2c, u_char *pos, goto error; } - if (rc == NGX_DECLINED) { - ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); - goto error; - } - return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR); } @@ -3229,282 +3214,6 @@ ngx_http_v2_get_closed_node(ngx_http_v2_connection_t *h2c) return node; } - -static ngx_int_t -ngx_http_v2_pseudo_header(ngx_http_request_t *r, ngx_http_v2_header_t *header) -{ - header->name.len--; - header->name.data++; - - switch (header->name.len) { - case 4: - if (ngx_memcmp(header->name.data, "path", sizeof("path") - 1) - == 0) - { - return ngx_http_v2_parse_path(r, &header->value); - } - - break; - - case 6: - if (ngx_memcmp(header->name.data, "method", sizeof("method") - 1) - == 0) - { - return ngx_http_v2_parse_method(r, &header->value); - } - - if (ngx_memcmp(header->name.data, "scheme", sizeof("scheme") - 1) - == 0) - { - return ngx_http_v2_parse_scheme(r, &header->value); - } - - break; - - case 9: - if (ngx_memcmp(header->name.data, "authority", sizeof("authority") - 1) - == 0) - { - return ngx_http_v2_parse_authority(r, &header->value); - } - - break; - } - - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent unknown pseudo-header \":%V\"", - &header->name); - - return NGX_DECLINED; -} - - -static ngx_int_t -ngx_http_v2_parse_path(ngx_http_request_t *r, ngx_str_t *value) -{ - if (r->unparsed_uri.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate :path header"); - - return NGX_DECLINED; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty :path header"); - - return NGX_DECLINED; - } - - r->uri_start = value->data; - r->uri_end = value->data + value->len; - - if (ngx_http_parse_uri(r) != NGX_OK) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid :path header: \"%V\"", value); - - return NGX_DECLINED; - } - - if (ngx_http_process_request_uri(r) != NGX_OK) { - /* - * request has been finalized already - * in ngx_http_process_request_uri() - */ - return NGX_ABORT; - } - - return NGX_OK; -} - - -static ngx_int_t -ngx_http_v2_parse_method(ngx_http_request_t *r, ngx_str_t *value) -{ - size_t k, len; - ngx_uint_t n; - const u_char *p, *m; - - /* - * This array takes less than 256 sequential bytes, - * and if typical CPU cache line size is 64 bytes, - * it is prefetched for 4 load operations. - */ - static const struct { - u_char len; - const u_char method[11]; - uint32_t value; - } tests[] = { - { 3, "GET", NGX_HTTP_GET }, - { 4, "POST", NGX_HTTP_POST }, - { 4, "HEAD", NGX_HTTP_HEAD }, - { 7, "OPTIONS", NGX_HTTP_OPTIONS }, - { 8, "PROPFIND", NGX_HTTP_PROPFIND }, - { 3, "PUT", NGX_HTTP_PUT }, - { 5, "MKCOL", NGX_HTTP_MKCOL }, - { 6, "DELETE", NGX_HTTP_DELETE }, - { 4, "COPY", NGX_HTTP_COPY }, - { 4, "MOVE", NGX_HTTP_MOVE }, - { 9, "PROPPATCH", NGX_HTTP_PROPPATCH }, - { 4, "LOCK", NGX_HTTP_LOCK }, - { 6, "UNLOCK", NGX_HTTP_UNLOCK }, - { 5, "PATCH", NGX_HTTP_PATCH }, - { 5, "TRACE", NGX_HTTP_TRACE }, - { 7, "CONNECT", NGX_HTTP_CONNECT } - }, *test; - - if (r->method_name.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate :method header"); - - return NGX_DECLINED; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty :method header"); - - return NGX_DECLINED; - } - - r->method_name.len = value->len; - r->method_name.data = value->data; - - len = r->method_name.len; - n = sizeof(tests) / sizeof(tests[0]); - test = tests; - - do { - if (len == test->len) { - p = r->method_name.data; - m = test->method; - k = len; - - do { - if (*p++ != *m++) { - goto next; - } - } while (--k); - - r->method = test->value; - return NGX_OK; - } - - next: - test++; - - } while (--n); - - p = r->method_name.data; - - do { - if ((*p < 'A' || *p > 'Z') && *p != '_' && *p != '-') { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid method: \"%V\"", - &r->method_name); - - return NGX_DECLINED; - } - - p++; - - } while (--len); - - return NGX_OK; -} - - -static ngx_int_t -ngx_http_v2_parse_scheme(ngx_http_request_t *r, ngx_str_t *value) -{ - u_char c, ch; - ngx_uint_t i; - - if (r->schema.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate :scheme header"); - - return NGX_DECLINED; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty :scheme header"); - - return NGX_DECLINED; - } - - for (i = 0; i < value->len; i++) { - ch = value->data[i]; - - c = (u_char) (ch | 0x20); - if (c >= 'a' && c <= 'z') { - continue; - } - - if (((ch >= '0' && ch <= '9') || ch == '+' || ch == '-' || ch == '.') - && i > 0) - { - continue; - } - - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid :scheme header: \"%V\"", value); - - return NGX_DECLINED; - } - - r->schema = *value; - - return NGX_OK; -} - - -static ngx_int_t -ngx_http_v2_parse_authority(ngx_http_request_t *r, ngx_str_t *value) -{ - ngx_table_elt_t *h; - ngx_http_header_t *hh; - ngx_http_core_main_conf_t *cmcf; - - static ngx_str_t host = ngx_string("host"); - - h = ngx_list_push(&r->headers_in.headers); - if (h == NULL) { - return NGX_ERROR; - } - - h->hash = ngx_hash(ngx_hash(ngx_hash('h', 'o'), 's'), 't'); - - h->key.len = host.len; - h->key.data = host.data; - - h->value.len = value->len; - h->value.data = value->data; - - h->lowcase_key = host.data; - - cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module); - - hh = ngx_hash_find(&cmcf->headers_in_hash, h->hash, - h->lowcase_key, h->key.len); - - if (hh == NULL) { - return NGX_ERROR; - } - - if (hh->handler(r, h, hh->offset) != NGX_OK) { - /* - * request has been finalized already - * in ngx_http_process_host() - */ - return NGX_ABORT; - } - - return NGX_OK; -} - - static ngx_int_t ngx_http_v2_construct_request_line(ngx_http_request_t *r) { diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c index fff5dd909..3e22b4414 100644 --- a/src/http/v3/ngx_http_v3_request.c +++ b/src/http/v3/ngx_http_v3_request.c @@ -28,31 +28,6 @@ static ngx_int_t ngx_http_v3_do_read_client_request_body(ngx_http_request_t *r); static ngx_int_t ngx_http_v3_request_body_filter(ngx_http_request_t *r, ngx_chain_t *in); - -static const struct { - ngx_str_t name; - ngx_uint_t method; -} ngx_http_v3_methods[] = { - - { ngx_string("GET"), NGX_HTTP_GET }, - { ngx_string("POST"), NGX_HTTP_POST }, - { ngx_string("HEAD"), NGX_HTTP_HEAD }, - { ngx_string("OPTIONS"), NGX_HTTP_OPTIONS }, - { ngx_string("PROPFIND"), NGX_HTTP_PROPFIND }, - { ngx_string("PUT"), NGX_HTTP_PUT }, - { ngx_string("MKCOL"), NGX_HTTP_MKCOL }, - { ngx_string("DELETE"), NGX_HTTP_DELETE }, - { ngx_string("COPY"), NGX_HTTP_COPY }, - { ngx_string("MOVE"), NGX_HTTP_MOVE }, - { ngx_string("PROPPATCH"), NGX_HTTP_PROPPATCH }, - { ngx_string("LOCK"), NGX_HTTP_LOCK }, - { ngx_string("UNLOCK"), NGX_HTTP_UNLOCK }, - { ngx_string("PATCH"), NGX_HTTP_PATCH }, - { ngx_string("TRACE"), NGX_HTTP_TRACE }, - { ngx_string("CONNECT"), NGX_HTTP_CONNECT } -}; - - void ngx_http_v3_init_stream(ngx_connection_t *c) { @@ -389,6 +364,14 @@ ngx_http_v3_wait_request_handler(ngx_event_t *rev) c->data = r; c->requests = (c->quic->id >> 2) + 1; + if (ngx_list_init(&r->headers_in.headers, r->pool, 20, + sizeof(ngx_table_elt_t)) + != NGX_OK) + { + ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + return; + } + cln = ngx_pool_cleanup_add(r->pool, 0); if (cln == NULL) { ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); @@ -694,153 +677,17 @@ static ngx_int_t ngx_http_v3_process_pseudo_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value) { - u_char ch, c; - ngx_uint_t i; - if (r->request_line.len) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, "client sent out of order pseudo-headers"); - goto failed; + ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); + return NGX_ERROR; } - if (name->len == 7 && ngx_strncmp(name->data, ":method", 7) == 0) { - - if (r->method_name.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate \":method\" header"); - goto failed; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty \":method\" header"); - goto failed; - } - - r->method_name = *value; - - for (i = 0; i < sizeof(ngx_http_v3_methods) - / sizeof(ngx_http_v3_methods[0]); i++) - { - if (value->len == ngx_http_v3_methods[i].name.len - && ngx_strncmp(value->data, - ngx_http_v3_methods[i].name.data, value->len) - == 0) - { - r->method = ngx_http_v3_methods[i].method; - break; - } - } - - for (i = 0; i < value->len; i++) { - ch = value->data[i]; - - if ((ch < 'A' || ch > 'Z') && ch != '_' && ch != '-') { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid method: \"%V\"", value); - goto failed; - } - } - - ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "http3 method \"%V\" %ui", value, r->method); - return NGX_OK; + if (ngx_http_v23_pseudo_header(r, name, value) != NGX_OK) { + return NGX_ERROR; } - - if (name->len == 5 && ngx_strncmp(name->data, ":path", 5) == 0) { - - if (r->uri_start) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate \":path\" header"); - goto failed; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty \":path\" header"); - goto failed; - } - - r->uri_start = value->data; - r->uri_end = value->data + value->len; - - if (ngx_http_parse_uri(r) != NGX_OK) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid \":path\" header: \"%V\"", - value); - goto failed; - } - - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "http3 path \"%V\"", value); - return NGX_OK; - } - - if (name->len == 7 && ngx_strncmp(name->data, ":scheme", 7) == 0) { - - if (r->schema.len) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate \":scheme\" header"); - goto failed; - } - - if (value->len == 0) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty \":scheme\" header"); - goto failed; - } - - for (i = 0; i < value->len; i++) { - ch = value->data[i]; - - c = (u_char) (ch | 0x20); - if (c >= 'a' && c <= 'z') { - continue; - } - - if (((ch >= '0' && ch <= '9') - || ch == '+' || ch == '-' || ch == '.') - && i > 0) - { - continue; - } - - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent invalid \":scheme\" header: \"%V\"", - value); - goto failed; - } - - r->schema = *value; - - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "http3 schema \"%V\"", value); - return NGX_OK; - } - - if (name->len == 10 && ngx_strncmp(name->data, ":authority", 10) == 0) { - - if (r->host_start) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate \":authority\" header"); - goto failed; - } - - r->host_start = value->data; - r->host_end = value->data + value->len; - - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "http3 authority \"%V\"", value); - return NGX_OK; - } - - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent unknown pseudo-header \"%V\"", name); - -failed: - - ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); - return NGX_ERROR; + return NGX_OK; } @@ -928,14 +775,6 @@ ngx_http_v3_init_pseudo_headers(ngx_http_request_t *r) r->headers_in.server = host; } - if (ngx_list_init(&r->headers_in.headers, r->pool, 20, - sizeof(ngx_table_elt_t)) - != NGX_OK) - { - ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); - return NGX_ERROR; - } - return NGX_OK; failed: @@ -975,38 +814,11 @@ ngx_http_v3_process_request_header(ngx_http_request_t *r) return NGX_ERROR; } - if (r->headers_in.server.len == 0) { - ngx_log_error(NGX_LOG_INFO, c->log, 0, - "client sent neither \":authority\" nor \"Host\" header"); - goto failed; + if (ngx_http_process_request_header(r) != NGX_OK) { + return NGX_ERROR; } - if (r->headers_in.host) { - if (r->headers_in.host->value.len != r->headers_in.server.len - || ngx_memcmp(r->headers_in.host->value.data, - r->headers_in.server.data, - r->headers_in.server.len) - != 0) - { - ngx_log_error(NGX_LOG_INFO, c->log, 0, - "client sent \":authority\" and \"Host\" headers " - "with different values"); - goto failed; - } - } - - if (r->headers_in.content_length) { - r->headers_in.content_length_n = - ngx_atoof(r->headers_in.content_length->value.data, - r->headers_in.content_length->value.len); - - if (r->headers_in.content_length_n == NGX_ERROR) { - ngx_log_error(NGX_LOG_INFO, c->log, 0, - "client sent invalid \"Content-Length\" header"); - goto failed; - } - - } else { + if (!r->headers_in.content_length) { b = r->header_in; n = b->last - b->pos; @@ -1029,24 +841,7 @@ ngx_http_v3_process_request_header(ngx_http_request_t *r) } } - if (r->method == NGX_HTTP_CONNECT) { - ngx_log_error(NGX_LOG_INFO, c->log, 0, "client sent CONNECT method"); - ngx_http_finalize_request(r, NGX_HTTP_NOT_ALLOWED); - return NGX_ERROR; - } - - if (r->method == NGX_HTTP_TRACE) { - ngx_log_error(NGX_LOG_INFO, c->log, 0, "client sent TRACE method"); - ngx_http_finalize_request(r, NGX_HTTP_NOT_ALLOWED); - return NGX_ERROR; - } - return NGX_OK; - -failed: - - ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); - return NGX_ERROR; } From 481d7e32141d38f5f42c9b999def60eec2dac189 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 7 Apr 2025 23:07:55 -0400 Subject: [PATCH 5/5] HTTP: Reject HTTP/2 and HTTP/3 requests with Transfer-Encoding RFC9113 and RFC9114 are clear that this header cannot be used in these versions of HTTP, and in other proxies accepting Transfer-Encoding has led to security vulnerabilities. NGINX is safe from the vulnerability because it ignores the header, but this is still wrong. Fixes: #612 --- src/http/ngx_http_request.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index ceac8d307..092d25509 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -2015,9 +2015,9 @@ ngx_http_process_request_header(ngx_http_request_t *r) } if (r->headers_in.transfer_encoding) { - if (r->http_version < NGX_HTTP_VERSION_11) { + if (r->http_version != NGX_HTTP_VERSION_11) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent HTTP/1.0 request with " + "client sent non-HTTP/1.1 request with " "\"Transfer-Encoding\" header"); ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); return NGX_ERROR;