[PATCH] Core: support for reading PROXY protocol v2 TLVs

[Maxim Dounin]

Hello!

On Wed, Nov 02, 2022 at 05:06:25PM +0400, Roman Arutyunyan wrote:

[...]

> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1667382376 -14400
> #      Wed Nov 02 13:46:16 2022 +0400
> # Node ID dc5f16e6a243c15f58e2c6a62f7a83f536729174
> # Parent  81b4326daac70d6de70abbc3fe36d4f6e3da54a2
> Increased maximum read PROXY protocol header size.
> 
> Maximum size for reading the PROXY protocol header is increased to 4096 to
> accommodate a bigger number of TLVs, which are supported since cca4c8a715de.
> 
> Maximum size for writing the PROXY protocol header is not changed since only
> version 1 is currently supported.
> 
> diff --git a/src/core/ngx_proxy_protocol.c b/src/core/ngx_proxy_protocol.c
> --- a/src/core/ngx_proxy_protocol.c
> +++ b/src/core/ngx_proxy_protocol.c
> @@ -281,7 +281,7 @@ ngx_proxy_protocol_write(ngx_connection_
>  {
>      ngx_uint_t  port, lport;
>  
> -    if (last - buf < NGX_PROXY_PROTOCOL_MAX_HEADER) {
> +    if (last - buf < NGX_PROXY_PROTOCOL_V1_MAX_HEADER) {
>          return NULL;
>      }

A side note: here an error is detected and returned, but no 
logging of the error happens neither in ngx_proxy_protocol_write() 
nor in its callers.  This needs to be fixed.

(Given that ngx_proxy_protocol_write() can also fail due to 
ngx_connection_local_sockaddr() failure, the logging should be 
added to ngx_proxy_protocol_write() itself.  Alternatively, the 
error detection can be completely removed, given that the error 
can never happen.)

>  
> diff --git a/src/core/ngx_proxy_protocol.h b/src/core/ngx_proxy_protocol.h
> --- a/src/core/ngx_proxy_protocol.h
> +++ b/src/core/ngx_proxy_protocol.h
> @@ -13,7 +13,8 @@
>  #include <ngx_core.h>
>  
>  
> -#define NGX_PROXY_PROTOCOL_MAX_HEADER  107
> +#define NGX_PROXY_PROTOCOL_V1_MAX_HEADER  107
> +#define NGX_PROXY_PROTOCOL_MAX_HEADER     4096
>  
>  
>  struct ngx_proxy_protocol_s {
> diff --git a/src/mail/ngx_mail_proxy_module.c b/src/mail/ngx_mail_proxy_module.c
> --- a/src/mail/ngx_mail_proxy_module.c
> +++ b/src/mail/ngx_mail_proxy_module.c
> @@ -890,7 +890,7 @@ ngx_mail_proxy_send_proxy_protocol(ngx_m
>      u_char            *p;
>      ssize_t            n, size;
>      ngx_connection_t  *c;
> -    u_char             buf[NGX_PROXY_PROTOCOL_MAX_HEADER];
> +    u_char             buf[NGX_PROXY_PROTOCOL_V1_MAX_HEADER];
>  
>      s->connection->log->action = "sending PROXY protocol header to upstream";
>  
> @@ -898,7 +898,7 @@ ngx_mail_proxy_send_proxy_protocol(ngx_m
>                     "mail proxy send PROXY protocol header");
>  
>      p = ngx_proxy_protocol_write(s->connection, buf,
> -                                 buf + NGX_PROXY_PROTOCOL_MAX_HEADER);
> +                                 buf + NGX_PROXY_PROTOCOL_V1_MAX_HEADER);
>      if (p == NULL) {
>          ngx_mail_proxy_internal_server_error(s);
>          return NGX_ERROR;
> diff --git a/src/stream/ngx_stream_proxy_module.c b/src/stream/ngx_stream_proxy_module.c
> --- a/src/stream/ngx_stream_proxy_module.c
> +++ b/src/stream/ngx_stream_proxy_module.c
> @@ -894,7 +894,7 @@ ngx_stream_proxy_init_upstream(ngx_strea
>              return;
>          }
>  
> -        p = ngx_pnalloc(c->pool, NGX_PROXY_PROTOCOL_MAX_HEADER);
> +        p = ngx_pnalloc(c->pool, NGX_PROXY_PROTOCOL_V1_MAX_HEADER);
>          if (p == NULL) {
>              ngx_stream_proxy_finalize(s, NGX_STREAM_INTERNAL_SERVER_ERROR);
>              return;
> @@ -902,7 +902,8 @@ ngx_stream_proxy_init_upstream(ngx_strea
>  
>          cl->buf->pos = p;
>  
> -        p = ngx_proxy_protocol_write(c, p, p + NGX_PROXY_PROTOCOL_MAX_HEADER);
> +        p = ngx_proxy_protocol_write(c, p,
> +                                     p + NGX_PROXY_PROTOCOL_V1_MAX_HEADER);
>          if (p == NULL) {
>              ngx_stream_proxy_finalize(s, NGX_STREAM_INTERNAL_SERVER_ERROR);
>              return;
> @@ -946,14 +947,15 @@ ngx_stream_proxy_send_proxy_protocol(ngx
>      ngx_connection_t             *c, *pc;
>      ngx_stream_upstream_t        *u;
>      ngx_stream_proxy_srv_conf_t  *pscf;
> -    u_char                        buf[NGX_PROXY_PROTOCOL_MAX_HEADER];
> +    u_char                        buf[NGX_PROXY_PROTOCOL_V1_MAX_HEADER];
>  
>      c = s->connection;
>  
>      ngx_log_debug0(NGX_LOG_DEBUG_STREAM, c->log, 0,
>                     "stream proxy send PROXY protocol header");
>  
> -    p = ngx_proxy_protocol_write(c, buf, buf + NGX_PROXY_PROTOCOL_MAX_HEADER);
> +    p = ngx_proxy_protocol_write(c, buf,
> +                                 buf + NGX_PROXY_PROTOCOL_V1_MAX_HEADER);
>      if (p == NULL) {
>          ngx_stream_proxy_finalize(s, NGX_STREAM_INTERNAL_SERVER_ERROR);
>          return NGX_ERROR;

Looks good.

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