Bitwarden_rs: Client IP address is wrong when server is behind proxy

[sercand]

Bitwarden uses rocket's client_id function of request which doesn't use X-Forwarded-For header to determine the client IP address.

Proxies like Envoy and Aws Elastic Load Balancing uses X-Forwarded-For header instead of X-Real-IP for forwarding client IP addresses.

[dani-garcia]

Well you could always configure the proxy to set the correct header, but you are right that X-Real-IP doesn't seem very popular compared to other headers, I wonder why Rocket decided to use that one.

We could offer an option to check extra headers, as the current situation has been confusing for some people.

[mprasil]

I feel like this should maybe be raised upstream in the rocket repository also?

[ryanjaeb]

This is tough to deal with when using Cloudflare and Traefik (v2). I can't find a way to rewrite the headers. Traefik adds the connecting IP as X-Real-IP if the header is missing and Cloudflare strips X-Real-IP. Between the two, X-Real-IP always ends up set to a Cloudflare IP.

It would be awesome if this were user settable via an environment or config variable since that would support provider specific headers like Cloudflare's CF-Connecting-IP.

[dani-garcia]

Okay seeing as the default header choice by rocket seems to be causing issues I've made it configurable in the latest commit, https://github.com/dani-garcia/bitwarden_rs/commit/88c56de97b48bb5b9b8af350d0d0e0d5f080ff0e.

Set IP_HEADER=CF-Connecting-IP or IP_HEADER=X-Forwarded-For to make it use those respective headers, or IP_HEADER=none to disable the header cheking. The default is still the same as before to avoid breaking anything.

[ryanjaeb]

@dani-garcia I just tested this with the latest bitwardenrs/server:alpine Docker image (sha256:64344a72bc0fc48df1ab421aa56154cfd18a331a066bc685a8def3f7f3adbca6). It works great with CF-Connecting-IP. Thank you!

However, I also tested it with X-Forwarded-For and I end up seeing the IP address of my Traefik container, which would be the connecting IP as seen by my bitwarden_rs container. My X-Forwarded-For header looks like this:

X-Forwarded-For: client_ip, cloudflare_proxy_ip

I turned on trace logging and see:

bitwardenrs    | [2019-12-28 07:11:15][hyper::header][TRACE] raw header: "X-Forwarded-For"= [ removed ]

The value shown in binary ([ removed ]) is correct and does not include the IP address of my Traefik container. I've never written any Rust, but my best guess is the comma separated IP list doesn't parse correctly and it falls back to using the connecting IP.

If that's correct, I think it's normal to treat the first IP address in the X-Forwarded-For header as the client IP, even though it can be spoofed on poorly configured proxies.

I really appreciate you adding the config option, so please let me know if I can do any testing or provide any config examples that would be useful.

[dani-garcia]

Ah, I didn't know X-Forwarded-For had a different format!

Your explanation sounds right, if the contents of the header can't parse as an IP, then I print a warning and fall back to the remote IP. https://github.com/dani-garcia/bitwarden_rs/blob/5c6081c4e2d287687a1be185306f6131fca33b4c/src/auth.rs#L433

It should work now in the latest master https://github.com/dani-garcia/bitwarden_rs/commit/cb6f3927745b8f5feb69662fe7aa2b355c818612.

Yeah all these headers can be sent directly by the user, so we depend on the proxies to be configured correctly to either remove them or replace them with the correct value. Now that you mention it, we should make sure to mention it at least in the Fail2Ban and Proxy wiki pages.

[sercand]

I think we can close this with the 1.13.1 release.

[dadatuputi]

It would be nice to have this option documented in the env template and wiki. This was necessary to get fail2ban working properly with caddy.