Re: PROXY protocol support

On Wed, Mar 3, 2021 at 10:00 AM Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>
> 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?

In order to figure that out, I hacked up a poc on that. Once again
without updates to the docs, but shows approximately how much code
complexity it adds (not much).

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