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

X-Forwarded-For doesn't seem to be working correctly #828

Open
far-blue opened this issue Apr 27, 2021 · 4 comments
Open

X-Forwarded-For doesn't seem to be working correctly #828

far-blue opened this issue Apr 27, 2021 · 4 comments

Comments

@far-blue
Copy link

I'm struggling with the Forwarded and X-Forwarded-For headers on http traffic passing through Fabio.

Our Fabio instances sit behind a Cloudflare style WAF/LB setup and I can see, via tcpdump, that this setup is correctly passing X-Forwarded-For headers. However, traffic passing through Fabio seems to drop all that info and the only entry in the X-Forwarded-For (and Forwarded) headers is the IP address of the WAF/LB.

For instance:
With a client IP of 1.2.3.4
and a WAF IP of 5.6.7.8
and a Fabio IP of 9.9.9.9

I see the following using tcpdump from the WAF:

X-Forwarded-For: 1.2.3.4
X-Forwarded-Proto: https

I see the following using tcpdump coming from Fabio:

X-Forwarded-For: 5.6.7.8
X-Forwarded-Proto: https
Forwarded: for=5.6.7.8; proto=https; by=9.9.9.9; httpproto=http/2.0;

I would expect instead to see:

X-Forwarded-For: 5.6.7.8, 1.2.3.4
X-Forwarded-Proto: https
Forwarded: for=1.2.3.4; proto=https; by=9.9.9.9, by=5.6.7.8, httpproto=http/2.0;

I've tried leaving proxy.header.clientip blank and also setting it to 'X-Forwarded-For'. Looking at the go http source code I would have expected a blank value to equate to the behaviour I'm seeing and 'X-Forwarded-For' to correctly retain and extend the X-Forwarded-For header. But this is not what I'm seeing.

Anyone got any ideas?

@aaronhurt
Copy link
Member

aaronhurt commented Apr 30, 2021

Hi Robert,

Thank you for the report. The HTTP header code hasn't changed in quite a while. Setting proxy.header.clientip to X-Forwarded-For or X-Real-Ip is going to be ignored. That's a noop.

// set configurable ClientIPHeader
// X-Real-Ip is set later and X-Forwarded-For is set
// by the Go HTTP reverse proxy.
if cfg.ClientIPHeader != "" &&
cfg.ClientIPHeader != "X-Forwarded-For" &&
cfg.ClientIPHeader != "X-Real-Ip" {
r.Header.Set(cfg.ClientIPHeader, remoteIP)
}

The Forwarded behavior is expected. We don't pull from X-Forwarded-For to add to the Forwarded header. The Forwarded header is always set but will preserve existing information already present in a Forwarded header.

fwd := r.Header.Get("Forwarded")
if fwd == "" {
fwd = "for=" + remoteIP + "; proto=" + proto
}
if cfg.LocalIP != "" {
fwd += "; by=" + cfg.LocalIP
}
if r.Proto != "" {
fwd += "; httpproto=" + strings.ToLower(r.Proto)
}
if r.TLS != nil && r.TLS.Version > 0 {
v := tlsver[r.TLS.Version]
if v == "" {
v = uint16base16(r.TLS.Version)
}
fwd += "; tlsver=" + v
}
if r.TLS != nil && r.TLS.CipherSuite != 0 {
fwd += "; tlscipher=" + uint16base16(r.TLS.CipherSuite)
}
r.Header.Set("Forwarded", fwd)

The one condition where we might replace an upstream X-Forwarded-For header is in the case of a websocket connection.

ws := r.Header.Get("Upgrade") == "websocket"
if ws {
r.Header.Set("X-Forwarded-For", remoteIP)
}

The only other handling of the X-Forwarded-For header is done in the golang standard http library.

https://golang.org/pkg/net/http/httputil/#ReverseProxy

If you have more information of a case with full headers I'd be happy to take a further look.

@far-blue
Copy link
Author

Hi :)

Where you say setting proxy.header.clientip to X-Forwarded-For is a noop, my understanding is that it was passed to the underlying go http proxy library and used as per lines 278-285 here: https://golang.org/src/net/http/httputil/reverseproxy.go in order to instruct the go proxy code to preserve the existing X-Forwarded-For content. But that isn't working.

What I start with, before a request passes through Fabio, is a correctly filled X-Forwarded-For header (including the correct source IP) and no Forwarded header. What I see coming out the other side is a Forwarded header that states the upstream proxy is is source IP and an X-Forwarded-For header that has lost all the previous content and which has been reset to also state the source IP is the upstream proxy. This is for an http connection (technically originally http/2.0), not a web socket.

All I really want to know is the original source IP address so I'm not really bothered how this is achieved but at the moment neither of the headers are giving me the info I need.

When you say you'd like full headers, could you be a bit more specific? I'm pulling these from the wire using tcpdump so I'll need to target the headers you want to see.

@aaronhurt
Copy link
Member

I'm saying that setting proxy.header.clientip to X-Forwarded-For is a noop because we ignore the proxy.header.clientip setting if it's set to X-Forwarded-For or X-Real-Ip. We always set X-Real-Ip but let reverseproxy.go handle setting X-Forwarded-For, unless it's a websocket, because websockets do not go through that same code you linked.

We look for and modify the Forwarded header but only touch X-Forwarded-For within Fabio for websocket requests. For standard http connections Fabio should not be modifying the upstream X-Forwarded-For header.

If the upstream X-Forwarded-For header is being lost I do not see where that would happen within Fabio. I'll mock up a test with curl and fabio pointing to an nc instance to see the full headers and post the results.

@far-blue
Copy link
Author

far-blue commented May 4, 2021

Hi :)

I wonder whether the problem might be that the string value of X-Forwarded-For isn't making it into the reverse proxy config in go. I notice there was a change of behaviour note in the code that a null (maybe also an empty string?) would cause the reverse proxy code to drop the upstream X-Forwarded-For header content and create a new one - which seems very much the behaviour I'm seeing.

I really appreciate your help and offer of some testing. If there's anything more I can provide please let me know and I'll do what I can :)

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

No branches or pull requests

2 participants