Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Roman Arutyunyan
January 22, 2024 10:50AM
Hi,

On Mon, Jan 22, 2024 at 02:59:21PM +0300, Maxim Dounin wrote:
> Hello!
>
> On Mon, Jan 22, 2024 at 02:49:54PM +0400, Roman Arutyunyan wrote:
>
> > # HG changeset patch
> > # User Roman Arutyunyan <arut@nginx.com>
> > # Date 1705916128 -14400
> > # Mon Jan 22 13:35:28 2024 +0400
> > # Node ID 2f12c929527b2337c15ef99d3a4dc97819b61fbd
> > # Parent ee40e2b1d0833b46128a357fbc84c6e23be9be07
> > Avoiding mixed socket families in PROXY protocol v1 (ticket #2594).
> >
> > When using realip module, remote and local addreses of a connection can belong
> > to different address families. This previously resulted in generating PROXY
> > protocol headers like this:
> >
> > PROXY TCP4 127.0.0.1 unix:/tmp/nginx1.sock 55544 0
> >
> > The PROXY protocol v1 specification does not allow mixed families. The change
> > will generate the unknown PROXY protocol header in this case:
> >
> > PROXY UNKNOWN
> >
> > Also, the above mentioned format for unix socket address is not specified in
> > PROXY protocol v1 and is a by-product of internal nginx representation of it.
> > The change eliminates such addresses from PROXY protocol headers as well.
>
> Nitpicking: double space in "from PROXY".

Yes, thanks.

> This change will essentially disable use of PROXY protocol in such
> configurations. While it is probably good enough from formal
> point of view, and better that what we have now, this might still
> be a surprise, especially when multiple address families are used
> on the original proxy server, and the configuration works for some
> of them, but not for others.
>
> Wouldn't it be better to remember if the PROXY protocol was used
> to set the address, and use $proxy_protocol_server_addr /
> $proxy_protocol_server_port in this case?
>
> Alternatively, we can use some dummy server address instead, so
> the client address will be still sent.

Another alternative is duplicating client address in this case, see patch.

--
Roman Arutyunyan
# HG changeset patch
# User Roman Arutyunyan <arut@nginx.com>
# Date 1705938401 -14400
# Mon Jan 22 19:46:41 2024 +0400
# Node ID 89ac89209d927b8a438780434a17a0677ef3bf4e
# Parent ee40e2b1d0833b46128a357fbc84c6e23be9be07
Avoiding mixed socket families in PROXY protocol v1 (ticket #2594).

When using realip module, remote and local addresses of a connection can belong
to different address families. This previously resulted in generating PROXY
protocol headers like this:

PROXY TCP4 127.0.0.1 unix:/tmp/nginx1.sock 55544 0

The PROXY protocol v1 specification does not allow mixed families. The change
substitutes server address with client address in this case:

PROXY TCP4 127.0.0.1 127.0.0.1 55544 55544

As an alternative, "PROXY UNKNOWN" header could be used, which unlike this
header does not contain any useful information about the client.

Also, the above mentioned format for unix socket address is not specified in
PROXY protocol v1 and is a by-product of internal nginx representation of it.
The change eliminates such addresses from PROXY protocol headers as well.

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
@@ -279,7 +279,9 @@ ngx_proxy_protocol_read_port(u_char *p,
u_char *
ngx_proxy_protocol_write(ngx_connection_t *c, u_char *buf, u_char *last)
{
- ngx_uint_t port, lport;
+ ngx_uint_t port, lport;
+ socklen_t local_socklen;
+ struct sockaddr *local_sockaddr;

if (last - buf < NGX_PROXY_PROTOCOL_V1_MAX_HEADER) {
ngx_log_error(NGX_LOG_ALERT, c->log, 0,
@@ -312,11 +314,19 @@ ngx_proxy_protocol_write(ngx_connection_

*buf++ = ' ';

- buf += ngx_sock_ntop(c->local_sockaddr, c->local_socklen, buf, last - buf,
- 0);
+ if (c->sockaddr->sa_family == c->local_sockaddr->sa_family) {
+ local_sockaddr = c->local_sockaddr;
+ local_socklen = c->local_socklen;
+
+ } else {
+ local_sockaddr = c->sockaddr;
+ local_socklen = c->socklen;
+ }
+
+ buf += ngx_sock_ntop(local_sockaddr, local_socklen, buf, last - buf, 0);

port = ngx_inet_get_port(c->sockaddr);
- lport = ngx_inet_get_port(c->local_sockaddr);
+ lport = ngx_inet_get_port(local_sockaddr);

return ngx_slprintf(buf, last, " %ui %ui" CRLF, port, lport);
}
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Roman Arutyunyan 354 January 22, 2024 05:52AM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Maxim Dounin 73 January 22, 2024 07:00AM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Roman Arutyunyan 67 January 22, 2024 10:50AM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Maxim Dounin 69 January 23, 2024 04:04PM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Roman Arutyunyan 64 February 21, 2024 08:32AM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

J Carter 75 February 21, 2024 09:00PM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Roman Arutyunyan 69 February 22, 2024 10:18AM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Sergey Kandaurov 62 March 06, 2024 09:52AM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Roman Arutyunyan 72 March 11, 2024 08:46AM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Sergey Kandaurov 68 March 13, 2024 01:10PM

Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)

Roman Arutyunyan 83 March 21, 2024 10:58AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 447
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 466 on July 09, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready