fix(egress): pin resolved public IP to close SSRF DNS-rebinding TOCTOU (S-1)#208
Merged
Conversation
…U (S-1) The webhook/alert delivery path validated the target URL and then let httpx re-resolve the same hostname to connect, so a low-TTL name that flips public->internal between the check and the connect slipped past the guard (audit S-1, P1). Resolve + validate once via resolve_public_ip() and pin the connection to that IP literal through _PinnedIPTransport, so httpx performs no second lookup. The original Host header and TLS SNI/certificate hostname are preserved via the sni_hostname request extension; allowlisted hosts stay connected by name (unpinned). Wired at both delivery sites. Test: test_pinned_transport_connects_to_validated_ip_not_rebound proves the socket target is the validated public IP with getaddrinfo consulted exactly once; resolve/allowlist paths covered too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DORA Metrics
|
The transport is only ever constructed with (hostname, pinned_ip), and typing **kwargs as `object` made the `super().__init__(**kwargs)` splat fail mypy against AsyncHTTPTransport's typed signature. Construct the base with no args. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
What
Closes the webhook/alert egress DNS-rebinding TOCTOU (audit S-1, P1). The delivery path validated the target URL and then let
httpxre-resolve the same hostname to open the socket, so a low-TTL name that flipspublic → internalbetween the check and the connect slipped past the guard and got fetched. Re-validating before each delivery only narrowed the window.How
egress_guard.resolve_public_ip()resolves + validates the host once and returns the approved public IP.pinned_transport()/_PinnedIPTransport(anhttpx.AsyncHTTPTransportsubclass) connects to that IP literal, so httpx performs no second name lookup — the rebinding window is closed by construction.Hostheader and TLS SNI / certificate hostname are preserved via thesni_hostnamerequest extension, so routing and cert verification still key on the hostname, not the IP.AGENTFLOW_EGRESS_ALLOWED_HOSTS) stay connected by name (unpinned).webhook_dispatcher.py,alerts/escalation.py.Tests
tests/unit/test_egress_guard.py::test_pinned_transport_connects_to_validated_ip_not_rebound— the socket target is the validated public IP withgetaddrinfoconsulted exactly once (a subsequent rebind flip is never reached); plus resolve / allowlist cases.ruff check+ruff format --checkclean. Integration URL contracts unchanged (the mock replacesAsyncClientwholesale and still sees the hostname URL).A live rebinding-harness run (real low-TTL DNS + TLS) belongs to the external pen-test; the mechanism is proven deterministically here.
🤖 Generated with Claude Code