core: Incorrect (?) handling of X-Forwarded-For

The problem

From https://github.com/home-assistant/core/pull/38696 , the trusted_proxies is used both to actually reject upstream proxies that are not trusted, and as a filter to determine the originator of the request, the first upstream proxy in the chain not being trusted being considered as the originator.

There doesn’t seem to be an RFC defining X-Forwarded-For, but both https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For and https://en.wikipedia.org/wiki/X-Forwarded-For define it as

X-Forwarded-For: client, proxy1, proxy2

So the originator would always be the first IP in the list.

With the aforementioned PR, the actual originator IP is lost and “Unauthorized access” messages will be logged with the address of a proxy unless the full list of possible proxies that could be used before reaching the actual upstream proxy is added to trusted_proxy, which is both a hassle and a security risk, as you are supposed to add proxies to the list which are not actually trusted, to get the actual originator of a request.

This seems an “in-between” and confusing situation. To add to confusion, the documentation for trusted_proxies says “List of trusted proxies, consisting of IP addresses or networks, that are allowed to set the X-Forwarded-For header”, which is not the actual behaviour (only the peer is considered, afaict).

I see 2 ways forward:

  • As mentionned in the PR, reject a request when any proxy in the X-Forwarded-For list is not trusted.
  • Remove the code that overrides the originator with an intermediate “untrusted” proxy.

What is version of Home Assistant Core has the issue?

2021.6.6

What was the last working version of Home Assistant Core?

?

What type of installation are you running?

Home Assistant Container

Integration causing the issue

http

Link to integration documentation on our website

https://www.home-assistant.io/integrations/http/

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 15 (14 by maintainers)

Commits related to this issue

Most upvoted comments

Yes, the first case can be rejected safely and protects against common misconfiguration. The second can be misconfiguration, however, could be something else 🤷