[PATCH 2 of 2] Stream: do not reallocate a parsed SNI host

[Sergey Kandaurov]

On Thu, May 30, 2024 at 08:48:07PM +0400, Roman Arutyunyan wrote:
> Hi,
> 
> On Mon, May 27, 2024 at 02:21:44PM +0400, Sergey Kandaurov wrote:
> > # HG changeset patch
> > # User Sergey Kandaurov <pluknet at nginx.com>
> > # Date 1716805288 -14400
> > #      Mon May 27 14:21:28 2024 +0400
> > # Node ID 88fa18a0f05f7dead38a127bb24e5cf861f3d66d
> > # Parent  e82a7318ed48fdbc1273771bc96357e9dc232975
> > Stream: do not reallocate a parsed SNI host.
> > 
> > Unlike in http SNI callback, it doesn't need to be saved here.
> > 
> > diff --git a/src/stream/ngx_stream_ssl_module.c b/src/stream/ngx_stream_ssl_module.c
> > --- a/src/stream/ngx_stream_ssl_module.c
> > +++ b/src/stream/ngx_stream_ssl_module.c
> > @@ -501,7 +501,7 @@ ngx_stream_ssl_servername(ngx_ssl_conn_t
> >  
> >      host.data = (u_char *) servername;
> >  
> > -    rc = ngx_stream_validate_host(&host, c->pool, 1);
> > +    rc = ngx_stream_validate_host(&host, c->pool, 0);
> >  
> >      if (rc == NGX_ERROR) {
> >          goto error;
> 
> While it's true the host variable is only used within this function, its
> content can be referenced by session after exiting it.  And that's why we
> should keep the reallocation here.
> 
> ngx_stream_find_virtual_server() calls ngx_stream_regex_exec() for server
> name regex, which can save references to parts of the host in capture variables.
> 

Indeed, this created unreferenced memory access.  Even though the passed
pointer appears to be valid until SSL shutdown, which follows stream log
handler, it's best to avoid.

# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1717681574 -14400
#      Thu Jun 06 17:46:14 2024 +0400
# Node ID 8a7d0ff7b75c664ddef2f985899612ef79643a59
# Parent  89277aea013e649cb2dfcb9621ce06e544ab1c31
Stream ssl_preread: do not reallocate a parsed SNI host.

We own this memory from the session pool.

diff --git a/src/stream/ngx_stream_ssl_preread_module.c b/src/stream/ngx_stream_ssl_preread_module.c
--- a/src/stream/ngx_stream_ssl_preread_module.c
+++ b/src/stream/ngx_stream_ssl_preread_module.c
@@ -519,7 +519,7 @@ ngx_stream_ssl_preread_servername(ngx_st
 
     host = *servername;
 
-    rc = ngx_stream_validate_host(&host, c->pool, 1);
+    rc = ngx_stream_validate_host(&host, c->pool, 0);
 
     if (rc == NGX_ERROR) {
         return NGX_ERROR;