caddyhttp: support X-Forwarded-Host

[kennethklee]

Adds support for the X-Forwarded-Host header in host matching. Should satisfy #3599.

This only checks either the header or the request host with preference to former.

Should we check both?

[CLAassistant]

All committers have signed the CLA.

[francislavoie]

I'll be honest, I have concerns about this from a security standpoint. That header isn't necessarily safe to trust, any client could send it. This would add some implicit behaviour that not all users would clearly understand is happening.

I think your approach with checking the header with a matcher in the issue you opened is more correct if your intention is specifically to trust that the X-Forwarded-Host header is correct.

Proxies should pass through the header with the actual Host header though, IMO it's strange if they only use X-Forwarded-Host, that's kinda non-standard.

[mholt]

Thanks for the PR!

But I'm not sure I understand... why can't you do this:

@fwdhost header X-Forwarded-For example.com

I'm not convinced that folding Host and X-Forwarded-For together is a good, universally-non-breaking idea... do you have any evidence that this will not break any other configs that rely on Host or authority headers?

[kennethklee]

@francislavoie in terms of a security standpoint, the Host header will have the same implications. as such, if you're using business logic that trusts those headers you'll have a security bug. fortunately Caddy's address matchers impose a whitelist, in essence, to validate the input. as such, there's no security issue.

in my use case, caddy is behind a gateway. from the gateway, the Host header turns into the internal address of the proxied request i.e. app.local, whereas the original request host moves to the forwarded host header. both host and forwarded host headers exist.

[kennethklee]

thinking more about the security implications, this PR could plug a security hole down the proxy chain. say the web server is PHP and blindly uses the request host to generate email links like password reset. if an attacker sends Host: app.com with X-Forwarded-Host: attack.com, caddy currently wouldn't validate the forwarded host header and the web server would send the link with the attack.com domain. regardless, that is not caddy's issue since caddy doesn't use host headers to generate any user consumed output.

[kennethklee]

@mholt you're absolutely right. that work around is effective. I've no proof either, as I was hoping a caddy dev who knew more about the rest of the code might have an idea. or even replace my PR (since I'm not too experienced with golang)

I feel the use case is common enough. say caddy is behind a load balancer, or API gateway, or anything that uses forwarded headers -- you would need this workaround.

the expectation I had was that caddy's native address matcher would match the intended request host anywhere down the line. the question is whether caddy should support the forwarded header standard.

[mholt]

The Host header and X-Forwarded-Host headers have different purposes. I think if you want to match on the X-Forwarded-Host header you should just do that, rather than us rolling them into one. I'm just not convinced that's a good idea (and there's a total lack of evidence here, the only argument is "it's what I expected to happen" but I'm not convinced that's a technically sound expectation).