SSL contexts reuse across locations

[Pavel Pautov]

Hi,

Attaching updated PoC patch, which also reuses contexts from 'http' level.

> -----Original Message-----
> From: Maxim Dounin <mdounin at mdounin.ru>
> Sent: Friday, May 20, 2022 16:23
> 
> Hello!
> 
> On Fri, May 20, 2022 at 06:52:54AM +0000, Pavel Pautov via nginx-devel wrote:
> 
> > > -----Original Message-----
> > > From: Maxim Dounin <mdounin at mdounin.ru>
> > > Sent: Wednesday, May 18, 2022 11:32
> > [..]
> > > > 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.
> >
> > Do you suggest to make ngx_http_proxy_set_ssl() responsible for
> > merging context related settings? I guess, we can do that, but
> > something like ngx_http_proxy_create_ssl() might be still
> > beneficial.
> 
> No, I'm suggesting to use ngx_http_proxy_set_ssl() for what it
> currently does: create plcf->upstream.ssl.  And extend it to
> inherit plcf->upstream.ssl when possible.

I went with ngx_http_proxy_create_ssl() for now (see patch), as I need to call it twice in some cases.
However, contexts reuse logic can be moved into separate function, as well. Especially if we are going to reuse it across modules.

> > > 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.
> >
> 
> > The patch shouldn't cause a blow up to my understanding, expect perhaps for
> very specific configs like:
> > server {
> >   proxy_ssl_session_reuse off;
> >   location / {
> >      proxy_ssl_session_reuse on;
> >       proxy_pass https://backend;
> >   }
> > }
> > Basically, one have to have "proxy_ssl_*" directives scattered
> > across hierarchy of locations with a single terminal "proxy_pass
> > https". If there are many terminal locations with different
> > proxy_passes, then increase shouldn't be that big. If there is
> > no terminal "proxy_pass https", then why are these proxy_ssl_*
> > options even there?
> 
> Ah, it looks like I've misread your patch.  With the existing
> approach it is not clear how do you expect it to actually fix the
> original problem as in the ticket, that is, a configuration like:
> 
> http {
>    proxy_ssl_trusted_certificate /etc/pki/tls/cert.pem;
> 
>    server {
>       ...
>       location ... { proxy_pass https://...; }
>       location ... { proxy_pass https://...; }
>       location ... { proxy_pass https://...; }
> 
>    }
> }
> 
> With your patch, no SSL context will be created for the server{}
> block, so each location will have to create its own SSL context
> for proxying: exactly what we are trying to avoid.

Indeed, somehow I've assumed there is going to be a merge into 'http' from some 'null' location.

> 
> [...]
> 
> > That being said, I think, we can actually satisfy "no SSL
> > contexts without https" requirement by linking location configs
> > and traversing them backwards in a search for location with
> > "proxy_ssl_*" directives. I'd update patch with that approach in
> > mind.
> 
> You may want to re-evaluate your understanding of the configuration
> merging process, and re-read the
> ngx_http_upstream_hide_headers_hash() function I've already
> mentioned above.

I did and I still find above a valid technique for achieving "no contexts without https proxy_pass" (not sure though, if it's still a requirement). There could be several layers of locations between "proxy_ssl_*" and proxy_pass and if we don't create context on the way down, we'll have to go up to create it for other nested locations to see it. Alternatively, I guess, we can use something like "ngx_ssl_t **" to have same placeholder for locations which would reuse context, and init it only if we hit "https proxy_pass".

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ssl_ctx_reuse.patch
Type: application/octet-stream
Size: 6615 bytes
Desc: ssl_ctx_reuse.patch
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20220524/5a1c60c6/attachment.obj>