SSL contexts reuse across locations

[Maxim Dounin]

Hello!

On Wed, May 18, 2022 at 07:20:51AM +0000, Pavel Pautov via nginx-devel wrote:

> Hello,
> 
> Attaching POC patch for 
> https://trac.nginx.org/nginx/ticket/1234.
> 
> At very least, ngx_http_proxy_set_ssl() needs to be converted 
> into ngx_http_proxy_create_ssl().

You may want to focus on actually making the code more readable 
and abstracting it into ngx_http_proxy_set_ssl() instead.  
Something like ngx_http_upstream_hide_headers_hash() might be a 
good example on how to do it properly.

Also, it should be a good idea to avoid creating SSL contexts if 
there is no SSL proxying configured.  Or, at very least, make sure 
only one context at the http level is used in such cases, so 
configurations with many servers won't suddenly blow up.

> But there are also a couple of things to discuss:
> 
> 1. Patch uses pretty straightforward reuse criteria (absence of 
> directives), but shall we go further, say, compare directive 
> arguments (with special treatment of complex values with 
> variables)?

Just checking if the relevant directives were inherited from the 
previous level should be enough, as it will allow creating 
memory-effective configurations.

> 2. Since similar change also makes sense for "grpc", "uwsgi" 
> (and may be "stream proxy") modules, perhaps it's time to factor 
> out SSL upstream settings code for all these modules to avoid 
> copypasting of above patch? We can introduce something like 
> "ngx_ssl_upstream_conf_t" to keep shared SSL settings and unite 
> ngx_http_(proxy|grpc|uwsgi)_set_ssl functions. Config merge 
> logic (together with attached patch) can be moved to something 
> like ngx_ssl_upstream_conf_merge. Optionally, 
> ngx_http_upstream_conf_t can be updated to contain 
> ngx_ssl_upstream_conf_t.

I don't think it the effort.  Further, there are protocol-specific 
differences, such as ALPN in gRPC proxy, so you can't fully 
abstract it anyway.

-- 
Maxim Dounin
http://mdounin.ru/