From dcdf7ec096f0998e689b7f0b0f7541e197eeff6a Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Thu, 17 Jun 2021 11:43:55 +0300 Subject: gRPC: handling GOAWAY with a higher last stream identifier. Previously, once received from upstream, it couldn't limit opening additional streams in a cached keepalive connection. --- src/http/modules/ngx_http_grpc_module.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src/http/modules') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 2e20e5ffd..42a540f19 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -121,6 +121,7 @@ typedef struct { unsigned done:1; unsigned status:1; unsigned rst:1; + unsigned goaway:1; ngx_http_request_t *request; @@ -1210,6 +1211,7 @@ ngx_http_grpc_reinit_request(ngx_http_request_t *r) ctx->done = 0; ctx->status = 0; ctx->rst = 0; + ctx->goaway = 0; ctx->connection = NULL; return NGX_OK; @@ -1565,6 +1567,7 @@ ngx_http_grpc_body_output_filter(void *data, ngx_chain_t *in) && ctx->out == NULL && ctx->output_closed && !ctx->output_blocked + && !ctx->goaway && ctx->state == ngx_http_grpc_st_start) { u->keepalive = 1; @@ -1714,6 +1717,8 @@ ngx_http_grpc_process_header(ngx_http_request_t *r) return NGX_HTTP_UPSTREAM_INVALID_HEADER; } + ctx->goaway = 1; + continue; } @@ -1907,6 +1912,7 @@ ngx_http_grpc_process_header(ngx_http_request_t *r) && ctx->out == NULL && ctx->output_closed && !ctx->output_blocked + && !ctx->goaway && b->last == b->pos) { u->keepalive = 1; @@ -2035,6 +2041,7 @@ ngx_http_grpc_filter(void *data, ssize_t bytes) if (ctx->in == NULL && ctx->output_closed && !ctx->output_blocked + && !ctx->goaway && ctx->state == ngx_http_grpc_st_start) { u->keepalive = 1; @@ -2204,6 +2211,8 @@ ngx_http_grpc_filter(void *data, ssize_t bytes) return NGX_ERROR; } + ctx->goaway = 1; + continue; } -- cgit From 693e4134a51b4fd30689ad1e31e6fdffe5ee1429 Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Thu, 17 Jun 2021 11:44:06 +0300 Subject: gRPC: RST_STREAM(NO_ERROR) handling micro-optimization. After 2096b21fcd10, a single RST_STREAM(NO_ERROR) may not result in an error. This change removes several unnecessary ctx->type checks for such a case. --- src/http/modules/ngx_http_grpc_module.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/http/modules') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 42a540f19..654bd17b9 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -2177,6 +2177,8 @@ ngx_http_grpc_filter(void *data, ssize_t bytes) } ctx->rst = 1; + + continue; } if (ctx->type == NGX_HTTP_V2_GOAWAY_FRAME) { @@ -3484,6 +3486,8 @@ ngx_http_grpc_parse_rst_stream(ngx_http_request_t *r, ngx_http_grpc_ctx_t *ctx, return NGX_AGAIN; } + ctx->state = ngx_http_grpc_st_start; + return NGX_OK; } -- cgit From 05395f4889cf0b66e8d049921ad19f1a08319150 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Mon, 28 Jun 2021 18:01:13 +0300 Subject: Disabled spaces in URIs (ticket #196). From now on, requests with spaces in URIs are immediately rejected rather than allowed. Spaces were allowed in 31e9677b15a1 (0.8.41) to handle bad clients. It is believed that now this behaviour causes more harm than good. --- src/http/modules/ngx_http_proxy_module.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/http/modules') diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c index 64190f1a0..d82f5ea21 100644 --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -1186,7 +1186,7 @@ ngx_http_proxy_create_key(ngx_http_request_t *r) loc_len = (r->valid_location && ctx->vars.uri.len) ? plcf->location.len : 0; - if (r->quoted_uri || r->space_in_uri || r->internal) { + if (r->quoted_uri || r->internal) { escape = 2 * ngx_escape_uri(NULL, r->uri.data + loc_len, r->uri.len - loc_len, NGX_ESCAPE_URI); } else { @@ -1299,7 +1299,7 @@ ngx_http_proxy_create_request(ngx_http_request_t *r) loc_len = (r->valid_location && ctx->vars.uri.len) ? plcf->location.len : 0; - if (r->quoted_uri || r->space_in_uri || r->internal) { + if (r->quoted_uri || r->internal) { escape = 2 * ngx_escape_uri(NULL, r->uri.data + loc_len, r->uri.len - loc_len, NGX_ESCAPE_URI); } -- cgit From 9ab4d368af63e9c4a0bebc0eda82d668adaa560a Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Mon, 28 Jun 2021 18:01:18 +0300 Subject: Disabled control characters and space in header names. Control characters (0x00-0x1f, 0x7f), space, and colon were never allowed in header names. The only somewhat valid use is header continuation which nginx never supported and which is explicitly obsolete by RFC 7230. Previously, such headers were considered invalid and were ignored by default (as per ignore_invalid_headers directive). With this change, such headers are unconditionally rejected. It is expected to make nginx more resilient to various attacks, in particular, with ignore_invalid_headers switched off (which is inherently unsecure, though nevertheless sometimes used in the wild). --- src/http/modules/ngx_http_grpc_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/http/modules') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 654bd17b9..65bd1e6c3 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -3384,7 +3384,7 @@ ngx_http_grpc_validate_header_name(ngx_http_request_t *r, ngx_str_t *s) return NGX_ERROR; } - if (ch == '\0' || ch == CR || ch == LF) { + if (ch <= 0x20 || ch == 0x7f) { return NGX_ERROR; } } -- cgit From 7587778a33bea0ce6f203a8c4de18e33f38b9582 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Mon, 28 Jun 2021 18:01:20 +0300 Subject: Improved logging of invalid headers. In 71edd9192f24 logging of invalid headers which were rejected with the NGX_HTTP_PARSE_INVALID_HEADER error was restricted to just the "client sent invalid header line" message, without any attempts to log the header itself. This patch returns logging of the header up to the invalid character and the character itself. The r->header_end pointer is now properly set in all cases to make logging possible. The same logging is also introduced when parsing headers from upstream servers. --- src/http/modules/ngx_http_fastcgi_module.c | 8 +++++--- src/http/modules/ngx_http_proxy_module.c | 8 +++++--- src/http/modules/ngx_http_scgi_module.c | 8 +++++--- src/http/modules/ngx_http_uwsgi_module.c | 8 +++++--- 4 files changed, 20 insertions(+), 12 deletions(-) (limited to 'src/http/modules') diff --git a/src/http/modules/ngx_http_fastcgi_module.c b/src/http/modules/ngx_http_fastcgi_module.c index 5191880e3..69ac0f72c 100644 --- a/src/http/modules/ngx_http_fastcgi_module.c +++ b/src/http/modules/ngx_http_fastcgi_module.c @@ -2019,10 +2019,12 @@ ngx_http_fastcgi_process_header(ngx_http_request_t *r) break; } - /* there was error while a header line parsing */ + /* rc == NGX_HTTP_PARSE_INVALID_HEADER */ - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream sent invalid header"); + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "upstream sent invalid header: \"%*s\\x%02xd...\"", + r->header_end - r->header_name_start, + r->header_name_start, *r->header_end); return NGX_HTTP_UPSTREAM_INVALID_HEADER; } diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c index d82f5ea21..368297e77 100644 --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -2019,10 +2019,12 @@ ngx_http_proxy_process_header(ngx_http_request_t *r) return NGX_AGAIN; } - /* there was error while a header line parsing */ + /* rc == NGX_HTTP_PARSE_INVALID_HEADER */ - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream sent invalid header"); + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "upstream sent invalid header: \"%*s\\x%02xd...\"", + r->header_end - r->header_name_start, + r->header_name_start, *r->header_end); return NGX_HTTP_UPSTREAM_INVALID_HEADER; } diff --git a/src/http/modules/ngx_http_scgi_module.c b/src/http/modules/ngx_http_scgi_module.c index 600999c88..570713df9 100644 --- a/src/http/modules/ngx_http_scgi_module.c +++ b/src/http/modules/ngx_http_scgi_module.c @@ -1140,10 +1140,12 @@ ngx_http_scgi_process_header(ngx_http_request_t *r) return NGX_AGAIN; } - /* there was error while a header line parsing */ + /* rc == NGX_HTTP_PARSE_INVALID_HEADER */ - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream sent invalid header"); + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "upstream sent invalid header: \"%*s\\x%02xd...\"", + r->header_end - r->header_name_start, + r->header_name_start, *r->header_end); return NGX_HTTP_UPSTREAM_INVALID_HEADER; } diff --git a/src/http/modules/ngx_http_uwsgi_module.c b/src/http/modules/ngx_http_uwsgi_module.c index 655be98c7..40a06c78e 100644 --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -1361,10 +1361,12 @@ ngx_http_uwsgi_process_header(ngx_http_request_t *r) return NGX_AGAIN; } - /* there was error while a header line parsing */ + /* rc == NGX_HTTP_PARSE_INVALID_HEADER */ - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "upstream sent invalid header"); + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "upstream sent invalid header: \"%*s\\x%02xd...\"", + r->header_end - r->header_name_start, + r->header_name_start, *r->header_end); return NGX_HTTP_UPSTREAM_INVALID_HEADER; } -- cgit