Hello!

On Thu, Jun 16, 2022 at 08:26:48AM +0000, Pavel Pautov via nginx-devel wrote:

> Looks like, we've made a full circle then... I've replied to
> that suggestion already and in last e-mail (with patch) I note
> that moving additional logic into the ngx_http_proxy_set_ssl()
> has its own drawbacks, but I can certainly move more stuff into
> it.
>
> So do you envision something like "ngx_http_proxy_set_ssl(cf,
> conf, prev, reuse_ssl)"? As previously we've established that
> directives merging stays out of ngx_http_proxy_set_ssl (and
> reuse_ssl calculation has to happen before it).

I don't think further attempts to explain how to write readable
code worth the effort.

Please see the patch below. Review and testing appreciated.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1655429915 -10800
# Fri Jun 17 04:38:35 2022 +0300
# Node ID e4a0eeb3edba037f0d090023a2242bda2f8dcb03
# Parent e23a385cd0ec866a3eb1d8c9c956991e1ed50d78
Upstream: optimized use of SSL contexts (ticket #1234).

To ensure optimal use of memory, SSL contexts for proxying are now
inherited from previous levels as long as relevant proxy_ssl_* directives
are not redefined.

Further, when no proxy_ssl_* directives are redefined in a server block,
we now preserve plcf->upstream.ssl in the "http" section configuration
to inherit it to all servers.

Similar changes made in uwsgi and grpc.

diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c
--- a/src/http/modules/ngx_http_grpc_module.c
+++ b/src/http/modules/ngx_http_grpc_module.c
@@ -210,7 +210,7 @@ static char *ngx_http_grpc_ssl_password_
static char *ngx_http_grpc_ssl_conf_command_check(ngx_conf_t *cf, void *post,
void *data);
static ngx_int_t ngx_http_grpc_set_ssl(ngx_conf_t *cf,
- ngx_http_grpc_loc_conf_t *glcf);
+ ngx_http_grpc_loc_conf_t *glcf, ngx_http_grpc_loc_conf_t *prev);
#endif


@@ -562,7 +562,7 @@ ngx_http_grpc_handler(ngx_http_request_t
ctx->host = glcf->host;

#if (NGX_HTTP_SSL)
- u->ssl = (glcf->upstream.ssl != NULL);
+ u->ssl = glcf->ssl;

if (u->ssl) {
ngx_str_set(&u->schema, "grpcs://");
@@ -4495,7 +4495,7 @@ ngx_http_grpc_merge_loc_conf(ngx_conf_t
ngx_conf_merge_ptr_value(conf->ssl_conf_commands,
prev->ssl_conf_commands, NULL);

- if (conf->ssl && ngx_http_grpc_set_ssl(cf, conf) != NGX_OK) {
+ if (ngx_http_grpc_set_ssl(cf, conf, prev) != NGX_OK) {
return NGX_CONF_ERROR;
}

@@ -4524,7 +4524,7 @@ ngx_http_grpc_merge_loc_conf(ngx_conf_t
conf->grpc_values = prev->grpc_values;

#if (NGX_HTTP_SSL)
- conf->upstream.ssl = prev->upstream.ssl;
+ conf->ssl = prev->ssl;
#endif
}

@@ -4874,10 +4874,37 @@ ngx_http_grpc_ssl_conf_command_check(ngx


static ngx_int_t
-ngx_http_grpc_set_ssl(ngx_conf_t *cf, ngx_http_grpc_loc_conf_t *glcf)
+ngx_http_grpc_set_ssl(ngx_conf_t *cf, ngx_http_grpc_loc_conf_t *glcf,
+ ngx_http_grpc_loc_conf_t *prev)
{
+ ngx_uint_t preserve;
ngx_pool_cleanup_t *cln;

+ if (glcf->ssl_protocols == prev->ssl_protocols
+ && glcf->ssl_ciphers.data == prev->ssl_ciphers.data
+ && glcf->upstream.ssl_certificate == prev->upstream.ssl_certificate
+ && glcf->upstream.ssl_certificate_key
+ == prev->upstream.ssl_certificate_key
+ && glcf->upstream.ssl_passwords == prev->upstream.ssl_passwords
+ && glcf->upstream.ssl_verify == prev->upstream.ssl_verify
+ && glcf->ssl_verify_depth == prev->ssl_verify_depth
+ && glcf->ssl_trusted_certificate.data
+ == prev->ssl_trusted_certificate.data
+ && glcf->ssl_crl.data == prev->ssl_crl.data
+ && glcf->upstream.ssl_session_reuse == prev->upstream.ssl_session_reuse
+ && glcf->ssl_conf_commands == prev->ssl_conf_commands)
+ {
+ if (prev->upstream.ssl) {
+ glcf->upstream.ssl = prev->upstream.ssl;
+ return NGX_OK;
+ }
+
+ preserve = 1;
+
+ } else {
+ preserve = 0;
+ }
+
glcf->upstream.ssl = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_t));
if (glcf->upstream.ssl == NULL) {
return NGX_ERROR;
@@ -4984,6 +5011,15 @@ ngx_http_grpc_set_ssl(ngx_conf_t *cf, ng
return NGX_ERROR;
}

+ /*
+ * special handling to preserve glcf->upstream.ssl
+ * in the "http" section to inherit it to all servers
+ */
+
+ if (preserve) {
+ prev->upstream.ssl = glcf->upstream.ssl;
+ }
+
return NGX_OK;
}

diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -237,7 +237,7 @@ static ngx_int_t ngx_http_proxy_rewrite_

#if (NGX_HTTP_SSL)
static ngx_int_t ngx_http_proxy_set_ssl(ngx_conf_t *cf,
- ngx_http_proxy_loc_conf_t *plcf);
+ ngx_http_proxy_loc_conf_t *plcf, ngx_http_proxy_loc_conf_t *prev);
#endif
static void ngx_http_proxy_set_vars(ngx_url_t *u, ngx_http_proxy_vars_t *v);

@@ -959,7 +959,7 @@ ngx_http_proxy_handler(ngx_http_request_
ctx->vars = plcf->vars;
u->schema = plcf->vars.schema;
#if (NGX_HTTP_SSL)
- u->ssl = (plcf->upstream.ssl != NULL);
+ u->ssl = plcf->ssl;
#endif

} else {
@@ -3756,7 +3756,7 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t
ngx_conf_merge_ptr_value(conf->ssl_conf_commands,
prev->ssl_conf_commands, NULL);

- if (conf->ssl && ngx_http_proxy_set_ssl(cf, conf) != NGX_OK) {
+ if (ngx_http_proxy_set_ssl(cf, conf, prev) != NGX_OK) {
return NGX_CONF_ERROR;
}

@@ -3857,7 +3857,7 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t
conf->proxy_values = prev->proxy_values;

#if (NGX_HTTP_SSL)
- conf->upstream.ssl = prev->upstream.ssl;
+ conf->ssl = prev->ssl;
#endif
}

@@ -4923,10 +4923,37 @@ ngx_http_proxy_ssl_conf_command_check(ng


static ngx_int_t
-ngx_http_proxy_set_ssl(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *plcf)
+ngx_http_proxy_set_ssl(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *plcf,
+ ngx_http_proxy_loc_conf_t *prev)
{
+ ngx_uint_t preserve;
ngx_pool_cleanup_t *cln;

+ if (plcf->ssl_protocols == prev->ssl_protocols
+ && plcf->ssl_ciphers.data == prev->ssl_ciphers.data
+ && plcf->upstream.ssl_certificate == prev->upstream.ssl_certificate
+ && plcf->upstream.ssl_certificate_key
+ == prev->upstream.ssl_certificate_key
+ && plcf->upstream.ssl_passwords == prev->upstream.ssl_passwords
+ && plcf->upstream.ssl_verify == prev->upstream.ssl_verify
+ && plcf->ssl_verify_depth == prev->ssl_verify_depth
+ && plcf->ssl_trusted_certificate.data
+ == prev->ssl_trusted_certificate.data
+ && plcf->ssl_crl.data == prev->ssl_crl.data
+ && plcf->upstream.ssl_session_reuse == prev->upstream.ssl_session_reuse
+ && plcf->ssl_conf_commands == prev->ssl_conf_commands)
+ {
+ if (prev->upstream.ssl) {
+ plcf->upstream.ssl = prev->upstream.ssl;
+ return NGX_OK;
+ }
+
+ preserve = 1;
+
+ } else {
+ preserve = 0;
+ }
+
plcf->upstream.ssl = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_t));
if (plcf->upstream.ssl == NULL) {
return NGX_ERROR;
@@ -5020,6 +5047,15 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n
return NGX_ERROR;
}

+ /*
+ * special handling to preserve plcf->upstream.ssl
+ * in the "http" section to inherit it to all servers
+ */
+
+ if (preserve) {
+ prev->upstream.ssl = plcf->upstream.ssl;
+ }
+
return NGX_OK;
}

diff --git a/src/http/modules/ngx_http_uwsgi_module.c b/src/http/modules/ngx_http_uwsgi_module.c
--- a/src/http/modules/ngx_http_uwsgi_module.c
+++ b/src/http/modules/ngx_http_uwsgi_module.c
@@ -97,7 +97,7 @@ static char *ngx_http_uwsgi_ssl_password
static char *ngx_http_uwsgi_ssl_conf_command_check(ngx_conf_t *cf, void *post,
void *data);
static ngx_int_t ngx_http_uwsgi_set_ssl(ngx_conf_t *cf,
- ngx_http_uwsgi_loc_conf_t *uwcf);
+ ngx_http_uwsgi_loc_conf_t *uwcf, ngx_http_uwsgi_loc_conf_t *prev);
#endif


@@ -668,7 +668,7 @@ ngx_http_uwsgi_handler(ngx_http_request_
if (uwcf->uwsgi_lengths == NULL) {

#if (NGX_HTTP_SSL)
- u->ssl = (uwcf->upstream.ssl != NULL);
+ u->ssl = uwcf->ssl;

if (u->ssl) {
ngx_str_set(&u->schema, "suwsgi://");
@@ -1897,7 +1897,7 @@ ngx_http_uwsgi_merge_loc_conf(ngx_conf_t
ngx_conf_merge_ptr_value(conf->ssl_conf_commands,
prev->ssl_conf_commands, NULL);

- if (conf->ssl && ngx_http_uwsgi_set_ssl(cf, conf) != NGX_OK) {
+ if (ngx_http_uwsgi_set_ssl(cf, conf, prev) != NGX_OK) {
return NGX_CONF_ERROR;
}

@@ -1927,7 +1927,7 @@ ngx_http_uwsgi_merge_loc_conf(ngx_conf_t
conf->uwsgi_values = prev->uwsgi_values;

#if (NGX_HTTP_SSL)
- conf->upstream.ssl = prev->upstream.ssl;
+ conf->ssl = prev->ssl;
#endif
}

@@ -2455,10 +2455,37 @@ ngx_http_uwsgi_ssl_conf_command_check(ng


static ngx_int_t
-ngx_http_uwsgi_set_ssl(ngx_conf_t *cf, ngx_http_uwsgi_loc_conf_t *uwcf)
+ngx_http_uwsgi_set_ssl(ngx_conf_t *cf, ngx_http_uwsgi_loc_conf_t *uwcf,
+ ngx_http_uwsgi_loc_conf_t *prev)
{
+ ngx_uint_t preserve;
ngx_pool_cleanup_t *cln;

+ if (uwcf->ssl_protocols == prev->ssl_protocols
+ && uwcf->ssl_ciphers.data == prev->ssl_ciphers.data
+ && uwcf->upstream.ssl_certificate == prev->upstream.ssl_certificate
+ && uwcf->upstream.ssl_certificate_key
+ == prev->upstream.ssl_certificate_key
+ && uwcf->upstream.ssl_passwords == prev->upstream.ssl_passwords
+ && uwcf->upstream.ssl_verify == prev->upstream.ssl_verify
+ && uwcf->ssl_verify_depth == prev->ssl_verify_depth
+ && uwcf->ssl_trusted_certificate.data
+ == prev->ssl_trusted_certificate.data
+ && uwcf->ssl_crl.data == prev->ssl_crl.data
+ && uwcf->upstream.ssl_session_reuse == prev->upstream.ssl_session_reuse
+ && uwcf->ssl_conf_commands == prev->ssl_conf_commands)
+ {
+ if (prev->upstream.ssl) {
+ uwcf->upstream.ssl = prev->upstream.ssl;
+ return NGX_OK;
+ }
+
+ preserve = 1;
+
+ } else {
+ preserve = 0;
+ }
+
uwcf->upstream.ssl = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_t));
if (uwcf->upstream.ssl == NULL) {
return NGX_ERROR;
@@ -2552,6 +2579,15 @@ ngx_http_uwsgi_set_ssl(ngx_conf_t *cf, n
return NGX_ERROR;
}

+ /*
+ * special handling to preserve uwcf->upstream.ssl
+ * in the "http" section to inherit it to all servers
+ */
+
+ if (preserve) {
+ prev->upstream.ssl = uwcf->upstream.ssl;
+ }
+
return NGX_OK;
}


--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org