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

Sync X-Forwarded-Proto and Forwarded header when possible #296

Closed
systemfreund opened this issue May 17, 2017 · 7 comments
Closed

Sync X-Forwarded-Proto and Forwarded header when possible #296

systemfreund opened this issue May 17, 2017 · 7 comments
Milestone

Comments

@systemfreund
Copy link
Contributor

systemfreund commented May 17, 2017

Situation:

  • AWS loadbalancer is configured to terminate TLS at the loadbalancer level
  • LB hands requests over to Fabio via http
  • When a https-request is made AWS LB sets following headers before calling fabio:
    • X-Forwarded-For: x.x.x.x
    • X-Forwarded-Proto: https
  • Fabio does not override the headers above, however it sets the following before passing the request to upstream:
    • Forwarded: for=x.x.x.x; proto=http; by=y.y.y.y

Now my application wrongly assumes that the "real" request is made via http and not https, because it consumes the Forwarded if its present, instead of X-Forwarded-For|Proto.

I am not sure if it's a problem in my application, or in fabio, as there's a discrepancy in both headers:

  • X-Forwarded-Proto says it's https
  • Forwarded says it's proto=http
@magiconair
Copy link
Contributor

I think the Forwarded header is specified in an RFC. There was another issue about that recently. I'll check after lunch.

@systemfreund
Copy link
Contributor Author

systemfreund commented May 24, 2017

I am quoting from the RFC 7239:

For example, in an environment where a reverse proxy is also used as
a crypto offloader, this allows the origin server to rewrite URLs in
a document to match the type of connection as the user agent
requested
, even though all connections to the origin server are
unencrypted HTTP

So obviously this is not what happens in fabio. As I described above the Forwarded header contains proto=http. However I'd expect it to be proto=https as that would be the "connection [type] as the user agent requested"

Ok admittedly, in my case the "crypto offloading" is done one layer above in the AWS loadbalancer. However, shouldn't we still forward the correct the user agent's "connection type" origin/upstream server?

@magiconair
Copy link
Contributor

The main problem is that the AWS LB sets some but not other headers which leads to the discrepancy. But fabio also uses the actual connection type to determine the scheme which can only work reliably when the connection is terminated on fabio.

We could use the following heuristic to determine the scheme:

  1. If X-Forwarded-Proto XOR Forwarded is set then use the value from the available header
  2. If both headers are set prefer Forwarded since it is well defined (arguments against?)
  3. If none is set, derive from connection

We might need to look at the remote ip as well even though this doesn't affect the AWS case since it uses the PROXY protocol.

@magiconair
Copy link
Contributor

I think this heuristic is flawed for case 2. When both headers are present fabio should not touch them since it isn't clear which is the source of truth.

@magiconair
Copy link
Contributor

I've pushed a change with the described behavior. Could you test it, please?

@magiconair magiconair changed the title Upstream is given the wrong proto Sync X-Forwarded-Proto and Forwarded header when possible May 29, 2017
@systemfreund
Copy link
Contributor Author

I have tested your change and it's working now.

Also I agree with your objection to case 2. It's probably better to not change anything, if both headers are passed to fabio

@hauleth
Copy link

hauleth commented Jun 21, 2017

I have somewheat similar problem, though I get http on both headers which prevents me from redirecting from HTTP to HTTPS as I have no way to distinguish origin protocol. However I am using tcp and ssl LB instead of http and https as I need to provide support for WebSockets also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants