diff --git a/homeassistant/components/http/forwarded.py b/homeassistant/components/http/forwarded.py index 134a31d1625..9d19ac3dcae 100644 --- a/homeassistant/components/http/forwarded.py +++ b/homeassistant/components/http/forwarded.py @@ -43,18 +43,22 @@ def async_setup_forwarded( 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: ` - e.g., `X-Forwarded-Host: example.com` + `X-Forwarded-Host: , , ` + e.g., `X-Forwarded-Host: example.com, proxy.example.com, backend.example.com` + OR `X-Forwarded-Host: example.com` (one entry, even with multiple proxies) If the previous headers are processed successfully, and the X-Forwarded-Host is - present, it will be used. + present, the last one in the list will be used (set by the proxy nearest to the backend). + + Multiple headers are valid as stated in https://www.rfc-editor.org/rfc/rfc7239#section-7.1 + If multiple headers are present, they are handled according to + https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Forwarded-For#parsing + > "split each X-Forwarded-For header by comma into lists and then join the lists." Additionally: - If no X-Forwarded-For header is found, the processing of all headers is skipped. - Throw HTTP 400 status when untrusted connected peer provides 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, @@ -111,15 +115,12 @@ def async_setup_forwarded( ) raise HTTPBadRequest - # Multiple X-Forwarded-For headers - if len(forwarded_for_headers) > 1: - _LOGGER.error( - "Too many headers for X-Forwarded-For: %s", forwarded_for_headers + # Process multiple X-Forwarded-For from the right side (by reversing the list) + forwarded_for_split = list( + reversed( + [addr for header in forwarded_for_headers for addr in header.split(",")] ) - raise HTTPBadRequest - - # Process X-Forwarded-For from the right side (by reversing the list) - forwarded_for_split = list(reversed(forwarded_for_headers[0].split(","))) + ) try: forwarded_for = [ip_address(addr.strip()) for addr in forwarded_for_split] except ValueError as err: @@ -148,14 +149,15 @@ def async_setup_forwarded( X_FORWARDED_PROTO, [] ) if forwarded_proto_headers: - if len(forwarded_proto_headers) > 1: - _LOGGER.error( - "Too many headers for X-Forward-Proto: %s", forwarded_proto_headers - ) - raise HTTPBadRequest - + # Process multiple X-Forwarded-Proto from the right side (by reversing the list) forwarded_proto_split = list( - reversed(forwarded_proto_headers[0].split(",")) + reversed( + [ + addr + for header in forwarded_proto_headers + for addr in header.split(",") + ] + ) ) forwarded_proto = [proto.strip() for proto in forwarded_proto_split] @@ -191,14 +193,16 @@ def async_setup_forwarded( # Handle X-Forwarded-Host forwarded_host_headers: list[str] = request.headers.getall(X_FORWARDED_HOST, []) if forwarded_host_headers: - # Multiple X-Forwarded-Host headers - if len(forwarded_host_headers) > 1: - _LOGGER.error( - "Too many headers for X-Forwarded-Host: %s", forwarded_host_headers + # Process multiple X-Forwarded-Host from the right side (by reversing the list) + forwarded_host = list( + reversed( + [ + addr.strip() + for header in forwarded_host_headers + for addr in header.split(",") + ] ) - raise HTTPBadRequest - - forwarded_host = forwarded_host_headers[0].strip() + )[0] if not forwarded_host: _LOGGER.error("Empty value received in X-Forward-Host header") raise HTTPBadRequest diff --git a/tests/components/http/test_forwarded.py b/tests/components/http/test_forwarded.py index ce9b8198377..b003730918d 100644 --- a/tests/components/http/test_forwarded.py +++ b/tests/components/http/test_forwarded.py @@ -90,6 +90,56 @@ async def test_x_forwarded_for_with_trusted_proxy( assert resp.status == HTTPStatus.OK +@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"], ["123.123.123.123", "2.2.2.2", "1.1.1.1"], "1.1.1.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", + ), + ], +) +async def test_x_multiple_forwarded_for_with_trusted_proxy( + trusted_proxies, x_forwarded_for, remote, aiohttp_client: ClientSessionGenerator +) -> None: + """Test that we get the IP from multiple forwarded for headers.""" + + 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 == remote + + return web.Response() + + app = web.Application() + app.router.add_get("/", 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, addr) for addr in x_forwarded_for] + ) + + assert resp.status == HTTPStatus.OK + + async def test_x_forwarded_for_disabled_with_proxy( aiohttp_client: ClientSessionGenerator, caplog: pytest.LogCaptureFixture ) -> None: @@ -176,28 +226,6 @@ async def test_x_forwarded_for_with_malformed_header( assert "Invalid IP address in X-Forwarded-For" in caplog.text -async def test_x_forwarded_for_with_multiple_headers( - aiohttp_client: ClientSessionGenerator, caplog: pytest.LogCaptureFixture -) -> None: - """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, True, [ip_network("127.0.0.1")]) - - mock_api_client = await aiohttp_client(app) - - resp = await mock_api_client.get( - "/", - headers=[ - (X_FORWARDED_FOR, "222.222.222.222"), - (X_FORWARDED_FOR, "123.123.123.123"), - ], - ) - - assert resp.status == HTTPStatus.BAD_REQUEST - assert "Too many headers for X-Forwarded-For" in caplog.text - - @pytest.mark.parametrize( ("x_forwarded_for", "remote", "x_forwarded_proto", "secure"), [ @@ -258,6 +286,65 @@ async def test_x_forwarded_proto_with_trusted_proxy( assert resp.status == HTTPStatus.OK +@pytest.mark.parametrize( + ("x_forwarded_for", "remote", "x_forwarded_proto", "secure"), + [ + ( + "10.10.10.10, 127.0.0.1, 127.0.0.2", + "10.10.10.10", + ["https", "http", "http"], + True, + ), + ( + "10.10.10.10, 127.0.0.1, 127.0.0.2", + "10.10.10.10", + ["http", "https", "https"], + False, + ), + ( + "255.255.255.255, 10.10.10.10, 127.0.0.1", + "10.10.10.10", + ["http", "https", "http"], + True, + ), + ( + "255.255.255.255, 10.10.10.10, 127.0.0.1", + "10.10.10.10", + ["https", "http", "https"], + False, + ), + ], +) +async def test_x_multiple_forwarded_proto_with_trusted_proxy( + x_forwarded_for, + remote, + x_forwarded_proto, + secure, + aiohttp_client: ClientSessionGenerator, +) -> None: + """Test that we get the proto header if proxy is trusted.""" + + async def handler(request): + assert request.remote == remote + assert request.scheme == ("https" if secure else "http") + assert request.secure == secure + + return web.Response() + + app = web.Application() + app.router.add_get("/", handler) + 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( + "/", + headers=[(X_FORWARDED_FOR, x_forwarded_for)] + + [(X_FORWARDED_PROTO, proto) for proto in x_forwarded_proto], + ) + + assert resp.status == HTTPStatus.OK + + async def test_x_forwarded_proto_with_trusted_proxy_multiple_for( aiohttp_client: ClientSessionGenerator, ) -> None: @@ -288,6 +375,38 @@ async def test_x_forwarded_proto_with_trusted_proxy_multiple_for( assert resp.status == HTTPStatus.OK +async def test_x_forwarded_proto_with_trusted_proxy_multiple_for_2( + aiohttp_client: ClientSessionGenerator, +) -> None: + """Test that we get the proto with 1 element in the proto, multiple in the for.""" + + async def handler(request): + url = mock_api_client.make_url("/") + assert request.host == f"{url.host}:{url.port}" + assert request.scheme == "https" + assert request.secure + assert request.remote == "255.255.255.255" + + return web.Response() + + app = web.Application() + app.router.add_get("/", handler) + 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( + "/", + headers=[ + (X_FORWARDED_FOR, "255.255.255.255"), + (X_FORWARDED_FOR, "127.0.0.1"), + (X_FORWARDED_FOR, "127.0.0.2"), + (X_FORWARDED_PROTO, "https"), + ], + ) + + assert resp.status == HTTPStatus.OK + + async def test_x_forwarded_proto_not_processed_without_for( aiohttp_client: ClientSessionGenerator, ) -> None: @@ -312,28 +431,6 @@ async def test_x_forwarded_proto_not_processed_without_for( assert resp.status == HTTPStatus.OK -async def test_x_forwarded_proto_with_multiple_headers( - aiohttp_client: ClientSessionGenerator, caplog: pytest.LogCaptureFixture -) -> None: - """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, True, [ip_network("127.0.0.1")]) - - mock_api_client = await aiohttp_client(app) - resp = await mock_api_client.get( - "/", - headers=[ - (X_FORWARDED_FOR, "222.222.222.222"), - (X_FORWARDED_PROTO, "https"), - (X_FORWARDED_PROTO, "http"), - ], - ) - - assert resp.status == HTTPStatus.BAD_REQUEST - assert "Too many headers for X-Forward-Proto" in caplog.text - - @pytest.mark.parametrize( "x_forwarded_proto", ["", ",", "https, , https", "https, https, "], @@ -447,7 +544,7 @@ async def test_x_forwarded_host_not_processed_without_for( async def test_x_forwarded_host_with_multiple_headers( aiohttp_client: ClientSessionGenerator, caplog: pytest.LogCaptureFixture ) -> None: - """Test that we get a HTTP 400 bad request with multiple headers.""" + """Test that we get a HTTP 200 OK with multiple headers.""" app = web.Application() app.router.add_get("/", mock_handler) async_setup_forwarded(app, True, [ip_network("127.0.0.1")]) @@ -462,8 +559,7 @@ async def test_x_forwarded_host_with_multiple_headers( ], ) - assert resp.status == HTTPStatus.BAD_REQUEST - assert "Too many headers for X-Forwarded-Host" in caplog.text + assert resp.status == HTTPStatus.OK async def test_x_forwarded_host_with_empty_header(