mirror of
https://github.com/home-assistant/core.git
synced 2026-02-15 07:36:16 +00:00
Fix incorrect forward header handling (#154793)
This commit is contained in:
@@ -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: <host>`
|
||||
e.g., `X-Forwarded-Host: example.com`
|
||||
`X-Forwarded-Host: <host1>, <host2>, <host3>`
|
||||
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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user