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

Reply via email to