Add support for the PROXY protocol #581

Tey · 2025-09-21T01:04:11Z

When there is a proxy in front of tinyproxy, the latter does not see the actual IP address of the client. This is a problem when using tools like fail2ban that parse the tinyproxy log to find and ban clients that try to break the authentication using brute force. Some proxies like stunnel or HAProxy support the PROXY protocol to overcome that issue, so that the next proxy in the chain can tell what the IP address of the original client is.

See https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt

This commit adds support for clients that use the PROXY protocol to extract the real client IP address and report it in the logs. This is only implemented when the new ClientUsesProxyProtocol parameter is set to Yes. It should only be enabled when the client is trusted and known to be a proxy that support that protocol.

When there is a proxy in front of tinyproxy, the latter does not see the actual IP address of the client. This is a problem when using tools like fail2ban that parse the tinyproxy log to find and ban clients that try to break the authentication using brute force. Some proxies like stunnel or HAProxy support the PROXY protocol to overcome that issue, so that the next proxy in the chain can tell what the IP address of the original client is. See https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt This commit adds support for clients that use the PROXY protocol to extract the real client IP address and report it in the logs. This is only implemented when the new ClientUsesProxyProtocol parameter is set to Yes. It should only be enabled when the client is trusted and known to be a proxy that support that protocol.

rofl0r · 2025-09-21T13:26:26Z

reading the document, it appears the protocol is widely enough supported so it may make sense to support it too. however, some questions arise:

should acl and loop checks be done before or after the peer address is rewritten?
should conn_init_contents be called after the PROXY line is already retrieved?

as for the implementation presented:

only the client ip string seems to be updated in the current impl, but not the port
read_request_line is being repurposed to do 2 different things at once. imo there should be a separate function called specifically when the PROXY protocol is enabled, called from handle_request.
from code flow it would appear that the "Connect (file descriptor %d): %s" is currently printed with the frontend proxy's address, not the one passed in the PROXY line.
while efficient, the current implementation modifies the original string twice (adding and removing nul terminators), which is kinda frail and requires some cognitive load to decipher for correctness. also it raises the basically same error message in 2 places.
so imo, the checking should be done once and the error raised immediately before modifying the string. we can for example compare whether it starts with "PROXY TCP" and then check the next char for '4' or '6' and scan the string for the right amount of spaces (eventually making a list of pointers or offsets of them) and only after it's checked proceed to the ip address field (and port, if it's needed) and only insert a NUL there.

also it needs to be documented that only v1 of the protocol is supported.

Tey · 2025-09-21T16:46:22Z

My main goal was the authentication failure log message, so I didn't think about the other things like ACL. What do you think of applying these changes:

Move the PROXY line parsing logic into a new function like read_proxy_line().
Call it just before the call to getpeer_information() in handle_connection(), so that it updates addr with the reported IP address of the client. That way, getpeer_information() will set peer_ipaddr to the reported IP address so all the log messages will use it and the ACL will be checked against it too.

I'm not sure it is useful to update the client port too. Is it ever used anywhere in the code?

I agree with the remarks about the implementation and will change it accordingly.

Reading the PROXY line is the first thing done in handle_connection() so that the client IP address is updated before it is needed by the rest of the code (ACL, logging, etc.). The parsing implementation is now less generic and updates the sockaddr_union structure directly.

src/reqs.c

etc/tinyproxy.conf.in

src/reqs.c

Tey · 2025-09-25T22:11:57Z

All of the review comments have been addressed in the latest commits.

Tey · 2025-11-17T19:12:47Z

@rofl0r Let me know if you need anything else from me to accept this PR.

rofl0r · 2025-11-19T11:33:08Z

looks mostly good now, but i still have no setup to test it. can you confirm whether the current impl here works in your setup ?

Tey · 2025-11-19T23:42:23Z

OK, take your time.

It works well on my 2 tinyproxy servers.

With fail2ban, I do see brute force bots getting banned from time to time, using the following configuration (based on that message):

/etc/fail2ban/filter.d/tinyproxy.conf

[Definition]
failregex = .* Failed auth attempt .*, ip <HOST>

/etc/fail2ban/jail.d/tinyproxy.conf

[tinyproxy]
enabled = true
#port = 8888
# stunnel+tinyproxy
port = 3128
filter = tinyproxy
logpath = /var/log/tinyproxy/tinyproxy.log
maxretry = 10
findtime = 60
bantime = 300
backend = auto

Tey added 2 commits September 21, 2025 21:30

Indicate which PROXY version is supported

13d0ff5