Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust HAProxy's existance to log when the proxy protocol is enabled … #1436

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

Codixer
Copy link
Contributor

@Codixer Codixer commented Sep 20, 2024

This is a VERY minor change, so minimal but VERY useful for system administrators. (I am one, and this was so confusing).
Other proxy softwares log when they enable proxy protocol, but velocity doesn't. I probably had some mandella effect, but it seemed like Velocity never used to do this.

But as a system administrator, it would be nice to check the console when the server boots to check if HAProxy is enabled. Hence this change.

@clrxbl
Copy link
Member

clrxbl commented Sep 20, 2024

At some point I remember complaining about this on the Discord aswell, but I think it'd be nicer to mimic what BungeeCord does and log it as a warning on startup, letting the user know proxy protocol is enabled and that it can be a security risk, see https://github.com/SpigotMC/BungeeCord/blob/acb85e30faf3aa10d21ab51efa8fe478f13b8361/proxy/src/main/java/net/md_5/bungee/BungeeCord.java#L335

Warning log would also have the advantage that it would be more visible

@Codixer
Copy link
Contributor Author

Codixer commented Sep 20, 2024

I didn't want to be to overbearing. This is a minimal change, but I can adjust it to be a WARNING if needs be

@JasmeowTheCat
Copy link

Fully approve this honestly, super helpful for me and my team ha. I swear Velocity used to have this but nope, other proxy software integrations do, with the following message for example:

00:04:18 [WARNING] Using PROXY protocol for listener /0.0.0.0:25565, please ensure this listener is adequately firewalled.
00:04:18 [WARNING] Since PROXY protocol is in use, internal connection throttle has been disabled.

I would recommend changing it to WARN.

@Codixer
Copy link
Contributor Author

Codixer commented Sep 20, 2024

@clrxbl @JasmeowTheCat
How's this? Better?

@astei astei merged commit ef1f500 into PaperMC:dev/3.0.0 Sep 23, 2024
1 check passed
@Codixer
Copy link
Contributor Author

Codixer commented Sep 23, 2024

Wohoo!
image

(You cannot imagine the pain I went through with this fata morgana of a feature)
I had vivid memories of a warning message in velocity. But seems like I was wrong.

pull bot pushed a commit to WiIIiam278/Velocity that referenced this pull request Sep 25, 2024
PaperMC#1436)

* Adjust HAProxy's existance to log when the proxy protocol is enabled during bind.

* Added additional warning message, instead of changing the main one. We can see what the preference would be.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants