Only use the X-Forwarded-For header if connection is from a trusted network

[colinodell]

Description:

As noted in #14345, Home Assistant blindly trusts the X-Forwarded-For header without checking who sent it or if they should be trusted. This allows anyone to spoof their IP address during authentication if http.use_x_forwarded_for is enabled which can lead to some major security problems: #14345 (comment)

This PR changes the behavior of real_ip_middleware to only use the X-Forwarded-For header if the incoming connection is from a trusted IP address (per the trusted_networks setting).

Related issue (if applicable): fixes #14345

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5618

Example entry for configuration.yaml (if applicable):

Assuming that a reverse proxy is running on the same machine, here's how you'd configure Home Assistant to detect the true IP address of the end-user:

http:
  api_password: YOUR_PASSWORD
  use_x_forwarded_for: True
  trusted_networks:
    - 127.0.0.1

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass

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

• Documentation added/updated in home-assistant.github.io

If the code does not interact with devices:

• Tests have been added to verify that the new code works.

[houndci-bot]

closing bracket does not match visual indentation

[houndci-bot]

line too long (82 > 79 characters)

[colinodell]

This seems to work locally for me, but I'd strongly prefer if somebody else could also test to confirm this works as expected.

[fabaff]

Doesn't this PR trying to do the same as #14379?

[colinodell]

I believe that #14379 is still vulnerable to spoofing because it never checks which IP the connection is being made from, unlike this PR. For example, given this configuration:

http:
  api_password: YOUR_PASSWORD
  use_x_forwarded_for: True
  trusted_networks:
    - 127.0.0.1

#14379 would still allow IP spoofing as long as the malicious header contains a trusted network like 127.0.0.1 within the list like this: X-Forwarded-For: 127.0.0.1, 12.34.56.78.

That approach does indeed prevent someone from gaining access by guessing a trusted network IP, but the IP spoofing still allows them to bypass rate limiting or perform a DoS attack.

[colinodell]

To clarify the difference further:

• Improve handling of x-forwarded-for header (fixes #14345) #14379 says "we'll (still) always blindly trust the header without checking who sent it, but skip values that match trusted_network"
• This PR says "we'll only trust the header if the connection actually comes from a trusted_network"

[mnoorenberghe]

IIUC, trusted_networks also bypasses authentication so doesn't that mean that there will be no way to use HA password authentication behind a proxy with X-Forwarded-For after this change?

[colinodell]

HA password authentication should still work, as that is a separate middleware which runs after we've determined the user's real IP.

When a proxied request comes in from a trusted proxy, this middleware will use the X-Forwarded-For value as the real IP. This "real IP" is then used by the authentication middleware to and checked against the list of trusted_networks.

There are only two scenarios I can envision where authentication would unexpectedly be bypassed:

Scenario 1 - Misconfiguration

The user has added the proxy IP to trusted_networks but failed to enable use_x_forwarded_for. This is a misconfiguration and HA is doing exactly what you requested.

Scenario 2 - Real IP and proxy IP are identical

If (for some reason) the HTTP requests originates from the same IP as the proxy server, then yes authentication will be bypassed. For example, let's say you're running HA on your local machine with this configuration:

http:
  api_password: YOUR_PASSWORD
  use_x_forwarded_for: True
  trusted_networks:
    - 127.0.0.1

And let's say you've also got nginx running locally as your proxy. Well, nginx is going to send an X-Forwarded-For: 127.0.0.1 header, so that'll be your real IP, and that'll also match the trusted_networks used by the auth middleware.

[colinodell]

I can submit a follow-up PR to add a separate trusted_proxies option, which would prevent scenario 2. This would also allow the user to trust a wider range of proxies (perhaps they're using a cloud proxy with multiple IP ranges) while keeping the auth-bypass range very small.

[mnoorenberghe]

HA password authentication should still work, as that is a separate middleware which runs after we've determined the user's real IP.

Thanks for clarifying, I didn't realize the auth middleware ran after. I hope there is a test for that since it seems like it would be easy to regress and accidentally bypass authentication for all users who use a proxy.

[colinodell]

@mnoorenberghe I have opened #15204 which adds a separate whitelist for proxies.

I hope there is a test for that since it seems like it would be easy to regress and accidentally bypass authentication for all users who use a proxy.

Because the two features would use two completely different lists if/when #15204 is merged, I don't think that test would be required as that edge case scenario would no longer be possible :)

[ayavilevich]

@colinodell thanks for trying to address this issue.

I don't have this setup myself so can't check, but I did a code review of the changes.
The issue I originally raised (#14345) was about spoofing the content of the header. Such spoofing can happen even if the proxy is trusted. This PR adds a check to verify that the proxy is in trusted_networks but it doesn't help with the spoofing.

Also, having the proxy in the trusted list might be undesirable, as the trusted list is basically a "password not required list" for the purpose of authentication. A separate "trusted_proxies" will help with that.

To handle spoofing you need to take the last IP address in the forwarded_for chain from the header when a trusted proxy (in a general term) is used. This is as simple as adding a "reverse()" call. Another solution was proposed by @mvn23 at #14379 . It ignores any IPs not in the trusted_networks. That PR is not accepted at this time, so I believe we are still vulnerable.

I think debugging and resolving this would be easier if you would have proper automated tests. I will prepare a list of use cases and the expected REAL_IP and post here and in #14345 .

[colinodell]

@ayavilevich I've submitted #15204 which should address both of your concerns

[ayavilevich]

@colinodell that looks good.

I have added a list of suggested test cases. Please take a look
#14345 (comment)