From 397c1ac30ae5e858812f45c14c7ee6316e37a2cb Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 28 Jul 2026 21:09:33 +0200 Subject: [PATCH 1/3] feat(core): put the first packet of a fresh UDP flow in targeting scope decide() step 1 asked the live socket map only when is_syn, and is_syn is set for TCP alone. UDP has no SYN, so it was never asked: a fresh flow was judged against a port set the resolver had not rebuilt yet. For a long-lived flow that costs one packet; for DNS over UDP and QUIC, which take a fresh ephemeral port per exchange, it costs every one of them. The fix is `(is_syn or not is_tcp)` - a TCP SYN, or anything that is not TCP. No new parameter on decide(). ProcessTargeting.syn_covers becomes owner_targeted, because the old name became a lie about when it runs. The SHAPE was chosen by measurement, and the measurement reversed the first recommendation. Each variant as a real byte patch of core.py, fresh subprocess, three traffic mixes x three map sizes, median of 5 (ns per decide(), all misses): 400/tcp-bulk today 799 every-miss 997 syn-or-not-tcp 788 10000/tcp-bulk today 796 every-miss 1056 syn-or-not-tcp 790 100000/tcp-bulk today 834 every-miss 1052 syn-or-not-tcp 833 100000/udp-heavy today 819 every-miss 1028 syn-or-not-tcp 1017 Asking on every miss costs +209..+266 ns/packet on TCP-heavy traffic, ~26% of decide(); this form is free there. Map size barely matters (400 -> 100k ports is ~35 ns), so it is about how often each variant asks, not the lookup. Price, named rather than implied: covering UDP widens the recycled-PID false positive from one SYN per connection to every UDP datagram of such a socket until the next rebuild. Ordinary TCP data is still never asked. - test_core.py::test_a_fresh_udp_flow_of_a_targeted_process_is_in_scope pins both halves, so nobody simplifies it into the expensive form - test_targeting_socketwatch.py::test_owner_targeted_says_no_for_portless_traffic guards the `port is None` line, which this change put on a live path for the first time: ICMP is not TCP, so every ping now calls owner_targeted(None) - four mutants, all caught - after the run showed the portless case was guarded by nothing, because the core test drives a fake with its own implementation Co-Authored-By: Claude Opus 5 --- CHANGELOG-INTERNAL.md | 42 +++++++++++++++++++++ CHANGELOG.md | 11 ++++++ README.md | 5 ++- README.pl.md | 3 +- beantester/core.py | 35 ++++++++++++----- beantester/targeting.py | 43 ++++++++++++++------- tests/test_core.py | 58 +++++++++++++++++++++++++++-- tests/test_targeting_socketwatch.py | 46 ++++++++++++++++++----- 8 files changed, 205 insertions(+), 38 deletions(-) diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index dd2e441..0c29aa2 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -97,6 +97,48 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre - Help text and the flag tables in both READMEs now state that the flag is valid on its own. - Version bump deliberately NOT taken (convention 34): the owner closes it in `VERSION.txt`. +### Fixed: the first packet of a fresh UDP flow is in targeting scope (handoff point 3) + +- **The gap.** `decide()` step 1 asked `syn_covers` only when `is_syn`, and `is_syn` is set for TCP + alone (`engine.py`). UDP has no SYN, so it was never asked: a fresh flow was judged against + `_ports`, which the resolver had not rebuilt yet. For a long-lived flow that costs one packet; + for **DNS over UDP and QUIC, which take a fresh ephemeral port per exchange, it costs all of them**. +- **The fix is `(is_syn or not is_tcp)`** - a TCP SYN, or anything that is not TCP. No new parameter + on `decide()`; ICMP lands here too and exits on `port is None` inside the callback. + `ProcessTargeting.syn_covers` is renamed **`owner_targeted`**, because the old name became a lie + about when it runs (`core._syn_covers` -> `_owner_targeted`). +- **The shape was chosen by MEASUREMENT, and the measurement reversed the first recommendation.** + Variant (a) "ask on every miss" was proposed on the strength of a microbenchmark of one call + (185 ns) plus reasoning that the difference was negligible. Measured properly - each variant as a + real byte patch of `core.py`, fresh subprocess, three traffic mixes x three map sizes, median + of 5: + + | map / mix | today | (a) every miss | (b) syn-or-not-tcp | (a) - (b) | + |---|---|---|---|---| + | 400 / tcp-bulk | 799 | 997 | 788 | **+209** | + | 10 000 / tcp-bulk | 796 | 1056 | 790 | **+266** | + | 100 000 / tcp-bulk | 834 | 1052 | 833 | **+219** | + | 100 000 / mixed | 865 | 1030 | 934 | +96 | + | 100 000 / udp-heavy | 819 | 1028 | 1017 | +11 | + + (ns per `decide()`, every packet missing.) (b) is **free** on TCP-heavy traffic - within noise of + today - while (a) costs ~26% of `decide()`. **Map SIZE barely matters** (400 -> 100 000 ports is + ~35 ns), so this is about how often each variant asks, not about the lookup. The lesson went into + PROJECT_NOTES rule 5: measuring a COMPONENT is not measuring the DIFFERENCE BETWEEN VARIANTS. +- **The price, named rather than implied:** covering UDP widens the recycled-PID false positive + from one SYN per connection to every UDP datagram of such a socket until the next rebuild (<=0.30 s). + Ordinary TCP data is still never asked, so an established connection cannot be dragged in. +- Two new guards. `test_core.py::test_a_fresh_udp_flow_of_a_targeted_process_is_in_scope` pins both + halves - UDP covered, ordinary TCP data NOT asked - so nobody "simplifies" it into the expensive + form. `test_targeting_socketwatch.py::test_owner_targeted_says_no_for_portless_traffic` guards the + `port is None` line, which **this change put on a live path for the first time**: ICMP is not TCP, + so every ping now calls `owner_targeted(None)` on the capture thread. +- **Four mutants, all caught - after the run exposed a real hole.** Mutating the REAL + `owner_targeted` to ignore the portless case was caught by NOTHING at first: the core test drives + a `_FakePorts` double with its own implementation, so it never touched the code under test. That + is the same trap F16 recorded, hit again from a different direction; the portless guard above + exists because of it, and the core test now says which half it actually pins. + ### ADR 2026-07-28: batched WinDivert I/O MEASURED and REJECTED (audit F18, part c) - **The question.** The engine does one `recv()` and one `send()` syscall per packet, and WinDivert diff --git a/CHANGELOG.md b/CHANGELOG.md index e430131..cab1e6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,17 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ### Fixed +- **Aiming at a process did not touch the first packet of a UDP exchange - and for DNS and QUIC + that meant it did not touch them at all.** Working out which connections belong to your target + means keeping a set of its ports, rebuilt in the background, and a brand-new port is not in it + yet. TCP had a way around this: a connection starts with a SYN, so one packet per connection was + checked against the live socket map. UDP has no SYN, so nothing was checked - and the traffic + where it matters takes a **fresh port every time**: every DNS query, every QUIC connection. Those + went past your target untouched, every one of them. They no longer do. If you have tested a game, + a video call or a browser over QUIC and the impairment seemed weaker than you configured, this is + why. Ordinary TCP data is deliberately still not re-checked - it does not need to be, and + measuring showed it would cost real throughput for nothing. + - **The "the driver held a packet" warning told you about lost accuracy when what you were losing was traffic.** It said the wait lands on the delay you are measuring - true, and not the half that matters. Measured here on a deliberately overloaded run: with 138,000 packets a second diff --git a/README.md b/README.md index a10dfea..637df30 100644 --- a/README.md +++ b/README.md @@ -968,8 +968,9 @@ seeded). explicit exclusion wins: `chrome, !chromedriver` will not pull in `chromedriver` via a parent. - **Process targeting is driven by socket events, not a slow poll.** WinDivert hands us a packet, not a PID, so we map a packet to its process by **local port**. On Windows the tool watches the - system's socket events (connect / bind / accept / close) as they happen, so a new connection of a - program that is **already** in scope is impaired from its very first packet. The exception is the + system's socket events (connect / bind / accept / close) as they happen, so a new connection **or + UDP flow** of a program that is **already** in scope is impaired from its very first packet - that + covers DNS queries and QUIC, each of which takes a fresh port. The exception is the program's *first* connection: until it owns at least one socket there is nothing to recognise it by, so that one connection goes through untouched. Without real WinDivert (the test / simulation path) it falls back to scanning the socket table a few times a second, where the first packet of diff --git a/README.pl.md b/README.pl.md index 6e34a94..2e5e5a0 100644 --- a/README.pl.md +++ b/README.pl.md @@ -846,7 +846,8 @@ wyznaczonym momencie. Wszystkie losowania idą przez jeden generator (opcjonalni - **Celowanie w proces napędzają zdarzenia gniazd, nie wolny polling.** WinDivert daje pakiet, nie PID, więc pakiet mapujemy na proces po **lokalnym porcie**. Na Windows narzędzie obserwuje zdarzenia gniazd systemu (connect / bind / accept / close) na bieżąco, więc nowe połączenie - programu, który **już** jest w zasięgu, jest psute od pierwszego pakietu. Wyjątkiem jest + **albo przepływ UDP** programu, który **już** jest w zasięgu, jest psute od pierwszego pakietu - + obejmuje to zapytania DNS i QUIC, z których każde bierze świeży port. Wyjątkiem jest *pierwsze* połączenie tego programu: dopóki nie ma ani jednego gniazda, nie ma po czym go poznać, więc to jedno połączenie przechodzi nietknięte. Bez realnego WinDivert (ścieżka testów / symulacji) narzędzie wraca do skanowania tabeli gniazd kilka razy na sekundę, gdzie pierwszy diff --git a/beantester/core.py b/beantester/core.py index 7d53380..b83bde9 100644 --- a/beantester/core.py +++ b/beantester/core.py @@ -214,7 +214,7 @@ def __init__(self): # None whenever the port container cannot answer for a fresh socket - a # plain set (tests, one-shot resolution), which is the behaviour that # existed before this check did. - self._syn_covers = None + self._owner_targeted = None self.dst_active = False self.dst_ip = "" # raw expression text (for summaries/reports) self.dst_port = "" # raw expression text @@ -304,7 +304,7 @@ def set_target(self, active, ports=None): self.target_ports = set(ports) else: self.target_ports = ports - self._syn_covers = getattr(self.target_ports, "syn_covers", None) + self._owner_targeted = getattr(self.target_ports, "owner_targeted", None) def set_dest(self, active, ip=None, port=None): """Destination targeting. ``ip``/``port`` are filter expressions (see @@ -497,14 +497,29 @@ def decide(self, size, is_outbound, local_port, now, rng, if self.target_active and local_port not in self.target_ports: # The port set is rebuilt on another thread, so a socket opened # microseconds ago is unknown HERE even when the live SOCKET map - # already knows its owner - and the first packet of every fresh - # connection was therefore judged out of scope. Measured end to - # end: 20 of 20 SYNs walked past a process target. For a SYN, and - # only a SYN (once per connection, not once per packet), ask that - # map. `not in` above has already flagged the miss and woken the - # resolver, so the rest of the connection takes the usual path. - if not (is_syn and self._syn_covers is not None - and self._syn_covers(local_port)): + # already knows its owner - and the first packet of a fresh flow + # was therefore judged out of scope. Measured end to end: 20 of 20 + # SYNs walked past a process target. So ask the live map - but not + # for every packet: `not in` above has already flagged the miss and + # woken the resolver, so an established flow takes the usual path. + # + # A TCP SYN (once per connection) and anything that is NOT TCP. + # UDP is the reason for the second half: it has no SYN, so it was + # not covered at all, and a fresh ephemeral port per datagram - DNS, + # QUIC - meant every one of them escaped. Portless traffic (ICMP) + # also lands here and exits on `port is None` inside the callback. + # + # Ordinary TCP data is deliberately NOT asked, and that is a + # MEASURED choice, not a guess: asking on every miss costs +209 to + # +266 ns per packet on a TCP-heavy mix (~26% of this function), + # while this form is free there - identical to not asking at all, + # within noise. It also keeps the recycled-PID exposure where it + # already was instead of widening it to every packet (see + # ``targeting.owner_targeted``). What it gives up is a TCP flow + # whose SYN predates the target being set; the resolver adopts that + # socket by itself at the next rebuild, within 0.30 s. + if not ((is_syn or not is_tcp) and self._owner_targeted is not None + and self._owner_targeted(local_port)): return Decision(False, False, [now], scoped=False) # 2) destination targeting (remote IP/port) - filter expressions if self.dst_active: diff --git a/beantester/targeting.py b/beantester/targeting.py index c6f8094..b1d0687 100644 --- a/beantester/targeting.py +++ b/beantester/targeting.py @@ -150,7 +150,7 @@ def set_table(self, table): back at the polling :class:`~beantester.portmap.PortTable` otherwise (no real WinDivert, or the SOCKET handle could not open). Both expose the same read surface (``snapshot`` / ``name_of`` / ``ancestors`` / ``refresh``, plus - ``pid_for`` since ``syn_covers`` exists - both real tables have always had + ``pid_for`` since ``owner_targeted`` exists - both real tables have always had it, but it is part of the contract now), which is why the swap is a one-line reference change. The resolved port set is left as it is until the next ``refresh()`` (the resolver runs those continuously), so @@ -159,20 +159,31 @@ def set_table(self, table): with self._lock: self.table = table if table is not None else portmap.default_table() - def syn_covers(self, port): + def owner_targeted(self, port): """Is this port a brand-new socket of a process we ALREADY target? + Named ``syn_covers`` until UDP was covered too, which made the old name a + lie about when it runs - see ``BeanCore.decide`` step 1 for the callers. + ``__contains__`` answers from ``_ports``, a frozenset rebuilt on another - thread, so the first packet of a fresh connection is judged BEFORE any - rebuild it triggers - it was never in scope, however early the SOCKET - event arrived. MEASURED end to end 2026-07-28: 20 fresh connections - against a process target with ``--syn-drop 100``, 20 SYNs straight - through, ``drop_syn`` 0. The live map knew each owner 0.02 ms before its - SYN; nothing consumed that. + thread, so the first packet of a fresh flow is judged BEFORE any rebuild + it triggers - it was never in scope, however early the SOCKET event + arrived. MEASURED end to end 2026-07-28: 20 fresh connections against a + process target with ``--syn-drop 100``, 20 SYNs straight through, + ``drop_syn`` 0. The live map knew each owner 0.02 ms before its SYN; + nothing consumed that. This is the one place that does. ``BeanCore.decide`` calls it for a TCP - SYN only - once per connection, not once per packet - and it costs a - lock-free dict read plus a frozenset lookup, no lock of its own. + SYN (once per connection) and for anything that is not TCP, and it costs + a lock-free dict read plus a frozenset lookup, no lock of its own. + + **Why not on every miss.** Asking for ordinary TCP data as well was + measured on the real code across three traffic mixes and three map sizes + (400 / 10 000 / 100 000 ports, median of 5): it costs **+209 to +266 ns + per packet** on a TCP-heavy mix, about 26% of ``decide()``, while the form + above is free there - within noise of not asking at all. The map's SIZE + barely matters (400 -> 100 000 ports moved ``decide()`` by ~35 ns), so + this is about how OFTEN each variant asks, not about the lookup. ``_pids`` is what the last rebuild concluded, so every expression form (name, PID, list, range, ``!`` exclusion, ancestor match) is already @@ -211,10 +222,16 @@ def syn_covers(self, port): that closes it holds the SYN until the answer is in, i.e. adds delay inside a tool whose job is to inject a PRECISE amount of it. * ``_pids`` is up to one resolver cycle stale (0.30 s). If the target - exits and Windows recycles its PID inside that window, one packet of an - unrelated new socket can be pulled into scope. That is a DIFFERENT false + exits and Windows recycles its PID inside that window, traffic of an + unrelated new socket can be pulled into scope. **Covering UDP widened + this, and that is the price of it:** it used to be one SYN per + connection, and it is now every UDP datagram of such a socket until the + next rebuild. Ordinary TCP data is still never asked, so an established + TCP connection cannot be dragged in this way. That is a DIFFERENT false positive from the stale ``_ports`` this code already lived with - named - here rather than implied. + here rather than implied. The bound is the rebuild interval and nothing + else: verifying identity means ``create_time()`` in the packet path, + which convention 20 forbids. """ if port is None: return False diff --git a/tests/test_core.py b/tests/test_core.py index 64d5daa..80cfc73 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -290,7 +290,7 @@ def __init__(self, ports=(), owners=None, pids=(), with_pid_for=True): self.owners = dict(owners or {}) self.pids = set(pids) self.misses = 0 - if not with_pid_for: # a table that predates syn_covers + if not with_pid_for: # a table that predates owner_targeted del self.__dict__["owners"] def __contains__(self, port): @@ -299,7 +299,7 @@ def __contains__(self, port): self.misses += 1 return False - def syn_covers(self, port): + def owner_targeted(self, port): owners = getattr(self, "owners", None) if owners is None: return False @@ -344,6 +344,58 @@ def test_a_syn_from_a_fresh_socket_of_a_targeted_process_is_in_scope(): f"(misses={ports.misses})") +def test_a_fresh_udp_flow_of_a_targeted_process_is_in_scope(): + """UDP has no SYN, so it was not covered at all - and that is where it hurts. + + A TCP connection has exactly one SYN, so before this at most one packet per + connection escaped and the next rebuild adopted the rest. UDP has no such + marker, and the traffic that matters takes a FRESH ephemeral port per + exchange: a DNS query, a QUIC connection. Every one of them was judged + against a port set that had never seen the port, so every one of them walked + past the target untouched. + + This also pins the SHAPE of the fix, which was chosen by measurement rather + than taste: ask for a SYN or for anything that is not TCP - never for + ordinary TCP data. Asking on every miss costs +209 to +266 ns per packet on a + TCP-heavy mix (~26% of ``decide()``, measured across three mixes and three map + sizes), while this form is free there, and it would widen the recycled-PID + window from one SYN to every packet. The last assertion here is what keeps + somebody from "simplifying" it into the expensive form. + """ + core = BeanCore() + rng = random.Random(0) + # 5000 is a brand-new UDP socket of pid 42 (the target); 6000 belongs to + # somebody else; 4000 is what the last rebuild already knew about. + ports = _FakePorts(ports={4000}, owners={5000: 42, 6000: 99, 7000: 42}, + pids={42}) + core.set_target(True, ports) + kw = dict(remote_ip="8.8.8.8", remote_port=53) + + fresh = core.decide(100, True, 5000, 0.0, rng, is_tcp=False, **kw) + check("a fresh UDP flow of the target is in scope", fresh.scoped is True, + f"(scoped={fresh.scoped})") + + stranger = core.decide(100, True, 6000, 0.0, rng, is_tcp=False, **kw) + check("a fresh UDP flow of ANOTHER process stays out", + stranger.scoped is False, f"(scoped={stranger.scoped})") + + # This one pins DECIDE's half only - that it hands a missing port through + # without crashing. The `port is None` guard inside the real + # ProcessTargeting is guarded separately, because _FakePorts has its own + # implementation and a mutation of the real one sails past here (checked). + # See test_targeting_socketwatch.py::test_owner_targeted_says_no_for_portless_traffic. + portless = core.decide(100, True, None, 0.0, rng, is_tcp=False, + remote_ip="8.8.8.8", remote_port=None) + check("portless traffic (ICMP) neither crashes nor lands in scope", + portless.scoped is False, f"(scoped={portless.scoped})") + + # The measured half of the decision: ordinary TCP data is NOT asked about. + tcp_data = core.decide(100, True, 7000, 0.0, rng, is_tcp=True, is_syn=False, + **kw) + check("ordinary TCP data on an unknown port is still not asked about", + tcp_data.scoped is False, f"(scoped={tcp_data.scoped})") + + def test_a_port_container_without_pid_for_does_not_break_the_packet_path(): """``set_table`` accepts anything with the read surface, and ``pid_for`` was not part of it until now. A container that cannot answer must fall back to the @@ -366,7 +418,7 @@ def test_a_plain_port_set_keeps_the_behaviour_it_always_had(): d = core.decide(100, True, 5000, 0.0, rng, remote_ip="1.1.1.1", remote_port=80, is_tcp=True, is_syn=True) check("plain set: a SYN on an unknown port is out of scope", d.scoped is False) - check("and the fast path was not even bound", core._syn_covers is None) + check("and the fast path was not even bound", core._owner_targeted is None) def test_a_reset_is_never_armed_from_a_syn(): diff --git a/tests/test_targeting_socketwatch.py b/tests/test_targeting_socketwatch.py index 3041c1f..d1eb8a8 100644 --- a/tests/test_targeting_socketwatch.py +++ b/tests/test_targeting_socketwatch.py @@ -142,13 +142,13 @@ def test_a_connection_is_targeted_the_moment_its_socket_event_arrives(): watcher.stop() -def test_syn_covers_reads_the_live_map_without_waiting_for_a_rebuild(): +def test_owner_targeted_reads_the_live_map_without_waiting_for_a_rebuild(): """`__contains__` answers from a set rebuilt on the resolver's thread, so the FIRST packet of a fresh connection is judged before any rebuild it triggers. Measured end to end 2026-07-28: 20 fresh connections against a process target with `--syn-drop 100` gave 20 established connections and `drop_syn` 0. - `syn_covers` is the one path that consults the live map instead, so it answers + `owner_targeted` is the one path that consults the live map instead, so it answers correctly with NO refresh having happened - which is what this asserts by never starting a resolver. """ @@ -163,21 +163,21 @@ def test_syn_covers_reads_the_live_map_without_waiting_for_a_rebuild(): # no resolver: the rebuilt port set is still empty, as it is for every # brand-new socket at the moment its SYN is judged check("the rebuilt set knows nothing yet", 5000 not in targeting) - check("...and syn_covers cannot help until a rebuild names the pid", - targeting.syn_covers(5000) is False) + check("...and owner_targeted cannot help until a rebuild names the pid", + targeting.owner_targeted(5000) is False) targeting.refresh() # what the resolver does on the miss above check("after the rebuild the fresh socket is covered", - targeting.syn_covers(5000) is True) + targeting.owner_targeted(5000) is True) check("another process's socket is not", - targeting.syn_covers(6000) is False) + targeting.owner_targeted(6000) is False) finally: watcher.stop() -def test_syn_covers_survives_a_table_that_cannot_answer(): +def test_owner_targeted_survives_a_table_that_cannot_answer(): """`set_table` takes anything with the read surface, and `pid_for` only became - part of that contract with `syn_covers`. This runs on the CAPTURE THREAD, so a + part of that contract with `owner_targeted`. This runs on the CAPTURE THREAD, so a table without it has to answer False - an AttributeError there would kill the capture thread and fail the session open (convention 20).""" class _NoPidFor: @@ -187,7 +187,35 @@ def snapshot(self): targeting = ProcessTargeting(bnt.parse_target("chrome"), table=_NoPidFor()) targeting._pids = frozenset({100}) check("a table without pid_for answers False instead of raising", - targeting.syn_covers(5000) is False) + targeting.owner_targeted(5000) is False) + + +def test_owner_targeted_says_no_for_portless_traffic(): + """ICMP reaches this now, and it did not before UDP was covered. + + Step 1 asks for a TCP SYN or for anything that is NOT TCP, and ICMP is not + TCP - so every ping packet calls ``owner_targeted(None)`` on the CAPTURE + THREAD. Previously only a TCP SYN could get here, so the ``port is None`` + guard sat on a path nothing ever took. + + It needs its own guard against the REAL class: mutating that line to + ``return True`` was caught by NOTHING (checked 2026-07-28), because the core + test drives a ``_FakePorts`` double with its own implementation. A portless + packet answering True would drag every ping on the machine into scope while a + process target is set - the exact false positive targeting exists to avoid. + """ + class _Table: + def snapshot(self): + return {5000: 100} + + def pid_for(self, port): + return {5000: 100}.get(port) + + targeting = ProcessTargeting(bnt.parse_target("chrome"), table=_Table()) + targeting._pids = frozenset({100}) + check("a port we have is still covered", targeting.owner_targeted(5000) is True) + check("portless traffic answers False, it does not fall through", + targeting.owner_targeted(None) is False) # -- engine binding ----------------------------------------------------------- # From 14277df46ef486a9a505ef8f5646a1360e36f8bd Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 28 Jul 2026 21:17:20 +0200 Subject: [PATCH 2/3] test(targeting): measure and bound the recycled-PID window The last "known edge, never reproduced" from the handoff. owner_targeted trusts the pid the live map reports without verifying identity (verifying is create_time() in the packet path, convention 20), so between a target exiting and the next rebuild a socket Windows hands that pid number is treated as the target's. The docstring claimed "up to one resolver cycle (0.30 s)" - a design statement with nothing behind it. Measured against the real socket table (no WinDivert, no admin: portmap reads iphlpapi and ProcessTargeting resolves against it). Seven rounds per transport, probe holding a real socket, killed, time until its pid leaves _pids: TCP median 309 ms (271-327) UDP median 315 ms (290-325) i.e. the 0.30 s routine tick and no more, and TIME_WAIT does NOT stretch it - worth checking rather than assuming, since a TCP socket can outlive its owner. Upper bound: with no traffic every rebuild is the routine tick, while a live session's misses shorten it toward the 0.05 s floor. Deliberately not done: chasing real Windows PID reuse. Hitting the same number inside a 0.30 s window needs a spawn storm at ~30-50 ms per process, and "we tried and it did not reproduce" is an expensive non-result. The window is the bound that matters and it is directly measurable. The new guard owns BOTH halves: the false positive is real, and it ENDS at the next rebuild. Adding identity verification later fails the first half, which is the intended way of being sent back to these docs. Three mutants, all caught: owner_targeted always False, _pids never forgetting a pid, and the rebuild re-adopting a pid on its stale cached name. Co-Authored-By: Claude Opus 5 --- CHANGELOG-INTERNAL.md | 31 ++++++++++++++ beantester/targeting.py | 32 ++++++++++----- tests/test_targeting_socketwatch.py | 63 +++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 11 deletions(-) diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index 0c29aa2..90beeda 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -97,6 +97,37 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre - Help text and the flag tables in both READMEs now state that the flag is valid on its own. - Version bump deliberately NOT taken (convention 34): the owner closes it in `VERSION.txt`. +### Tests: the recycled-PID window is measured and bounded, not just asserted (handoff point C) + +- The last "known edge, never reproduced" from the handoff. `owner_targeted` trusts the pid the live + map reports without verifying identity (verifying = `create_time()` in the packet path, convention + 20), so between a target exiting and the next rebuild a socket Windows hands that pid number is + treated as the target's. The docstring claimed "up to one resolver cycle (0.30 s)" - a design + statement with nothing behind it. +- **Measured against the REAL socket table** (no WinDivert and no admin needed: `portmap` reads + `iphlpapi` and `ProcessTargeting` resolves against it). Seven rounds per transport, probe holding + a real socket, killed, time until its pid leaves `_pids`: + + | transport | median | range | + |---|---|---| + | TCP | **309 ms** | 271-327 | + | UDP | **315 ms** | 290-325 | + + i.e. the 0.30 s routine tick and no more. **TIME_WAIT does not stretch it** - worth checking + rather than assuming, since a TCP socket can outlive its owner. Upper bound: with no traffic every + rebuild is the routine tick, while a live session's constant misses shorten it toward the 0.05 s + floor. +- **Deliberately NOT done: chasing real Windows PID reuse.** Hitting the same number inside a 0.30 s + window needs a spawn storm at ~30-50 ms per process - a dozen low-probability attempts, and + "we tried and it did not reproduce" is an expensive non-result. The window is the bound that + matters, and it is measurable directly. +- New guard `test_targeting_socketwatch.py::test_a_recycled_pid_is_in_scope_until_the_next_rebuild_and_no_longer` + owns BOTH halves: the false positive is real (or the docs promise a hazard that does not exist) + and it ENDS at the next rebuild (or the bound is fiction). Adding identity verification later + fails the first half, which is the intended way of being sent back to these docs. +- Three mutants, all caught: `owner_targeted` always False (no window at all), `_pids` never + forgetting a pid (window never closes), and the rebuild re-adopting a pid on its stale cached name. + ### Fixed: the first packet of a fresh UDP flow is in targeting scope (handoff point 3) - **The gap.** `decide()` step 1 asked `syn_covers` only when `is_syn`, and `is_syn` is set for TCP diff --git a/beantester/targeting.py b/beantester/targeting.py index b1d0687..a380326 100644 --- a/beantester/targeting.py +++ b/beantester/targeting.py @@ -221,17 +221,27 @@ def owner_targeted(self, port): watcher thread does not help - the window is the same. The only design that closes it holds the SYN until the answer is in, i.e. adds delay inside a tool whose job is to inject a PRECISE amount of it. - * ``_pids`` is up to one resolver cycle stale (0.30 s). If the target - exits and Windows recycles its PID inside that window, traffic of an - unrelated new socket can be pulled into scope. **Covering UDP widened - this, and that is the price of it:** it used to be one SYN per - connection, and it is now every UDP datagram of such a socket until the - next rebuild. Ordinary TCP data is still never asked, so an established - TCP connection cannot be dragged in this way. That is a DIFFERENT false - positive from the stale ``_ports`` this code already lived with - named - here rather than implied. The bound is the rebuild interval and nothing - else: verifying identity means ``create_time()`` in the packet path, - which convention 20 forbids. + * ``_pids`` goes stale when the target exits, and until the next rebuild a + socket Windows hands that pid number is treated as the target's. + **MEASURED against the real socket table 2026-07-28** (seven rounds per + transport, no traffic, so every rebuild is the routine tick and this is + the UPPER bound): a dead process leaves ``_pids`` after a median of + **309 ms** (TCP, 271-327) and **315 ms** (UDP, 290-325) - the 0.30 s + tick, and no more. TIME_WAIT does not stretch it, which was worth + checking rather than assuming, since a TCP socket can outlive its owner. + A live session misses constantly, which wakes the resolver and only + shortens this toward the 0.05 s floor. This used to be a design claim + ("up to one resolver cycle") with nothing behind it. + + **Covering UDP widened what fits in that window, and that is the price + of it:** it used to be one SYN per connection, and it is now every UDP + datagram of such a socket. Ordinary TCP data is still never asked, so an + established TCP connection cannot be dragged in this way. That is a + DIFFERENT false positive from the stale ``_ports`` this code already + lived with - named here rather than implied. Nothing narrows the bound + further: verifying identity means ``create_time()`` in the packet path, + which convention 20 forbids. Guarded both ways by + ``test_targeting_socketwatch.py::test_a_recycled_pid_is_in_scope_until_the_next_rebuild_and_no_longer``. """ if port is None: return False diff --git a/tests/test_targeting_socketwatch.py b/tests/test_targeting_socketwatch.py index d1eb8a8..fa526ae 100644 --- a/tests/test_targeting_socketwatch.py +++ b/tests/test_targeting_socketwatch.py @@ -218,6 +218,69 @@ def pid_for(self, port): targeting.owner_targeted(None) is False) +def test_a_recycled_pid_is_in_scope_until_the_next_rebuild_and_no_longer(): + """The recycled-PID window, pinned so its BOUND cannot drift unnoticed. + + ``owner_targeted`` trusts the pid the live map reports without verifying that + it is still the same process - verifying means ``create_time()`` in the packet + path, which convention 20 forbids. So between a target exiting and the next + rebuild, a socket Windows hands that pid number is, as far as targeting is + concerned, the target's. + + Until now that was a docstring claim and nothing more ("nieodtworzone" in the + handoff). MEASURED against the real socket table 2026-07-28, seven rounds per + transport: a dead process leaves ``_pids`` after a median of **309 ms** (TCP, + 271-327) and **315 ms** (UDP, 290-325) - the 0.30 s routine tick, with a live + session's constant misses only shortening it. TIME_WAIT does NOT stretch it, + which was worth checking rather than assuming, since a TCP socket can outlive + its owner. + + This test owns both halves: the false positive is REAL (or the docs promise a + hazard that does not exist), and it ENDS at the next rebuild (or the bound is + fiction). Add identity verification one day and the first half fails, which is + the intended way to be forced back to these docs. + """ + class _Table: + """Port 5000 belongs to chrome; after the swap, pid 100 is somebody else.""" + + def __init__(self): + self.ports = {5000: 100} + self.names = {100: "chrome.exe"} + + def refresh(self, now=None, force=False): + return True + + def snapshot(self): + return dict(self.ports) + + def name_of(self, pid, cheap=False): + return self.names.get(pid, "") + + def ancestors(self, pid, depth=8): + return [] + + def pid_for(self, port): + return self.ports.get(port) + + table = _Table() + targeting = ProcessTargeting(bnt.parse_target("chrome"), table=table) + targeting.refresh() + check("the target resolved", targeting.pids() == {100}, sorted(targeting.pids())) + + # chrome exits; Windows hands pid 100 to an unrelated process, which opens a + # socket of its own. Nothing has rebuilt yet - this is the window. + table.ports = {7000: 100} + table.names = {100: "innocent.exe"} + check("inside the window, a stranger's socket IS pulled into scope", + targeting.owner_targeted(7000) is True) + + targeting.refresh() # what the resolver does at its next tick + check("the rebuild names the pid and drops it", targeting.pids() == set(), + sorted(targeting.pids())) + check("after the rebuild the stranger is out of scope again", + targeting.owner_targeted(7000) is False) + + # -- engine binding ----------------------------------------------------------- # def test_engine_binds_targeting_to_the_watcher_when_present(): eng = BeanEngine() From b91ce677fbf7459685f7ba0f574def4442724a28 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 28 Jul 2026 21:36:04 +0200 Subject: [PATCH 3/3] docs(core): record the UDP acceptance run Master worktree against this tree, same machine, same probe: 8 DNS queries from FRESH sockets with the probe holding one socket open, so _pids cannot be the hidden variable. --target --dst-ip 8.8.8.8 --dst-port 53 --loss 100 --filter out, outbound only so the reply comes back untouched and the result stays binary. master 8 of 8 answered scoped_seen 0 of 1944 captured drop_loss 0 this tree 0 of 8 answered scoped_seen 8 drop_loss 8 The master column is the gap in its purest form: with a process target set, not one packet of 1944 was ever considered in scope. The branch column cross-checks exactly - 8 queries, 8 scoped, 8 dropped. Co-Authored-By: Claude Opus 5 --- CHANGELOG-INTERNAL.md | 14 ++++++++++++++ beantester/core.py | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/CHANGELOG-INTERNAL.md b/CHANGELOG-INTERNAL.md index 90beeda..2a3408d 100644 --- a/CHANGELOG-INTERNAL.md +++ b/CHANGELOG-INTERNAL.md @@ -156,6 +156,20 @@ a `### BREAKING` section placed FIRST in that version, and each such line is pre today - while (a) costs ~26% of `decide()`. **Map SIZE barely matters** (400 -> 100 000 ports is ~35 ns), so this is about how often each variant asks, not about the lookup. The lesson went into PROJECT_NOTES rule 5: measuring a COMPONENT is not measuring the DIFFERENCE BETWEEN VARIANTS. +- **Acceptance, master worktree against this tree on the same machine.** 8 DNS queries from FRESH + sockets (the realistic shape: a new ephemeral port each), the probe holding one socket open so + `_pids` cannot be the hidden variable, `--target --dst-ip 8.8.8.8 --dst-port 53 + --loss 100 --filter out` (outbound only, so the reply comes back untouched and the result stays + binary): + + | tree | queries answered | `scoped_seen` | `drop_loss` | + |---|---|---|---| + | master | **8 of 8** | **0** of 1944 captured | 0 | + | this branch | **0 of 8** | 8 | 8 | + + The master column is the finding in its purest form: with a process target set, **not one packet + of 1944 was ever considered in scope**. The branch column cross-checks exactly - 8 queries, + 8 scoped, 8 dropped. - **The price, named rather than implied:** covering UDP widens the recycled-PID false positive from one SYN per connection to every UDP datagram of such a socket until the next rebuild (<=0.30 s). Ordinary TCP data is still never asked, so an established connection cannot be dragged in. diff --git a/beantester/core.py b/beantester/core.py index b83bde9..234eb53 100644 --- a/beantester/core.py +++ b/beantester/core.py @@ -509,6 +509,15 @@ def decide(self, size, is_outbound, local_port, now, rng, # QUIC - meant every one of them escaped. Portless traffic (ICMP) # also lands here and exits on `port is None` inside the callback. # + # ACCEPTED end to end 2026-07-28, a master worktree against this + # tree on the same machine: 8 DNS queries from fresh sockets, the + # probe holding one socket open so `_pids` cannot be the variable, + # against `--target --dst-ip 8.8.8.8 --dst-port 53 + # --loss 100 --filter out`. Before: **8 of 8 replied**, with + # `scoped_seen` 0 of 1944 packets captured - with a process target + # set, NOTHING was ever in scope. After: **0 of 8 replied**, + # `scoped_seen` 8, `drop_loss` 8. + # # Ordinary TCP data is deliberately NOT asked, and that is a # MEASURED choice, not a guess: asking on every miss costs +209 to # +266 ns per packet on a TCP-heavy mix (~26% of this function),