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

Switch to go-proxyproto #176

Open
polarathene opened this issue Feb 14, 2024 · 4 comments
Open

Switch to go-proxyproto #176

polarathene opened this issue Feb 14, 2024 · 4 comments
Labels
good first issue Good for newcomers

Comments

@polarathene
Copy link

polarathene commented Feb 14, 2024

Caddy 2.8 has changed their PROXY protocol dependency to go-proxyproto which is a bit more featureful (a port can support PROXY protocol connections but optionally still accept connections without PROXY header present based on a configured policy, where the PROXY protocol header is only accepted from trusted hosts).

caddy-l4 should probably follow that change?

@mholt
Copy link
Owner

mholt commented Feb 14, 2024

Yeah, we can look into switching over (I don't have time to do it at the moment though).

@mohammed90 mohammed90 added the good first issue Good for newcomers label Feb 14, 2024
@mohammed90
Copy link
Collaborator

I'm having a hard time following the three threads (@polarathene issue on Caddy repo, the conversation on the PR, and the conversation on HAProxy repo), but don't we need to settle on the conclusion of those conversations before we bring the same implementation to layer4?

@mholt
Copy link
Owner

mholt commented Feb 14, 2024

Yes, probably a good idea to wait until we figure out what the deal is.

@polarathene
Copy link
Author

before we bring the same implementation to layer4?

There is no rush 👍

I just thought it was worthwhile to add a tracking issue here while I was engaging in the topic.


I'm having a hard time following the three threads (@polarathene issue on Caddy repo, the conversation on the PR, and the conversation on HAProxy repo),

Sorry, I know I can be very verbose 😓

From what I can tell there was a miscommunication with the vulnerability concern raised. At least with the present discussion, there is still no clear justification that a trust policy cannot apply to a host where the PROXY header is optional.

It wasn't my intention for that discussion to spill across into the Caddy project. I had reached out directly to HAProxy on Github to clarify a concern with the specification wording, and only made them aware of popular projects that had flexible policies that conflicted with the specification (which appears acceptable to an extent from their response, thus the specification probably needs an update).


Technical Details (proper summary relevant to Caddy devs)

In the example we've been discussing, the concern appears more to do with the trusted host (Traefik) being hands-off with PROXY protocol, such that a malicious client can provide their own PROXY protocol header that Traefik simply forwards to Caddy that trusts all connections to the Caddy server port from Traefik.

Having a more strict policy on Caddy only avoids the issue as a side-effect of requiring Traefik to always provide PROXY header itself for connections to Caddy. But that is not possible for any HTTP router/service config in Traefik which lacks this capability (as it's for Layer 7), only Layer 4 TCP router/service in Traefik can send traffic out with PROXY protocol.

Correct fix and prevention is to ensure Traefik does not allow untrusted clients to sneak in their own PROXY protocol header. Configuration issue of the trusted host, nothing to do with Caddy allowing the PROXY header to be optional for a trusted host.

The trust policy serves to establish when to accept the PROXY protocol header from a host, supporting the omission of the header does not contribute to the vulnerability concern; only in the sense that the trusted host can be vulnerable from misconfiguration and the lack of enforcement by Caddy makes that less obvious to realize.

  • REQUIRE as a default would reject the connections from Traefik that do not explicitly provide the PROXY header, requiring configuration to be corrected for legitimate traffic.
  • A malicious client sending their own PROXY header would then have Caddy receive two, process only one, then fail to handle HTTP protocol or TLS termination due to the remaining PROXY header.
  • If REQUIRE were a default, but the user wanted Caddy to have USE for supporting connections to Caddy that don't / can't provide the PROXY header, all that's required in this vulnerable Traefik configuration is to ensure it uses IGNORE for untrusted clients (I will raise a bug report as their docs don't communicate the default behaviour doesn't check for a PROXY header at all).

Ultimately, not a Caddy concern. Just a misconfigured trusted host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants