Re: relayd: receive the PROXY protocol (v1 and v2) on listeners

On Tue, 14 Jul 2026 13:54:02 +0200,
Romain FABBRI <[email protected]> wrote:
> 
> Thanks for the quick review!
> 
> Revised diff on codeberg (I hope it's ok my email client sucks): 
> https://codeberg.org/cybercloud/openbsd-relayd-proxyproto/src/branch/main/patches/current/0001-relayd-proxy-protocol-receive.patch
>

I had used a file from commit a0a2babbaa4029b637eb787459244cc9bc6262ef

I won't go to small details, just highlight the major issues.

1)

> +	/* Seed with the real endpoints so a LOCAL/UNSPEC header is a no-op. */
> +	src = con->se_in.ss;
> +	dst = con->se_sockname;
> +
> +	ret = proxy_protocol_read(fd, &src, &dst);
> +	if (ret == 0) {			/* header not complete yet */
> +		relay_proxy_read(con);
> +		return;
> +	}
> +	if (ret == -1) {
> +		relay_close(con, "invalid PROXY header", 1);
> +		return;
> +	}

...

> +int
> +proxy_protocol_read(int fd, struct sockaddr_storage *src,
> +    struct sockaddr_storage *dst)
> +{
> +	u_int8_t		 buf[PROXY_HEADER_MAX];
> +	struct proxy_v2_hdr	*hdr;
> +	union proxy_v2_addr	*addr;
> +	ssize_t			 n;
> +	size_t			 hlen, alen, matchlen;
> +
> +	n = recv(fd, buf, sizeof(buf), MSG_PEEK);
> +	if (n == -1)
> +		return (errno == EAGAIN || errno == EWOULDBLOCK) ? 0 : -1;
> +	if (n == 0)
> +		return -1;
> +


After recv() returns an incomplete header, the first quoted code rearms
EV_READ without consuming anything. The socket remains readable, so libevent
repeatedly invokes the callback on the same bytes until the timeout.

Literally one byte can monopolise a relay process.

Am I wrong?

2)

>  relay_session(struct rsession *con)
>  {
>  	struct relay		*rlay = con->se_relay;
>  	struct ctl_relay_event	*in = &con->se_in, *out = &con->se_out;
>  
> +	/*
> +	 * Consume the PROXY protocol header (if the listener expects one)
> +	 * before anything reads the socket, so the advertised client address
> +	 * replaces the accept(2) peer for hashing, $REMOTE_ADDR and logging.
> +	 */
> +	if (rlay->rl_conf.proxyproto && !con->se_proxydone) {
> +		relay_proxy_read(con);
> +		return;
> +	}
> +
>  	if (bcmp(&rlay->rl_conf.ss, &out->ss, sizeof(out->ss)) == 0 &&
>  	    out->port == rlay->rl_conf.port) {
>  		log_debug("%s: session %d: looping", __func__, con->se_id);
> 

Why you haven't put it in relay_accept()? I think current design will work
only after upstream is selected.

3)

UDP is interesting story. It is calling relay_session() when the packed is
alread copied. So, the new stream gate consequently schedules EV_READ on a
possible invalid descriptor because UDP is packed based, not stream based.

I think your changes brokes UDP because your works based on assamtioon that
after you parsed ProxyProtocol header, you may continue read which is not
true for UDP.

Also, I've checked
https://github.com/haproxy/haproxy/blob/master/doc/proxy-protocol.txt and it
requires the UDP header and payload in the same package.

-- 
wbr, Kirill