caddyhttp: Pluggable trusted proxy IP range sources

[francislavoie]

Followup to #5103, I realized that the work there was somewhat incomplete.

We've already had requests for making it easy to configure trusted proxy ranges for e.g. CloudFlare, which publish their IP ranges at https://www.cloudflare.com/ips/. This PR makes it possible for a plugin to implement a module that can fetch those IP ranges and feed it to the global trusted proxies in the server.

This should be merged ASAP, before the next release, because otherwise this would be a breaking change. But since we haven't released this yet, it's the best time to make this pluggable.

Regarding the interface, I decided that it should just return a slice of netip.Prefix and not an error, because returning any error would mess with the HTTP server being able to function properly. See the godoc comment on the interface where I wrote some implementation recommendations for plugins which load the source from elsewhere. This godoc comment could possibly use some language polish.

Maybe TODO, should we implement a MultiSource right away, which would merge IP ranges from multiple sources? That might be handy for trusting something like CloudFlare + some other IPs from servers controlled by the user.

[aadnehovda]

What if a plugin could simply provide yes/no to whether a proxy is trusted, instead of just the dynamic IP ranges? Could the plugin then include additional checks such as TLS auth, secret headers (#5193), etc.? . A rogue proxy instance (e.g. for spoofing real client IPs to bypass geowalls) on cloudflare will be using the same IP ranges as your own instance.

[francislavoie]

Well the nice thing about the design using a var is that a plugin could just set trusted_proxy to true using whatever conditions it wants. You could even do it in your config with the vars handler with something like vars @some-condition trusted_proxy true.

I thought about passing the req to this interface, but then it's more push than pull, and acts more like a request matcher. Then the question would be, is there a point to the interface if it's just a request matcher? I think this is simpler, because it abstracts away the checking of the remote IP by just feeding Caddy some IP ranges which is easier to configure and harder to mess up.

[mholt]

This is a great improvement. Thanks for the enhancement Francis!

Responding to the discussion in a moment.

[mholt]

Code LGTM -- I think there's some good discussion points in here.

The matcher idea is interesting. In the future, maybe we could use this to "feed" a matcher for a trusted proxy? I.e. it can pull from this global option and use it to determine whether a requests matches or not.

I thought about passing the req to this interface, but then it's more push than pull, and acts more like a request matcher.

True, it does feel like a matcher -- however I have not regretted passing the *Request in, because it does open a lot more possibilities and still grants the same functionality at no higher cost or complexity. (We just have to trust that the request being passed in isn't being changed inadvertently.)

And actually, the "push" model feels more enticing to me because of the possibilities for dynamic answers. Kind of like how we do for reverse proxy upstreams, being able to get them dynamically at every request.

Would it be so bad for the trusted proxies implementation to actually be a set of request matchers instead? That way we're not limited to just IP ranges in the future.

[francislavoie]

Would it be so bad for the trusted proxies implementation to actually be a set of request matchers instead?

It would certainly make it more awkward to configure in the Caddyfile. That's my main concern. It might not be syntactically obvious that a matcher is what's used at that spot in the config.

But I guess for the CloudFlare case it would probably mean someone should implement a http.matchers.is_cloudflare module, then it would look like trusted_proxy is_cloudflare, I guess. They would have to implement the remote address matching logic themselves in that matcher, whereas we can abstract that away currently in Caddy.

I dunno. I'm lukewarm on it. There's good arguments for it, but 🤔

[mholt]

Just one suggestion from our call today :)

[aadnehovda]

A plugin could check the request for a header and just return an empty IP range if it is missing/has the wrong value?

[francislavoie]

@aadnehovda A plugin could check the request for a header and just return an empty IP range if it is missing/has the wrong value?

An empty list of IP ranges would make the request untrusted. Setting the range to 0.0.0.0/0 would make the request trusted.

From my chat with Matt, we decided to go with an IP range fetching API instead of request matchers, because really, the only part of the request that can actually be trusted is the remote address, since it's the only part of the request that is essentially tamper-proof. Headers can be spoofed.

If you as a user have your own external guarantees (networking isolation) to ensure no other connections can reach your server except through the proxy, then if you feel it's necessary to look at the request to make a decision about trust, now that's possible. But I still wouldn't recommend it in general.

[adryd325]

how does one create an ip range source in a Caddyfile at the moment?
this syntax no-longer works

trusted_proxies {
    140.238.155.71/32
}

[francislavoie]

Currently in v2.6.2, you must specify trusted proxies as a subdirective to reverse_proxy. You have to put them on the same line, no { } block.

example.com {
	reverse_proxy localhost:8080 {
		trusted_proxies 140.238.155.71/32
	}
}

As of v2.6.3 you'll be able to specify it as a global option. That way it'll apply to every proxy, while only having it once in the config. It looks like this:

{
	servers {
		trusted_proxies static 140.238.155.71/32
	}
}

example.com {
	reverse_proxy localhost:8080
}