Proxy IP taken instead of the client one if X-Forwarded-For contains a list

numo68 commented 6 months ago

The problem

My installation is fronted by a reverse proxy (traefik) that is further proxied by Cloudflare, with the usual

http:
  use_x_forwarded_for: true
  trusted_proxies:
    - 192.168.xx.yy/24

The HTTP request headers arriving at the Home Assistant are

Cf-Connecting-Ip: MY-CLIENT-IP
X-Forwarded-For: MY-CLIENT-IP, CLOUDFLARE-IP
X-Real-Ip: MY-CLIENT-IP

If I try to log with invalid credentials, the log however contains

2024-01-04 12:18:55.919 WARNING (MainThread) [homeassistant.components.http.ban] Login attempt or request with invalid authentication from CLOUDFLARE-IP (CLOUDFLARE-IP)

meaning that while the X-Forwarded-For header is trusted, the incorrect component of it is taken.

The X-Forwarded-For header has no formal specification and Home Assistant is by far not the only software that gets it wrong; the commonly used definition however does allow for a list and sorts the addresses real-client-first (see e.g. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For ).

Proposal:

either parse and use the first address of the list
or - better - allow to define the trusted header name itself, instead of a boolean use_x_forwarded_for. I am imagining something like real_ip_from: X-Real-Ip.

Thanks

What version of Home Assistant Core has the issue?

core-2023.12.4

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

No response

Link to integration documentation on our website

No response

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

matejx commented 5 months ago

I don't know about the logs, but you're using trusted_proxies wrong. trusted_proxies means trusted proxy IPs, not trusted client IPs. A local subnet should never appear in the trusted_proxies list. In your case CLOUDFLARE-IP should be the only IP in the list.

numo68 commented 5 months ago

I don't know about the logs, but you're using trusted_proxies wrong. trusted_proxies means trusted proxy IPs, not trusted client IPs. A local subnet should never appear in the trusted_proxies list. In your case CLOUDFLARE-IP should be the only IP in the list.

That IP on the local subnet belongs to a proxy on a docker network used for the proxy -> server (not just HA) connections. This is the IP the HA will see the proxied connections coming from.

The thing here is that there are two proxies - cloudflare forwarding to the traefik and traefik then forwarding to the HA. Meaning traefik needs to trust cloudflare and HA needs to trust the traefik; the HA never sees a connection coming from CLOUDFLARE-IP.

The config might be a bit unusual but it will happen in containerized environments.

matejx commented 5 months ago

That IP on the local subnet belongs to a proxy on a docker network used for the proxy -> server (not just HA) connections. This is the IP the HA will see the proxied connections coming from.

I see. Disregard my comment please.

malkstar commented 4 months ago

@numo68 I run a similar setup and ran into an issue with seeing cloudflare's IP today as the client today. I haven't looked too much into it yet but I'm going to setup my reverse proxy to overwrite the X-Forwarded-For with CF-Connecting-IP instead of the remote IP to simplify the parsing. https://developers.cloudflare.com/support/troubleshooting/restoring-visitor-ips/restoring-original-visitor-ips/#web-server-instructions

That being said the behaviour in HA is strange, it takes the first non-trusted address from right to left. This is documented but a little ambiguous. When it says first, it means parsed right-to-left.

issue-triage-workflows[bot] commented 1 month ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

numo68 commented 1 month ago

If neither of the proposals, i.e.

either parse and use the first address of the list
or - better - allow to define the trusted header name itself, instead of a boolean use_x_forwarded_for. I am imagining something like real_ip_from: X-Real-Ip.

is being considered, please a human close this, not a bot :) Thanks

home-assistant / core