From f512483571a922372d65ab31933c877fa862fa3d Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Thu, 23 Jan 2025 21:14:38 -0800 Subject: [PATCH 1/2] SSL: minimize the number of preserved password copies. This also fixes dynamic loading of password protected certificates via proxy_ssl_certificate and friends. After d791b4a (1.23.1), when upstream SSL context was shared between several locations, only the first one had properly preserved passwords. The ngx_ssl_preserve_passwords() interface is kept the same for compatibility sake, but will be simplified in future commit. --- src/event/ngx_event_openssl.c | 58 +++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index 0681ca3a2..4ec7a5054 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -565,12 +565,13 @@ ngx_ssl_connection_certificate(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *cert, ngx_str_t *key, ngx_ssl_cache_t *cache, ngx_array_t *passwords) { - char *err; - X509 *x509; - u_long n; - EVP_PKEY *pkey; - ngx_uint_t mask; - STACK_OF(X509) *chain; + char *err; + X509 *x509; + u_long n; + EVP_PKEY *pkey; + ngx_uint_t mask; + STACK_OF(X509) *chain; + static ngx_array_t empty_passwords; mask = 0; @@ -618,6 +619,16 @@ retry: #endif + if (passwords == NULL) { + + /* + * Make sure OpenSSL's default password callback + * won't block on reading from stdin. + */ + + passwords = &empty_passwords; + } + pkey = ngx_ssl_cache_connection_fetch(cache, pool, NGX_SSL_CACHE_PKEY | mask, &err, key, passwords); @@ -1108,7 +1119,7 @@ ngx_ssl_read_password_file(ngx_conf_t *cf, ngx_str_t *file) ssize_t n; ngx_fd_t fd; ngx_str_t *pwd; - ngx_array_t *passwords; + ngx_array_t *passwords, *temp_passwords; ngx_pool_cleanup_t *cln; u_char buf[NGX_SSL_PASSWORD_BUFFER_SIZE]; @@ -1226,6 +1237,14 @@ cleanup: ngx_explicit_memzero(buf, NGX_SSL_PASSWORD_BUFFER_SIZE); + if (passwords != NULL) { + temp_passwords = passwords; + passwords = ngx_palloc(cf->pool, sizeof(ngx_array_t)); + if (passwords != NULL) { + *passwords = *temp_passwords; + } + } + return passwords; } @@ -1235,7 +1254,7 @@ ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) { ngx_str_t *opwd, *pwd; ngx_uint_t i; - ngx_array_t *pwds; + ngx_array_t opwds; ngx_pool_cleanup_t *cln; static ngx_array_t empty_passwords; @@ -1256,8 +1275,15 @@ ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) * runtime they have to be copied to the configuration pool. */ - pwds = ngx_array_create(cf->pool, passwords->nelts, sizeof(ngx_str_t)); - if (pwds == NULL) { + if (passwords->pool == cf->pool || passwords == &empty_passwords) { + return passwords; + } + + opwds = *passwords; + + if (ngx_array_init(passwords, cf->pool, opwds.nelts, sizeof(ngx_str_t)) + != NGX_OK) + { return NULL; } @@ -1267,13 +1293,13 @@ ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) } cln->handler = ngx_ssl_passwords_cleanup; - cln->data = pwds; + cln->data = passwords; - opwd = passwords->elts; + opwd = opwds.elts; - for (i = 0; i < passwords->nelts; i++) { + for (i = 0; i < opwds.nelts; i++) { - pwd = ngx_array_push(pwds); + pwd = ngx_array_push(passwords); if (pwd == NULL) { return NULL; } @@ -1282,14 +1308,14 @@ ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) pwd->data = ngx_pnalloc(cf->pool, pwd->len); if (pwd->data == NULL) { - pwds->nelts--; + passwords->nelts--; return NULL; } ngx_memcpy(pwd->data, opwd[i].data, opwd[i].len); } - return pwds; + return passwords; } From b6ac92d28dfcdcb8933a09ef077fa5155c375960 Mon Sep 17 00:00:00 2001 From: Pavel Pautov Date: Thu, 23 Jan 2025 22:07:58 -0800 Subject: [PATCH 2/2] SSL: simplify ngx_ssl_preserve_passwords() interface. --- src/event/ngx_event_openssl.c | 28 +++++++----------------- src/event/ngx_event_openssl.h | 3 +-- src/http/modules/ngx_http_grpc_module.c | 6 ++--- src/http/modules/ngx_http_proxy_module.c | 6 ++--- src/http/modules/ngx_http_ssl_module.c | 3 +-- src/http/modules/ngx_http_uwsgi_module.c | 6 ++--- src/stream/ngx_stream_proxy_module.c | 4 +--- src/stream/ngx_stream_ssl_module.c | 3 +-- 8 files changed, 21 insertions(+), 38 deletions(-) diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index 4ec7a5054..7a6934252 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -1249,24 +1249,16 @@ cleanup: } -ngx_array_t * +ngx_int_t ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) { ngx_str_t *opwd, *pwd; ngx_uint_t i; ngx_array_t opwds; ngx_pool_cleanup_t *cln; - static ngx_array_t empty_passwords; - if (passwords == NULL) { - - /* - * If there are no passwords, an empty array is used - * to make sure OpenSSL's default password callback - * won't block on reading from stdin. - */ - - return &empty_passwords; + if (passwords == NULL || passwords->pool == cf->pool) { + return NGX_OK; } /* @@ -1275,21 +1267,17 @@ ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) * runtime they have to be copied to the configuration pool. */ - if (passwords->pool == cf->pool || passwords == &empty_passwords) { - return passwords; - } - opwds = *passwords; if (ngx_array_init(passwords, cf->pool, opwds.nelts, sizeof(ngx_str_t)) != NGX_OK) { - return NULL; + return NGX_ERROR; } cln = ngx_pool_cleanup_add(cf->pool, 0); if (cln == NULL) { - return NULL; + return NGX_ERROR; } cln->handler = ngx_ssl_passwords_cleanup; @@ -1301,7 +1289,7 @@ ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) pwd = ngx_array_push(passwords); if (pwd == NULL) { - return NULL; + return NGX_ERROR; } pwd->len = opwd[i].len; @@ -1309,13 +1297,13 @@ ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords) if (pwd->data == NULL) { passwords->nelts--; - return NULL; + return NGX_ERROR; } ngx_memcpy(pwd->data, opwd[i].data, opwd[i].len); } - return passwords; + return NGX_OK; } diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h index 9ad4d177b..6213be7d7 100644 --- a/src/event/ngx_event_openssl.h +++ b/src/event/ngx_event_openssl.h @@ -248,8 +248,7 @@ void *ngx_ssl_cache_connection_fetch(ngx_ssl_cache_t *cache, ngx_pool_t *pool, ngx_uint_t index, char **err, ngx_str_t *path, void *data); ngx_array_t *ngx_ssl_read_password_file(ngx_conf_t *cf, ngx_str_t *file); -ngx_array_t *ngx_ssl_preserve_passwords(ngx_conf_t *cf, - ngx_array_t *passwords); +ngx_int_t ngx_ssl_preserve_passwords(ngx_conf_t *cf, ngx_array_t *passwords); ngx_int_t ngx_ssl_dhparam(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file); ngx_int_t ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *name); ngx_int_t ngx_ssl_early_data(ngx_conf_t *cf, ngx_ssl_t *ssl, diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c index 8e246c3cf..f54f551de 100644 --- a/src/http/modules/ngx_http_grpc_module.c +++ b/src/http/modules/ngx_http_grpc_module.c @@ -5080,9 +5080,9 @@ ngx_http_grpc_set_ssl(ngx_conf_t *cf, ngx_http_grpc_loc_conf_t *glcf) if (glcf->upstream.ssl_certificate->lengths || glcf->upstream.ssl_certificate_key->lengths) { - glcf->upstream.ssl_passwords = - ngx_ssl_preserve_passwords(cf, glcf->upstream.ssl_passwords); - if (glcf->upstream.ssl_passwords == NULL) { + if (ngx_ssl_preserve_passwords(cf, 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 27c34fef2..a221da8c3 100644 --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -5340,9 +5340,9 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *plcf) if (plcf->upstream.ssl_certificate->lengths || plcf->upstream.ssl_certificate_key->lengths) { - plcf->upstream.ssl_passwords = - ngx_ssl_preserve_passwords(cf, plcf->upstream.ssl_passwords); - if (plcf->upstream.ssl_passwords == NULL) { + if (ngx_ssl_preserve_passwords(cf, plcf->upstream.ssl_passwords) + != NGX_OK) + { return NGX_ERROR; } diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c index dbfe5c08b..1dd76b1fb 100644 --- a/src/http/modules/ngx_http_ssl_module.c +++ b/src/http/modules/ngx_http_ssl_module.c @@ -988,8 +988,7 @@ found: } } - conf->passwords = ngx_ssl_preserve_passwords(cf, conf->passwords); - if (conf->passwords == NULL) { + if (ngx_ssl_preserve_passwords(cf, conf->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 14aae5bf1..d4d0ecc5e 100644 --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -2688,9 +2688,9 @@ ngx_http_uwsgi_set_ssl(ngx_conf_t *cf, ngx_http_uwsgi_loc_conf_t *uwcf) if (uwcf->upstream.ssl_certificate->lengths || uwcf->upstream.ssl_certificate_key->lengths) { - uwcf->upstream.ssl_passwords = - ngx_ssl_preserve_passwords(cf, uwcf->upstream.ssl_passwords); - if (uwcf->upstream.ssl_passwords == NULL) { + if (ngx_ssl_preserve_passwords(cf, uwcf->upstream.ssl_passwords) + != NGX_OK) + { return NGX_ERROR; } diff --git a/src/stream/ngx_stream_proxy_module.c b/src/stream/ngx_stream_proxy_module.c index 7f8bfc4e0..c2185502e 100644 --- a/src/stream/ngx_stream_proxy_module.c +++ b/src/stream/ngx_stream_proxy_module.c @@ -2421,9 +2421,7 @@ ngx_stream_proxy_set_ssl(ngx_conf_t *cf, ngx_stream_proxy_srv_conf_t *pscf) if (pscf->ssl_certificate->lengths || pscf->ssl_certificate_key->lengths) { - pscf->ssl_passwords = - ngx_ssl_preserve_passwords(cf, pscf->ssl_passwords); - if (pscf->ssl_passwords == NULL) { + if (ngx_ssl_preserve_passwords(cf, pscf->ssl_passwords) != NGX_OK) { return NGX_ERROR; } diff --git a/src/stream/ngx_stream_ssl_module.c b/src/stream/ngx_stream_ssl_module.c index b84995d61..e7eceebae 100644 --- a/src/stream/ngx_stream_ssl_module.c +++ b/src/stream/ngx_stream_ssl_module.c @@ -1207,8 +1207,7 @@ found: } } - conf->passwords = ngx_ssl_preserve_passwords(cf, conf->passwords); - if (conf->passwords == NULL) { + if (ngx_ssl_preserve_passwords(cf, conf->passwords) != NGX_OK) { return NGX_ERROR; }