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
- Clarifies usage of trusted_proxies As discussed in https://github.com/home-assistant/core/issues/52736 — committed to koying/home-assistant.io by koying 3 years ago
- Clarifies usage of trusted_proxies (#18447) * Clarifies usage of trusted_proxies As discussed in https://github.com/home-assistant/core/issues/52736 * Tweak Co-authored-by: Franck Nijhof <fr... — committed to home-assistant/home-assistant.io by koying 3 years ago
- Clarifies usage of trusted_proxies (#18447) * Clarifies usage of trusted_proxies As discussed in https://github.com/home-assistant/core/issues/52736 * Tweak Co-authored-by: Franck Nijhof <fr... — committed to hesselonline/home-assistant.io by koying 3 years ago
Yes, the first case can be rejected safely and protects against common misconfiguration. The second can be misconfiguration, however, could be something else 🤷