Repository navigation
fix(client-ip): align client identity resolution with the reference - #86
Merged
Merged
Conversation
The forwarded chain follows the reference rules (ports and brackets stripped, empty entries kept, right to left walk when the depth over-counts hops, metacharacter and invalid entries rejected), the unix token is accepted by trusted_proxies, an untrusted peer sending X-Forwarded-For is reported as a spoof attempt, trusted proxy matching follows the reference raw-address rules, zoned addresses parse and render like ipaddress, and list matching compares canonical forms. Adds the generated reference expectation tables as a test runner.
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.
Closes #85
I aligned the client identity the engine resolves with the reference (
guard_core/_utils/ip_extraction.pyand_ip_in_list). I generated 104 extraction rows and 78 list-matching rows by running the reference functions directly (IPv4-mapped, bracketed and zoned addresses, ports, X-Forwarded-For chains with trusted proxies and depth, untrusted peers, theunixtoken, malformed entries) and replayed them against the port in the newbin/test_client_ip.php(tables intests/data/).What differed on master
26 extraction rows failed. The IPv4 blacklist case from the TypeScript report already worked here (the checks read the canonical identity), so the test pins it. The differences were in the resolver:
fe80::1%eth0) and a mapped zoned address rendered in hex instead of the reference form.trusted_proxieswas trusted, and a malformed CIDR reached before a match was skipped. The reference trusts neither.trusted_proxiesvalidation rejected theunixtoken, so a request with no peer address could not resolve through the chain. Entries now keep the reference spelling (no IPv4-mapped collapse).suspicious_request/spoofing_detectedevent (handlerip_extraction, canonical peer, redacted chain) and no spoof log, and none of the once-only chain warnings were logged. The route_config check now passes its logger and the event bus to the resolver, andEventBus::sendRequestHandlerEventis the referencesend_agent_eventshape.CanonicalIp::ipInListis the reference_ip_in_list. The block-hook payload fallback canonicalizes the raw peer like the reference.Tests
bin/test_client_ip.php(454 checks, added to the CI runners, the coverage suites and the Makefile) replays both tables, stores the engine identity for each row, and adds engine-level tests: an IPv4 blacklist entry, an IPv4 CIDR and a mapped entry block a client that connects as::ffff:127.0.0.1; a whitelist CIDR admits and denies mapped clients; a ban stored for the IPv4 form blocks the mapped form; the emergency whitelist admits a mapped client; rate-limit keys are shared by both spellings; a forwarded identity from a trusted proxy and theunixchain reach the checks; a spoofed header from an untrusted peer reaches no check and is reported once. On master the runner reports 26 failing extraction rows, theunixconfiguration is rejected and the spoof event is missing. One existing assertion inbin/test_state.phppinned the hex rendering of a zoned mapped address and now expects the reference form::ffff:1.2.3.4%eth0(checked on Python 3.10 to 3.14).Verification
PHP 8.5.11 on the host (CI runs 8.2 to 8.4):
php -loversrcandbin,composer validate --strict, every CI runner (with a privateredis-serveron a free port, never the shared 6379), the pcov 100% line gate, andcomposer audit --lockedall exit 0.bin/test_support.phpneedsmemory_limit=-1on this host (it exhausts the 128M default on master too). The coverage waivers move with identical code:SecurityConfig.php1162 and 1186 to 1171 and 1195,CanonicalIp.php56 and 108 to 154 and 248 plus 219 (the sameinet_pton-after-FILTER_VALIDATE_IPproof),ClientIpResolver.phpswaps the sixunixlines (now reachable and covered) for one dead trailing return (212, proof in the file). Waived lines go from 87 to 83.