Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security issue, unauthorized trusted access by spoofing x-forwarded-for header #14345

Closed
ayavilevich opened this issue May 8, 2018 · 20 comments · Fixed by #15182
Closed

Security issue, unauthorized trusted access by spoofing x-forwarded-for header #14345

ayavilevich opened this issue May 8, 2018 · 20 comments · Fixed by #15182

Comments

@ayavilevich
Copy link

Home Assistant release with the issue:
0.68.1 but probably older versions too

Last working Home Assistant release (if known):
N/A

Operating environment (Hass.io/Docker/Windows/etc.):
Hassbian but should be applicable to other deployment methods

Component/platform:
http

Description of problem:
There is a bug in HA that in certain conditions will allow a person to spoof a trusted source address and bypass password authentication.

Conditions:

  • “trusted_networks” is defined with IP values
  • “use_x_forwarded_for: True”
  • A reverse proxy is used (i.e nginx)
  • Attacker can guess one of the IPs in the trusted list (i.e. 127.0.0.1)
  • API password is set (or else there is nothing to bypass)

Use case:
Attacker adds a spoofed x-forwarded-for header with an IP that is in the trusted list. HA grants access without asking for a password.

Cause:
https://github.com/home-assistant/home-assistant/blob/master/homeassistant/components/http/real_ip.py#L22
Uses the first IP in the header which can be spoofed.
Only the last IP in the header is guaranteed to be non-spoofed assuming a valid reverse proxy is in place.

https://en.wikipedia.org/wiki/X-Forwarded-For : Format :
“where the value is a comma+space separated list of IP addresses, the left-most being the original client, and each successive proxy that passed the request adding the IP address where it received the request from. In this example, the request passed through proxy1, proxy2, and then proxy3 (not shown in the header). proxy3 appears as remote address of the request.
Since it is easy to forge an X-Forwarded-For field the given information should be used with care. The last IP address is always the IP address that connects to the last proxy, which means it is the most reliable source of information. X-Forwarded-For data can be used in a forward or reverse proxy scenario.”

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

http:
  # Secrets are defined in the file secrets.yaml
  api_password: !secret http_password
  use_x_forwarded_for: True
  trusted_networks:
    - 127.0.0.1

Traceback (if applicable):


Additional information:
Work around: Disable trusted_network setting, remove reverse proxy or limit network access to authorized individuals only.

Problem first noticed first by https://community.home-assistant.io/u/NightRanger . Reference: https://community.home-assistant.io/t/trusted-networks-and-x-forwarded-for/20850

I would issue a PR but I don’t have the right environment to test this. Hopefully somebody who uses this setup can apply the fix and test it.

@rofrantz
Copy link
Contributor

rofrantz commented May 8, 2018

This used to work (last known working version 0.62)...but after upgrading recently and checking it in the latest dev version I can confirm that it is an issue

@rofrantz
Copy link
Contributor

rofrantz commented May 8, 2018

The thing is that I didnt even hacked the headers and still no password is asked when connecting from external network

@ayavilevich
Copy link
Author

@rofrantz if you are not spoofing the headers then I believe you are having a different issue.
Do you use a reverse proxy? What is your "http" HA configuration?

@rofrantz
Copy link
Contributor

rofrantz commented May 8, 2018

I use nginx as reverse proxy
My http config uses api password and trusted network

@ayavilevich
Copy link
Author

Do you have use_x_forwarded_for: True in your config?

@rofrantz
Copy link
Contributor

rofrantz commented May 8, 2018

api_password: [redacted]
server_port: [redacted]
base_url: [redacted]
ssl_certificate: [redacted]
ssl_key: [redacted]
cors_allowed_origins:
  - https://google.com
  - https://home-assistant.io
use_x_forwarded_for: True
trusted_networks:
  - 127.0.0.1
  - ::1
  - 192.168.4.0/24
  - 2001:DB8:ABCD::/48
ip_ban_enabled: True
login_attempts_threshold: 3

@NightRang3r
Copy link

NightRang3r commented May 9, 2018

@rofrantz

I made several tests in the past with different configurations, Here are the results:

Test 1:

use_x_forwarded_for: True
Using reverse proxy - Bypassed Login
Without reverse proxy - Bypassed Login

Test2:
use_x_forwarded_for: False
Using reverse proxy - Bypassed Login
Without reverse proxy - No bypass

Test3:
No use_x_forwarded_for in configuration file at all
Using reverse proxy - Bypassed Login
Without reverse proxy - No bypass

I would avoid using the trusted networks configuration when exposing HA to the internet (specially when using reverse proxy)

@rofrantz
Copy link
Contributor

rofrantz commented May 9, 2018

But it used to work and I had the same network setup :((((
Only HA changed

@ayavilevich
Copy link
Author

Proposed solution:

In https://github.com/home-assistant/home-assistant/blob/master/homeassistant/components/http/real_ip.py#L22 , change

request.headers.get(X_FORWARDED_FOR).split(',')[0])

to

request.headers.get(X_FORWARDED_FOR).split(',').reverse()[0])

A more complete solution would be to allow the user to specify, in the configuration, which index (from the end) in the X-Forwarded-For header to use. This will also allow cases where there are two or more reverse proxies inline. Such a use case requires deep understanding of reverse proxies.

@fabaff
Copy link
Member

fabaff commented May 10, 2018

What's the difference between Test2 and Test3? The are booth the same as use_x_forwarded_for is False by default.

At the moment I don't see how replacing the client's source IP with the IP address of the last proxy in the chain would increase the security. The requests are coming from the proxy anyway. Usually the proxy is in the trustworthy network environment but not the client if we are talking about external access from the outside. At first glance it looks like it would get even worse.

Also, the implication about using use_x_forwarded_for and trusted_networks are well documented already.

@NightRang3r
Copy link

@fabaff Where is it documented properly?

It looks like most people that use reverse proxy are not aware of this documentation, browsing some sample users configurations on the HA forums shows that this configuration is enabled and that they have no clue they are exposed to auth bypass.

Here's an example:

https://community.home-assistant.io/t/my-home-assistant-configuration-it-s-alive/52670/7

https://github.com/ntalekt/homeassistant/blob/6f5b927701c89c2be54bacbb294082f4abd726e3/configuration.yaml#L36

@mvn23
Copy link
Contributor

mvn23 commented May 10, 2018

I can confirm this issue as well. The solution @ayavilevich suggested would work, but - as noted - will have some problems when multiple proxies are chained together (either by design or as part of an attack).
A more elaborate solution would be to use the first untrusted IP in the x-forwarded-for header if there are any, and otherwise keep the current behaviour.

As a workaround for the simplest setup with only one proxy, you can change your nginx config as below:
Replace all occurrences of
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
with
proxy_set_header X-Forwarded-For $remote_addr;
This will replace the header the client provides with only the ip that nginx sees instead of appending to the provided header.

@fabaff
Copy link
Member

fabaff commented May 10, 2018

@fabaff Where is it documented properly?

Like here.

To reproduce it with no proxy:

http:
  api_password: !secret http_password
  trusted_networks:
    - 192.168.100.0/24
  use_x_forwarded_for: false
$ curl http://192.168.0.1:8123/api/
401: Unauthorized
http:
  api_password: !secret http_password
  trusted_networks:
    - 192.168.100.0/24
  use_x_forwarded_for: true
$ curl http://192.168.0.1:8123/api/
401: Unauthorized
$ curl --header "X-Forwarded-For: 192.168.0.1" http://192.168.0.242:8123/api/
401: Unauthorized
$ curl --header "X-Forwarded-For: 192.168.100.1" http://192.168.0.1:8123/api/
{"message": "API running."}

@NightRang3r
Copy link

NightRang3r commented May 10, 2018

@fabaff I think you are missing the point, This is not documented properly, Properly is BIG BOLD RED WARNING MESSAGE explaining the DANGER of using trusted networks with reverse proxy and the risk.

"Therefore in a reverse proxy scenario, this option should be used with extreme care." or " You should only enable this in a trustworthy network environment" for standard users is not clear what is "extreme care" ? what is a "trustworthy network environment" ?

Not explaining the security risks PROPERLY to users in the age of "Cyber" is making them vulnerable targets.

@fabaff
Copy link
Member

fabaff commented May 10, 2018

Feel free to improve the formatting or wording. But bigger warnings usually bring nothing because people tend to ignore them anyway.

A "regular/standard" user will never setup a reverse proxy. It's too much work and just enabling port forwarding is easier. For the advance users we need to assume that they know what they are doing and understand the description of the configuration entries. Till now this was the case as there were no complains about that.

@mvn23
Copy link
Contributor

mvn23 commented May 11, 2018

A "regular/standard" user will never setup a reverse proxy. It's too much work and just enabling port forwarding is easier. For the advance users we need to assume that they know what they are doing and understand the description of the configuration entries. Till now this was the case as there were no complains about that.

There's also the people who tend to find some guide or howto on the internet and adapt it to their needs until it works. Those are the people who don't dive into the official docs and read every word written about the options they are enabling.
I'm not saying that it's a good idea to do things this way, but I know for a fact that it happens. If there is a way to make software a bit more "fool-proof" to protect the users then I'm all for it, hence my PR.
In the majority of cases it will not change anything. In a normal reverse proxy setup with no spoofing going on it will still use the first ip in the header. In a trusted environment where all proxies are trusted (e.g. reverse proxy in your home network), it will still use the first ip in the header. Only when the first n addresses are trusted and there is also an untrusted proxy in the chain will it change the current behaviour.
The PR does not mitigate all the risks though, for example it is still possible for an attacker to trigger an ip ban on any untrusted ip if the ip banning function is enabled (this was already possible anyway), but at least the most common and dangerous pitfall in using a reverse proxy is mitigated.

@ayavilevich
Copy link
Author

@fabaff

At the moment I don't see how replacing the client's source IP with the IP address of the last proxy in the chain would increase the security. The requests are coming from the proxy anyway. Usually the proxy is in the trustworthy network environment but not the client if we are talking about external access from the outside. At first glance it looks like it would get even worse.

Happy to explain further. LMK if I am understanding your terminology correctly:
client's source IP: is this the IP of the actual browser client? Is this his private or public IP? Or is this the IP of the party that connects to HA? Which may be a proxy or a real browser client?
IP address of the last proxy: Is this the IP address of the last proxy server or the client IP address that the last proxy server reports?

The current implementation takes an IP address from the header which it assumes is coming from a trusted proxy however due to a bug it is not a trusted IP address and can be spoofed. Therefore the current implementation allows for authentication bypass (under some conditions described above). If you agree to this base understanding then it is clear that fixing this will increase the security.

I understand that there are warnings in the documentation, but the current situation prevents even a person who is an experienced network engineer from setting up a proper secure configuration. So no amount of knowledge or reading of the warnings will be an alternative to fixing this bug if a person wishes to use proper reverse proxy networking.
The warnings are there for other reasons, one of which is to prevent people who don't have a proxy from enabling this configuration as it will also create a securty problem for them.

I don't see how it would get worse. Please provide a use case so we can discuss. If needed I can provide use case for the current state and suggested solution.

@colinodell
Copy link
Contributor

Wow, this is a nasty flaw. The current implementation of setup_real_ip() blindly trusts the X-Forwarded-For header regardless of who sent it. As a result, the following are possible:

  • Gaining access (without knowing the password) by guessing a valid trusted_network IP and sending that in the header
  • Brute-forcing the api_password with unlimited attempts, even if ip_ban_enabled is on (since the attacker can spoof/change the IP which gets banned)
  • DoS attack to block user access by sending multiple bad password guesses using a spoofed IP of a legit user

It is extremely important that the X-Forwarded-For header is *only used if the connection is coming from a whitelisted IP of a known/trusted upstream proxy.

I'll create a pull request with a potential implementation to resolve this issue, along with a PR to the docs explaining the change.

@colinodell
Copy link
Contributor

I have submitted #15182 as a potential solution to this issue. @ayavilevich if you'd be able to review my approach (and possibly help test it) I'd greatly appreciate it!

As a short-term alternative, I have also submitted home-assistant/home-assistant.io#5617 to help further prevent users from using this setting without understanding it.

@ayavilevich
Copy link
Author

Some PRs were made to resolve this. Thanks to everybody who did.
#14379
#15182
#15204

@colinodell
I don't have this setup so can't help QA. I strongly suggest adding more automatic test cases. I have compiled a list of cases which I am including below.

People report unauthorized access which could be due to this vulnerability. I know it only affects a minority of the users, but it is critical nevertheless
https://community.home-assistant.io/t/home-assistant-security-concern/57914

Test cases:

Glosary

use_x_forwarded_for - config, should we look at the header at all, false by default for most users
trusted_networks - config, who do we trust to by pass authentication (note new trusted_proxies field)
peername - input, request.transport.get_extra_info('peername')[0], the IP of who we directly connect to
FORWARDED_FOR - input, the list of IPs as claimed by the peername
REAL_IP - output, request[KEY_REAL_IP] with the IP of the client (direct or in-direct) to consider for authentication

Note
This list doesn't address the new "trusted_proxies" configuration field.

case 1a - no use_x_forwarded_for

use_x_forwarded_for: false
trusted_networks: [127.0.0.1]
peername: 1.1.1.1
FORWARDED_FOR: none
=> REAL_IP: 1.1.1.1

case 1b - no use_x_forwarded_for with spoof or proxy

use_x_forwarded_for: false
trusted_networks: [127.0.0.1]
peername: 1.1.1.1
FORWARDED_FOR: 2.2.2.2 or 2.2.2.2, 3.3.3.3
=> REAL_IP: 1.1.1.1
note: if proper proxy is in line then REAL_IP will always be peername. If proxy is in trusted_network then this will cause lack of authentication for everybody. If proxy is not in trusted_network then this will cause forced authentications, even for clients in trusted_network. This is a case of bad user configuration, not a bug in HA. Users of reverse proxy must configure use_x_forwarded_for properly for their setup.

case 2a - use_x_forwarded_for, normal case with proxy

use_x_forwarded_for: true
trusted_networks: [127.0.0.1, 192.168.0.1/24]
peername: 192.168.0.10
FORWARDED_FOR: 2.2.2.2
=> REAL_IP: 2.2.2.2
note: assuming peername is a trusted proxy

case 2b - use_x_forwarded_for, spoof case, no proxy

use_x_forwarded_for: true
trusted_networks: [127.0.0.1, 192.168.0.1/24]
peername: 1.1.1.1
FORWARDED_FOR: 127.0.0.1
=> REAL_IP: 127.0.0.1
note: this will result in authentication bypass! This case is due to user miss-configuration, not a bug in HA. If you don't use a trusted proxy you should not set use_x_forwarded_for to true. For automated tests sack the output should be 127.0.0.1 since the test can't know if a proxy is or isn't there.

case 3a - use_x_forwarded_for, spoof case, proxy that chains ips

use_x_forwarded_for: true
trusted_networks: [127.0.0.1, 192.168.0.1/24]
peername: 192.168.0.10
FORWARDED_FOR: 127.0.0.1, 1.1.1.1 or 127.0.0.1, 2.2.2.2, 1.1.1.1
=> REAL_IP: 1.1.1.1
note: this is where HA failed before #14345. The wrong output is 127.0.0.1 as it will result in an authentication bypass, even though the user config is proper.

case 3b - use_x_forwarded_for, spoof case, proxy that replaces ips

use_x_forwarded_for: true
trusted_networks: [127.0.0.1, 192.168.0.1/24]
peername: 192.168.0.10
FORWARDED_FOR: 1.1.1.1
=> REAL_IP: 1.1.1.1
note: reverse proxies can be configured to chain or "reset" the IP list. Both are valid per spec, though chaining is the documented method. See example for nginx at #14345 (comment) . If "reset" mode is used, then the bug from #14345 doesn't manifest. Could be a workaround for those who are vulnerable.

case 4a - use_x_forwarded_for, double proxy setup

use_x_forwarded_for: true
trusted_networks: [127.0.0.1, 192.168.0.1/24]
peername: 192.168.0.10 (proxy A)
FORWARDED_FOR: 1.1.1.1, 192.168.0.20 (proxy B)
=> REAL_IP: 1.1.1.1 (real client)
note: This cannot be implemented at this time with HA. A concept of more than one chained proxy is not supported in HA. This is not very common in real life. Perhaps using trusted_proxies this can be supported as well.
note: using a double proxy setup with this configuration today will result in output of "192.168.0.20" which will lead to authentication bypass for this case!

case 4b - use_x_forwarded_for, double proxy setup, one trusted

use_x_forwarded_for: true
trusted_networks: [127.0.0.1, 192.168.0.1/24]
peername: 192.168.0.10 (proxy A)
FORWARDED_FOR: 1.1.1.1, 192.168.1.20 (proxy B)
=> REAL_IP: 1.1.1.1 (real client)
note: This cannot be implemented at this time with HA. A concept of more than one chained proxy is not supported in HA. This is not very common in real life. Perhaps using trusted_proxies this can be supported as well.
note: using a double proxy setup with this configuration today will result in output of "192.168.1.20" which will lead to forced authentication for all!

@home-assistant home-assistant locked and limited conversation to collaborators Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants