[PATCH] Added warning about redefinition of listen socket protocol options

Maxim Dounin mdounin at mdounin.ru
Fri Jan 13 01:31:00 UTC 2023


Hello!

On Sat, Dec 31, 2022 at 05:35:59PM +0300, Maxim Dounin wrote:

> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1672497248 -10800
> #      Sat Dec 31 17:34:08 2022 +0300
> # Node ID c215d5cf25732ece1819cf1cd48ebb480bb642c7
> # Parent  07b0bee87f32be91a33210bc06973e07c4c1dac9
> Added warning about redefinition of listen socket protocol options.
> 
> The "listen" directive in the http module can be used multiple times
> in different server blocks.  Originally, it was supposed to be specified
> once with various socket options, and without any parameters in virtual
> server blocks.  For example:
> 
>     server { listen 80 backlog=1024; server_name foo; ... }
>     server { listen 80; server_name bar; ... }
>     server { listen 80; server_name bazz; ... }
> 
> The address part of the syntax ("address[:port]" / "port" / "unix:path")
> uniquely identifies the listening socket, and therefore is enough for
> name-based virtual servers (to let nginx know that the virtual server
> accepts requests on the listening socket in question).
> 
> To ensure that listening options do not conflict between virtual servers,
> they were allowed only once.  For example, the following configuration
> will be rejected ("duplicate listen options for 0.0.0.0:80 in ..."):
> 
>     server { listen 80 backlog=1024; server_name foo; ... }
>     server { listen 80 backlog=512; server_name bar; ... }
> 
> At some point it was, however, noticed, that it is sometimes convenient
> to repeat some options for clarity.  In nginx 0.8.51 the "ssl" parameter
> was allowed to be specified multiple times, e.g.:
> 
>     server { listen 443 ssl backlog=1024; server_name foo; ... }
>     server { listen 443 ssl; server_name bar; ... }
>     server { listen 443 ssl; server_name bazz; ... }
> 
> This approach makes configuration more readable, since SSL sockets are
> immediately visible in the configuration.  If this is not needed, just the
> address can still be used.
> 
> Later, additional protocol-specific options similar to "ssl" were
> introduced, notably "http2" and "proxy_protocol".  With these options,
> one can write:
> 
>     server { listen 443 ssl backlog=1024; server_name foo; ... }
>     server { listen 443 http2; server_name bar; ... }
>     server { listen 443 proxy_protocol; server_name bazz; ... }
> 
> The resulting socket will use ssl, http2, and proxy_protocol, but this
> is not really obvious from the configuration.
> 
> To ensure such misleading configurations are not allowed, nginx now
> warns as long as the "listen" directive is used with options different
> from the options previously used if these are potentially confusing.
> 
> In particular, the following configurations are allowed:
> 
>     server { listen 8401 ssl backlog=1024; server_name foo; }
>     server { listen 8401 ssl; server_name bar; }
>     server { listen 8401 ssl; server_name bazz; }
> 
>     server { listen 8402 ssl http2 backlog=1024; server_name foo; }
>     server { listen 8402 ssl; server_name bar; }
>     server { listen 8402 ssl; server_name bazz; }
> 
>     server { listen 8403 ssl; server_name bar; }
>     server { listen 8403 ssl; server_name bazz; }
>     server { listen 8403 ssl http2; server_name foo; }
> 
>     server { listen 8404 ssl http2 backlog=1024; server_name foo; }
>     server { listen 8404 http2; server_name bar; }
>     server { listen 8404 http2; server_name bazz; }
> 
>     server { listen 8405 ssl http2 backlog=1024; server_name foo; }
>     server { listen 8405 ssl http2; server_name bar; }
>     server { listen 8405 ssl http2; server_name bazz; }
> 
>     server { listen 8406 ssl; server_name foo; }
>     server { listen 8406; server_name bar; }
>     server { listen 8406; server_name bazz; }
> 
> And the following configurations will generate warnings:
> 
>     server { listen 8501 ssl http2 backlog=1024; server_name foo; }
>     server { listen 8501 http2; server_name bar; }
>     server { listen 8501 ssl; server_name bazz; }
> 
>     server { listen 8502 backlog=1024; server_name foo; }
>     server { listen 8502 ssl; server_name bar; }
> 
>     server { listen 8503 ssl; server_name foo; }
>     server { listen 8503 http2; server_name bar; }
> 
>     server { listen 8504 ssl; server_name foo; }
>     server { listen 8504 http2; server_name bar; }
>     server { listen 8504 proxy_protocol; server_name bazz; }
> 
>     server { listen 8505 ssl http2 proxy_protocol; server_name foo; }
>     server { listen 8505 ssl http2; server_name bar; }
>     server { listen 8505 ssl; server_name bazz; }
> 
>     server { listen 8506 ssl http2; server_name foo; }
>     server { listen 8506 ssl; server_name bar; }
>     server { listen 8506; server_name bazz; }
> 
>     server { listen 8507 ssl; server_name bar; }
>     server { listen 8507; server_name bazz; }
>     server { listen 8507 ssl http2; server_name foo; }
> 
>     server { listen 8508 ssl; server_name bar; }
>     server { listen 8508; server_name bazz; }
>     server { listen 8508 ssl backlog=1024; server_name foo; }
> 
>     server { listen 8509; server_name bazz; }
>     server { listen 8509 ssl; server_name bar; }
>     server { listen 8509 ssl backlog=1024; server_name foo; }
> 
> The basic idea is that at most two sets of protocol options are allowed:
> the main one (with socket options, if any), and a shorter one, with options
> being a subset of the main options, repeated for clarity.  As long as the
> shorter set of protocol options is used, all listen directives except the
> main one should use it.
> 
> diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c
> --- a/src/http/ngx_http.c
> +++ b/src/http/ngx_http.c
> @@ -1228,7 +1228,8 @@ static ngx_int_t
>  ngx_http_add_addresses(ngx_conf_t *cf, ngx_http_core_srv_conf_t *cscf,
>      ngx_http_conf_port_t *port, ngx_http_listen_opt_t *lsopt)
>  {
> -    ngx_uint_t             i, default_server, proxy_protocol;
> +    ngx_uint_t             i, default_server, proxy_protocol,
> +                           protocols, protocols_prev;
>      ngx_http_conf_addr_t  *addr;
>  #if (NGX_HTTP_SSL)
>      ngx_uint_t             ssl;
> @@ -1264,12 +1265,18 @@ ngx_http_add_addresses(ngx_conf_t *cf, n
>          default_server = addr[i].opt.default_server;
>  
>          proxy_protocol = lsopt->proxy_protocol || addr[i].opt.proxy_protocol;
> +        protocols = lsopt->proxy_protocol;
> +        protocols_prev = addr[i].opt.proxy_protocol;
>  
>  #if (NGX_HTTP_SSL)
>          ssl = lsopt->ssl || addr[i].opt.ssl;
> +        protocols |= lsopt->ssl << 1;
> +        protocols_prev |= addr[i].opt.ssl << 1;
>  #endif
>  #if (NGX_HTTP_V2)
>          http2 = lsopt->http2 || addr[i].opt.http2;
> +        protocols |= lsopt->http2 << 2;
> +        protocols_prev |= addr[i].opt.http2 << 2;
>  #endif
>  
>          if (lsopt->set) {
> @@ -1299,6 +1306,55 @@ ngx_http_add_addresses(ngx_conf_t *cf, n
>              addr[i].default_server = cscf;
>          }
>  
> +        /* check for conflicting protocol options */
> +
> +        if ((protocols | protocols_prev) != protocols_prev) {
> +
> +            /* options added */
> +
> +            if ((addr[i].opt.set && !lsopt->set)
> +                || addr[i].protocols_changed
> +                || (protocols | protocols_prev) != protocols)
> +            {
> +                ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
> +                                   "protocol options redefined for %V",
> +                                   &addr[i].opt.addr_text);
> +            }
> +
> +            addr[i].protocols = protocols_prev;
> +            addr[i].protocols_set = 1;
> +            addr[i].protocols_changed = 1;
> +
> +        } else if ((protocols_prev | protocols) != protocols) {
> +
> +            /* options removed */
> +
> +            if (lsopt->set
> +                || (addr[i].protocols_set && protocols != addr[i].protocols))
> +            {
> +                ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
> +                                   "protocol options redefined for %V",
> +                                   &addr[i].opt.addr_text);
> +            }
> +
> +            addr[i].protocols = protocols;
> +            addr[i].protocols_set = 1;
> +            addr[i].protocols_changed = 1;
> +
> +        } else {
> +
> +            /* the same options */
> +
> +            if (lsopt->set && addr[i].protocols_changed) {
> +                ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
> +                                   "protocol options redefined for %V",
> +                                   &addr[i].opt.addr_text);
> +            }
> +
> +            addr[i].protocols = protocols;
> +            addr[i].protocols_set = 1;
> +        }
> +
>          addr[i].opt.default_server = default_server;
>          addr[i].opt.proxy_protocol = proxy_protocol;
>  #if (NGX_HTTP_SSL)
> @@ -1355,6 +1411,9 @@ ngx_http_add_address(ngx_conf_t *cf, ngx
>      }
>  
>      addr->opt = *lsopt;
> +    addr->protocols = 0;
> +    addr->protocols_set = 0;
> +    addr->protocols_changed = 0;
>      addr->hash.buckets = NULL;
>      addr->hash.size = 0;
>      addr->wc_head = NULL;
> diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h
> --- a/src/http/ngx_http_core_module.h
> +++ b/src/http/ngx_http_core_module.h
> @@ -274,6 +274,10 @@ typedef struct {
>  typedef struct {
>      ngx_http_listen_opt_t      opt;
>  
> +    unsigned                   protocols:3;
> +    unsigned                   protocols_set:1;
> +    unsigned                   protocols_changed:1;
> +
>      ngx_hash_t                 hash;
>      ngx_hash_wildcard_t       *wc_head;
>      ngx_hash_wildcard_t       *wc_tail;

Ping.

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


More information about the nginx-devel mailing list