From d339e3bd8c0d1030fe1f686af26e9f890f8ff687 Mon Sep 17 00:00:00 2001
From: Joakim Plate <elupus@ecce.se>
Date: Fri, 2 Jul 2021 11:49:42 +0200
Subject: [PATCH] Reject trusted network access from proxies (#52388)
---
.../auth/providers/trusted_networks.py | 14 ++++++++
homeassistant/components/http/forwarded.py | 8 ++---
tests/auth/providers/test_trusted_networks.py | 25 ++++++++++++++
tests/components/http/test_forwarded.py | 33 ++++---------------
4 files changed, 48 insertions(+), 32 deletions(-)
diff --git a/homeassistant/auth/providers/trusted_networks.py b/homeassistant/auth/providers/trusted_networks.py
index fd2014667f8..7b609f371ef 100644
--- a/homeassistant/auth/providers/trusted_networks.py
+++ b/homeassistant/auth/providers/trusted_networks.py
@@ -81,6 +81,17 @@ class TrustedNetworksAuthProvider(AuthProvider):
"""Return trusted users per network."""
return cast(Dict[IPNetwork, Any], self.config[CONF_TRUSTED_USERS])
+ @property
+ def trusted_proxies(self) -> list[IPNetwork]:
+ """Return trusted proxies in the system."""
+ if not self.hass.http:
+ return []
+
+ return [
+ ip_network(trusted_proxy)
+ for trusted_proxy in self.hass.http.trusted_proxies
+ ]
+
@property
def support_mfa(self) -> bool:
"""Trusted Networks auth provider does not support MFA."""
@@ -178,6 +189,9 @@ class TrustedNetworksAuthProvider(AuthProvider):
):
raise InvalidAuthError("Not in trusted_networks")
+ if any(ip_addr in trusted_proxy for trusted_proxy in self.trusted_proxies):
+ raise InvalidAuthError("Can't allow access from a proxy server")
+
@callback
def async_validate_refresh_token(
self, refresh_token: RefreshToken, remote_ip: str | None = None
diff --git a/homeassistant/components/http/forwarded.py b/homeassistant/components/http/forwarded.py
index 684dbbb9e2b..18bc51af1d1 100644
--- a/homeassistant/components/http/forwarded.py
+++ b/homeassistant/components/http/forwarded.py
@@ -129,11 +129,9 @@ def async_setup_forwarded(
overrides["remote"] = str(forwarded_ip)
break
else:
- _LOGGER.warning(
- "Request originated directly from a trusted proxy included in X-Forwarded-For: %s, this is likely a miss configuration and will be rejected",
- forwarded_for_headers,
- )
- raise HTTPBadRequest()
+ # If all the IP addresses are from trusted networks, take the left-most.
+ forwarded_for_index = -1
+ overrides["remote"] = str(forwarded_for[-1])
# Handle X-Forwarded-Proto
forwarded_proto_headers: list[str] = request.headers.getall(
diff --git a/tests/auth/providers/test_trusted_networks.py b/tests/auth/providers/test_trusted_networks.py
index 412f660adc3..d7574bf0da1 100644
--- a/tests/auth/providers/test_trusted_networks.py
+++ b/tests/auth/providers/test_trusted_networks.py
@@ -8,7 +8,9 @@ import voluptuous as vol
from homeassistant import auth
from homeassistant.auth import auth_store
from homeassistant.auth.providers import trusted_networks as tn_auth
+from homeassistant.components.http import CONF_TRUSTED_PROXIES, CONF_USE_X_FORWARDED_FOR
from homeassistant.data_entry_flow import RESULT_TYPE_ABORT, RESULT_TYPE_CREATE_ENTRY
+from homeassistant.setup import async_setup_component
@pytest.fixture
@@ -144,6 +146,29 @@ async def test_validate_access(provider):
provider.async_validate_access(ip_address("2001:db8::ff00:42:8329"))
+async def test_validate_access_proxy(hass, provider):
+ """Test validate access from trusted networks are blocked from proxy."""
+
+ await async_setup_component(
+ hass,
+ "http",
+ {
+ "http": {
+ CONF_TRUSTED_PROXIES: ["192.168.128.0/31", "fd00::1"],
+ CONF_USE_X_FORWARDED_FOR: True,
+ }
+ },
+ )
+ provider.async_validate_access(ip_address("192.168.128.2"))
+ provider.async_validate_access(ip_address("fd00::2"))
+ with pytest.raises(tn_auth.InvalidAuthError):
+ provider.async_validate_access(ip_address("192.168.128.0"))
+ with pytest.raises(tn_auth.InvalidAuthError):
+ provider.async_validate_access(ip_address("192.168.128.1"))
+ with pytest.raises(tn_auth.InvalidAuthError):
+ provider.async_validate_access(ip_address("fd00::1"))
+
+
async def test_validate_refresh_token(provider):
"""Verify re-validation of refresh token."""
with patch.object(provider, "async_validate_access") as mock:
diff --git a/tests/components/http/test_forwarded.py b/tests/components/http/test_forwarded.py
index 8d8467b699f..400a1f32729 100644
--- a/tests/components/http/test_forwarded.py
+++ b/tests/components/http/test_forwarded.py
@@ -43,9 +43,15 @@ async def test_x_forwarded_for_without_trusted_proxy(aiohttp_client, caplog):
@pytest.mark.parametrize(
"trusted_proxies,x_forwarded_for,remote",
[
+ (
+ ["127.0.0.0/24", "1.1.1.1", "10.10.10.0/24"],
+ "10.10.10.10, 1.1.1.1",
+ "10.10.10.10",
+ ),
(["127.0.0.0/24", "1.1.1.1"], "123.123.123.123, 2.2.2.2, 1.1.1.1", "2.2.2.2"),
(["127.0.0.0/24", "1.1.1.1"], "123.123.123.123,2.2.2.2,1.1.1.1", "2.2.2.2"),
(["127.0.0.0/24"], "123.123.123.123, 2.2.2.2, 1.1.1.1", "1.1.1.1"),
+ (["127.0.0.0/24"], "127.0.0.1", "127.0.0.1"),
(["127.0.0.1", "1.1.1.1"], "123.123.123.123, 1.1.1.1", "123.123.123.123"),
(["127.0.0.1", "1.1.1.1"], "123.123.123.123, 2.2.2.2, 1.1.1.1", "2.2.2.2"),
(["127.0.0.1"], "255.255.255.255", "255.255.255.255"),
@@ -77,33 +83,6 @@ async def test_x_forwarded_for_with_trusted_proxy(
assert resp.status == 200
-@pytest.mark.parametrize(
- "trusted_proxies,x_forwarded_for",
- [
- (
- ["127.0.0.0/24", "1.1.1.1", "10.10.10.0/24"],
- "10.10.10.10, 1.1.1.1",
- ),
- (["127.0.0.0/24"], "127.0.0.1"),
- ],
-)
-async def test_x_forwarded_for_from_trusted_proxy_rejected(
- trusted_proxies, x_forwarded_for, aiohttp_client
-):
- """Test that we reject forwarded requests from proxy server itself."""
-
- app = web.Application()
- app.router.add_get("/", mock_handler)
- async_setup_forwarded(
- app, True, [ip_network(trusted_proxy) for trusted_proxy in trusted_proxies]
- )
-
- mock_api_client = await aiohttp_client(app)
- resp = await mock_api_client.get("/", headers={X_FORWARDED_FOR: x_forwarded_for})
-
- assert resp.status == 400
-
-
async def test_x_forwarded_for_disabled_with_proxy(aiohttp_client, caplog):
"""Test that we warn when processing is disabled, but proxy has been detected."""