From 8288714853952aca6b3511268b9d772f1b7f489f Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Fri, 4 Sep 2026 13:44:12 +0200 Subject: fix(node): make ice_interfaces match adapters on Windows (W9) `ice_interfaces` compared the operator's entry against ifaddr's `adapter.name` only -- the kernel name on Linux (`wlp3s0f0`), but the adapter GUID on Windows (`{846EE342-...}`). A setting written on Linux, or copied into a Windows node's node.toml, matched no adapter at all. The failure was silent and total rather than partial: aioice binds one socket per host address, so an empty list means no sockets, no host candidates, and an SDP offering only a reflexive address. The settings field is free text with no picker, and on Windows the operator sees neither the GUID nor the description -- `ipconfig` shows the connection name -- so an entry now matches the adapter name, the device description, or one of the adapter's own IPv4 addresses, case-insensitively. An address is the one identifier visible on every platform. A filter that matches nothing now falls back to the unfiltered list with a warning. Losing the 5 s timeout saving is a regression; being silently unconnectable is a defect. Also fixes IPv4/IPv6 discrimination in the same loop: the two were told apart by falling through to an `elif` that index-probed `ip.ip[0]` and `ip.ip[2]`, which on an IPv4 str yields characters that compared unequal by luck rather than by design. Now discriminated by isinstance. WINDOWS-PORT.md claimed Transport had "no platform dependency"; it does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DtfG7z6wHWj8RKHCvxQtY1 --- docs/WINDOWS-PORT.md | 28 +++- packages/meshbay-node/src/meshbay_node/config.py | 8 +- .../src/meshbay_node/transport/ice_filter.py | 117 ++++++++++++--- packages/meshbay-node/tests/test_ice_filter.py | 163 +++++++++++++++++++++ 4 files changed, 295 insertions(+), 21 deletions(-) create mode 100644 packages/meshbay-node/tests/test_ice_filter.py diff --git a/docs/WINDOWS-PORT.md b/docs/WINDOWS-PORT.md index 239dbd1..ceba2b8 100644 --- a/docs/WINDOWS-PORT.md +++ b/docs/WINDOWS-PORT.md @@ -27,7 +27,7 @@ it matters most: | `meshbay_common/` (all other modules) | **Ready** | Crypto, protocol, keyderive — no OS dependency | | `roots.py` | **Ready** | Uses `fold()`, `portable_name_problem()`, `as_posix()` for virtual paths, Windows drive examples in docstrings | | `indexer/indexer.py` | **Ready** | `as_posix()` for index paths, `long_path()` for file I/O, reconciliation loop for dropped `ReadDirectoryChangesW` events | -| Transport (WebRTC, QUIC, MNP) | **Ready** | asyncio + aiortc, no platform dependency | +| Transport (WebRTC, QUIC, MNP) | **Ready** | asyncio + aiortc. One platform dependency: `ice_filter.py` matches adapters by `ifaddr` name, which is a kernel name on Linux and a GUID on Windows — see W9 | | Roster, bundle store, audit, chat store | **Ready** | SQLite + pathlib throughout | | `test_paths.py` | **Ready** | Case folding, NFC, reserved names, reserved characters, trailing dot/space — already covers Windows filesystem rules | | Electron `preload.js` | **Ready** | Pure IPC bridge, no platform code | @@ -394,6 +394,30 @@ when `sys.platform == "win32"`. **Estimated scope:** ~20 lines, cosmetic. +### 5.8 ICE interface naming (W9) + +**Scope:** `transport/ice_filter.py` + +`ice_interfaces` restricts ICE gathering to named adapters. The filter compared +the operator's entry against `ifaddr`'s `adapter.name` only — the kernel name +on Linux (`wlp3s0f0`), but the adapter **GUID** on Windows +(`{846EE342-7039-11DE-9D20-806E6F6E6963}`). A setting written on Linux, or +copied into a Windows guest's `node.toml`, therefore matched no adapter at all. + +The failure was silent and total rather than partial: aioice binds one socket +per host address, so an empty list means no sockets, no host candidates, and an +SDP offering only a server-reflexive address. Behind two NATs — a VM on a +libvirt NAT inside a LAN — that address is unreachable for a peer on the +intermediate LAN, so ICE never completes and the symptom reads as a network +fault, not a config one. + +An entry now matches the adapter name, the device description +(`adapter.nice_name`, which is what Windows populates), or one of the adapter's +own IPv4 addresses — the settings field is free text with no picker, so +operators write whichever identifier they can see. A filter that matches +nothing falls back to the unfiltered list with a warning: losing the 5 s +timeout saving is a regression, being unconnectable is a defect. + --- ## 6. Execution order @@ -405,6 +429,7 @@ W5 File permissions ✅ done W7 CLI messages ✅ done W6 ffmpeg discovery ✅ done W8 test suite green on win32 ✅ done (encoding sweep + skipif; 784 pass) +W9 ICE interface naming ✅ done (name/description/IP match + fail-open) ──── milestone: daemon runs on Windows ✅ (verified end to end) ──── W3 Daemon lifecycle ✅ done — Startup-folder .vbs (Task Scheduler needs admin) ──── milestone: daemon starts/stops on Windows ✅ ──── @@ -417,6 +442,7 @@ W4 Packaging ✅ built — one per-user NSIS installer, client ``` W1–W2–W5–W6–W7–W8 shipped as a run of mechanical commits, testable on Linux. +W9 came out of a live guest that could not connect at all. W3 and W4 were the real work. --- diff --git a/packages/meshbay-node/src/meshbay_node/config.py b/packages/meshbay-node/src/meshbay_node/config.py index 7be68a4..a5220cd 100644 --- a/packages/meshbay-node/src/meshbay_node/config.py +++ b/packages/meshbay-node/src/meshbay_node/config.py @@ -66,8 +66,12 @@ transcode_incompatible_video = true # ICE candidate gathering. By default, virtual/VPN interfaces (Tailscale, # libvirt, Docker) are auto-excluded — a STUN request that can't reach the # server holds the WebRTC answer for 5 seconds. Set this to restrict -# gathering to specific interfaces (by OS adapter name). -# ice_interfaces = ["wlp0s20f3", "eth0"] +# gathering to specific interfaces. An entry matches the OS adapter name, the +# device description, or one of the adapter's own IPv4 addresses, so a Windows +# host can be named by description or address rather than by its GUID. An +# entry that matches nothing is ignored with a warning rather than leaving the +# node with no candidates at all. +# ice_interfaces = ["wlp0s20f3", "eth0", "192.168.1.22"] # STUN servers for WebRTC ICE candidate gathering (NAT traversal). By default # four public servers are used; set this to override. Every server in the list diff --git a/packages/meshbay-node/src/meshbay_node/transport/ice_filter.py b/packages/meshbay-node/src/meshbay_node/transport/ice_filter.py index a91724e..f0f369e 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/ice_filter.py +++ b/packages/meshbay-node/src/meshbay_node/transport/ice_filter.py @@ -15,6 +15,24 @@ the interfaces the operator actually wants. Two modes: Auto-excluded adapter name patterns: tailscale*, wt*, virbr*, docker*, veth*, br-*, podman*, cni*. + +Matching is "by adapter name", but an adapter does not have one name. ifaddr +reports `adapter.name` as the kernel name on Linux (`eth0`) and as a GUID on +Windows (`{846EE342-7039-11DE-9D20-806E6F6E6963}`), while `adapter.nice_name` +repeats the kernel name on Linux and carries the device *description* on +Windows. Neither is the connection name a Windows operator reads off +`ipconfig` ("Ethernet", "Wi-Fi"). So an include-list entry is compared, case +-insensitively, against the adapter's name, its nice_name, and its own IPv4 +addresses — an address is the one identifier visible on every platform, and +the settings field is free text, so operators write whichever they can see. + +A filter that matches nothing is worse than no filter: aioice binds one socket +per host address, so zero addresses means zero sockets, zero host candidates, +and an ICE session that can only ever offer a server-reflexive address. Behind +two layers of NAT that address is unreachable for a peer on the intermediate +LAN, so the connection cannot succeed at all — the failure looks like a network +problem and is silent. A filter that would return nothing therefore falls back +to the unfiltered list and logs a warning: slow beats unreachable. """ import ipaddress @@ -42,41 +60,104 @@ def _is_cgnat(addr: str) -> bool: return False +def _names(adapter) -> list[str]: + """Both identifiers ifaddr exposes; the same string twice on Linux.""" + return [n for n in (adapter.name, adapter.nice_name) if isinstance(n, str)] + + +def _ipv4s(adapter) -> list[str]: + return [ip.ip for ip in adapter.ips if isinstance(ip.ip, str)] + + +def _is_virtual(adapter) -> bool: + return any(_VIRTUAL_ADAPTER_RE.match(n) for n in _names(adapter)) + + +def _matches(adapter, include_set: set[str]) -> bool: + """True if the operator named this adapter, however they spelled it.""" + return any( + value.casefold() in include_set + for value in _names(adapter) + _ipv4s(adapter) + ) + + +def _addresses_of(adapter, use_ipv4: bool, use_ipv6: bool, *, + drop_cgnat: bool) -> list[str]: + """ + The usable addresses of one adapter. ifaddr gives IPv4 as a str and IPv6 + as an (address, flowinfo, scope_id) tuple, so the two are told apart by + type rather than by indexing — indexing a str yields characters, which + silently compare unequal to anything useful. + """ + out: list[str] = [] + for ip in adapter.ips: + if isinstance(ip.ip, str): + if not use_ipv4 or ip.ip == "127.0.0.1": + continue + if drop_cgnat and _is_cgnat(ip.ip): + continue + out.append(ip.ip) + elif isinstance(ip.ip, tuple) and len(ip.ip) >= 3: + # scope_id 0 == global; link-local needs a scope aioice has no use for + if use_ipv6 and ip.ip[0] != "::1" and ip.ip[2] == 0: + out.append(ip.ip[0]) + return out + + def install(include: list[str] | None = None) -> None: """Monkey-patch aioice.ice.get_host_addresses with a filtered version.""" import ifaddr - include_set = set(include) if include else None + include_set = {s.casefold() for s in include} if include else None def filtered_get_host_addresses( use_ipv4: bool = True, use_ipv6: bool = True, ) -> list[str]: + adapters = ifaddr.get_adapters() addresses: list[str] = [] - for adapter in ifaddr.get_adapters(): + + for adapter in adapters: if include_set is not None: - if adapter.name not in include_set: + if not _matches(adapter, include_set): continue - elif _VIRTUAL_ADAPTER_RE.match(adapter.name): - continue - - for ip in adapter.ips: - if isinstance(ip.ip, str) and use_ipv4 and ip.ip != "127.0.0.1": - if include_set is None and _is_cgnat(ip.ip): - continue - addresses.append(ip.ip) - elif use_ipv6 and ip.ip[0] != "::1" and ip.ip[2] == 0: - addresses.append(ip.ip[0]) - return addresses + # An explicitly named adapter is kept whatever its address range. + drop_cgnat = False + else: + if _is_virtual(adapter): + continue + drop_cgnat = True + addresses.extend( + _addresses_of(adapter, use_ipv4, use_ipv6, drop_cgnat=drop_cgnat)) + + if addresses: + return addresses + + # Nothing matched. Rather than gather no candidates at all, fall back to + # the unfiltered set — the operator loses the timeout saving, not the + # ability to connect. + fallback: list[str] = [] + for adapter in adapters: + fallback.extend( + _addresses_of(adapter, use_ipv4, use_ipv6, drop_cgnat=False)) + if fallback: + log.warning( + "ICE interface filter (%s) matched no address on this host; " + "using every interface instead. Adapters seen: %s", + "ice_interfaces=" + ", ".join(sorted(include)) if include + else "auto-exclude", + ", ".join(sorted({n for a in adapters for n in _names(a)})), + ) + return fallback aioice.ice.get_host_addresses = filtered_get_host_addresses if include_set: - log.info("ICE interfaces (explicit): %s", ", ".join(sorted(include_set))) + log.info("ICE interfaces (explicit): %s", ", ".join(sorted(include))) else: - all_adapters = [a.name for a in ifaddr.get_adapters()] excluded = [ - n for n in all_adapters - if _VIRTUAL_ADAPTER_RE.match(n) + a.nice_name or a.name + for a in ifaddr.get_adapters() + if _is_virtual(a) ] if excluded: log.info("ICE auto-excluded interfaces: %s", ", ".join(excluded)) diff --git a/packages/meshbay-node/tests/test_ice_filter.py b/packages/meshbay-node/tests/test_ice_filter.py new file mode 100644 index 0000000..a995c2b --- /dev/null +++ b/packages/meshbay-node/tests/test_ice_filter.py @@ -0,0 +1,163 @@ +""" +transport/ice_filter — the interface filter must name the same adapter on +every platform, and must never leave the gather with no address at all. +""" + +import ifaddr +import aioice.ice +import pytest +from meshbay_node.transport import ice_filter + + +class _IP: + def __init__(self, ip): + self.ip = ip + + +class _Adapter: + def __init__(self, name, nice_name, ips): + self.name, self.nice_name = name, nice_name + self.ips = [_IP(ip) for ip in ips] + + +def _linux(name, *ips): + """On Linux ifaddr repeats the kernel name in nice_name.""" + return _Adapter(name, name, list(ips)) + + +def _windows(guid, description, *ips): + """On Windows `name` is a GUID and `nice_name` the device description.""" + return _Adapter(guid, description, list(ips)) + + +@pytest.fixture(autouse=True) +def _restore(): + saved = aioice.ice.get_host_addresses + yield + aioice.ice.get_host_addresses = saved + + +@pytest.fixture +def adapters(monkeypatch): + """Install a fake adapter set; returns a setter the test calls.""" + def _set(*adapters): + monkeypatch.setattr(ifaddr, "get_adapters", lambda: list(adapters)) + return _set + + +def _gather(include=None): + ice_filter.install(include) + return aioice.ice.get_host_addresses() + + +def test_auto_excludes_virtual_adapters(adapters): + adapters( + _linux("lo", "127.0.0.1"), + _linux("wlp3s0f0", "192.168.1.22"), + _linux("virbr0", "192.168.200.254"), + _linux("docker0", "172.17.0.1"), + ) + assert _gather() == ["192.168.1.22"] + + +def test_auto_drops_cgnat_range(adapters): + adapters( + _linux("eth0", "192.168.1.22"), + _linux("tun0", "100.101.102.103"), + ) + assert _gather() == ["192.168.1.22"] + + +def test_include_list_by_kernel_name(adapters): + adapters( + _linux("eth0", "192.168.1.22"), + _linux("wlp3s0f0", "192.168.1.23"), + ) + assert _gather(["wlp3s0f0"]) == ["192.168.1.23"] + + +def test_include_list_keeps_explicitly_named_cgnat_adapter(adapters): + """An explicit name beats the auto-exclusion rules.""" + adapters(_linux("eth0", "192.168.1.22"), _linux("tun0", "100.101.102.103")) + assert _gather(["tun0"]) == ["100.101.102.103"] + + +def test_include_list_matches_windows_description(adapters): + """ + The Windows adapter GUID is not something an operator ever types, so the + device description has to match too. + """ + adapters( + _windows("{846EE342-7039-11DE-9D20-806E6F6E6963}", + "Gigabit Network Connection", "192.168.200.173"), + _windows("{0C6A2C4B-1111-2222-3333-444455556666}", + "Loopback Pseudo-Interface", "127.0.0.1"), + ) + assert _gather(["Gigabit Network Connection"]) == ["192.168.200.173"] + + +def test_include_list_matching_is_case_insensitive(adapters): + adapters(_windows("{GUID}", "Gigabit Network Connection", "192.168.200.173")) + assert _gather(["gigabit network connection"]) == ["192.168.200.173"] + + +def test_include_list_matches_an_ip_literal(adapters): + """The address is the one identifier visible on every platform.""" + adapters( + _windows("{GUID-A}", "Gigabit Network Connection", "192.168.200.173"), + _windows("{GUID-B}", "Virtual Adapter", "10.0.0.5"), + ) + assert _gather(["192.168.200.173"]) == ["192.168.200.173"] + + +def test_unmatched_include_list_falls_back_to_every_interface(adapters): + """ + The regression: a Linux-style ice_interfaces carried to a Windows guest + matched no GUID and no description, leaving the node with zero host + candidates — silently unconnectable rather than merely slow. + """ + adapters( + _windows("{846EE342-7039-11DE-9D20-806E6F6E6963}", + "Gigabit Network Connection", "192.168.200.173"), + ) + assert _gather(["wlp0s20f3"]) == ["192.168.200.173"] + + +def test_fallback_when_every_adapter_is_virtual(adapters): + """Auto-exclude must not empty the list either.""" + adapters(_linux("virbr0", "192.168.200.254"), _linux("docker0", "172.17.0.1")) + assert _gather() == ["192.168.200.254", "172.17.0.1"] + + +def test_fallback_warns(adapters, caplog): + adapters(_windows("{GUID}", "Gigabit Network Connection", "192.168.200.173")) + with caplog.at_level("WARNING", logger=ice_filter.log.name): + _gather(["wlp0s20f3"]) + assert "matched no address" in caplog.text + + +def test_loopback_only_host_yields_nothing_and_does_not_warn(adapters, caplog): + """No address to offer is not a filter misconfiguration.""" + adapters(_linux("lo", "127.0.0.1")) + with caplog.at_level("WARNING", logger=ice_filter.log.name): + assert _gather() == [] + assert caplog.text == "" + + +def test_ipv6_global_kept_link_local_dropped(adapters): + adapters(_Adapter("eth0", "eth0", [ + ("2a01:cb15:80e8:de00::1", 0, 0), + ("fe80::af4c:4912:c1ec:b6dc", 0, 3), + ("::1", 0, 0), + ])) + assert _gather() == ["2a01:cb15:80e8:de00::1"] + + +def test_ipv4_is_not_index_probed_as_ipv6(adapters): + """ + An IPv4 str must be discriminated by type: the old code fell through to the + IPv6 branch and indexed characters out of it. + """ + ice_filter.install(None) + adapters(_linux("eth0", "192.168.1.22")) + assert aioice.ice.get_host_addresses(use_ipv4=False, use_ipv6=True) == [] -- cgit v1.2.3