Re: PROXY protocol support

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