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