Re: PROXY protocol support

On Wed, Mar 3, 2021 at 1:50 AM Jacob Champion <pchampion(at)vmware(dot)com> wrote:
>
> 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?

I definitely missed that part of the spec. Ugh.

That said, I'm not sure it's *actually* an issue in the case of
PostgreSQL. Given that doing what you're suggesting, accidentally
passing connections without PROXY, will get caught in pg_hba.conf.

That said, I agree with your interpretation, and it's pretty easy to
change it to that. Basically we just have to do the IP check *before*
doing the PROXY protocol check. It makes testing a bit more difficult
though, but maybe worth it?

I've attached a POC that does that. Note that I have *not* updated the docs!

Another option would of course be to listen on a separate port for it,
which seems to be the "haproxy way". That would be slightly more code
(we'd still want to keep the code for validating the list of trusted
proxies I'd say), but maybe worth doing?

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/