From ead9ab09255042559c5568cb5959a487fbef2fab Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 21 Apr 2021 23:24:48 +0300 Subject: Version bump. --- src/core/nginx.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/core/nginx.h b/src/core/nginx.h index 8a3560179..d8c8289a8 100644 --- a/src/core/nginx.h +++ b/src/core/nginx.h @@ -9,8 +9,8 @@ #define _NGINX_H_INCLUDED_ -#define nginx_version 1019010 -#define NGINX_VERSION "1.19.10" +#define nginx_version 1021000 +#define NGINX_VERSION "1.21.0" #define NGINX_VER "nginx/" NGINX_VERSION #ifdef NGX_BUILD -- cgit From 7b9920aad80299d79f4dba08de36693804f8751c Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 21 Apr 2021 23:24:59 +0300 Subject: Mail: fixed reading with fully filled buffer (ticket #2159). With SMTP pipelining, ngx_mail_read_command() can be called with s->buffer without any space available, to parse additional commands received to the buffer on previous calls. Previously, this resulted in recv() being called with zero length, resulting in zero being returned, which was interpreted as a connection close by the client, so nginx silently closed connection. Fix is to avoid calling c->recv() if there is no free space in the buffer, but continue parsing of the already received commands. --- src/mail/ngx_mail_handler.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/mail/ngx_mail_handler.c b/src/mail/ngx_mail_handler.c index 0aaa0e786..57503e9a6 100644 --- a/src/mail/ngx_mail_handler.c +++ b/src/mail/ngx_mail_handler.c @@ -833,20 +833,23 @@ ngx_mail_read_command(ngx_mail_session_t *s, ngx_connection_t *c) ngx_str_t l; ngx_mail_core_srv_conf_t *cscf; - n = c->recv(c, s->buffer->last, s->buffer->end - s->buffer->last); + if (s->buffer->last < s->buffer->end) { - if (n == NGX_ERROR || n == 0) { - ngx_mail_close_connection(c); - return NGX_ERROR; - } + n = c->recv(c, s->buffer->last, s->buffer->end - s->buffer->last); - if (n > 0) { - s->buffer->last += n; - } + if (n == NGX_ERROR || n == 0) { + ngx_mail_close_connection(c); + return NGX_ERROR; + } - if (n == NGX_AGAIN) { - if (s->buffer->pos == s->buffer->last) { - return NGX_AGAIN; + if (n > 0) { + s->buffer->last += n; + } + + if (n == NGX_AGAIN) { + if (s->buffer->pos == s->buffer->last) { + return NGX_AGAIN; + } } } -- cgit From f02e2a734ef472f0dcf83ab2e8ce96d1acead8a5 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Thu, 22 Apr 2021 16:12:52 +0300 Subject: Restored zeroing of ngx_channel_t in ngx_pass_open_channel(). Due to structure's alignment, some uninitialized memory contents may have been passed between processes. Zeroing was removed in 0215ec9aaa8a. Reported by Johnny Wang. --- src/os/unix/ngx_process_cycle.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/os/unix/ngx_process_cycle.c b/src/os/unix/ngx_process_cycle.c index b31485f88..07cd05e80 100644 --- a/src/os/unix/ngx_process_cycle.c +++ b/src/os/unix/ngx_process_cycle.c @@ -398,6 +398,8 @@ ngx_pass_open_channel(ngx_cycle_t *cycle) ngx_int_t i; ngx_channel_t ch; + ngx_memzero(&ch, sizeof(ngx_channel_t)); + ch.command = NGX_CMD_OPEN_CHANNEL; ch.pid = ngx_processes[ngx_process_slot].pid; ch.slot = ngx_process_slot; -- cgit From 4faa84085379346fa1cf322907a1f9b6a7fbd583 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Thu, 6 May 2021 02:22:03 +0300 Subject: Changed complex value slots to use NGX_CONF_UNSET_PTR. With this change, it is now possible to use ngx_conf_merge_ptr_value() to merge complex values. This change follows much earlier changes in ngx_conf_merge_ptr_value() and ngx_conf_set_str_array_slot() in 1452:cd586e963db0 (0.6.10) and 1701:40d004d95d88 (0.6.22), and the change in ngx_conf_set_keyval_slot() (7728:485dba3e2a01, 1.19.4). To preserve compatibility with existing 3rd party modules, both NULL and NGX_CONF_UNSET_PTR are accepted for now. --- src/http/modules/ngx_http_auth_basic_module.c | 6 +++--- src/http/modules/ngx_http_grpc_module.c | 8 +++----- src/http/modules/ngx_http_proxy_module.c | 20 +++++++++----------- src/http/modules/ngx_http_secure_link_module.c | 17 ++++++++--------- src/http/modules/ngx_http_uwsgi_module.c | 7 +++---- src/http/ngx_http_core_module.c | 14 +++++--------- src/http/ngx_http_script.c | 2 +- src/stream/ngx_stream_proxy_module.c | 18 ++++++------------ src/stream/ngx_stream_script.c | 2 +- 9 files changed, 39 insertions(+), 55 deletions(-) (limited to 'src') diff --git a/src/http/modules/ngx_http_auth_basic_module.c b/src/http/modules/ngx_http_auth_basic_module.c index ed9df3430..4e28b6ce6 100644 --- a/src/http/modules/ngx_http_auth_basic_module.c +++ b/src/http/modules/ngx_http_auth_basic_module.c @@ -357,6 +357,8 @@ ngx_http_auth_basic_create_loc_conf(ngx_conf_t *cf) return NULL; } + conf->realm = NGX_CONF_UNSET_PTR; + return conf; } @@ -367,9 +369,7 @@ ngx_http_auth_basic_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_http_auth_basic_loc_conf_t *prev = parent; ngx_http_auth_basic_loc_conf_t *conf = child; - if (conf->realm == NULL) { - conf->realm = prev->realm; - } + ngx_conf_merge_ptr_value(conf->realm, prev->realm, NULL); if (conf->user_file.value.data == NULL) { conf->user_file = prev->user_file; diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 53bc54710..8db35631c 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -4331,7 +4331,6 @@ ngx_http_grpc_create_loc_conf(ngx_conf_t *cf) * conf->upstream.ignore_headers = 0; * conf->upstream.next_upstream = 0; * conf->upstream.hide_headers_hash = { NULL, 0 }; - * conf->upstream.ssl_name = NULL; * * conf->headers.lengths = NULL; * conf->headers.values = NULL; @@ -4364,6 +4363,7 @@ ngx_http_grpc_create_loc_conf(ngx_conf_t *cf) #if (NGX_HTTP_SSL) conf->upstream.ssl_session_reuse = NGX_CONF_UNSET; + conf->upstream.ssl_name = NGX_CONF_UNSET_PTR; conf->upstream.ssl_server_name = NGX_CONF_UNSET; conf->upstream.ssl_verify = NGX_CONF_UNSET; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; @@ -4459,10 +4459,8 @@ ngx_http_grpc_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_str_value(conf->ssl_ciphers, prev->ssl_ciphers, "DEFAULT"); - if (conf->upstream.ssl_name == NULL) { - conf->upstream.ssl_name = prev->upstream.ssl_name; - } - + ngx_conf_merge_ptr_value(conf->upstream.ssl_name, + prev->upstream.ssl_name, NULL); ngx_conf_merge_value(conf->upstream.ssl_server_name, prev->upstream.ssl_server_name, 0); ngx_conf_merge_value(conf->upstream.ssl_verify, diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c index a63c3ed54..97c168b45 100644 --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -3327,9 +3327,7 @@ ngx_http_proxy_create_loc_conf(ngx_conf_t *cf) * conf->upstream.hide_headers_hash = { NULL, 0 }; * conf->upstream.store_lengths = NULL; * conf->upstream.store_values = NULL; - * conf->upstream.ssl_name = NULL; * - * conf->method = NULL; * conf->location = NULL; * conf->url = { 0, NULL }; * conf->headers.lengths = NULL; @@ -3400,6 +3398,7 @@ ngx_http_proxy_create_loc_conf(ngx_conf_t *cf) #if (NGX_HTTP_SSL) conf->upstream.ssl_session_reuse = NGX_CONF_UNSET; + conf->upstream.ssl_name = NGX_CONF_UNSET_PTR; conf->upstream.ssl_server_name = NGX_CONF_UNSET; conf->upstream.ssl_verify = NGX_CONF_UNSET; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; @@ -3410,10 +3409,13 @@ ngx_http_proxy_create_loc_conf(ngx_conf_t *cf) /* "proxy_cyclic_temp_file" is disabled */ conf->upstream.cyclic_temp_file = 0; + conf->upstream.change_buffering = 1; + conf->headers_source = NGX_CONF_UNSET_PTR; + conf->method = NGX_CONF_UNSET_PTR; + conf->redirect = NGX_CONF_UNSET; - conf->upstream.change_buffering = 1; conf->cookie_domains = NGX_CONF_UNSET_PTR; conf->cookie_paths = NGX_CONF_UNSET_PTR; @@ -3708,10 +3710,6 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) #endif - if (conf->method == NULL) { - conf->method = prev->method; - } - ngx_conf_merge_value(conf->upstream.pass_request_headers, prev->upstream.pass_request_headers, 1); ngx_conf_merge_value(conf->upstream.pass_request_body, @@ -3732,10 +3730,8 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_str_value(conf->ssl_ciphers, prev->ssl_ciphers, "DEFAULT"); - if (conf->upstream.ssl_name == NULL) { - conf->upstream.ssl_name = prev->upstream.ssl_name; - } - + ngx_conf_merge_ptr_value(conf->upstream.ssl_name, + prev->upstream.ssl_name, NULL); ngx_conf_merge_value(conf->upstream.ssl_server_name, prev->upstream.ssl_server_name, 0); ngx_conf_merge_value(conf->upstream.ssl_verify, @@ -3761,6 +3757,8 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) #endif + ngx_conf_merge_ptr_value(conf->method, prev->method, NULL); + ngx_conf_merge_value(conf->redirect, prev->redirect, 1); if (conf->redirect) { diff --git a/src/http/modules/ngx_http_secure_link_module.c b/src/http/modules/ngx_http_secure_link_module.c index 536e09a72..4d4ce6af1 100644 --- a/src/http/modules/ngx_http_secure_link_module.c +++ b/src/http/modules/ngx_http_secure_link_module.c @@ -302,11 +302,12 @@ ngx_http_secure_link_create_conf(ngx_conf_t *cf) /* * set by ngx_pcalloc(): * - * conf->variable = NULL; - * conf->md5 = NULL; * conf->secret = { 0, NULL }; */ + conf->variable = NGX_CONF_UNSET_PTR; + conf->md5 = NGX_CONF_UNSET_PTR; + return conf; } @@ -318,6 +319,9 @@ ngx_http_secure_link_merge_conf(ngx_conf_t *cf, void *parent, void *child) ngx_http_secure_link_conf_t *conf = child; if (conf->secret.data) { + ngx_conf_init_ptr_value(conf->variable, NULL); + ngx_conf_init_ptr_value(conf->md5, NULL); + if (conf->variable || conf->md5) { ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "\"secure_link_secret\" cannot be mixed with " @@ -328,13 +332,8 @@ ngx_http_secure_link_merge_conf(ngx_conf_t *cf, void *parent, void *child) return NGX_CONF_OK; } - if (conf->variable == NULL) { - conf->variable = prev->variable; - } - - if (conf->md5 == NULL) { - conf->md5 = prev->md5; - } + ngx_conf_merge_ptr_value(conf->variable, prev->variable, NULL); + ngx_conf_merge_ptr_value(conf->md5, prev->md5, NULL); if (conf->variable == NULL && conf->md5 == NULL) { conf->secret = prev->secret; diff --git a/src/http/modules/ngx_http_uwsgi_module.c b/src/http/modules/ngx_http_uwsgi_module.c index 1334f44c9..dbcbd06d2 100644 --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -1509,6 +1509,7 @@ ngx_http_uwsgi_create_loc_conf(ngx_conf_t *cf) #if (NGX_HTTP_SSL) conf->upstream.ssl_session_reuse = NGX_CONF_UNSET; + conf->upstream.ssl_name = NGX_CONF_UNSET_PTR; conf->upstream.ssl_server_name = NGX_CONF_UNSET; conf->upstream.ssl_verify = NGX_CONF_UNSET; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; @@ -1824,10 +1825,8 @@ ngx_http_uwsgi_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_str_value(conf->ssl_ciphers, prev->ssl_ciphers, "DEFAULT"); - if (conf->upstream.ssl_name == NULL) { - conf->upstream.ssl_name = prev->upstream.ssl_name; - } - + ngx_conf_merge_ptr_value(conf->upstream.ssl_name, + prev->upstream.ssl_name, NULL); ngx_conf_merge_value(conf->upstream.ssl_server_name, prev->upstream.ssl_server_name, 0); ngx_conf_merge_value(conf->upstream.ssl_verify, diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c index 6664fa6c3..96a945be4 100644 --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -3479,8 +3479,6 @@ ngx_http_core_create_loc_conf(ngx_conf_t *cf) * clcf->exact_match = 0; * clcf->auto_redirect = 0; * clcf->alias = 0; - * clcf->limit_rate = NULL; - * clcf->limit_rate_after = NULL; * clcf->gzip_proxied = 0; * clcf->keepalive_disable = 0; */ @@ -3512,6 +3510,8 @@ ngx_http_core_create_loc_conf(ngx_conf_t *cf) clcf->send_timeout = NGX_CONF_UNSET_MSEC; clcf->send_lowat = NGX_CONF_UNSET_SIZE; clcf->postpone_output = NGX_CONF_UNSET_SIZE; + clcf->limit_rate = NGX_CONF_UNSET_PTR; + clcf->limit_rate_after = NGX_CONF_UNSET_PTR; clcf->keepalive_time = NGX_CONF_UNSET_MSEC; clcf->keepalive_timeout = NGX_CONF_UNSET_MSEC; clcf->keepalive_header = NGX_CONF_UNSET; @@ -3743,13 +3743,9 @@ ngx_http_core_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_size_value(conf->postpone_output, prev->postpone_output, 1460); - if (conf->limit_rate == NULL) { - conf->limit_rate = prev->limit_rate; - } - - if (conf->limit_rate_after == NULL) { - conf->limit_rate_after = prev->limit_rate_after; - } + ngx_conf_merge_ptr_value(conf->limit_rate, prev->limit_rate, NULL); + ngx_conf_merge_ptr_value(conf->limit_rate_after, + prev->limit_rate_after, NULL); ngx_conf_merge_msec_value(conf->keepalive_time, prev->keepalive_time, 3600000); diff --git a/src/http/ngx_http_script.c b/src/http/ngx_http_script.c index 13c57d6d9..e94de7385 100644 --- a/src/http/ngx_http_script.c +++ b/src/http/ngx_http_script.c @@ -250,7 +250,7 @@ ngx_http_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) cv = (ngx_http_complex_value_t **) (p + cmd->offset); - if (*cv != NULL) { + if (*cv != NGX_CONF_UNSET_PTR && *cv != NULL) { return "is duplicate"; } diff --git a/src/stream/ngx_stream_proxy_module.c b/src/stream/ngx_stream_proxy_module.c index 01cda7a36..6f6623aa8 100644 --- a/src/stream/ngx_stream_proxy_module.c +++ b/src/stream/ngx_stream_proxy_module.c @@ -1977,14 +1977,11 @@ ngx_stream_proxy_create_srv_conf(ngx_conf_t *cf) * * conf->ssl_protocols = 0; * conf->ssl_ciphers = { 0, NULL }; - * conf->ssl_name = NULL; * conf->ssl_trusted_certificate = { 0, NULL }; * conf->ssl_crl = { 0, NULL }; * conf->ssl_certificate = { 0, NULL }; * conf->ssl_certificate_key = { 0, NULL }; * - * conf->upload_rate = NULL; - * conf->download_rate = NULL; * conf->ssl = NULL; * conf->upstream = NULL; * conf->upstream_value = NULL; @@ -1994,6 +1991,8 @@ ngx_stream_proxy_create_srv_conf(ngx_conf_t *cf) conf->timeout = NGX_CONF_UNSET_MSEC; conf->next_upstream_timeout = NGX_CONF_UNSET_MSEC; conf->buffer_size = NGX_CONF_UNSET_SIZE; + conf->upload_rate = NGX_CONF_UNSET_PTR; + conf->download_rate = NGX_CONF_UNSET_PTR; conf->requests = NGX_CONF_UNSET_UINT; conf->responses = NGX_CONF_UNSET_UINT; conf->next_upstream_tries = NGX_CONF_UNSET_UINT; @@ -2005,6 +2004,7 @@ ngx_stream_proxy_create_srv_conf(ngx_conf_t *cf) #if (NGX_STREAM_SSL) conf->ssl_enable = NGX_CONF_UNSET; conf->ssl_session_reuse = NGX_CONF_UNSET; + conf->ssl_name = NGX_CONF_UNSET_PTR; conf->ssl_server_name = NGX_CONF_UNSET; conf->ssl_verify = NGX_CONF_UNSET; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; @@ -2034,13 +2034,9 @@ ngx_stream_proxy_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_size_value(conf->buffer_size, prev->buffer_size, 16384); - if (conf->upload_rate == NULL) { - conf->upload_rate = prev->upload_rate; - } + ngx_conf_merge_ptr_value(conf->upload_rate, prev->upload_rate, NULL); - if (conf->download_rate == NULL) { - conf->download_rate = prev->download_rate; - } + ngx_conf_merge_ptr_value(conf->download_rate, prev->download_rate, NULL); ngx_conf_merge_uint_value(conf->requests, prev->requests, 0); @@ -2073,9 +2069,7 @@ ngx_stream_proxy_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_str_value(conf->ssl_ciphers, prev->ssl_ciphers, "DEFAULT"); - if (conf->ssl_name == NULL) { - conf->ssl_name = prev->ssl_name; - } + ngx_conf_merge_ptr_value(conf->ssl_name, prev->ssl_name, NULL); ngx_conf_merge_value(conf->ssl_server_name, prev->ssl_server_name, 0); diff --git a/src/stream/ngx_stream_script.c b/src/stream/ngx_stream_script.c index a15f772b5..76d347599 100644 --- a/src/stream/ngx_stream_script.c +++ b/src/stream/ngx_stream_script.c @@ -252,7 +252,7 @@ ngx_stream_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd, cv = (ngx_stream_complex_value_t **) (p + cmd->offset); - if (*cv != NULL) { + if (*cv != NGX_CONF_UNSET_PTR && *cv != NULL) { return "is duplicate"; } -- cgit From a6bce8c2274c971d4d61b78e002857d1ec69a901 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Thu, 6 May 2021 02:22:07 +0300 Subject: Auth basic: changed alcf->user_file to be a pointer. This saves some memory in typical case when auth_basic_user_file is not explicitly set, and unifies the code with alcf->realm. --- src/http/modules/ngx_http_auth_basic_module.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/http/modules/ngx_http_auth_basic_module.c b/src/http/modules/ngx_http_auth_basic_module.c index 4e28b6ce6..069331982 100644 --- a/src/http/modules/ngx_http_auth_basic_module.c +++ b/src/http/modules/ngx_http_auth_basic_module.c @@ -16,7 +16,7 @@ typedef struct { ngx_http_complex_value_t *realm; - ngx_http_complex_value_t user_file; + ngx_http_complex_value_t *user_file; } ngx_http_auth_basic_loc_conf_t; @@ -107,7 +107,7 @@ ngx_http_auth_basic_handler(ngx_http_request_t *r) alcf = ngx_http_get_module_loc_conf(r, ngx_http_auth_basic_module); - if (alcf->realm == NULL || alcf->user_file.value.data == NULL) { + if (alcf->realm == NULL || alcf->user_file == NULL) { return NGX_DECLINED; } @@ -133,7 +133,7 @@ ngx_http_auth_basic_handler(ngx_http_request_t *r) return NGX_HTTP_INTERNAL_SERVER_ERROR; } - if (ngx_http_complex_value(r, &alcf->user_file, &user_file) != NGX_OK) { + if (ngx_http_complex_value(r, alcf->user_file, &user_file) != NGX_OK) { return NGX_ERROR; } @@ -358,6 +358,7 @@ ngx_http_auth_basic_create_loc_conf(ngx_conf_t *cf) } conf->realm = NGX_CONF_UNSET_PTR; + conf->user_file = NGX_CONF_UNSET_PTR; return conf; } @@ -370,10 +371,7 @@ ngx_http_auth_basic_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_http_auth_basic_loc_conf_t *conf = child; ngx_conf_merge_ptr_value(conf->realm, prev->realm, NULL); - - if (conf->user_file.value.data == NULL) { - conf->user_file = prev->user_file; - } + ngx_conf_merge_ptr_value(conf->user_file, prev->user_file, NULL); return NGX_CONF_OK; } @@ -406,17 +404,22 @@ ngx_http_auth_basic_user_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) ngx_str_t *value; ngx_http_compile_complex_value_t ccv; - if (alcf->user_file.value.data) { + if (alcf->user_file != NGX_CONF_UNSET_PTR) { return "is duplicate"; } + alcf->user_file = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t)); + if (alcf->user_file == NULL) { + return NGX_CONF_ERROR; + } + value = cf->args->elts; ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t)); ccv.cf = cf; ccv.value = &value[1]; - ccv.complex_value = &alcf->user_file; + ccv.complex_value = alcf->user_file; ccv.zero = 1; ccv.conf_prefix = 1; -- cgit From c7de65228f798c3c5391370fcd2d10032aa6eaf8 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Thu, 6 May 2021 02:22:09 +0300 Subject: Upstream: variables support in certificates. --- src/http/modules/ngx_http_grpc_module.c | 62 +++++++++++-------- src/http/modules/ngx_http_proxy_module.c | 62 +++++++++++-------- src/http/modules/ngx_http_uwsgi_module.c | 60 +++++++++++------- src/http/ngx_http_script.c | 38 ++++++++++++ src/http/ngx_http_script.h | 2 + src/http/ngx_http_upstream.c | 51 ++++++++++++++++ src/http/ngx_http_upstream.h | 4 ++ src/stream/ngx_stream_proxy_module.c | 102 +++++++++++++++++++++++++------ src/stream/ngx_stream_script.c | 38 ++++++++++++ src/stream/ngx_stream_script.h | 2 + 10 files changed, 331 insertions(+), 90 deletions(-) (limited to 'src') diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 8db35631c..2e20e5ffd 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -37,9 +37,6 @@ typedef struct { ngx_uint_t ssl_verify_depth; ngx_str_t ssl_trusted_certificate; ngx_str_t ssl_crl; - ngx_str_t ssl_certificate; - ngx_str_t ssl_certificate_key; - ngx_array_t *ssl_passwords; ngx_array_t *ssl_conf_commands; #endif } ngx_http_grpc_loc_conf_t; @@ -425,16 +422,16 @@ static ngx_command_t ngx_http_grpc_commands[] = { { ngx_string("grpc_ssl_certificate"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_http_set_complex_value_zero_slot, NGX_HTTP_LOC_CONF_OFFSET, - offsetof(ngx_http_grpc_loc_conf_t, ssl_certificate), + offsetof(ngx_http_grpc_loc_conf_t, upstream.ssl_certificate), NULL }, { ngx_string("grpc_ssl_certificate_key"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_http_set_complex_value_zero_slot, NGX_HTTP_LOC_CONF_OFFSET, - offsetof(ngx_http_grpc_loc_conf_t, ssl_certificate_key), + offsetof(ngx_http_grpc_loc_conf_t, upstream.ssl_certificate_key), NULL }, { ngx_string("grpc_ssl_password_file"), @@ -4342,8 +4339,6 @@ ngx_http_grpc_create_loc_conf(ngx_conf_t *cf) * conf->ssl_ciphers = { 0, NULL }; * conf->ssl_trusted_certificate = { 0, NULL }; * conf->ssl_crl = { 0, NULL }; - * conf->ssl_certificate = { 0, NULL }; - * conf->ssl_certificate_key = { 0, NULL }; */ conf->upstream.local = NGX_CONF_UNSET_PTR; @@ -4367,7 +4362,9 @@ ngx_http_grpc_create_loc_conf(ngx_conf_t *cf) conf->upstream.ssl_server_name = NGX_CONF_UNSET; conf->upstream.ssl_verify = NGX_CONF_UNSET; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; - conf->ssl_passwords = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_certificate = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_certificate_key = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_passwords = NGX_CONF_UNSET_PTR; conf->ssl_conf_commands = NGX_CONF_UNSET_PTR; #endif @@ -4471,11 +4468,12 @@ ngx_http_grpc_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) prev->ssl_trusted_certificate, ""); ngx_conf_merge_str_value(conf->ssl_crl, prev->ssl_crl, ""); - ngx_conf_merge_str_value(conf->ssl_certificate, - prev->ssl_certificate, ""); - ngx_conf_merge_str_value(conf->ssl_certificate_key, - prev->ssl_certificate_key, ""); - ngx_conf_merge_ptr_value(conf->ssl_passwords, prev->ssl_passwords, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_certificate, + prev->upstream.ssl_certificate, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_certificate_key, + prev->upstream.ssl_certificate_key, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_passwords, + prev->upstream.ssl_passwords, NULL); ngx_conf_merge_ptr_value(conf->ssl_conf_commands, prev->ssl_conf_commands, NULL); @@ -4831,15 +4829,15 @@ ngx_http_grpc_ssl_password_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) ngx_str_t *value; - if (glcf->ssl_passwords != NGX_CONF_UNSET_PTR) { + if (glcf->upstream.ssl_passwords != NGX_CONF_UNSET_PTR) { return "is duplicate"; } value = cf->args->elts; - glcf->ssl_passwords = ngx_ssl_read_password_file(cf, &value[1]); + glcf->upstream.ssl_passwords = ngx_ssl_read_password_file(cf, &value[1]); - if (glcf->ssl_passwords == NULL) { + if (glcf->upstream.ssl_passwords == NULL) { return NGX_CONF_ERROR; } @@ -4885,20 +4883,34 @@ ngx_http_grpc_set_ssl(ngx_conf_t *cf, ngx_http_grpc_loc_conf_t *glcf) cln->handler = ngx_ssl_cleanup_ctx; cln->data = glcf->upstream.ssl; - if (glcf->ssl_certificate.len) { + if (glcf->upstream.ssl_certificate) { - if (glcf->ssl_certificate_key.len == 0) { + if (glcf->upstream.ssl_certificate_key == NULL) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "no \"grpc_ssl_certificate_key\" is defined " - "for certificate \"%V\"", &glcf->ssl_certificate); + "for certificate \"%V\"", + &glcf->upstream.ssl_certificate->value); return NGX_ERROR; } - if (ngx_ssl_certificate(cf, glcf->upstream.ssl, &glcf->ssl_certificate, - &glcf->ssl_certificate_key, glcf->ssl_passwords) - != NGX_OK) + if (glcf->upstream.ssl_certificate->lengths + || glcf->upstream.ssl_certificate_key->lengths) { - return NGX_ERROR; + glcf->upstream.ssl_passwords = + ngx_ssl_preserve_passwords(cf, glcf->upstream.ssl_passwords); + if (glcf->upstream.ssl_passwords == NULL) { + return NGX_ERROR; + } + + } else { + if (ngx_ssl_certificate(cf, glcf->upstream.ssl, + &glcf->upstream.ssl_certificate->value, + &glcf->upstream.ssl_certificate_key->value, + glcf->upstream.ssl_passwords) + != NGX_OK) + { + return NGX_ERROR; + } } } diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c index 97c168b45..64190f1a0 100644 --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -124,9 +124,6 @@ typedef struct { ngx_uint_t ssl_verify_depth; ngx_str_t ssl_trusted_certificate; ngx_str_t ssl_crl; - ngx_str_t ssl_certificate; - ngx_str_t ssl_certificate_key; - ngx_array_t *ssl_passwords; ngx_array_t *ssl_conf_commands; #endif } ngx_http_proxy_loc_conf_t; @@ -753,16 +750,16 @@ static ngx_command_t ngx_http_proxy_commands[] = { { ngx_string("proxy_ssl_certificate"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_http_set_complex_value_zero_slot, NGX_HTTP_LOC_CONF_OFFSET, - offsetof(ngx_http_proxy_loc_conf_t, ssl_certificate), + offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_certificate), NULL }, { ngx_string("proxy_ssl_certificate_key"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_http_set_complex_value_zero_slot, NGX_HTTP_LOC_CONF_OFFSET, - offsetof(ngx_http_proxy_loc_conf_t, ssl_certificate_key), + offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_certificate_key), NULL }, { ngx_string("proxy_ssl_password_file"), @@ -3345,8 +3342,6 @@ ngx_http_proxy_create_loc_conf(ngx_conf_t *cf) * conf->ssl_ciphers = { 0, NULL }; * conf->ssl_trusted_certificate = { 0, NULL }; * conf->ssl_crl = { 0, NULL }; - * conf->ssl_certificate = { 0, NULL }; - * conf->ssl_certificate_key = { 0, NULL }; */ conf->upstream.store = NGX_CONF_UNSET; @@ -3401,8 +3396,10 @@ ngx_http_proxy_create_loc_conf(ngx_conf_t *cf) conf->upstream.ssl_name = NGX_CONF_UNSET_PTR; conf->upstream.ssl_server_name = NGX_CONF_UNSET; conf->upstream.ssl_verify = NGX_CONF_UNSET; + conf->upstream.ssl_certificate = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_certificate_key = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_passwords = NGX_CONF_UNSET_PTR; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; - conf->ssl_passwords = NGX_CONF_UNSET_PTR; conf->ssl_conf_commands = NGX_CONF_UNSET_PTR; #endif @@ -3742,11 +3739,12 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) prev->ssl_trusted_certificate, ""); ngx_conf_merge_str_value(conf->ssl_crl, prev->ssl_crl, ""); - ngx_conf_merge_str_value(conf->ssl_certificate, - prev->ssl_certificate, ""); - ngx_conf_merge_str_value(conf->ssl_certificate_key, - prev->ssl_certificate_key, ""); - ngx_conf_merge_ptr_value(conf->ssl_passwords, prev->ssl_passwords, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_certificate, + prev->upstream.ssl_certificate, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_certificate_key, + prev->upstream.ssl_certificate_key, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_passwords, + prev->upstream.ssl_passwords, NULL); ngx_conf_merge_ptr_value(conf->ssl_conf_commands, prev->ssl_conf_commands, NULL); @@ -4857,15 +4855,15 @@ ngx_http_proxy_ssl_password_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) ngx_str_t *value; - if (plcf->ssl_passwords != NGX_CONF_UNSET_PTR) { + if (plcf->upstream.ssl_passwords != NGX_CONF_UNSET_PTR) { return "is duplicate"; } value = cf->args->elts; - plcf->ssl_passwords = ngx_ssl_read_password_file(cf, &value[1]); + plcf->upstream.ssl_passwords = ngx_ssl_read_password_file(cf, &value[1]); - if (plcf->ssl_passwords == NULL) { + if (plcf->upstream.ssl_passwords == NULL) { return NGX_CONF_ERROR; } @@ -4944,20 +4942,34 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *plcf) cln->handler = ngx_ssl_cleanup_ctx; cln->data = plcf->upstream.ssl; - if (plcf->ssl_certificate.len) { + if (plcf->upstream.ssl_certificate) { - if (plcf->ssl_certificate_key.len == 0) { + if (plcf->upstream.ssl_certificate_key == NULL) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "no \"proxy_ssl_certificate_key\" is defined " - "for certificate \"%V\"", &plcf->ssl_certificate); + "for certificate \"%V\"", + &plcf->upstream.ssl_certificate->value); return NGX_ERROR; } - if (ngx_ssl_certificate(cf, plcf->upstream.ssl, &plcf->ssl_certificate, - &plcf->ssl_certificate_key, plcf->ssl_passwords) - != NGX_OK) + if (plcf->upstream.ssl_certificate->lengths + || plcf->upstream.ssl_certificate_key->lengths) { - return NGX_ERROR; + plcf->upstream.ssl_passwords = + ngx_ssl_preserve_passwords(cf, plcf->upstream.ssl_passwords); + if (plcf->upstream.ssl_passwords == NULL) { + return NGX_ERROR; + } + + } else { + if (ngx_ssl_certificate(cf, plcf->upstream.ssl, + &plcf->upstream.ssl_certificate->value, + &plcf->upstream.ssl_certificate_key->value, + plcf->upstream.ssl_passwords) + != NGX_OK) + { + return NGX_ERROR; + } } } diff --git a/src/http/modules/ngx_http_uwsgi_module.c b/src/http/modules/ngx_http_uwsgi_module.c index dbcbd06d2..655be98c7 100644 --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -54,9 +54,6 @@ typedef struct { ngx_uint_t ssl_verify_depth; ngx_str_t ssl_trusted_certificate; ngx_str_t ssl_crl; - ngx_str_t ssl_certificate; - ngx_str_t ssl_certificate_key; - ngx_array_t *ssl_passwords; ngx_array_t *ssl_conf_commands; #endif } ngx_http_uwsgi_loc_conf_t; @@ -548,16 +545,16 @@ static ngx_command_t ngx_http_uwsgi_commands[] = { { ngx_string("uwsgi_ssl_certificate"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_http_set_complex_value_zero_slot, NGX_HTTP_LOC_CONF_OFFSET, - offsetof(ngx_http_uwsgi_loc_conf_t, ssl_certificate), + offsetof(ngx_http_uwsgi_loc_conf_t, upstream.ssl_certificate), NULL }, { ngx_string("uwsgi_ssl_certificate_key"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_http_set_complex_value_zero_slot, NGX_HTTP_LOC_CONF_OFFSET, - offsetof(ngx_http_uwsgi_loc_conf_t, ssl_certificate_key), + offsetof(ngx_http_uwsgi_loc_conf_t, upstream.ssl_certificate_key), NULL }, { ngx_string("uwsgi_ssl_password_file"), @@ -1513,7 +1510,9 @@ ngx_http_uwsgi_create_loc_conf(ngx_conf_t *cf) conf->upstream.ssl_server_name = NGX_CONF_UNSET; conf->upstream.ssl_verify = NGX_CONF_UNSET; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; - conf->ssl_passwords = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_certificate = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_certificate_key = NGX_CONF_UNSET_PTR; + conf->upstream.ssl_passwords = NGX_CONF_UNSET_PTR; conf->ssl_conf_commands = NGX_CONF_UNSET_PTR; #endif @@ -1837,11 +1836,12 @@ ngx_http_uwsgi_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) prev->ssl_trusted_certificate, ""); ngx_conf_merge_str_value(conf->ssl_crl, prev->ssl_crl, ""); - ngx_conf_merge_str_value(conf->ssl_certificate, - prev->ssl_certificate, ""); - ngx_conf_merge_str_value(conf->ssl_certificate_key, - prev->ssl_certificate_key, ""); - ngx_conf_merge_ptr_value(conf->ssl_passwords, prev->ssl_passwords, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_certificate, + prev->upstream.ssl_certificate, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_certificate_key, + prev->upstream.ssl_certificate_key, NULL); + ngx_conf_merge_ptr_value(conf->upstream.ssl_passwords, + prev->upstream.ssl_passwords, NULL); ngx_conf_merge_ptr_value(conf->ssl_conf_commands, prev->ssl_conf_commands, NULL); @@ -2376,15 +2376,15 @@ ngx_http_uwsgi_ssl_password_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) ngx_str_t *value; - if (uwcf->ssl_passwords != NGX_CONF_UNSET_PTR) { + if (uwcf->upstream.ssl_passwords != NGX_CONF_UNSET_PTR) { return "is duplicate"; } value = cf->args->elts; - uwcf->ssl_passwords = ngx_ssl_read_password_file(cf, &value[1]); + uwcf->upstream.ssl_passwords = ngx_ssl_read_password_file(cf, &value[1]); - if (uwcf->ssl_passwords == NULL) { + if (uwcf->upstream.ssl_passwords == NULL) { return NGX_CONF_ERROR; } @@ -2430,20 +2430,34 @@ ngx_http_uwsgi_set_ssl(ngx_conf_t *cf, ngx_http_uwsgi_loc_conf_t *uwcf) cln->handler = ngx_ssl_cleanup_ctx; cln->data = uwcf->upstream.ssl; - if (uwcf->ssl_certificate.len) { + if (uwcf->upstream.ssl_certificate) { - if (uwcf->ssl_certificate_key.len == 0) { + if (uwcf->upstream.ssl_certificate_key == NULL) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "no \"uwsgi_ssl_certificate_key\" is defined " - "for certificate \"%V\"", &uwcf->ssl_certificate); + "for certificate \"%V\"", + &uwcf->upstream.ssl_certificate->value); return NGX_ERROR; } - if (ngx_ssl_certificate(cf, uwcf->upstream.ssl, &uwcf->ssl_certificate, - &uwcf->ssl_certificate_key, uwcf->ssl_passwords) - != NGX_OK) + if (uwcf->upstream.ssl_certificate->lengths + || uwcf->upstream.ssl_certificate_key->lengths) { - return NGX_ERROR; + uwcf->upstream.ssl_passwords = + ngx_ssl_preserve_passwords(cf, uwcf->upstream.ssl_passwords); + if (uwcf->upstream.ssl_passwords == NULL) { + return NGX_ERROR; + } + + } else { + if (ngx_ssl_certificate(cf, uwcf->upstream.ssl, + &uwcf->upstream.ssl_certificate->value, + &uwcf->upstream.ssl_certificate_key->value, + uwcf->upstream.ssl_passwords) + != NGX_OK) + { + return NGX_ERROR; + } } } diff --git a/src/http/ngx_http_script.c b/src/http/ngx_http_script.c index e94de7385..bebdbd92b 100644 --- a/src/http/ngx_http_script.c +++ b/src/http/ngx_http_script.c @@ -275,6 +275,44 @@ ngx_http_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) } +char * +ngx_http_set_complex_value_zero_slot(ngx_conf_t *cf, ngx_command_t *cmd, + void *conf) +{ + char *p = conf; + + ngx_str_t *value; + ngx_http_complex_value_t **cv; + ngx_http_compile_complex_value_t ccv; + + cv = (ngx_http_complex_value_t **) (p + cmd->offset); + + if (*cv != NGX_CONF_UNSET_PTR) { + return "is duplicate"; + } + + *cv = ngx_palloc(cf->pool, sizeof(ngx_http_complex_value_t)); + if (*cv == NULL) { + return NGX_CONF_ERROR; + } + + value = cf->args->elts; + + ngx_memzero(&ccv, sizeof(ngx_http_compile_complex_value_t)); + + ccv.cf = cf; + ccv.value = &value[1]; + ccv.complex_value = *cv; + ccv.zero = 1; + + if (ngx_http_compile_complex_value(&ccv) != NGX_OK) { + return NGX_CONF_ERROR; + } + + return NGX_CONF_OK; +} + + char * ngx_http_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) diff --git a/src/http/ngx_http_script.h b/src/http/ngx_http_script.h index a6b345e2d..43600383c 100644 --- a/src/http/ngx_http_script.h +++ b/src/http/ngx_http_script.h @@ -216,6 +216,8 @@ size_t ngx_http_complex_value_size(ngx_http_request_t *r, ngx_int_t ngx_http_compile_complex_value(ngx_http_compile_complex_value_t *ccv); char *ngx_http_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +char *ngx_http_set_complex_value_zero_slot(ngx_conf_t *cf, ngx_command_t *cmd, + void *conf); char *ngx_http_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c index 1016afa5b..61fad5ca3 100644 --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -187,6 +187,8 @@ static void ngx_http_upstream_ssl_handshake(ngx_http_request_t *, static void ngx_http_upstream_ssl_save_session(ngx_connection_t *c); static ngx_int_t ngx_http_upstream_ssl_name(ngx_http_request_t *r, ngx_http_upstream_t *u, ngx_connection_t *c); +static ngx_int_t ngx_http_upstream_ssl_certificate(ngx_http_request_t *r, + ngx_http_upstream_t *u, ngx_connection_t *c); #endif @@ -1692,6 +1694,16 @@ ngx_http_upstream_ssl_init_connection(ngx_http_request_t *r, } } + if (u->conf->ssl_certificate && (u->conf->ssl_certificate->lengths + || u->conf->ssl_certificate_key->lengths)) + { + if (ngx_http_upstream_ssl_certificate(r, u, c) != NGX_OK) { + ngx_http_upstream_finalize_request(r, u, + NGX_HTTP_INTERNAL_SERVER_ERROR); + return; + } + } + if (u->conf->ssl_session_reuse) { c->ssl->save_session = ngx_http_upstream_ssl_save_session; @@ -1912,6 +1924,45 @@ done: return NGX_OK; } + +static ngx_int_t +ngx_http_upstream_ssl_certificate(ngx_http_request_t *r, + ngx_http_upstream_t *u, ngx_connection_t *c) +{ + ngx_str_t cert, key; + + if (ngx_http_complex_value(r, u->conf->ssl_certificate, &cert) + != NGX_OK) + { + return NGX_ERROR; + } + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, + "http upstream ssl cert: \"%s\"", cert.data); + + if (*cert.data == '\0') { + return NGX_OK; + } + + if (ngx_http_complex_value(r, u->conf->ssl_certificate_key, &key) + != NGX_OK) + { + return NGX_ERROR; + } + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, + "http upstream ssl key: \"%s\"", key.data); + + if (ngx_ssl_connection_certificate(c, r->pool, &cert, &key, + u->conf->ssl_passwords) + != NGX_OK) + { + return NGX_ERROR; + } + + return NGX_OK; +} + #endif diff --git a/src/http/ngx_http_upstream.h b/src/http/ngx_http_upstream.h index fd642c2d2..3db7b0643 100644 --- a/src/http/ngx_http_upstream.h +++ b/src/http/ngx_http_upstream.h @@ -234,6 +234,10 @@ typedef struct { ngx_http_complex_value_t *ssl_name; ngx_flag_t ssl_server_name; ngx_flag_t ssl_verify; + + ngx_http_complex_value_t *ssl_certificate; + ngx_http_complex_value_t *ssl_certificate_key; + ngx_array_t *ssl_passwords; #endif ngx_str_t module; diff --git a/src/stream/ngx_stream_proxy_module.c b/src/stream/ngx_stream_proxy_module.c index 6f6623aa8..8c686ab20 100644 --- a/src/stream/ngx_stream_proxy_module.c +++ b/src/stream/ngx_stream_proxy_module.c @@ -46,8 +46,8 @@ typedef struct { ngx_uint_t ssl_verify_depth; ngx_str_t ssl_trusted_certificate; ngx_str_t ssl_crl; - ngx_str_t ssl_certificate; - ngx_str_t ssl_certificate_key; + ngx_stream_complex_value_t *ssl_certificate; + ngx_stream_complex_value_t *ssl_certificate_key; ngx_array_t *ssl_passwords; ngx_array_t *ssl_conf_commands; @@ -101,6 +101,7 @@ static void ngx_stream_proxy_ssl_init_connection(ngx_stream_session_t *s); static void ngx_stream_proxy_ssl_handshake(ngx_connection_t *pc); static void ngx_stream_proxy_ssl_save_session(ngx_connection_t *c); static ngx_int_t ngx_stream_proxy_ssl_name(ngx_stream_session_t *s); +static ngx_int_t ngx_stream_proxy_ssl_certificate(ngx_stream_session_t *s); static ngx_int_t ngx_stream_proxy_set_ssl(ngx_conf_t *cf, ngx_stream_proxy_srv_conf_t *pscf); @@ -318,14 +319,14 @@ static ngx_command_t ngx_stream_proxy_commands[] = { { ngx_string("proxy_ssl_certificate"), NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_stream_set_complex_value_zero_slot, NGX_STREAM_SRV_CONF_OFFSET, offsetof(ngx_stream_proxy_srv_conf_t, ssl_certificate), NULL }, { ngx_string("proxy_ssl_certificate_key"), NGX_STREAM_MAIN_CONF|NGX_STREAM_SRV_CONF|NGX_CONF_TAKE1, - ngx_conf_set_str_slot, + ngx_stream_set_complex_value_zero_slot, NGX_STREAM_SRV_CONF_OFFSET, offsetof(ngx_stream_proxy_srv_conf_t, ssl_certificate_key), NULL }, @@ -1060,6 +1061,15 @@ ngx_stream_proxy_ssl_init_connection(ngx_stream_session_t *s) } } + if (pscf->ssl_certificate && (pscf->ssl_certificate->lengths + || pscf->ssl_certificate_key->lengths)) + { + if (ngx_stream_proxy_ssl_certificate(s) != NGX_OK) { + ngx_stream_proxy_finalize(s, NGX_STREAM_INTERNAL_SERVER_ERROR); + return; + } + } + if (pscf->ssl_session_reuse) { pc->ssl->save_session = ngx_stream_proxy_ssl_save_session; @@ -1247,6 +1257,50 @@ done: return NGX_OK; } + +static ngx_int_t +ngx_stream_proxy_ssl_certificate(ngx_stream_session_t *s) +{ + ngx_str_t cert, key; + ngx_connection_t *c; + ngx_stream_proxy_srv_conf_t *pscf; + + c = s->upstream->peer.connection; + + pscf = ngx_stream_get_module_srv_conf(s, ngx_stream_proxy_module); + + if (ngx_stream_complex_value(s, pscf->ssl_certificate, &cert) + != NGX_OK) + { + return NGX_ERROR; + } + + ngx_log_debug1(NGX_LOG_DEBUG_STREAM, c->log, 0, + "stream upstream ssl cert: \"%s\"", cert.data); + + if (*cert.data == '\0') { + return NGX_OK; + } + + if (ngx_stream_complex_value(s, pscf->ssl_certificate_key, &key) + != NGX_OK) + { + return NGX_ERROR; + } + + ngx_log_debug1(NGX_LOG_DEBUG_STREAM, c->log, 0, + "stream upstream ssl key: \"%s\"", key.data); + + if (ngx_ssl_connection_certificate(c, c->pool, &cert, &key, + pscf->ssl_passwords) + != NGX_OK) + { + return NGX_ERROR; + } + + return NGX_OK; +} + #endif @@ -1979,8 +2033,6 @@ ngx_stream_proxy_create_srv_conf(ngx_conf_t *cf) * conf->ssl_ciphers = { 0, NULL }; * conf->ssl_trusted_certificate = { 0, NULL }; * conf->ssl_crl = { 0, NULL }; - * conf->ssl_certificate = { 0, NULL }; - * conf->ssl_certificate_key = { 0, NULL }; * * conf->ssl = NULL; * conf->upstream = NULL; @@ -2008,6 +2060,8 @@ ngx_stream_proxy_create_srv_conf(ngx_conf_t *cf) conf->ssl_server_name = NGX_CONF_UNSET; conf->ssl_verify = NGX_CONF_UNSET; conf->ssl_verify_depth = NGX_CONF_UNSET_UINT; + conf->ssl_certificate = NGX_CONF_UNSET_PTR; + conf->ssl_certificate_key = NGX_CONF_UNSET_PTR; conf->ssl_passwords = NGX_CONF_UNSET_PTR; conf->ssl_conf_commands = NGX_CONF_UNSET_PTR; #endif @@ -2083,11 +2137,11 @@ ngx_stream_proxy_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_str_value(conf->ssl_crl, prev->ssl_crl, ""); - ngx_conf_merge_str_value(conf->ssl_certificate, - prev->ssl_certificate, ""); + ngx_conf_merge_ptr_value(conf->ssl_certificate, + prev->ssl_certificate, NULL); - ngx_conf_merge_str_value(conf->ssl_certificate_key, - prev->ssl_certificate_key, ""); + ngx_conf_merge_ptr_value(conf->ssl_certificate_key, + prev->ssl_certificate_key, NULL); ngx_conf_merge_ptr_value(conf->ssl_passwords, prev->ssl_passwords, NULL); @@ -2131,20 +2185,34 @@ ngx_stream_proxy_set_ssl(ngx_conf_t *cf, ngx_stream_proxy_srv_conf_t *pscf) cln->handler = ngx_ssl_cleanup_ctx; cln->data = pscf->ssl; - if (pscf->ssl_certificate.len) { + if (pscf->ssl_certificate) { - if (pscf->ssl_certificate_key.len == 0) { + if (pscf->ssl_certificate_key == NULL) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "no \"proxy_ssl_certificate_key\" is defined " - "for certificate \"%V\"", &pscf->ssl_certificate); + "for certificate \"%V\"", + &pscf->ssl_certificate->value); return NGX_ERROR; } - if (ngx_ssl_certificate(cf, pscf->ssl, &pscf->ssl_certificate, - &pscf->ssl_certificate_key, pscf->ssl_passwords) - != NGX_OK) + if (pscf->ssl_certificate->lengths + || pscf->ssl_certificate_key->lengths) { - return NGX_ERROR; + pscf->ssl_passwords = + ngx_ssl_preserve_passwords(cf, pscf->ssl_passwords); + if (pscf->ssl_passwords == NULL) { + return NGX_ERROR; + } + + } else { + if (ngx_ssl_certificate(cf, pscf->ssl, + &pscf->ssl_certificate->value, + &pscf->ssl_certificate_key->value, + pscf->ssl_passwords) + != NGX_OK) + { + return NGX_ERROR; + } } } diff --git a/src/stream/ngx_stream_script.c b/src/stream/ngx_stream_script.c index 76d347599..c447e152f 100644 --- a/src/stream/ngx_stream_script.c +++ b/src/stream/ngx_stream_script.c @@ -277,6 +277,44 @@ ngx_stream_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd, } +char * +ngx_stream_set_complex_value_zero_slot(ngx_conf_t *cf, ngx_command_t *cmd, + void *conf) +{ + char *p = conf; + + ngx_str_t *value; + ngx_stream_complex_value_t **cv; + ngx_stream_compile_complex_value_t ccv; + + cv = (ngx_stream_complex_value_t **) (p + cmd->offset); + + if (*cv != NGX_CONF_UNSET_PTR) { + return "is duplicate"; + } + + *cv = ngx_palloc(cf->pool, sizeof(ngx_stream_complex_value_t)); + if (*cv == NULL) { + return NGX_CONF_ERROR; + } + + value = cf->args->elts; + + ngx_memzero(&ccv, sizeof(ngx_stream_compile_complex_value_t)); + + ccv.cf = cf; + ccv.value = &value[1]; + ccv.complex_value = *cv; + ccv.zero = 1; + + if (ngx_stream_compile_complex_value(&ccv) != NGX_OK) { + return NGX_CONF_ERROR; + } + + return NGX_CONF_OK; +} + + char * ngx_stream_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) diff --git a/src/stream/ngx_stream_script.h b/src/stream/ngx_stream_script.h index a481ca3ab..d8f374047 100644 --- a/src/stream/ngx_stream_script.h +++ b/src/stream/ngx_stream_script.h @@ -112,6 +112,8 @@ ngx_int_t ngx_stream_compile_complex_value( ngx_stream_compile_complex_value_t *ccv); char *ngx_stream_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +char *ngx_stream_set_complex_value_zero_slot(ngx_conf_t *cf, ngx_command_t *cmd, + void *conf); char *ngx_stream_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); -- cgit From 60a5a6f0d37b2e10d8d4aab97b64f4048e99c99a Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:12 +0300 Subject: Mail: fixed SMTP pipelining to send the response immediately. Previously, if there were some pipelined SMTP data in the buffer when a proxied connection with the backend was established, nginx called ngx_mail_proxy_handler() to send these data, and not tried to send the response to the last command. In most cases, this response was later sent along with the response to the pipelined command, but if for some reason client decides to wait for the response before finishing the next command this might result in a connection hang. Fix is to always call ngx_mail_proxy_handler() to send the response, and additionally post an event to send the pipelined data if needed. --- src/mail/ngx_mail_proxy_module.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/mail/ngx_mail_proxy_module.c b/src/mail/ngx_mail_proxy_module.c index 66aa0ba09..38164af95 100644 --- a/src/mail/ngx_mail_proxy_module.c +++ b/src/mail/ngx_mail_proxy_module.c @@ -813,13 +813,12 @@ ngx_mail_proxy_smtp_handler(ngx_event_t *rev) c->log->action = NULL; ngx_log_error(NGX_LOG_INFO, c->log, 0, "client logged in"); - if (s->buffer->pos == s->buffer->last) { - ngx_mail_proxy_handler(s->connection->write); - - } else { - ngx_mail_proxy_handler(c->write); + if (s->buffer->pos < s->buffer->last) { + ngx_post_event(c->write, &ngx_posted_events); } + ngx_mail_proxy_handler(s->connection->write); + return; default: -- cgit From 204f944add09c43223de317451175d5fdb7d1fb0 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:15 +0300 Subject: Mail: fixed handling of invalid SMTP commands split between reads. Previously, if an invalid SMTP command was split between reads, nginx failed to wait for LF before returning an error, and interpreted the rest of the command received later as a separate command. The sw_invalid state in ngx_mail_smtp_parse_command(), introduced in 04e43d03e153, did not work, since ngx_mail_smtp_auth_state() clears s->state when returning an error due to NGX_MAIL_PARSE_INVALID_COMMAND. And not clearing s->state will introduce another problem: the rest of the command would trigger duplicate error when rest of the command is received. Fix is to return NGX_AGAIN from ngx_mail_smtp_parse_command() until full command is received. --- src/mail/ngx_mail_parse.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index 2c2cdffa1..0712ad5dc 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -846,14 +846,14 @@ invalid: for (p = s->buffer->pos; p < s->buffer->last; p++) { if (*p == LF) { s->state = sw_start; - p++; - break; + s->buffer->pos = p + 1; + return NGX_MAIL_PARSE_INVALID_COMMAND; } } s->buffer->pos = p; - return NGX_MAIL_PARSE_INVALID_COMMAND; + return NGX_AGAIN; } -- cgit From 317223cb56f70f3fc2eb646a78a8f0161cafa70b Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:17 +0300 Subject: Mail: optimized discarding invalid SMTP commands. There is no need to scan buffer from s->buffer->pos, as we already scanned the buffer till "p" and wasn't able to find an LF. There is no real need for this change in SMTP, since it is at most a microoptimization of a non-common code path. Similar code in IMAP, however, will have to start scanning from "p" to be correct, since there can be newlines in IMAP literals. --- src/mail/ngx_mail_parse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index 0712ad5dc..c4e8f0eae 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -843,7 +843,7 @@ invalid: /* skip invalid command till LF */ - for (p = s->buffer->pos; p < s->buffer->last; p++) { + for ( /* void */ ; p < s->buffer->last; p++) { if (*p == LF) { s->state = sw_start; s->buffer->pos = p + 1; -- cgit From d96d60d2e0a41a4e01163f7e5d1835e028f94b72 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:18 +0300 Subject: Mail: POP3 pipelining support. The change is mostly the same as the SMTP one (04e43d03e153 and 3f5d0af4e40a), and ensures that nginx is able to properly handle or reject multiple POP3 commands, as required by the PIPELINING capability (RFC 2449). The s->cmd field is not really used and set for consistency. --- src/mail/ngx_mail_parse.c | 32 +++++++++++++++++++++++++++++--- src/mail/ngx_mail_pop3_handler.c | 15 ++++++++++++--- src/mail/ngx_mail_proxy_module.c | 4 ++++ 3 files changed, 45 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index c4e8f0eae..e55010c60 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -21,6 +21,8 @@ ngx_mail_pop3_parse_command(ngx_mail_session_t *s) ngx_str_t *arg; enum { sw_start = 0, + sw_command, + sw_invalid, sw_spaces_before_argument, sw_argument, sw_almost_done @@ -35,8 +37,14 @@ ngx_mail_pop3_parse_command(ngx_mail_session_t *s) /* POP3 command */ case sw_start: + s->cmd_start = p; + state = sw_command; + + /* fall through */ + + case sw_command: if (ch == ' ' || ch == CR || ch == LF) { - c = s->buffer->start; + c = s->cmd_start; if (p - c == 4) { @@ -85,6 +93,9 @@ ngx_mail_pop3_parse_command(ngx_mail_session_t *s) goto invalid; } + s->cmd.data = s->cmd_start; + s->cmd.len = p - s->cmd_start; + switch (ch) { case ' ': state = sw_spaces_before_argument; @@ -104,6 +115,9 @@ ngx_mail_pop3_parse_command(ngx_mail_session_t *s) break; + case sw_invalid: + goto invalid; + case sw_spaces_before_argument: switch (ch) { case ' ': @@ -205,10 +219,22 @@ done: invalid: - s->state = sw_start; + s->state = sw_invalid; s->arg_start = NULL; - return NGX_MAIL_PARSE_INVALID_COMMAND; + /* skip invalid command till LF */ + + for ( /* void */ ; p < s->buffer->last; p++) { + if (*p == LF) { + s->state = sw_start; + s->buffer->pos = p + 1; + return NGX_MAIL_PARSE_INVALID_COMMAND; + } + } + + s->buffer->pos = p; + + return NGX_AGAIN; } diff --git a/src/mail/ngx_mail_pop3_handler.c b/src/mail/ngx_mail_pop3_handler.c index edfd98681..226e7419b 100644 --- a/src/mail/ngx_mail_pop3_handler.c +++ b/src/mail/ngx_mail_pop3_handler.c @@ -262,6 +262,10 @@ ngx_mail_pop3_auth_state(ngx_event_t *rev) } } + if (s->buffer->pos < s->buffer->last) { + s->blocked = 1; + } + switch (rc) { case NGX_DONE: @@ -283,11 +287,14 @@ ngx_mail_pop3_auth_state(ngx_event_t *rev) case NGX_OK: s->args.nelts = 0; - s->buffer->pos = s->buffer->start; - s->buffer->last = s->buffer->start; + + if (s->buffer->pos == s->buffer->last) { + s->buffer->pos = s->buffer->start; + s->buffer->last = s->buffer->start; + } if (s->state) { - s->arg_start = s->buffer->start; + s->arg_start = s->buffer->pos; } if (ngx_handle_read_event(c->read, 0) != NGX_OK) { @@ -400,6 +407,8 @@ ngx_mail_pop3_stls(ngx_mail_session_t *s, ngx_connection_t *c) if (c->ssl == NULL) { sslcf = ngx_mail_get_module_srv_conf(s, ngx_mail_ssl_module); if (sslcf->starttls) { + s->buffer->pos = s->buffer->start; + s->buffer->last = s->buffer->start; c->read->handler = ngx_mail_starttls_handler; return NGX_OK; } diff --git a/src/mail/ngx_mail_proxy_module.c b/src/mail/ngx_mail_proxy_module.c index 38164af95..0f56d299e 100644 --- a/src/mail/ngx_mail_proxy_module.c +++ b/src/mail/ngx_mail_proxy_module.c @@ -327,6 +327,10 @@ ngx_mail_proxy_pop3_handler(ngx_event_t *rev) c->log->action = NULL; ngx_log_error(NGX_LOG_INFO, c->log, 0, "client logged in"); + if (s->buffer->pos < s->buffer->last) { + ngx_post_event(c->write, &ngx_posted_events); + } + ngx_mail_proxy_handler(s->connection->write); return; -- cgit From 3c660ef59b545bf76f2fb2b6a37ca02bcae4b2fb Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:20 +0300 Subject: Mail: fixed s->arg_start clearing on invalid IMAP commands. Previously, s->arg_start was left intact after invalid IMAP commands, and this might result in an argument incorrectly added to the following command. Similarly, s->backslash was left intact as well, leading to unneeded backslash removal. For example (LFs from the client are explicitly shown as ""): S: * OK IMAP4 ready C: a01 login "\ S: a01 BAD invalid command C: a0000000000\2 authenticate S: a00000000002 aBAD invalid command The backslash followed by LF generates invalid command with s->arg_start and s->backslash set, the following command incorrectly treats anything from the old s->arg_start to the space after the command as an argument, and removes the backslash from the tag. If there is no space, s->arg_end will be NULL. Both things seem to be harmless though. In particular: - This can be used to provide an incorrect argument to a command without arguments. The only command which seems to look at the single argument is AUTHENTICATE, and it checks the argument length before trying to access it. - Backslash removal uses the "end" pointer, and stops due to "src < end" condition instead of scanning all the process memory if s->arg_end is NULL (and arg[0].len is huge). - There should be no backslashes in unquoted strings. An obvious fix is to clear s->arg_start and s->backslash on invalid commands, similarly to how it is done in POP3 parsing (added in 810:e3aa8f305d21) and SMTP parsing. This, however, makes it clear that s->arg_start handling in the "done" label is wrong: s->arg_start cannot be legitimately set there, as it is expected to be cleared in all possible cases when the "done" label is reached. The relevant code is dead and will be removed by the following change. --- src/mail/ngx_mail_parse.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index e55010c60..5d5f3b460 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -637,7 +637,9 @@ done: invalid: s->state = sw_start; + s->arg_start = NULL; s->quoted = 0; + s->backslash = 0; s->no_sync_literal = 0; s->literal_len = 0; -- cgit From fabe28259f4e191fee660a240ba347a0a8d2f1dc Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:22 +0300 Subject: Mail: removed dead s->arg_start handling. As discussed in the previous change, s->arg_start handling in the "done" labels of ngx_mail_pop3_parse_command(), ngx_mail_imap_parse_command(), and ngx_mail_smtp_parse_command() is wrong: s->arg_start cannot be set there, as it is handled and cleared on all code paths where the "done" labels are reached. The relevant code is dead and now removed. --- src/mail/ngx_mail.h | 1 - src/mail/ngx_mail_parse.c | 47 ----------------------------------------------- 2 files changed, 48 deletions(-) (limited to 'src') diff --git a/src/mail/ngx_mail.h b/src/mail/ngx_mail.h index b865a3b9e..0a0bde290 100644 --- a/src/mail/ngx_mail.h +++ b/src/mail/ngx_mail.h @@ -238,7 +238,6 @@ typedef struct { ngx_uint_t state; u_char *cmd_start; u_char *arg_start; - u_char *arg_end; ngx_uint_t literal_len; } ngx_mail_session_t; diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index 5d5f3b460..299858c14 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -124,10 +124,8 @@ ngx_mail_pop3_parse_command(ngx_mail_session_t *s) break; case CR: state = sw_almost_done; - s->arg_end = p; break; case LF: - s->arg_end = p; goto done; default: if (s->args.nelts <= 2) { @@ -202,17 +200,6 @@ ngx_mail_pop3_parse_command(ngx_mail_session_t *s) done: s->buffer->pos = p + 1; - - if (s->arg_start) { - arg = ngx_array_push(&s->args); - if (arg == NULL) { - return NGX_ERROR; - } - arg->len = s->arg_end - s->arg_start; - arg->data = s->arg_start; - s->arg_start = NULL; - } - s->state = (s->command != NGX_POP3_AUTH) ? sw_start : sw_argument; return NGX_OK; @@ -220,7 +207,6 @@ done: invalid: s->state = sw_invalid; - s->arg_start = NULL; /* skip invalid command till LF */ @@ -436,10 +422,8 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) break; case CR: state = sw_almost_done; - s->arg_end = p; break; case LF: - s->arg_end = p; goto done; case '"': if (s->args.nelts <= 2) { @@ -614,22 +598,6 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) done: s->buffer->pos = p + 1; - - if (s->arg_start) { - arg = ngx_array_push(&s->args); - if (arg == NULL) { - return NGX_ERROR; - } - arg->len = s->arg_end - s->arg_start; - arg->data = s->arg_start; - - s->arg_start = NULL; - s->cmd_start = NULL; - s->quoted = 0; - s->no_sync_literal = 0; - s->literal_len = 0; - } - s->state = (s->command != NGX_IMAP_AUTHENTICATE) ? sw_start : sw_argument; return NGX_OK; @@ -637,7 +605,6 @@ done: invalid: s->state = sw_start; - s->arg_start = NULL; s->quoted = 0; s->backslash = 0; s->no_sync_literal = 0; @@ -786,10 +753,8 @@ ngx_mail_smtp_parse_command(ngx_mail_session_t *s) break; case CR: state = sw_almost_done; - s->arg_end = p; break; case LF: - s->arg_end = p; goto done; default: if (s->args.nelts <= 10) { @@ -849,17 +814,6 @@ ngx_mail_smtp_parse_command(ngx_mail_session_t *s) done: s->buffer->pos = p + 1; - - if (s->arg_start) { - arg = ngx_array_push(&s->args); - if (arg == NULL) { - return NGX_ERROR; - } - arg->len = s->arg_end - s->arg_start; - arg->data = s->arg_start; - s->arg_start = NULL; - } - s->state = (s->command != NGX_SMTP_AUTH) ? sw_start : sw_argument; return NGX_OK; @@ -867,7 +821,6 @@ done: invalid: s->state = sw_invalid; - s->arg_start = NULL; /* skip invalid command till LF */ -- cgit From 82840d165144584d1b288521266051a6b5a462eb Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:23 +0300 Subject: Mail: fixed backslash handling in IMAP literals. Previously, s->backslash was set if any of the arguments was a quoted string with a backslash character. After successful command parsing this resulted in all arguments being filtered to remove backslashes. This is, however, incorrect, as backslashes should not be removed from IMAP literals. For example: S: * OK IMAP4 ready C: a01 login {9} S: + OK C: user\name "pass\"word" S: * BAD internal server error resulted in "Auth-User: username" instead of "Auth-User: user\name" as it should. Fix is to apply backslash filtering on per-argument basis during parsing. --- src/mail/ngx_mail_imap_handler.c | 26 ++------------------------ src/mail/ngx_mail_parse.c | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/mail/ngx_mail_imap_handler.c b/src/mail/ngx_mail_imap_handler.c index 5dfdd7601..ba66b898c 100644 --- a/src/mail/ngx_mail_imap_handler.c +++ b/src/mail/ngx_mail_imap_handler.c @@ -101,10 +101,9 @@ ngx_mail_imap_init_protocol(ngx_event_t *rev) void ngx_mail_imap_auth_state(ngx_event_t *rev) { - u_char *p, *dst, *src, *end; - ngx_str_t *arg; + u_char *p; ngx_int_t rc; - ngx_uint_t tag, i; + ngx_uint_t tag; ngx_connection_t *c; ngx_mail_session_t *s; @@ -158,27 +157,6 @@ ngx_mail_imap_auth_state(ngx_event_t *rev) ngx_log_debug1(NGX_LOG_DEBUG_MAIL, c->log, 0, "imap auth command: %i", s->command); - if (s->backslash) { - - arg = s->args.elts; - - for (i = 0; i < s->args.nelts; i++) { - dst = arg[i].data; - end = dst + arg[i].len; - - for (src = dst; src < end; dst++) { - *dst = *src; - if (*src++ == '\\') { - *dst = *src++; - } - } - - arg[i].len = dst - arg[i].data; - } - - s->backslash = 0; - } - switch (s->mail_state) { case ngx_imap_start: diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index 299858c14..cc5293093 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -227,7 +227,7 @@ invalid: ngx_int_t ngx_mail_imap_parse_command(ngx_mail_session_t *s) { - u_char ch, *p, *c; + u_char ch, *p, *c, *dst, *src, *end; ngx_str_t *arg; enum { sw_start = 0, @@ -470,6 +470,22 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) } arg->len = p - s->arg_start; arg->data = s->arg_start; + + if (s->backslash) { + dst = s->arg_start; + end = p; + + for (src = dst; src < end; dst++) { + *dst = *src; + if (*src++ == '\\') { + *dst = *src++; + } + } + + arg->len = dst - s->arg_start; + s->backslash = 0; + } + s->arg_start = NULL; switch (ch) { -- cgit From 4617dd64b863df111e33b1b395709f4c2f427350 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:26 +0300 Subject: Mail: stricter checking of IMAP tags. Only "A-Za-z0-9-._" characters now allowed (which is stricter than what RFC 3501 requires, but expected to be enough for all known clients), and tags shouldn't be longer than 32 characters. --- src/mail/ngx_mail_parse.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'src') diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index cc5293093..47c9e3a90 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -265,6 +265,17 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) case LF: s->state = sw_start; return NGX_MAIL_PARSE_INVALID_COMMAND; + default: + if ((ch < 'A' || ch > 'Z') && (ch < 'a' || ch > 'z') + && (ch < '0' || ch > '9') && ch != '-' && ch != '.' + && ch != '_') + { + goto invalid; + } + if (p - s->buffer->start > 31) { + goto invalid; + } + break; } break; -- cgit From 5015209054f68141cd4f5f61e874d4497d4ef49c Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:28 +0300 Subject: Mail: IMAP pipelining support. The change is mostly the same as the SMTP one (04e43d03e153 and 3f5d0af4e40a), and ensures that nginx is able to properly handle or reject multiple IMAP commands. The s->cmd field is not really used and set for consistency. Non-synchronizing literals handling in invalid/unknown commands is limited, so when a non-synchronizing literal is detected at the end of a discarded line, the connection is closed. --- src/mail/ngx_mail.h | 1 + src/mail/ngx_mail_imap_handler.c | 17 +++++++---- src/mail/ngx_mail_parse.c | 61 +++++++++++++++++++++++++++++++--------- src/mail/ngx_mail_proxy_module.c | 4 +++ 4 files changed, 65 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/mail/ngx_mail.h b/src/mail/ngx_mail.h index 0a0bde290..07104df68 100644 --- a/src/mail/ngx_mail.h +++ b/src/mail/ngx_mail.h @@ -236,6 +236,7 @@ typedef struct { /* used to parse POP3/IMAP/SMTP command */ ngx_uint_t state; + u_char *tag_start; u_char *cmd_start; u_char *arg_start; ngx_uint_t literal_len; diff --git a/src/mail/ngx_mail_imap_handler.c b/src/mail/ngx_mail_imap_handler.c index ba66b898c..291e87a4d 100644 --- a/src/mail/ngx_mail_imap_handler.c +++ b/src/mail/ngx_mail_imap_handler.c @@ -226,6 +226,10 @@ ngx_mail_imap_auth_state(ngx_event_t *rev) ngx_str_set(&s->out, imap_next); } + if (s->buffer->pos < s->buffer->last) { + s->blocked = 1; + } + switch (rc) { case NGX_DONE: @@ -275,13 +279,14 @@ ngx_mail_imap_auth_state(ngx_event_t *rev) if (s->state) { /* preserve tag */ - s->arg_start = s->buffer->start + s->tag.len; - s->buffer->pos = s->arg_start; - s->buffer->last = s->arg_start; + s->arg_start = s->buffer->pos; } else { - s->buffer->pos = s->buffer->start; - s->buffer->last = s->buffer->start; + if (s->buffer->pos == s->buffer->last) { + s->buffer->pos = s->buffer->start; + s->buffer->last = s->buffer->start; + } + s->tag.len = 0; } } @@ -459,6 +464,8 @@ ngx_mail_imap_starttls(ngx_mail_session_t *s, ngx_connection_t *c) if (c->ssl == NULL) { sslcf = ngx_mail_get_module_srv_conf(s, ngx_mail_ssl_module); if (sslcf->starttls) { + s->buffer->pos = s->buffer->start; + s->buffer->last = s->buffer->start; c->read->handler = ngx_mail_starttls_handler; return NGX_OK; } diff --git a/src/mail/ngx_mail_parse.c b/src/mail/ngx_mail_parse.c index 47c9e3a90..4db1f18d3 100644 --- a/src/mail/ngx_mail_parse.c +++ b/src/mail/ngx_mail_parse.c @@ -231,6 +231,8 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) ngx_str_t *arg; enum { sw_start = 0, + sw_tag, + sw_invalid, sw_spaces_before_command, sw_command, sw_spaces_before_argument, @@ -253,18 +255,21 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) /* IMAP tag */ case sw_start: + s->tag_start = p; + state = sw_tag; + + /* fall through */ + + case sw_tag: switch (ch) { case ' ': - s->tag.len = p - s->buffer->start + 1; - s->tag.data = s->buffer->start; + s->tag.len = p - s->tag_start + 1; + s->tag.data = s->tag_start; state = sw_spaces_before_command; break; case CR: - s->state = sw_start; - return NGX_MAIL_PARSE_INVALID_COMMAND; case LF: - s->state = sw_start; - return NGX_MAIL_PARSE_INVALID_COMMAND; + goto invalid; default: if ((ch < 'A' || ch > 'Z') && (ch < 'a' || ch > 'z') && (ch < '0' || ch > '9') && ch != '-' && ch != '.' @@ -272,23 +277,23 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) { goto invalid; } - if (p - s->buffer->start > 31) { + if (p - s->tag_start > 31) { goto invalid; } break; } break; + case sw_invalid: + goto invalid; + case sw_spaces_before_command: switch (ch) { case ' ': break; case CR: - s->state = sw_start; - return NGX_MAIL_PARSE_INVALID_COMMAND; case LF: - s->state = sw_start; - return NGX_MAIL_PARSE_INVALID_COMMAND; + goto invalid; default: s->cmd_start = p; state = sw_command; @@ -408,6 +413,9 @@ ngx_mail_imap_parse_command(ngx_mail_session_t *s) goto invalid; } + s->cmd.data = s->cmd_start; + s->cmd.len = p - s->cmd_start; + switch (ch) { case ' ': state = sw_spaces_before_argument; @@ -631,13 +639,40 @@ done: invalid: - s->state = sw_start; + s->state = sw_invalid; s->quoted = 0; s->backslash = 0; s->no_sync_literal = 0; s->literal_len = 0; - return NGX_MAIL_PARSE_INVALID_COMMAND; + /* skip invalid command till LF */ + + for ( /* void */ ; p < s->buffer->last; p++) { + if (*p == LF) { + s->state = sw_start; + s->buffer->pos = p + 1; + + /* detect non-synchronizing literals */ + + if ((size_t) (p - s->buffer->start) > sizeof("{1+}") - 1) { + p--; + + if (*p == CR) { + p--; + } + + if (*p == '}' && *(p - 1) == '+') { + s->quit = 1; + } + } + + return NGX_MAIL_PARSE_INVALID_COMMAND; + } + } + + s->buffer->pos = p; + + return NGX_AGAIN; } diff --git a/src/mail/ngx_mail_proxy_module.c b/src/mail/ngx_mail_proxy_module.c index 0f56d299e..a7ab0776e 100644 --- a/src/mail/ngx_mail_proxy_module.c +++ b/src/mail/ngx_mail_proxy_module.c @@ -486,6 +486,10 @@ ngx_mail_proxy_imap_handler(ngx_event_t *rev) c->log->action = NULL; ngx_log_error(NGX_LOG_INFO, c->log, 0, "client logged in"); + if (s->buffer->pos < s->buffer->last) { + ngx_post_event(c->write, &ngx_posted_events); + } + ngx_mail_proxy_handler(s->connection->write); return; -- cgit From 173f16f736c10eae46cd15dd861b04b82d91a37a Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 19 May 2021 03:13:31 +0300 Subject: Mail: max_errors directive. Similarly to smtpd_hard_error_limit in Postfix and smtp_max_unknown_commands in Exim, specifies the number of errors after which the connection is closed. --- src/mail/ngx_mail.h | 3 +++ src/mail/ngx_mail_core_module.c | 10 ++++++++++ src/mail/ngx_mail_handler.c | 15 ++++++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/mail/ngx_mail.h b/src/mail/ngx_mail.h index 07104df68..21178c3e2 100644 --- a/src/mail/ngx_mail.h +++ b/src/mail/ngx_mail.h @@ -115,6 +115,8 @@ typedef struct { ngx_msec_t timeout; ngx_msec_t resolver_timeout; + ngx_uint_t max_errors; + ngx_str_t server_name; u_char *file_name; @@ -231,6 +233,7 @@ typedef struct { ngx_uint_t command; ngx_array_t args; + ngx_uint_t errors; ngx_uint_t login_attempt; /* used to parse POP3/IMAP/SMTP command */ diff --git a/src/mail/ngx_mail_core_module.c b/src/mail/ngx_mail_core_module.c index 408312423..115671ca4 100644 --- a/src/mail/ngx_mail_core_module.c +++ b/src/mail/ngx_mail_core_module.c @@ -85,6 +85,13 @@ static ngx_command_t ngx_mail_core_commands[] = { offsetof(ngx_mail_core_srv_conf_t, resolver_timeout), NULL }, + { ngx_string("max_errors"), + NGX_MAIL_MAIN_CONF|NGX_MAIL_SRV_CONF|NGX_CONF_TAKE1, + ngx_conf_set_num_slot, + NGX_MAIL_SRV_CONF_OFFSET, + offsetof(ngx_mail_core_srv_conf_t, max_errors), + NULL }, + ngx_null_command }; @@ -163,6 +170,8 @@ ngx_mail_core_create_srv_conf(ngx_conf_t *cf) cscf->timeout = NGX_CONF_UNSET_MSEC; cscf->resolver_timeout = NGX_CONF_UNSET_MSEC; + cscf->max_errors = NGX_CONF_UNSET_UINT; + cscf->resolver = NGX_CONF_UNSET_PTR; cscf->file_name = cf->conf_file->file.name.data; @@ -182,6 +191,7 @@ ngx_mail_core_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_msec_value(conf->resolver_timeout, prev->resolver_timeout, 30000); + ngx_conf_merge_uint_value(conf->max_errors, prev->max_errors, 5); ngx_conf_merge_str_value(conf->server_name, prev->server_name, ""); diff --git a/src/mail/ngx_mail_handler.c b/src/mail/ngx_mail_handler.c index 57503e9a6..246ba97cf 100644 --- a/src/mail/ngx_mail_handler.c +++ b/src/mail/ngx_mail_handler.c @@ -874,7 +874,20 @@ ngx_mail_read_command(ngx_mail_session_t *s, ngx_connection_t *c) return NGX_MAIL_PARSE_INVALID_COMMAND; } - if (rc == NGX_IMAP_NEXT || rc == NGX_MAIL_PARSE_INVALID_COMMAND) { + if (rc == NGX_MAIL_PARSE_INVALID_COMMAND) { + + s->errors++; + + if (s->errors >= cscf->max_errors) { + ngx_log_error(NGX_LOG_INFO, c->log, 0, + "client sent too many invalid commands"); + s->quit = 1; + } + + return rc; + } + + if (rc == NGX_IMAP_NEXT) { return rc; } -- cgit From 6029e211c63895c44a942bacc32c6d2f565cc3e3 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Wed, 19 May 2021 16:24:13 +0300 Subject: Core: fixed comment about msie_refresh escaping. After 12a656452ad1, the "%" character is no longer escaped by ngx_escape_uri(NGX_ESCAPE_REFRESH). --- src/core/ngx_string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/core/ngx_string.c b/src/core/ngx_string.c index 93f32ea0c..5cc9b26f9 100644 --- a/src/core/ngx_string.c +++ b/src/core/ngx_string.c @@ -1573,7 +1573,7 @@ ngx_escape_uri(u_char *dst, u_char *src, size_t size, ngx_uint_t type) 0xffffffff /* 1111 1111 1111 1111 1111 1111 1111 1111 */ }; - /* " ", """, "%", "'", %00-%1F, %7F-%FF */ + /* " ", """, "'", %00-%1F, %7F-%FF */ static uint32_t refresh[] = { 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ -- cgit From ecbe06b9fee57207d84be9ac39d1aa10b2d0fbd8 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Thu, 20 May 2021 19:59:16 +0300 Subject: Stream: the "fastopen" parameter of the "listen" directive. Based on a patch by Anbang Wen. --- src/stream/ngx_stream.c | 4 ++++ src/stream/ngx_stream.h | 3 +++ src/stream/ngx_stream_core_module.c | 25 +++++++++++++++++++++++++ 3 files changed, 32 insertions(+) (limited to 'src') diff --git a/src/stream/ngx_stream.c b/src/stream/ngx_stream.c index 78356754e..3304c843c 100644 --- a/src/stream/ngx_stream.c +++ b/src/stream/ngx_stream.c @@ -510,6 +510,10 @@ ngx_stream_optimize_servers(ngx_conf_t *cf, ngx_array_t *ports) ls->ipv6only = addr[i].opt.ipv6only; #endif +#if (NGX_HAVE_TCP_FASTOPEN) + ls->fastopen = addr[i].opt.fastopen; +#endif + #if (NGX_HAVE_REUSEPORT) ls->reuseport = addr[i].opt.reuseport; #endif diff --git a/src/stream/ngx_stream.h b/src/stream/ngx_stream.h index 9e3583295..46c362296 100644 --- a/src/stream/ngx_stream.h +++ b/src/stream/ngx_stream.h @@ -65,6 +65,9 @@ typedef struct { int backlog; int rcvbuf; int sndbuf; +#if (NGX_HAVE_TCP_FASTOPEN) + int fastopen; +#endif int type; } ngx_stream_listen_t; diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c index 9b6afe974..d96d27ab5 100644 --- a/src/stream/ngx_stream_core_module.c +++ b/src/stream/ngx_stream_core_module.c @@ -615,6 +615,10 @@ ngx_stream_core_listen(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) ls->type = SOCK_STREAM; ls->ctx = cf->ctx; +#if (NGX_HAVE_TCP_FASTOPEN) + ls->fastopen = -1; +#endif + #if (NGX_HAVE_INET6) ls->ipv6only = 1; #endif @@ -635,6 +639,21 @@ ngx_stream_core_listen(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) continue; } +#if (NGX_HAVE_TCP_FASTOPEN) + if (ngx_strncmp(value[i].data, "fastopen=", 9) == 0) { + ls->fastopen = ngx_atoi(value[i].data + 9, value[i].len - 9); + ls->bind = 1; + + if (ls->fastopen == NGX_ERROR) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "invalid fastopen \"%V\"", &value[i]); + return NGX_CONF_ERROR; + } + + continue; + } +#endif + if (ngx_strncmp(value[i].data, "backlog=", 8) == 0) { ls->backlog = ngx_atoi(value[i].data + 8, value[i].len - 8); ls->bind = 1; @@ -859,6 +878,12 @@ ngx_stream_core_listen(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) if (ls->proxy_protocol) { return "\"proxy_protocol\" parameter is incompatible with \"udp\""; } + +#if (NGX_HAVE_TCP_FASTOPEN) + if (ls->fastopen != -1) { + return "\"fastopen\" parameter is incompatible with \"udp\""; + } +#endif } als = cmcf->listen.elts; -- cgit From 52d0ec7d1799cc67452c32052e96b8cdace0c7b7 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Mon, 24 May 2021 18:23:42 +0300 Subject: Fixed log action when using SSL certificates with variables. When variables are used in ssl_certificate or ssl_certificate_key, a request is created in the certificate callback to evaluate the variables, and then freed. Freeing it, however, updates c->log->action to "closing request", resulting in confusing error messages like "client timed out ... while closing request" when a client times out during the SSL handshake. Fix is to restore c->log->action after calling ngx_http_free_request(). --- src/http/ngx_http_request.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 684fabdd6..81b27a386 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -1043,12 +1043,14 @@ ngx_http_ssl_certificate(ngx_ssl_conn_t *ssl_conn, void *arg) } ngx_http_free_request(r, 0); + c->log->action = "SSL handshaking"; c->destroyed = 0; return 1; failed: ngx_http_free_request(r, 0); + c->log->action = "SSL handshaking"; c->destroyed = 0; return 0; } -- cgit From 41a241b3ef74dbbe3d82ab2ebbe682919e4a0b90 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Mon, 24 May 2021 21:55:20 +0300 Subject: Location header escaping in redirects (ticket #882). The header is escaped in redirects based on request URI or location name (auto redirect). --- src/http/modules/ngx_http_dav_module.c | 25 ++++++++++++++++++++- src/http/modules/ngx_http_static_module.c | 17 +++++++++++--- src/http/ngx_http.c | 37 +++++++++++++++++++++++++++++++ src/http/ngx_http_core_module.c | 7 +++--- src/http/ngx_http_core_module.h | 1 + 5 files changed, 80 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/http/modules/ngx_http_dav_module.c b/src/http/modules/ngx_http_dav_module.c index 8b69e6f38..0cc9ae18b 100644 --- a/src/http/modules/ngx_http_dav_module.c +++ b/src/http/modules/ngx_http_dav_module.c @@ -1072,6 +1072,10 @@ ngx_http_dav_error(ngx_log_t *log, ngx_err_t err, ngx_int_t not_found, static ngx_int_t ngx_http_dav_location(ngx_http_request_t *r) { + u_char *p; + size_t len; + uintptr_t escape; + r->headers_out.location = ngx_list_push(&r->headers_out.headers); if (r->headers_out.location == NULL) { return NGX_ERROR; @@ -1079,7 +1083,26 @@ ngx_http_dav_location(ngx_http_request_t *r) r->headers_out.location->hash = 1; ngx_str_set(&r->headers_out.location->key, "Location"); - r->headers_out.location->value = r->uri; + + escape = 2 * ngx_escape_uri(NULL, r->uri.data, r->uri.len, NGX_ESCAPE_URI); + + if (escape) { + len = r->uri.len + escape; + + p = ngx_pnalloc(r->pool, len); + if (p == NULL) { + ngx_http_clear_location(r); + return NGX_ERROR; + } + + r->headers_out.location->value.len = len; + r->headers_out.location->value.data = p; + + ngx_escape_uri(p, r->uri.data, r->uri.len, NGX_ESCAPE_URI); + + } else { + r->headers_out.location->value = r->uri; + } return NGX_OK; } diff --git a/src/http/modules/ngx_http_static_module.c b/src/http/modules/ngx_http_static_module.c index 282d6ee98..cf29d5a6d 100644 --- a/src/http/modules/ngx_http_static_module.c +++ b/src/http/modules/ngx_http_static_module.c @@ -50,6 +50,7 @@ ngx_http_static_handler(ngx_http_request_t *r) { u_char *last, *location; size_t root, len; + uintptr_t escape; ngx_str_t path; ngx_int_t rc; ngx_uint_t level; @@ -155,14 +156,18 @@ ngx_http_static_handler(ngx_http_request_t *r) return NGX_HTTP_INTERNAL_SERVER_ERROR; } - len = r->uri.len + 1; + escape = 2 * ngx_escape_uri(NULL, r->uri.data, r->uri.len, + NGX_ESCAPE_URI); - if (!clcf->alias && r->args.len == 0) { + if (!clcf->alias && r->args.len == 0 && escape == 0) { + len = r->uri.len + 1; location = path.data + root; *last = '/'; } else { + len = r->uri.len + escape + 1; + if (r->args.len) { len += r->args.len + 1; } @@ -173,7 +178,13 @@ ngx_http_static_handler(ngx_http_request_t *r) return NGX_HTTP_INTERNAL_SERVER_ERROR; } - last = ngx_copy(location, r->uri.data, r->uri.len); + if (escape) { + last = (u_char *) ngx_escape_uri(location, r->uri.data, + r->uri.len, NGX_ESCAPE_URI); + + } else { + last = ngx_copy(location, r->uri.data, r->uri.len); + } *last = '/'; diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c index e1d3d0034..3d5c4ac16 100644 --- a/src/http/ngx_http.c +++ b/src/http/ngx_http.c @@ -37,6 +37,8 @@ static ngx_int_t ngx_http_init_locations(ngx_conf_t *cf, ngx_http_core_srv_conf_t *cscf, ngx_http_core_loc_conf_t *pclcf); static ngx_int_t ngx_http_init_static_location_trees(ngx_conf_t *cf, ngx_http_core_loc_conf_t *pclcf); +static ngx_int_t ngx_http_escape_location_name(ngx_conf_t *cf, + ngx_http_core_loc_conf_t *clcf); static ngx_int_t ngx_http_cmp_locations(const ngx_queue_t *one, const ngx_queue_t *two); static ngx_int_t ngx_http_join_exact_locations(ngx_conf_t *cf, @@ -882,6 +884,41 @@ ngx_http_add_location(ngx_conf_t *cf, ngx_queue_t **locations, ngx_queue_insert_tail(*locations, &lq->queue); + if (ngx_http_escape_location_name(cf, clcf) != NGX_OK) { + return NGX_ERROR; + } + + return NGX_OK; +} + + +static ngx_int_t +ngx_http_escape_location_name(ngx_conf_t *cf, ngx_http_core_loc_conf_t *clcf) +{ + u_char *p; + size_t len; + uintptr_t escape; + + escape = 2 * ngx_escape_uri(NULL, clcf->name.data, clcf->name.len, + NGX_ESCAPE_URI); + + if (escape) { + len = clcf->name.len + escape; + + p = ngx_pnalloc(cf->pool, len); + if (p == NULL) { + return NGX_ERROR; + } + + clcf->escaped_name.len = len; + clcf->escaped_name.data = p; + + ngx_escape_uri(p, clcf->name.data, clcf->name.len, NGX_ESCAPE_URI); + + } else { + clcf->escaped_name = clcf->name; + } + return NGX_OK; } diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c index 96a945be4..bad43ea5c 100644 --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -1010,10 +1010,10 @@ ngx_http_core_find_config_phase(ngx_http_request_t *r, ngx_str_set(&r->headers_out.location->key, "Location"); if (r->args.len == 0) { - r->headers_out.location->value = clcf->name; + r->headers_out.location->value = clcf->escaped_name; } else { - len = clcf->name.len + 1 + r->args.len; + len = clcf->escaped_name.len + 1 + r->args.len; p = ngx_pnalloc(r->pool, len); if (p == NULL) { @@ -1025,7 +1025,7 @@ ngx_http_core_find_config_phase(ngx_http_request_t *r, r->headers_out.location->value.len = len; r->headers_out.location->value.data = p; - p = ngx_cpymem(p, clcf->name.data, clcf->name.len); + p = ngx_cpymem(p, clcf->escaped_name.data, clcf->escaped_name.len); *p++ = '?'; ngx_memcpy(p, r->args.data, r->args.len); } @@ -3467,6 +3467,7 @@ ngx_http_core_create_loc_conf(ngx_conf_t *cf) /* * set by ngx_pcalloc(): * + * clcf->escaped_name = { 0, NULL }; * clcf->root = { 0, NULL }; * clcf->limit_except = 0; * clcf->post_action = { 0, NULL }; diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h index 2341fd463..002d4193a 100644 --- a/src/http/ngx_http_core_module.h +++ b/src/http/ngx_http_core_module.h @@ -299,6 +299,7 @@ typedef struct { struct ngx_http_core_loc_conf_s { ngx_str_t name; /* location name */ + ngx_str_t escaped_name; #if (NGX_PCRE) ngx_http_regex_t *regex; -- cgit From 9f1dcb0c0473641730b871dee984016ff19d2c53 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:36 +0300 Subject: Resolver: fixed off-by-one write in ngx_resolver_copy(). Reported by Luis Merino, Markus Vervier, Eric Sesterhenn, X41 D-Sec GmbH. --- src/core/ngx_resolver.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 793907010..63b26193d 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -4008,15 +4008,15 @@ done: n = *src++; } else { + if (dst != name->data) { + *dst++ = '.'; + } + ngx_strlow(dst, src, n); dst += n; src += n; n = *src++; - - if (n != 0) { - *dst++ = '.'; - } } if (n == 0) { -- cgit From 077a890a76fff4f071776184aed881b5f314c98a Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:38 +0300 Subject: Resolver: fixed off-by-one read in ngx_resolver_copy(). It is believed to be harmless, and in the worst case it uses some uninitialized memory as a part of the compression pointer length, eventually leading to the "name is out of DNS response" error. --- src/core/ngx_resolver.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 63b26193d..9b1317234 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -3958,6 +3958,11 @@ ngx_resolver_copy(ngx_resolver_t *r, ngx_str_t *name, u_char *buf, u_char *src, } if (n & 0xc0) { + if (p >= last) { + err = "name is out of DNS response"; + goto invalid; + } + n = ((n & 0x3f) << 8) + *p; p = &buf[n]; -- cgit From bbd403a7ab3810fe82ecd1d6ebca9fc34d68126a Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:41 +0300 Subject: Resolver: fixed label types handling in ngx_resolver_copy(). Previously, anything with any of the two high bits set were interpreted as compression pointers. This is incorrect, as RFC 1035 clearly states that "The 10 and 01 combinations are reserved for future use". Further, the 01 combination is actually allocated for EDNS extended label type (see RFC 2671 and RFC 6891), not really used though. Fix is to reject unrecognized label types rather than misinterpreting them as compression pointers. --- src/core/ngx_resolver.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 9b1317234..12dab09ea 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -3958,6 +3958,11 @@ ngx_resolver_copy(ngx_resolver_t *r, ngx_str_t *name, u_char *buf, u_char *src, } if (n & 0xc0) { + if ((n & 0xc0) != 0xc0) { + err = "invalid label type in DNS response"; + goto invalid; + } + if (p >= last) { err = "name is out of DNS response"; goto invalid; -- cgit From f1dd1d50e090b32a765295daea5f167f1077d706 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:43 +0300 Subject: Resolver: reworked ngx_resolver_copy() copy loop. To make the code easier to read, reworked the ngx_resolver_copy() copy loop to match the one used to calculate length. No functional changes. --- src/core/ngx_resolver.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 12dab09ea..0d7fd7915 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -4008,15 +4008,18 @@ done: name->data = dst; - n = *src++; - for ( ;; ) { + n = *src++; + + if (n == 0) { + name->len = dst - name->data; + return NGX_OK; + } + if (n & 0xc0) { n = ((n & 0x3f) << 8) + *src; src = &buf[n]; - n = *src++; - } else { if (dst != name->data) { *dst++ = '.'; @@ -4025,13 +4028,6 @@ done: ngx_strlow(dst, src, n); dst += n; src += n; - - n = *src++; - } - - if (n == 0) { - name->len = dst - name->data; - return NGX_OK; } } } -- cgit From f85d7016949b34119b5f4c53ddbfac4f199b4343 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:45 +0300 Subject: Resolver: simplified ngx_resolver_copy(). Instead of checking on each label if we need to place a dot or not, now it always adds a dot after a label, and reduces the resulting length afterwards. --- src/core/ngx_resolver.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 0d7fd7915..9ce53b930 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -3939,11 +3939,11 @@ ngx_resolver_copy(ngx_resolver_t *r, ngx_str_t *name, u_char *buf, u_char *src, { char *err; u_char *p, *dst; - ssize_t len; + size_t len; ngx_uint_t i, n; p = src; - len = -1; + len = 0; /* * compression pointers allow to create endless loop, so we set limit; @@ -3996,7 +3996,7 @@ done: return NGX_OK; } - if (len == -1) { + if (len == 0) { ngx_str_null(name); return NGX_OK; } @@ -4012,7 +4012,7 @@ done: n = *src++; if (n == 0) { - name->len = dst - name->data; + name->len = dst - name->data - 1; return NGX_OK; } @@ -4021,13 +4021,10 @@ done: src = &buf[n]; } else { - if (dst != name->data) { - *dst++ = '.'; - } - ngx_strlow(dst, src, n); dst += n; src += n; + *dst++ = '.'; } } } -- cgit From e860ecce82f1ee9cffb228d29d3ad61375b29aff Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:50 +0300 Subject: Resolver: explicit check for compression pointers in question. Since nginx always uses exactly one entry in the question section of a DNS query, and never uses compression pointers in this entry, parsing of a DNS response in ngx_resolver_process_response() does not expect compression pointers to appear in the question section of the DNS response. Indeed, compression pointers in the first name of a DNS response hardly make sense, do not seem to be allowed by RFC 1035 (which says "a pointer to a prior occurance of the same name", note "prior"), and were never observed in practice. Added an explicit check to ngx_resolver_process_response()'s parsing of the question section to properly report an error if compression pointers nevertheless appear in the question section. --- src/core/ngx_resolver.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 9ce53b930..58d5f3ec4 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -1798,6 +1798,12 @@ ngx_resolver_process_response(ngx_resolver_t *r, u_char *buf, size_t n, i = sizeof(ngx_resolver_hdr_t); while (i < (ngx_uint_t) n) { + + if (buf[i] & 0xc0) { + err = "unexpected compression pointer in DNS response"; + goto done; + } + if (buf[i] == '\0') { goto found; } -- cgit