Re: PROXY protocol support

On Thu, 2021-11-04 at 12:03 +0100, Magnus Hagander wrote:
> Thanks for the pointer, PFA a rebase.

I think the Unix socket handling needs the same "success" fix that you
applied to the TCP socket handling above it:

> @@ -1328,9 +1364,23 @@ PostmasterMain(int argc, char *argv[])
> ereport(WARNING,
> (errmsg("could not create Unix-domain socket in directory \"%s\"",
> socketdir)));
> +
> + if (ProxyPortNumber)
> + {
> + socket = StreamServerPort(AF_UNIX, NULL,
> + (unsigned short) ProxyPortNumber,
> + socketdir,
> + ListenSocket, MAXLISTEN);
> + if (socket)
> + socket->isProxy = true;
> + else
> + ereport(WARNING,
> + (errmsg("could not create Unix-domain PROXY socket for \"%s\"",
> + socketdir)));
> + }
> }
>
> - if (!success && elemlist != NIL)
> + if (socket == NULL && elemlist != NIL)
> ereport(FATAL,
> (errmsg("could not create any Unix-domain sockets")));

Other than that, I can find nothing else to improve, and I think this
is ready for more eyes than mine. :)

--

To tie off some loose ends from upthread:

I didn't find any MAXLISTEN documentation either, so I guess it's only
a documentation issue if someone runs into it, heh.

I was not able to find any other cases (besides ident) where using
daddr instead of laddr would break things. I am going a bit snow-blind
on the patch, though, and there's a lot of auth code.

I never did hear back from the PROXY spec maintainer on how strict to
be with LOCAL; another contributor did chime in but only to add that
they didn't know the answer. That conversation is at [1], in case
someone picks it up in the future.

A summary of possible improvements talked about upthread, for a future
v2:

- SQL functions to get the laddr info (scoped to superusers, somehow),
if there's a use case for them

- Setting up PROXY Unix socket permissions separately from the "main"
socket

- Allowing PROXY-only communication (disable the "main" port)

Thanks,
--Jacob

[1] https://www.mail-archive.com/haproxy(at)formilux(dot)org/msg40899.html