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
#!/usr/bin/env python3
"""Cliente que habla PROXY protocol v1 y v2 contra un PostgreSQL parchado.
proxy_client.py <puerto> <caso>
No usa libpq: arma el header PROXY a mano y despues el StartupMessage, que es
lo unico que hace falta para comprobar que el servidor toma la direccion del
header y no la del socket. Asi se puede probar tambien lo que un cliente
normal no puede mandar: headers invalidos, truncados o de familias raras.
Casos:
v1 header de texto valido, IPv4
v1_ipv6 header de texto valido, IPv6
v2 header binario valido, IPv4
v2_local header binario con comando LOCAL (el proxy habla de si mismo)
sin_header conexion directa, sin header
v1_basura "PROXY " seguido de basura
v1_corto header v1 cortado a la mitad
v2_mal_sig header binario con la firma equivocada
v2_largo v2 que declara mas bytes de los que manda
"""
import socket
import struct
import sys
V2_SIG = b"\r\n\r\n\x00\r\nQUIT\n"
def startup(user=b"postgres", db=b"postgres"):
body = struct.pack("!i", 196608) + b"user\x00" + user + b"\x00" \
+ b"database\x00" + db + b"\x00\x00"
return struct.pack("!i", len(body) + 4) + body
def header(caso):
if caso == "v1":
return b"PROXY TCP4 198.51.100.7 203.0.113.9 51234 5432\r\n"
if caso == "v1_ipv6":
return b"PROXY TCP6 2001:db8::7 2001:db8::9 51234 5432\r\n"
if caso == "v2":
# ver 2 / PROXY, TCP over IPv4, 12 bytes de direcciones
addr = socket.inet_aton("198.51.100.7") +
socket.inet_aton("203.0.113.9") \
+ struct.pack("!HH", 51234, 5432)
return V2_SIG + bytes([0x21, 0x11]) + struct.pack("!H", len(addr)) +
addr
if caso == "v2_local":
return V2_SIG + bytes([0x20, 0x00]) + struct.pack("!H", 0)
if caso == "sin_header":
return b""
if caso == "v1_basura":
return b"PROXY estoesbasura\r\n"
if caso == "v1_corto":
return b"PROXY TCP4 198.51.100.7 203."
if caso == "v2_mal_sig":
addr = socket.inet_aton("198.51.100.7") +
socket.inet_aton("203.0.113.9") \
+ struct.pack("!HH", 51234, 5432)
return b"\x00" * 12 + bytes([0x21, 0x11]) + struct.pack("!H",
len(addr)) + addr
if caso == "v2_largo":
return V2_SIG + bytes([0x21, 0x11]) + struct.pack("!H", 200) + b"\x00"
* 4
raise SystemExit(f"caso desconocido: {caso}")
def main(port, caso):
s = socket.create_connection(("127.0.0.1", int(port)), timeout=5)
h = header(caso)
if h:
s.sendall(h)
s.sendall(startup())
try:
data = s.recv(4096)
except socket.timeout:
print(f"{caso}: TIMEOUT (el servidor no contesto)")
return
if not data:
print(f"{caso}: el servidor cerro sin responder")
return
tipo = chr(data[0])
if tipo == "E":
# mensaje de error: campos separados por \0
campos = data[5:].split(b"\x00")
msg = next((c[1:].decode(errors="replace") for c in campos
if c[:1] in (b"M",)), "?")
print(f"{caso}: ERROR -> {msg}")
elif tipo == "R":
print(f"{caso}: conexion aceptada (mensaje de autenticacion)")
else:
print(f"{caso}: respuesta tipo '{tipo}'")
s.close()
if __name__ == "__main__":
main(sys.argv[1], sys.argv[2])
