CORS does not deal with Forwarded or X-Forwarded-xxx headers

[tjquinno]

Environment Details

• Helidon Version: 2.x, 3.x
• Helidon SE or Helidon MP
• JDK version:
• OS:
• Docker version (if applicable):

---

Problem Description

Helidon's CORS support relies on the Host header (among other things) to make its decisions.

If intermediaries--load balancers, etc.--lie between the real client and the service, then when the request reaches the Helidon service the Host value no longer reflects the actual host.

HTTP specifies the Forwarded header which conveys by, for, host (which might include the port), and protocol as specified by the originating client.

Non-standard but widespread headers X-Forwarded-Host, X-Forwarded-For, X-Forwarded-Proto, and X-Forwarded-Port express the same information.

Helidon's CORS processing can be enhanced to prefer Forwarded if present, then X-Forwarded-xxx if present, and then Host for deriving the "effective host" to use in CORS decision-making.

Depends on #5824

[tomas-langer]

My recommended design (for discussion):

• add a configuration option to use non-Host headers (defaults to false)
• add a configuration option to provide a list of headers that can be used to determine "frontend host", will be used in order of definition; if any of them is Forwarded, other headers would be ignored if Forwarded is present (as it should cover all cases); e.g. the options should be "forwarded, x-forwarded-host, x-forwarded-port, x-forwarded-proto" etc., so we always have a complete list of headers we should support (security measure)
• add a method ServerRequest.usedAuthority() (or originalAuthority or similar) to obtain the authority as constructed from configured HTTP headers (may be same as HOST header in the simple case of no proxying or default config)
• add a method ServerRequest.usedProtocol() to obtain the original protocol (such as http or https)

We may add additional methods, or we may add a single method that just returns URI - this would be a bit more expensive, and would have to be done lazily (as should any access to request headers anyway)

[tjquinno]

1. Adding usedXXX methods which take the headers into account implies that most parts of Helidon should use the raw values, ignoring the headers, and those parts would continue to invoke authority() and protocol as they do now.

   Is that true? Could the argument be made that most parts of Helidon should consider the headers, and any that do not should invoke rawAuthority and rawProtocol explicitly?

2. As described, would this config approach allow the user to configure the list of headers to be x-forwarded-host only, without x-forwarded-port or x-forwarded-proto? There are, of course, other combinations. Is this a use case we should support? An alternative would be for the config value to contain zero or more of Forwarded, X-Forwarded with the second indicating that the combination of all the X-Forwarded-* headers should be used (if Forwarded is absent from a given request).

3. Both X-Forwarded-For https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For and Forwarded https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded can either appear multiple times or can contain multiple values. Especially the discussion for X-Forwarded-For goes into some detail about security concerns and "how far back" in the chain is safe to use. We will need to keep this in mind as we implement a solution.

[tomas-langer]

The following information is needed:

• protocol (scheme), such as http or https
• host
• port
• path (some proxies can remove a path prefix which must be honored when redirecting)

The reason I do not like to combine X-Forwarded and Forwarded is that each supports a different set of options, and combining them may create mayhem when in the same request (e.g. we could take protocol from X-Forwarded-Proto and host from Forwarded). It seems to me that when a header is defined and we want to use it, we should honor all of the options from that header.

In addition we should follow the proxy whitelist approach for X-Forwarded-For to make sure all proxies on the list are trusted (with an option to trust all). The main reason is that the original IP address may be used for security purposes (such as whitelisted client IP addresses), and if we have non-trusted proxy on the path, user could inject anything they want.

I agree we may want to use existing methods to obtain the computed information, and add methods for "raw". This requires a bit of an analysis of usages of methods that provide such information (for example isSecure on webserver in such a case may change semantics, which would be a backward incompatible change).

[tjquinno]

I was not suggesting combining X-Forwarded-* with Forwarded. My comment about "other combinations" was different combinations of the X-Forwarded-* headers with each other.

I agree that, if both types of headers are present--that is, both Forwarded and X-Forwarded-*--we should use only one of them as directed by user settings and config, not combine some values from one and some from the other.

You mentioned using a whitelist for X-Forwarded-For. Would we want whitelisting for Forwarded: for also? Because both are just headers, is seems that both would be equally susceptible to corruption by bad actors.

[tomas-langer]

yes, we should do the same to trust the intermediaries regardless of header types