Reject non-loopback Host headers for loopback web servers#5289
Open
petrmarinec wants to merge 1 commit intogoogle:mainfrom
Open
Reject non-loopback Host headers for loopback web servers#5289petrmarinec wants to merge 1 commit intogoogle:mainfrom
petrmarinec wants to merge 1 commit intogoogle:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.