search

mholt / caddy-l4

Layer 4 (TCP/UDP) app for Caddy
Apache License 2.0
855 stars 68 forks source link

Regression: PROXY protocol line sent only when first data arrives from client #212

Open thegcat opened 2 weeks ago

thegcat commented 2 weeks ago

We have noticed a regression between 145ec36251a4
 and ca3e2f38f6e5 when using proxy_protocol.

In 145ec36251a4 the PROXY protocol line would be sent directly on connection from Caddy to the backend. In ca3e2f38f6e5 it seems the PROXY protocol line is sent to the backend only when the client connected to Caddy sends data.

This is problematic with Exim at least, as the SMTP protocol with PROXY (or at least the EXIM implementation of it) expects the mail server/backend to first receive the PROXY line, then answers with the version and host information of the mail server/backend, and then expects a HELO, EHLO or the like. Sending the PROXY line with the first line the client sends breaks this dance.

mholt commented 2 weeks ago

Ah, sorry about that. We are refactoring the matching/peeking logic and it is still undergoing some improvements. @ydylla amd @WeidiDeng are working on it and I am behind on reviewing their patches 😅

thegcat commented 2 weeks ago

No worries, we reverted to 145ec36251a44286f05a10d231d8bfb3a8192e09 after we understood the problem, this was more a heads-up for you if this was unintended and maybe for other users if they also encounter a similar issue.

ydylla commented 2 weeks ago

Hi, I don't think this has something to do with changes in the proxy_protocol matcher/handler, because there are none.
Your config likely already fails in the matching phase because the new matching behavior is not compatible with connections where the server has to speak first (and the client sends nothing). I did not consider this case during the rewrite. But without data to match on the use of caddy-l4 is also rather limited.
Can you please post the config you are using?

To fix this for at least some simple configs, we could likely add a special case that detects if the config has only one possible route (without matchers) and then don't call prefetch here. Which is what probably blocks your connections currently until the matching timeout is reached and matching is aborted.