Reject non-loopback Host headers for loopback web servers#5289
Reject non-loopback Host headers for loopback web servers#5289petrmarinec wants to merge 2 commits into
Conversation
|
Hi @petrmarinec , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix formatting errors by running autoformat.sh and fix the failing unit tests. |
|
Hi, quick follow-up here: the formatting/unit-test issues mentioned earlier have been addressed on the current PR head, and the PR checks are green now. Happy to provide any additional detail if that would help with review. |
|
Hi @xuanyang15 , can you please review this. |
|
Hi @petrmarinec Thanks for creating this PR! Could you please help rebase the PR to 2.0 (i.e. the latest main branch)? @wyf7107 Could you please help review after the rebase? |
a15646d to
dbfe472
Compare
Hi, rebased to the latest I also reran the targeted validation locally after the rebase:
Happy to provide anything else that would help with review. |
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
When ADK is bound to loopback, such as the default
127.0.0.1, the web server currently relies on an origin check that derives the request origin from the incomingHostheader. A DNS-rebound request can use the same non-loopback hostname in bothHostandOrigin, causing the request to be treated as same-origin.Solution:
This change adds a loopback Host check for loopback-bound ADK servers. When enabled, requests must use a loopback
Hostheader such aslocalhost,127.0.0.1, or::1; non-loopback hostnames are rejected before the existing origin check runs. The check is enabled only when the server is configured to bind to a loopback host, preserving behavior for explicitly non-loopback binds such as0.0.0.0.Testing Plan
Unit Tests:
Passed pytest results:
PYTHONPATH=src python -m pytest tests/unittests/cli/test_fast_api.py -k "builder_save_rejects_cross_origin_post or builder_save_rejects_dns_rebound_host or builder_save_allows_same_origin_post or builder_get_allows_cross_origin_get or independent_telemetry_context" -vvon Windows:5 passeddocker run --rm -v "${PWD}:/workspace" -w /workspace python:3.11-bookworm bash -lc "python -m pip install --upgrade pip >/tmp/pip-upgrade.log && python -m pip install -e '.[test]' >/tmp/pip-install.log && PYTHONPATH=src python -m pytest tests/unittests/cli/test_fast_api.py -vv":63 passedpython -m pyink --check src/google/adk/cli/adk_web_server.py src/google/adk/cli/fast_api.py tests/unittests/cli/test_fast_api.py: unchangedManual End-to-End (E2E) Tests:
Manual validation was performed with an in-process FastAPI test client:
origin/main: a request withHost: rebind.attacker.example:8000and matchingOrigin: http://rebind.attacker.example:8000returned200 true403 Forbidden: host not allowedHost: rebind.attacker.example:8000without anOriginheader also returned403 Forbidden: host not allowedHost: localhostreturned200for the same local test client setupChecklist
Additional context
The full repository unit test suite was not run locally. The relevant CLI web server unit test file passes in Linux Docker.