Re: PROXY protocol support - Mailing list pgsql-hackers

Hi Julien,

I tested the rewrite. It works, including the parts that are easy to
claim and tedious to check, so here is the evidence rather than a "looks
good".

I did not use libpq for this: the attached script builds the PROXY
header by hand and then sends a StartupMessage, which makes it possible
to send what a normal client cannot - truncated headers, wrong
signatures, a length that lies. Everything below is on your v12 applied
to master (one conflict, typedefs.list, trivial).

1. It does what it says

With proxy_networks = '127.0.0.1/32', a pg_hba.conf that only trusts the
address inside the header, and a connection arriving from 127.0.0.1:

  client_addr  | client_port | proxy_addr | proxy_port
  --------------+-------------+------------+------------
  198.51.100.7 |       51234 | 127.0.0.1  |      50664

and authentication matched the pg_hba line for 198.51.100.7, not the one
for 127.0.0.1. That is the whole point of the feature and it holds.

Per header type, same setup:

  v1 IPv4        server uses the header address
  v1 IPv6        server uses the header address
  v2 IPv4        server uses the header address
  v2 LOCAL       connection accepted with the real address, per spec
  no header      rejected
  garbage v1     rejected
  bad v2 signature rejected

2. The three claims in your message

  one port for both       with proxy_networks empty, an ordinary
                          connection works and a PROXY header is
                          rejected as the garbage it is at that point
  trusted, no header      rejected
  untrusted, with header  rejected, and nothing is leaked

For the last one I set proxy_networks = '10.0.0.0/8' and connected from
127.0.0.1, so the client is outside the trusted set:

  no header   accepted, ordinary connection
  v1 header   server closes without replying
  v2 header   server closes without replying

and an unpatched master, same client, same v1 header, also closes
without replying. The two are indistinguishable from the outside, so a
scanner cannot tell the feature is compiled in. That is the claim, and
it holds.

3. Truncated headers: not a problem, but I had to check

Two cases left the connection hanging, which is what a streaming parser
should do:

  v1 cut before the CRLF
  v2 announcing 200 bytes of addresses and sending 4

The question is whether anything closes them. With
authentication_timeout = 3s, all of them are closed by the server at
3.0s, same as a connection that sends nothing at all. So the existing
mechanism covers it and there is nothing to fix. I am mentioning it
because it is the first thing a reviewer worried about resource
exhaustion will ask, and now it is answered.

4. One thing to decide about the logs

Since the header is parsed late, "connection received" still prints the
proxy's address, and the next line authenticates against the client's:

  LOG: connection received: host=127.0.0.1 port=50664
  LOG: connection authenticated: ... (pg_hba.conf:3)   <- the 198.51.100.7 rule

The same connection appears with two different addresses in consecutive
lines. Both are true and it follows from parsing late, which I agree is
the right call, but an operator reading logs will trip on it. Either
"connection received" should be emitted after the header is parsed, or
the documentation should say that this line carries the proxy address.

src/test/protocol passes, 2 files, 71 tests.

I have not benchmarked the "no performance regression expected" claim
yet. If that is a blocker for anyone I can measure connection setup with
and without proxy_networks set.

Regards,
Manu