Improve X-Forwarded-* request headers handling

[frenck]

Breaking change

The processing of data received from reverse proxies is now more strictly handled. Invalid or malformed X-Forwarded-For headers will now result in an HTTP 400 error (Bad Request). Support for X-Forwarded-Proto and X-Forwarded-Host has been added.

Additionally, Home Assistant will now log cases of a reverse proxy being used, but not configured with Home Assistant. Make sure, you set the use_x_forwarded_for and trusted_proxies in your Home Assistant http configuration correctly to avoid warnings.

Proposed change

Improves the handling of X-Forwarded-* headers, replacing the real_ip middleware with a forwarded middleware.

• Modifies the existing request, instead of setting a request[KEY_REAL_IP] "hack".
• Only initialize middleware in case x_forwarded_for is enabled in the http configuration and trusted_proxies are set.
• Improves the handling of X-Forwarded-For (see processing info below)
• Add support for handling X-Forwarded-Proto
• Add support for handling X-Forwarded-Host

The processing is more strict and will throw an HTTP 400 error in case something is wrong. This might be a breaking change, before this PR, Home Assistant would accept about anything.

How headers are processed

To add clarification to the PR, this PR processes the headers in the following way:

X-Forwarded-For: <client>, <proxy1>, <proxy2>
e.g., X-Forwarded-For: 203.0.113.195, 70.41.3.18, 150.172.238.178

We go through the list from the right side, and skip all entries that are in our trusted proxies list. The first non-trusted IP is used as the client IP. If all items in the X-Forwarded-For are trusted, including the most left item (<client>), the most left item is used. In the latter case, the client connection originated from an IP that is also listed as a trusted proxy IP or network.

X-Forwarded-Proto: <client>, <proxy1>, <proxy2>
e.g., X-Forwarded-Proto: https, http, http
OR X-Forwarded-Proto: https (one entry, even with multiple proxies)

The X-Forwarded-Proto is determined based on the corresponding entry of the X-Forwarded-For header that is used/chosen as the client IP. However, some proxies, for example, Kubernetes NGINX ingress, only retain one element in the X-Forwarded-Proto header. In that case, we'll just use what we have.

X-Forwarded-Host: <host>
e.g., X-Forwarded-Host: example.com

If the previous headers are processed successfully, and the X-Forwarded-Host is present, it will be used.

Additionally:

• If no X-Forwarded-For header is found, the processing of all headers is skipped.
• Log a warning when untrusted connected peer provices X-Forwarded-For headers.
• If multiple instances of X-Forwarded-For, X-Forwarded-Proto or X-Forwarded-Host are found, an HTTP 400 status code is thrown.
• If malformed or invalid (IP) data in X-Forwarded-For header is found, an HTTP 400 status code is thrown.
• The connected client peer on the socket of the incoming connection, must be trusted for any processing to take place.
• If the number of elements in X-Forwarded-Proto does not equal 1 or is equal to the number of elements in X-Forwarded-For, an HTTP 400 status code is thrown.
• If an empty X-Forwarded-Host is provided, a HTTP 400 status code is thrown.
• If an empty X-Forwarded-Proto is provided, or an empty element in the list, a HTTP 400 status code is thrown.

Alternatives/Additions to consider

• Require the whole X-Forwarded-For chain to be valid.
  Right now we stop processing at the first untrusted IP, however, we can improve security a bit, by requiring the whole chain to be valid (excluding <client> as that one is expected to be untrusted). This might add configuration complexity for the end-user.
• Require specification for the number of hops/proxies.
  We now go through the list of IP addresses in X-Forwarded-For and accept any amount of values/hops. Ideally, we can improve security a bit by requiring specification on the number of hops expected. This can be problematic when Home Assistant is deployed in a more advanced and dynamic environment where this isn't known. This also adds configuration complexity for the end-user.
• Require order of hops/proxies.
  Right now, we don't look at the order of reverse proxies in the X-Forwarded-For. We could use the trusted_proxies list as the order and number of proxies expected which greatly improves expected results, however, this is a larger breaking change and can be problematic when Home Assistant is deployed in a more advanced and dynamic environment where this isn't known. The level of breaking change is most like to cause quite a lot of configuration confusing with the end-user.
• Processing of X-Forwarded-Port.
• Processing of X-Real-Ip.
  This could be done if the client connected to the socket is trusted and no X-Forwarded-For is set.
• Processing of RFC7239 Forwarded headers.
  While X-Forwarded-For is the defacto standard, however, Forwarded is standardized in RFC7239. It provides more features as well and is easier to process. If Forwarded is present, X-Forwarded-* could be ignored.

References

• Wikipedia: https://www.wikiwand.com/en/X-Forwarded-For
• Mozilla MDN web docs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For
• RFC7239: Forwarded HTTP Extension: https://tools.ietf.org/html/rfc7239
• Reference implementation from aiohttp: https://github.com/aio-libs/aiohttp-remotes/blob/master/aiohttp_remotes/x_forwarded.py

Todo

• Adding lots of tests for the forwarded middleware
• Correct all existing tests that used request[HA_REAL_IP]
• Fix tests/components/emulated_hue/test_hue_api.py::test_external_ip_blocked, external IP needs to be patched differently.
• Log warning when X-Forwarded-For header is received from untrusted peer

Manual regression tests

• UI works, including lovelace + camera's
  • Without proxy
  • With trusted proxy
  • With untrusted proxy
• Shows warnings for untrusted proxies in the log
• IP ban still works
  • Bans proxy when its not trusted
  • Bans client when proxy is trusted
  • Bans client without proxy
• Trusted networks still works
  • Without proxy
  • With proxy
  • Blocks from outside trusted network without proxy
  • Blocks from outside trusted network with proxy

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
http:
  use_x_forwarded_for: true
  trusted_proxies:
    - 10.10.100.10

Additional information

• This PR fixes or closes issue: fixes X-Forwarded-For is parsed incorrectly / incompletely #29868
• This PR is related to issue: X-Forwarded-For parsed incorrectly when using multiple proxies? #25963 (closed), Fix forwarded-for ip extraction #26193 (closed)
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

• No score or internal
• 🥈 Silver
• 🥇 Gold
• 🏆 Platinum

[probot-home-assistant]

Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with an integration (http) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

[pvizeli]

2 small comments, but other looks fine 👍 good work

[bdraco]

Looks good (discussed out of band)

[errolsancaktar]

i think i may be missing part of the config here. Caching is handled by cloudflare on my side. i tried adding their proxy ips however still getting Too many headers for X-Forwarded-For

[frenck]

@errolsancaktar if you have an issue, please create an issue. Thanks!

[koying]

Hi @frenck . Sorry to bother you.

X-Forwarded-For: <client>, <proxy1>, <proxy2>:
We go through the list from the right side, and skip all entries that are in our trusted proxies list. The first non-trusted IP is used as the client IP.

I was wondering what was the reasoning behind this. According to specs, the first IP is definitely the originator. Why replacing/loosing this information by replacing it by an intermediate proxy?

That also has the inconvenience that "trusted_proxy" has 2 different meanings:

• The actual trusted proxies, so that if the upstream proxy is not one of those, it's an error
• A way to filter out intermediate proxies?

There are a couple of threads on the forum where users are using cloudflare proxies upstream (don't ask me why/how), and, unless they put in their config the full list of possible cloudflare proxies, they end up with "Login attempt or request with invalid authentication" with a cloudflare address, which is quite confusing.

[frenck]

According to specs, the first IP is definitely the originator.

That is incorrect.

Nevertheless, this is an PR from August 2020, many things have changed in between. If you want to report a bug, please raise an issue. This PR has been closed and handled a long time ago.

Thanks 👍