Support Forwarded in Host extractor

[davidpdrsn]

Based on https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded

[jwangnz]

It would be great if Axum also provides an Scheme extractor. We tried to use the Host extractor but in most of our cases, we also need to extractor the scheme from the x-forwarded-proto header, so we end up writing our own extractor to extract both.

[davidpdrsn]

@Tsing please file a separate issue about that.

[davidpdrsn]

cc @neoeinstein what do you think about this?

[jplatte]

I have no idea whether this makes sense conceptually, but code looks good 😄

[neoeinstein]

Did some initial review. The current implementation is insecure by default and overly-trusting of the X-Forwarded-Host headers, which take precedence of the Host header and can potentially allow someone to spoof access to a different virtual host via a CDN, for example. This also has allusions to dealing with Proto and For for the original scheme and IP address respectively.

I'd recommend that the default, no-configuration implementation should only trust the Host header. There's some configurability warranted here, and we should consider using extensions to allow injecting that configuration, or exposing that configuration through generic type variables that can be aliased by a user.

If configured to trust proxy headers, then parsing Forwarded is preferred, as it can be directly associated with proxy IPs that indicate trust. If only X-Forwarded-Host is available, then that value can be used as most will replace the value through forwards. If there are multiple entries, there is no real way with just this header to identify if one was spoofed.

A proper configuration for proxy forwarded information should account for:

• Trusting private IPs by default (does require checking the first-hop connection address first)
• Trusting additional or fewer IP addresses/CIDR blocks
• Trusting a constant number of known proxies

Using the above, the "trusted" information should take the Forwarded header and virtually append the peer IP, Host, scheme as a last value. Then iterating from the tail, consume the constant number of know proxies. Then for each entry beyond this, evaluate whether it is a "trusted IP". Upon reaching the first non-trusted IP, take the first value specified for each in the trusted tail. As an example:

Peer IP: 127.0.0.1
Scheme: HTTP
Host: host.internal
Forwarded: for=255.0.0.255, for=254.0.1.1;host=spoofed.public, for=154.33.22.1;host=real.public, for=10.0.2.2;proto=https

With a default trust profile of private IPs and 0 trust, we evaluate as follows:

1. 127.0.0.1: private, trusted
2. 10.0.2.2: private, trusted
3. 154.33.22.1: non-private, not trusted
   "Real" client IP is: 154.33.22.1, "Real" host is: real.public, "Real" proto is: https

With just X-Forwarded-* headers, we lose the correlation beyond the first hop, so can only use this pattern to evaluate the "Real" client IP, and then need to implicitly trust that an intermediate proxy from a trusted first hop has overwritten any spoofed X-Forwarded-Host and X-Forwarded-Proto as appropriate. In practice, host and proto are less likely to be multiply overwritten (just one CDN layer, for example), but this is why Forwarded is preferred over the older X-Forwarded-* headers.

[davidpdrsn]

Can't all headers be spoofed including Host? Why is only looking at that by default safer?

I imagine its up to users to not blindly trust the data that comes through headers. The docs currently say

Note that user agents can set X-Forwarded-Host and Host headers to arbitrary values so make sure to validate them to avoid security issues.

[neoeinstein]

The primary question is where you place the trust boundary. With only a direct connection and no headers indicating the request was forwarded, we trust that the first hop is the "real" client and they provided the header. Things get tricky as you insert proxies in the middle, and especially with a CDN that might need to replace the original host header to avoid self-reference.

A malicious client can spoof arbitrary information, but proxies will always tack their information onto the end for Forwarded and X-Forwarded-For. Using the rules above, you can establish your trust boundary, where you will trust that the hop is actually proxying a request and not spoofing being a proxy. This is important if you're implementing something like IP-based request limits/quotas and need to identify an IP as responsible for the request, or if your CDN makes some choice about how to map an incoming host to a backend server and thus gain unintended access to a different virtual host on the same server.

[davidpdrsn]

Perhaps I'm being naive but I don't think adding a bunch of configuration to this is necessary. If users wanna look at things in a different order or exclude addresses they don't trust then they can still do that by writing their own extractor.

I have compared with actix-web and they do basically the same thing that we do. See https://github.com/actix/actix-web/blob/master/actix-web/src/info.rs#L75

[neoeinstein]

I think that's a reasonable take. I tend to lean toward preferring secure defaults, especially if others are going to be relying on the value for potentially critical functionality, hence my recommendation on the default pattern. It can be done such that no configuration is required (in which case it would apply the conservative defaults), but that customization could be provided just by adding an extension layer.

With this way of doing things, the security admonition is important, as people will rely on this for security-related purposes whether the documentation says to or not. With a conservative default, 99% of user use cases will be correct and safe out of the box without needing configuration.

Whichever approach axum ultimately decides on is fine by me, though.

[davidpdrsn]

Thanks for you input 😊 Perhaps a more conservative Host extractor would be interesting to explore in axum-extra. I don't think we can change things in axum without breakage but we can get started in axum-extra and move things into axum for 0.6.