diff --git a/homeassistant/auth/providers/trusted_networks.py b/homeassistant/auth/providers/trusted_networks.py index e93587e91ca..fd2014667f8 100644 --- a/homeassistant/auth/providers/trusted_networks.py +++ b/homeassistant/auth/providers/trusted_networks.py @@ -14,13 +14,10 @@ from ipaddress import ( ip_address, ip_network, ) -import logging from typing import Any, Dict, List, Union, cast -from aiohttp import hdrs import voluptuous as vol -from homeassistant.components import http from homeassistant.core import callback from homeassistant.data_entry_flow import FlowResult from homeassistant.exceptions import HomeAssistantError @@ -89,31 +86,9 @@ class TrustedNetworksAuthProvider(AuthProvider): """Trusted Networks auth provider does not support MFA.""" return False - @callback - def is_allowed_request(self) -> bool: - """Return if it is an allowed request.""" - request = http.current_request.get() - if request is not None and ( - self.hass.http.use_x_forwarded_for - or hdrs.X_FORWARDED_FOR not in request.headers - ): - return True - - logging.getLogger(__name__).warning( - "A request contained an x-forwarded-for header but your HTTP integration is not set-up " - "for reverse proxies. This usually means that you have not configured your reverse proxy " - "correctly. This request will be blocked in Home Assistant 2021.7 unless you configure " - "your HTTP integration to allow this header." - ) - return True - async def async_login_flow(self, context: dict | None) -> LoginFlow: """Return a flow to login.""" assert context is not None - - if not self.is_allowed_request(): - return MisconfiguredTrustedNetworksLoginFlow(self) - ip_addr = cast(IPAddress, context.get("ip_address")) users = await self.store.async_get_users() available_users = [ @@ -195,11 +170,6 @@ class TrustedNetworksAuthProvider(AuthProvider): Raise InvalidAuthError if not. Raise InvalidAuthError if trusted_networks is not configured. """ - if not self.is_allowed_request(): - raise InvalidAuthError( - "No request or it contains x-forwarded-for header and that's not allowed by configuration" - ) - if not self.trusted_networks: raise InvalidAuthError("trusted_networks is not configured") @@ -220,16 +190,6 @@ class TrustedNetworksAuthProvider(AuthProvider): self.async_validate_access(ip_address(remote_ip)) -class MisconfiguredTrustedNetworksLoginFlow(LoginFlow): - """Login handler for misconfigured trusted networks.""" - - async def async_step_init( - self, user_input: dict[str, str] | None = None - ) -> FlowResult: - """Handle the step of the form.""" - return self.async_abort(reason="forwared_for_header_not_allowed") - - class TrustedNetworksLoginFlow(LoginFlow): """Handler for the login flow.""" diff --git a/homeassistant/components/http/__init__.py b/homeassistant/components/http/__init__.py index db198cb334a..19e3437b79c 100644 --- a/homeassistant/components/http/__init__.py +++ b/homeassistant/components/http/__init__.py @@ -232,10 +232,7 @@ class HomeAssistantHTTP: # forwarded middleware needs to go second. setup_security_filter(app) - # Only register middleware if `use_x_forwarded_for` is enabled - # and trusted proxies are provided - if use_x_forwarded_for and trusted_proxies: - async_setup_forwarded(app, trusted_proxies) + async_setup_forwarded(app, use_x_forwarded_for, trusted_proxies) setup_request_context(app, current_request) @@ -252,7 +249,6 @@ class HomeAssistantHTTP: self.ssl_key = ssl_key self.server_host = server_host self.server_port = server_port - self.use_x_forwarded_for = use_x_forwarded_for self.trusted_proxies = trusted_proxies self.is_ban_enabled = is_ban_enabled self.ssl_profile = ssl_profile diff --git a/homeassistant/components/http/forwarded.py b/homeassistant/components/http/forwarded.py index 5c5726a2597..5c62a469924 100644 --- a/homeassistant/components/http/forwarded.py +++ b/homeassistant/components/http/forwarded.py @@ -14,7 +14,9 @@ _LOGGER = logging.getLogger(__name__) @callback -def async_setup_forwarded(app: Application, trusted_proxies: list[str]) -> None: +def async_setup_forwarded( + app: Application, use_x_forwarded_for: bool | None, trusted_proxies: list[str] +) -> None: """Create forwarded middleware for the app. Process IP addresses, proto and host information in the forwarded for headers. @@ -73,15 +75,37 @@ def async_setup_forwarded(app: Application, trusted_proxies: list[str]) -> None: # No forwarding headers, continue as normal return await handler(request) - # Ensure the IP of the connected peer is trusted - assert request.transport is not None + # Get connected IP + if ( + request.transport is None + or request.transport.get_extra_info("peername") is None + ): + # Connected IP isn't retrieveable from the request transport, continue + return await handler(request) + connected_ip = ip_address(request.transport.get_extra_info("peername")[0]) - if not any(connected_ip in trusted_proxy for trusted_proxy in trusted_proxies): + + # We have X-Forwarded-For, but config does not agree + if not use_x_forwarded_for: _LOGGER.warning( - "Received X-Forwarded-For header from untrusted proxy %s, headers not processed", + "A request from a reverse proxy was received from %s, but your " + "HTTP integration is not set-up for reverse proxies; " + "This request will be blocked in Home Assistant 2021.7 unless " + "you configure your HTTP integration to allow this header", connected_ip, ) - # Not trusted, continue as normal + # Block this request in the future, for now we pass. + return await handler(request) + + # Ensure the IP of the connected peer is trusted + if not any(connected_ip in trusted_proxy for trusted_proxy in trusted_proxies): + _LOGGER.warning( + "Received X-Forwarded-For header from untrusted proxy %s, headers not processed; " + "This request will be blocked in Home Assistant 2021.7 unless you configure " + "your HTTP integration to allow this proxy to reverse your Home Assistant instance", + connected_ip, + ) + # Not trusted, Block this request in the future, continue as normal return await handler(request) # Multiple X-Forwarded-For headers diff --git a/tests/auth/providers/test_trusted_networks.py b/tests/auth/providers/test_trusted_networks.py index cceb3d720c0..412f660adc3 100644 --- a/tests/auth/providers/test_trusted_networks.py +++ b/tests/auth/providers/test_trusted_networks.py @@ -5,13 +5,10 @@ from unittest.mock import Mock, patch import pytest import voluptuous as vol -from homeassistant import auth, const +from homeassistant import auth from homeassistant.auth import auth_store from homeassistant.auth.providers import trusted_networks as tn_auth from homeassistant.data_entry_flow import RESULT_TYPE_ABORT, RESULT_TYPE_CREATE_ENTRY -from homeassistant.setup import async_setup_component - -FORWARD_FOR_IS_WARNING = (const.MAJOR_VERSION, const.MINOR_VERSION) < (2021, 8) @pytest.fixture @@ -115,17 +112,7 @@ def manager_bypass_login(hass, store, provider_bypass_login): ) -@pytest.fixture -def mock_allowed_request(): - """Mock that the request is allowed.""" - with patch( - "homeassistant.auth.providers.trusted_networks.TrustedNetworksAuthProvider.is_allowed_request", - return_value=True, - ): - yield - - -async def test_trusted_networks_credentials(manager, provider, mock_allowed_request): +async def test_trusted_networks_credentials(manager, provider): """Test trusted_networks credentials related functions.""" owner = await manager.async_create_user("test-owner") tn_owner_cred = await provider.async_get_or_create_credentials({"user": owner.id}) @@ -142,7 +129,7 @@ async def test_trusted_networks_credentials(manager, provider, mock_allowed_requ await provider.async_get_or_create_credentials({"user": "invalid-user"}) -async def test_validate_access(provider, mock_allowed_request): +async def test_validate_access(provider): """Test validate access from trusted networks.""" provider.async_validate_access(ip_address("192.168.0.1")) provider.async_validate_access(ip_address("192.168.128.10")) @@ -157,7 +144,7 @@ async def test_validate_access(provider, mock_allowed_request): provider.async_validate_access(ip_address("2001:db8::ff00:42:8329")) -async def test_validate_refresh_token(provider, mock_allowed_request): +async def test_validate_refresh_token(provider): """Verify re-validation of refresh token.""" with patch.object(provider, "async_validate_access") as mock: with pytest.raises(tn_auth.InvalidAuthError): @@ -167,7 +154,7 @@ async def test_validate_refresh_token(provider, mock_allowed_request): mock.assert_called_once_with(ip_address("127.0.0.1")) -async def test_login_flow(manager, provider, mock_allowed_request): +async def test_login_flow(manager, provider): """Test login flow.""" owner = await manager.async_create_user("test-owner") user = await manager.async_create_user("test-user") @@ -194,9 +181,7 @@ async def test_login_flow(manager, provider, mock_allowed_request): assert step["data"]["user"] == user.id -async def test_trusted_users_login( - manager_with_user, provider_with_user, mock_allowed_request -): +async def test_trusted_users_login(manager_with_user, provider_with_user): """Test available user list changed per different IP.""" owner = await manager_with_user.async_create_user("test-owner") sys_user = await manager_with_user.async_create_system_user( @@ -276,9 +261,7 @@ async def test_trusted_users_login( assert schema({"user": sys_user.id}) -async def test_trusted_group_login( - manager_with_user, provider_with_user, mock_allowed_request -): +async def test_trusted_group_login(manager_with_user, provider_with_user): """Test config trusted_user with group_id.""" owner = await manager_with_user.async_create_user("test-owner") # create a user in user group @@ -331,9 +314,7 @@ async def test_trusted_group_login( assert schema({"user": user.id}) -async def test_bypass_login_flow( - manager_bypass_login, provider_bypass_login, mock_allowed_request -): +async def test_bypass_login_flow(manager_bypass_login, provider_bypass_login): """Test login flow can be bypass if only one user available.""" owner = await manager_bypass_login.async_create_user("test-owner") @@ -364,42 +345,3 @@ async def test_bypass_login_flow( # both owner and user listed assert schema({"user": owner.id}) assert schema({"user": user.id}) - - -async def test_allowed_request(hass, provider, current_request, caplog): - """Test allowing requests.""" - assert await async_setup_component(hass, "http", {}) - - provider.async_validate_access(ip_address("192.168.0.1")) - - current_request.get.return_value = current_request.get.return_value.clone( - headers={ - **current_request.get.return_value.headers, - "x-forwarded-for": "1.2.3.4", - } - ) - - if FORWARD_FOR_IS_WARNING: - caplog.clear() - provider.async_validate_access(ip_address("192.168.0.1")) - assert "This request will be blocked" in caplog.text - else: - with pytest.raises(tn_auth.InvalidAuthError): - provider.async_validate_access(ip_address("192.168.0.1")) - - hass.http.use_x_forwarded_for = True - - provider.async_validate_access(ip_address("192.168.0.1")) - - -@pytest.mark.skipif(FORWARD_FOR_IS_WARNING, reason="Currently a warning") -async def test_login_flow_no_request(provider): - """Test getting a login flow.""" - login_flow = await provider.async_login_flow({"ip_address": ip_address("1.1.1.1")}) - assert await login_flow.async_step_init() == { - "description_placeholders": None, - "flow_id": None, - "handler": None, - "reason": "forwared_for_header_not_allowed", - "type": "abort", - } diff --git a/tests/components/http/test_auth.py b/tests/components/http/test_auth.py index 71c01630a67..6bd1d622b12 100644 --- a/tests/components/http/test_auth.py +++ b/tests/components/http/test_auth.py @@ -53,7 +53,7 @@ def app(hass): app = web.Application() app["hass"] = hass app.router.add_get("/", mock_handler) - async_setup_forwarded(app, []) + async_setup_forwarded(app, True, []) return app diff --git a/tests/components/http/test_forwarded.py b/tests/components/http/test_forwarded.py index 2946c0b383c..4b7a3421b0a 100644 --- a/tests/components/http/test_forwarded.py +++ b/tests/components/http/test_forwarded.py @@ -28,7 +28,7 @@ async def test_x_forwarded_for_without_trusted_proxy(aiohttp_client, caplog): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, []) + async_setup_forwarded(app, True, []) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get("/", headers={X_FORWARDED_FOR: "255.255.255.255"}) @@ -74,7 +74,7 @@ async def test_x_forwarded_for_with_trusted_proxy( app = web.Application() app.router.add_get("/", handler) async_setup_forwarded( - app, [ip_network(trusted_proxy) for trusted_proxy in trusted_proxies] + app, True, [ip_network(trusted_proxy) for trusted_proxy in trusted_proxies] ) mock_api_client = await aiohttp_client(app) @@ -83,6 +83,33 @@ async def test_x_forwarded_for_with_trusted_proxy( assert resp.status == 200 +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.""" + + async def handler(request): + url = mock_api_client.make_url("/") + assert request.host == f"{url.host}:{url.port}" + assert request.scheme == "http" + assert not request.secure + assert request.remote == "127.0.0.1" + + return web.Response() + + app = web.Application() + app.router.add_get("/", handler) + + async_setup_forwarded(app, False, []) + + mock_api_client = await aiohttp_client(app) + resp = await mock_api_client.get("/", headers={X_FORWARDED_FOR: "255.255.255.255"}) + + assert resp.status == 200 + assert ( + "A request from a reverse proxy was received from 127.0.0.1, but your HTTP " + "integration is not set-up for reverse proxies" in caplog.text + ) + + async def test_x_forwarded_for_with_untrusted_proxy(aiohttp_client): """Test that we get the IP from transport with untrusted proxy.""" @@ -97,7 +124,7 @@ async def test_x_forwarded_for_with_untrusted_proxy(aiohttp_client): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, [ip_network("1.1.1.1")]) + async_setup_forwarded(app, True, [ip_network("1.1.1.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get("/", headers={X_FORWARDED_FOR: "255.255.255.255"}) @@ -119,7 +146,7 @@ async def test_x_forwarded_for_with_spoofed_header(aiohttp_client): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -148,7 +175,7 @@ async def test_x_forwarded_for_with_malformed_header( """Test that we get a HTTP 400 bad request with a malformed header.""" app = web.Application() app.router.add_get("/", mock_handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) @@ -162,7 +189,7 @@ async def test_x_forwarded_for_with_multiple_headers(aiohttp_client, caplog): """Test that we get a HTTP 400 bad request with multiple headers.""" app = web.Application() app.router.add_get("/", mock_handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) @@ -193,7 +220,7 @@ async def test_x_forwarded_proto_without_trusted_proxy(aiohttp_client): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, []) + async_setup_forwarded(app, True, []) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -245,7 +272,7 @@ async def test_x_forwarded_proto_with_trusted_proxy( app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, [ip_network("127.0.0.0/24")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.0/24")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -273,7 +300,7 @@ async def test_x_forwarded_proto_with_trusted_proxy_multiple_for(aiohttp_client) app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, [ip_network("127.0.0.0/24")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.0/24")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -301,7 +328,7 @@ async def test_x_forwarded_proto_not_processed_without_for(aiohttp_client): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get("/", headers={X_FORWARDED_PROTO: "https"}) @@ -313,7 +340,7 @@ async def test_x_forwarded_proto_with_multiple_headers(aiohttp_client, caplog): """Test that we get a HTTP 400 bad request with multiple headers.""" app = web.Application() app.router.add_get("/", mock_handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -339,7 +366,7 @@ async def test_x_forwarded_proto_empty_element( """Test that we get a HTTP 400 bad request with empty proto.""" app = web.Application() app.router.add_get("/", mock_handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -364,7 +391,7 @@ async def test_x_forwarded_proto_incorrect_number_of_elements( """Test that we get a HTTP 400 bad request with incorrect number of elements.""" app = web.Application() app.router.add_get("/", mock_handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -397,7 +424,7 @@ async def test_x_forwarded_host_without_trusted_proxy(aiohttp_client): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, []) + async_setup_forwarded(app, True, []) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -421,7 +448,7 @@ async def test_x_forwarded_host_with_trusted_proxy(aiohttp_client): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -446,7 +473,7 @@ async def test_x_forwarded_host_not_processed_without_for(aiohttp_client): app = web.Application() app.router.add_get("/", handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get("/", headers={X_FORWARDED_HOST: "example.com"}) @@ -458,7 +485,7 @@ async def test_x_forwarded_host_with_multiple_headers(aiohttp_client, caplog): """Test that we get a HTTP 400 bad request with multiple headers.""" app = web.Application() app.router.add_get("/", mock_handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get( @@ -478,7 +505,7 @@ async def test_x_forwarded_host_with_empty_header(aiohttp_client, caplog): """Test that we get a HTTP 400 bad request with empty host value.""" app = web.Application() app.router.add_get("/", mock_handler) - async_setup_forwarded(app, [ip_network("127.0.0.1")]) + async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) mock_api_client = await aiohttp_client(app) resp = await mock_api_client.get(