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

feat: expose actual remote address #175

Open
james-callahan opened this issue May 16, 2024 · 5 comments
Open

feat: expose actual remote address #175

james-callahan opened this issue May 16, 2024 · 5 comments

Comments

@james-callahan
Copy link
Contributor

I was hoping to use go-httpbin today to figure out what the ip of my load balancer actually comes through as, but it was masked by the cloudflare CDN the request went through

Could you return the .RemoteAddr field somewhere in the response?

Code

func getClientIP(r *http.Request) string {
// Special case some hosting platforms that provide the value directly.
if clientIP := r.Header.Get("Fly-Client-IP"); clientIP != "" {
return clientIP
}
if clientIP := r.Header.Get("CF-Connecting-IP"); clientIP != "" {
return clientIP
}
if clientIP := r.Header.Get("Fastly-Client-IP"); clientIP != "" {
return clientIP
}
if clientIP := r.Header.Get("True-Client-IP"); clientIP != "" {
return clientIP
}
// Try to pull a reasonable value from the X-Forwarded-For header, if
// present, by taking the first entry in a comma-separated list of IPs.
if forwardedFor := r.Header.Get("X-Forwarded-For"); forwardedFor != "" {
return strings.TrimSpace(strings.SplitN(forwardedFor, ",", 2)[0])
}
// Finally, fall back on the actual remote addr from the request.
return r.RemoteAddr

@mccutchen
Copy link
Owner

Ah, so you'd like go-httpbin sitting behind your load balancer to report the IP address of the load balancer itself, rather than make an attempt to return the actual client IP address? That's interesting, I'd be curious to hear more about your use case here if you're willing and able to share!

I'm reluctant to enable something like this by default, as it would expose potentially sensitive details about the network and infrastructure where an instance of go-httpbin is deployed, but if you or someone else opened a pull request adding a new, optional /remote-addr endpoint (or something along those lines), I'd consider it.

I'd probably want it modeled after the /hostname endpoint added in #81, which returns a dummy value by unless a deployment explicitly enables returning the real hostname in its configuration. (That pull request is a bit noisier than it should be, but 1334011 would be a good starting point for anyone interested in making this kind of change!)

@mccutchen mccutchen changed the title Show remote address feat: expose actual remote address May 16, 2024
@james-callahan
Copy link
Contributor Author

Ah, so you'd like go-httpbin sitting behind your load balancer to report the IP address of the load balancer itself, rather than make an attempt to return the actual client IP address? That's interesting, I'd be curious to hear more about your use case here if you're willing and able to share!

I was trying to figure out what the load balancer(s) were: my thought to debug/discover the system was to throw up a go-httpbin image using the existing CI pipelines, and see where the connections were coming from

I'm reluctant to enable something like this by default, as it would expose potentially sensitive details about the network and infrastructure where an instance of go-httpbin is deployed, but if you or someone else opened a pull request adding a new, optional /remote-addr endpoint (or something along those lines), I'd consider it.

Isn't that usually already available in X-Forwarded-For? At least in the case of multiple reverse proxies. Only the most recent hop would be missing.


I also note that at the moment the client can easily fake their IP as long as you aren't behind fly.io (the first preference in the code) e.g. curl https://my-go-httpbin.internal/get -H "Fly-Client-IP: 4.5.6.7.8"
go-httpbin doesn't have any sort of "trust these IPs when they present a forwarded-for header" (and for many cases, an IP list isn't enough!)

@mccutchen
Copy link
Owner

mccutchen commented May 18, 2024

Isn't that usually already available in X-Forwarded-For? At least in the case of multiple reverse proxies. Only the most recent hop would be missing.

Ha, yes, that's a good point! But doesn't that also eliminate the need for adding a new endpoint?

# original example:
# $ curl -s https://httpbingo.org/headers | jq -r '.headers["X-Forwarded-For"][0]' | awk '{print $NF}'

# slightly more robust/correct example:
$ curl -s https://httpbingo.org/headers | jq -r '.headers["X-Forwarded-For"][0]' | awk -F, '{print $NF}' | tr -d ' '
66.241.125.232

I also note that at the moment the client can easily fake their IP as long as you aren't behind fly.io (the first preference in the code) e.g. curl https://my-go-httpbin.internal/get -H "Fly-Client-IP: 4.5.6.7.8"

Sure, but in this case, the client is only tricking itself, at least in terms of getting "risky" output out of go-httpbin.

The other downside is that the go-httpbin server might log the incorrect remote addr in its logs, but if that's a real concern operators can deploy a "hardened" go-httpbin instance that adds middleware to prevent it.

It would probably be reasonable to add an option to control this, if anyone is interested in making the effort. It would also probably be reasonable to document this potential downside in the Production Considerations section of the README.

@james-callahan
Copy link
Contributor Author

Only the most recent hop would be missing.

Ha, yes, that's a good point! But doesn't that also eliminate the need for adding a new endpoint?

The most recent hop is missing, i.e. the remoteaddr as seen by go-httpbin.

Sure, but in this case, the client is only tricking itself, at least in terms of getting "risky" output out of go-httpbin.

I'm thinking of a malicious or misconfigured load balancer where the wrong e.g. Fly-Client-IP header is getting added

@mccutchen
Copy link
Owner

The most recent hop is missing, i.e. the remoteaddr as seen by go-httpbin.

👍 ah yeah, you're right, my mistake. In this case, I'm back to thinking that we could add an additional endpoint here, but that it should be opt-in rather than opt-out.

Sure, but in this case, the client is only tricking itself, at least in terms of getting "risky" output out of go-httpbin.

I'm thinking of a malicious or misconfigured load balancer where the wrong e.g. Fly-Client-IP header is getting added

Maybe I'm missing something, but the use case of deploying a go-httpbin on untrusted/malicious infrastructure is not something I'm particularly worried about; if it's a real concern, I'd be in favor of adding an option to control how exactly go-httpbin determines the "client address" (e.g. "use header Fly-Client-Ip" or "trust these hops in X-Forwarded-For").

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

2 participants