fix: evict stale port-decision cache entry when an ephemeral port is reused - #215
Open
rsyuzyov wants to merge 1 commit into
Open
fix: evict stale port-decision cache entry when an ephemeral port is reused#215rsyuzyov wants to merge 1 commit into
rsyuzyov wants to merge 1 commit into
Conversation
…reused
The per-port decision fast-path caches a DIRECT verdict in
port_decided_bitmap/port_direct_bitmap and clears it on outbound FIN/RST.
That eviction trigger is incomplete:
* a server-initiated close arrives on the inbound branch, which never
touches the bitmaps;
* loopback and abortive teardowns are not seen by the filter at all.
So a DIRECT verdict can outlive the connection that produced it. Windows
hands the ephemeral port to another process later, and the first SYN of the
new connection hits the fast-path, is sent unproxied and never runs its own
rule - traffic that a PROXY rule covers leaks out directly. Entries also
accumulate over time, so the longer the process runs the more often a fresh
connection lands on a stale port.
The SYN path already evicts the PID cache for exactly this reason (port
recycling); evict the decision cache in the same place. Both IP stacks share
the bitmaps, so the IPv6 branch needs the same eviction - a verdict cached
for an IPv4 connection must not be inherited by an IPv6 one reusing the port
number.
Observed on Windows 11 with a per-process SOCKS5 rule: after a few hours of
uptime new connections of the ruled processes started going out with the
host WAN address instead of the proxy exit; verified with an instrumented
build that logged the fast-path hits and the cache lifecycle. With this
change: 63 h uptime, 1.4 M connections, zero leaks (exit IP checked every
5 min, 757 samples, no deviation).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Roman Syuzyov <rsyuzyov@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a long-uptime Windows routing defect where a cached per-source-port DIRECT fast-path verdict can outlive the TCP connection that produced it and be incorrectly inherited when Windows reuses the ephemeral port, causing traffic that should be proxied to leak direct.
Changes:
- Evicts
port_decided_bitmap/port_direct_bitmapentries on a fresh TCP SYN (Syn && !Ack) to treat SYN as the port “ownership boundary” (IPv4). - Adds the same SYN-based eviction to the IPv6 outbound TCP path to prevent cross-stack inheritance since the decision bitmaps are shared.
- Adds in-code commentary explaining why FIN/RST is not a reliable eviction trigger and why SYN-based eviction is required.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
|
Thanks, but the Windows version is currently under rewrite. The project is moving from Windivert to its own kernel driver. I am working on a trying to get kernel signing till then, Windows dev is on hold. Currently, the custom kernel version is on the driver branch - https://github.com/InterceptSuite/ProxyBridge/tree/Driver If you open you can contribute to the driver for now. |
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.
Fixes #206.
Problem
The fast path caches a per-source-port rule decision in
port_decided_bitmap/port_direct_bitmap. A cached DIRECT verdict is only evicted on an outbound FIN/RST, which is not a reliable end-of-life signal:port_clear(sp)on FIN/RST the same iteration falls through tocheck_process_rule()and re-caches DIRECT viaport_set_direct(sp), so even an observed teardown does not necessarily leave the entry cleared (thanks to @dentatli for spotting this one).So a DIRECT verdict outlives the connection that produced it. When Windows recycles the ephemeral port to a different process, the new SYN hits the fast path, is sent unmodified, and that process's own rule never runs — traffic that should be proxied goes out direct. It accumulates with uptime: on my workstation the effect was invisible for the first hours and near-total after a day.
Fix
Treat a fresh SYN (
Syn && !Ack) as the ownership boundary and evict the cached decision there, right next to the existingremove_cached_pid()call that already handles exactly this scenario for the PID cache. Applied to both stacks, because IPv4 and IPv6 share the same decision bitmaps and would otherwise inherit each other's verdicts.Cost is one bitmap test plus one clear on new connections only; the steady-state fast path for established flows is untouched.
Verification
Built with MSVC 2022 + WinDivert 2.2.2-A, run on Windows 11 for ~63 hours without a restart:
dev— identical patch ID, clean build.Relation to #209
Complementary, not overlapping. #209 is about flow identity being keyed on
src_portalone inconnection_hash_table; this change only bounds how long a decision may live. Both are worth having, and neither depends on the other.AI-assisted code disclosure
Per CONTRIBUTING § AI-Generated Code: this patch was written with Claude Opus under my direction, instrumentation and testing. It is 2 hunks / ~15 lines, reuses the existing eviction point rather than adding new state or abstractions, and I have read and understood every line of it. Happy to iterate on placement or wording of the comments if you would prefer them shorter.
Checklist
git commit -s)