Re: PROXY protocol support

On Tue, 2021-03-02 at 18:43 +0100, Magnus Hagander wrote:
> PFA a simple patch that implements support for the PROXY protocol.

I'm not all the way through the patch yet, but this part jumped out at
me:

> + if (memcmp(proxyheader.sig, "\x0d\x0a\x0d\x0a\x00\x0d\x0a\x51\x55\x49\x54\x0a", sizeof(proxyheader.sig)) != 0)
> + {
> + /*
> + * Data is there but it wasn't a proxy header. Also fall through to
> + * normal processing
> + */
> + pq_endmsgread();
> + return STATUS_OK;

From my reading, the spec explicitly disallows this sort of fallback
behavior:

> The receiver MUST be configured to only receive the protocol described in this
> specification and MUST not try to guess whether the protocol header is present
> or not. This means that the protocol explicitly prevents port sharing between
> public and private access.

You might say, "if we already trust the proxy server, why should we
care?" but I think the point is that you want to catch
misconfigurations where the middlebox is forwarding bare TCP without
adding a PROXY header of its own, which will "work" for innocent
clients but in reality is a ticking timebomb. If you've decided to
trust an intermediary to use PROXY connections, then you must _only_
accept PROXY connections from that intermediary. Does that seem like a
reasonable interpretation?

--Jacob