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

[Maxim Dounin]

Hello!

On Mon, Jan 22, 2024 at 07:48:01PM +0400, Roman Arutyunyan wrote:

> 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 at 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).

Also nitpicking: ticket #2010 might be a better choice.

The #2594 is actually a duplicate (with a side issue noted that 
using long unix socket path might result in a PROXY protocol 
header without ports and CRLF) and should be closed as such.

> > > 
> > > 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.

I don't think it is a good idea.  Using some meaningful real 
address might easily mislead users.  I would rather use a clearly 
dummy address instead, such as INADDR_ANY with port 0.

Also, as suggested, using the server address as obtained via PROXY 
protocol from the client might be a better solution as long as the 
client address was set via PROXY protocol (regardless of whether 
address families match or not), and what users expect from the 
"proty_protocol on;" when chaining stream proxies in the first 
place.

[...]

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