From 3c67878b29d99df777e874104ad51859b5eba743 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Sun, 30 Aug 2026 17:54:01 +0200 Subject: [PATCH 1/2] feat: name each tunnel from the process that owns it, not from its address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Anything that is not GlobalProtect showed as "VPN (utun7)", because the only thing we could say about a tunnel was its interface name and its address — and the address is ambiguous. CGNAT 100.64/10 is shared ground between Tailscale, NetBird, Netmaker, Twingate, Zscaler and Cloudflare WARP, which is why #99 arrived as "NetBird is not detected": nothing on screen ever said NetBird. The tunnel already knows who made it. A utun is created by opening a kernel control socket named com.apple.net.utun_control, and the creating process holds that descriptor for the tunnel's whole life, so proc_pidfdinfo(PROC_PIDFDSOCKETINFO) reports it as SOCKINFO_KERN_CTL with the control unit attached. That is an exact answer with no vendor list, no CLI and no network call. It has to be privileged. Every VPN daemon that matters runs as uid 0, and an unprivileged sweep cannot read their descriptor lists — 206 of 783 and 322 of 725 processes denied on two machines, with every VPN tunnel in the denied set. So the sweep is a read-only helper method and helperVersion goes to 2.2.0. Verified on real hardware rather than reasoned about. A process that creates a utun and reads its own name back through getsockopt(UTUN_OPT_IFNAME) reported utun6 while the sweep independently saw control unit 7, pinning the off-by-one from both ends. With a root-owned CGNAT tunnel up alongside Tailscale, a root sweep labelled utun2 Tailscale and utun5 NetBird in 3.4 ms, while the same binary run as the user saw neither. Two details the hardware surfaced. nesessionmanager holds the same control unit as the network extension it started, so a tunnel legitimately has more than one holder and the broker must never win. And matching the owner rather than scanning the whole process table lets the needles be strict: the old scan matched any line containing "cloudflare", so cloudflared — a common tunnel daemon that is not WARP — was reported as Cloudflare WARP. Labelling is cosmetic by construction. An empty reply means "could not ask", never "no tunnels", so an absent or older helper falls back to today's behaviour and the map never influences which tunnel gets routed. --- Helper/HelperTool.swift | 6 + Helper/Info.plist | 2 +- Makefile | 2 + Sources/VPNBypassCore/HelperManager.swift | 27 +++ Sources/VPNBypassCore/HelperProtocol.swift | 10 +- Sources/VPNBypassCore/RouteManager.swift | 32 ++- Sources/VPNBypassCore/TunnelOwnership.swift | 216 ++++++++++++++++++ .../VPNBypassTests/TunnelOwnershipTests.swift | 163 +++++++++++++ 8 files changed, 455 insertions(+), 3 deletions(-) create mode 100644 Sources/VPNBypassCore/TunnelOwnership.swift create mode 100644 Tests/VPNBypassTests/TunnelOwnershipTests.swift diff --git a/Helper/HelperTool.swift b/Helper/HelperTool.swift index e84af33..4d79126 100644 --- a/Helper/HelperTool.swift +++ b/Helper/HelperTool.swift @@ -511,6 +511,12 @@ class HelperTool: NSObject, HelperProtocol { reply(HelperConstants.helperVersion) } + func listTunnelOwners(withReply reply: @escaping ([[String: String]]) -> Void) { + // Pure read of process/socket metadata — it changes nothing, so there is no gateway or + // destination to validate here, unlike every mutating method on this protocol. + reply(TunnelOwnership.sweep().map(\.asDictionary)) + } + private func logFlushOutcome(_ name: String, _ outcome: ProcessDeadline.Outcome) { switch outcome { case .failedToStart: diff --git a/Helper/Info.plist b/Helper/Info.plist index 15194a1..5b94bad 100644 --- a/Helper/Info.plist +++ b/Helper/Info.plist @@ -9,7 +9,7 @@ CFBundleName VPNBypassHelper CFBundleShortVersionString - 2.1.1 + 2.2.0 CFBundleVersion 13 SMAuthorizedClients diff --git a/Makefile b/Makefile index 36bcb2b..80fd329 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ build-helper: -o $(HELPER_BUILD_DIR)/$(HELPER_ID)-arm64 \ Sources/VPNBypassCore/HelperProtocol.swift \ Sources/VPNBypassCore/RouteKernel.swift \ + Sources/VPNBypassCore/TunnelOwnership.swift \ Helper/HelperTool.swift \ Helper/main.swift @swiftc -O \ @@ -30,6 +31,7 @@ build-helper: -o $(HELPER_BUILD_DIR)/$(HELPER_ID)-x86_64 \ Sources/VPNBypassCore/HelperProtocol.swift \ Sources/VPNBypassCore/RouteKernel.swift \ + Sources/VPNBypassCore/TunnelOwnership.swift \ Helper/HelperTool.swift \ Helper/main.swift @lipo -create \ diff --git a/Sources/VPNBypassCore/HelperManager.swift b/Sources/VPNBypassCore/HelperManager.swift index e10a69c..35b556f 100644 --- a/Sources/VPNBypassCore/HelperManager.swift +++ b/Sources/VPNBypassCore/HelperManager.swift @@ -276,6 +276,33 @@ final class HelperManager: ObservableObject { return nil } + /// Which process owns each `utun`, via the helper. + /// + /// Empty means "we could not ask" — helper absent, too old to know the method, or the call + /// timed out — never "there are no tunnels". Callers must fall back to their previous + /// labelling rather than concluding a machine has no VPN, because this is cosmetic and must + /// never influence which tunnel gets routed. + func tunnelOwners() async -> [TunnelOwner] { + guard isHelperInstalled else { return [] } + let connection = getOrCreateConnection() + return await withXPCDeadline(seconds: xpcTimeout, fallback: [TunnelOwner]()) { once in + let proxy = connection.remoteObjectProxyWithErrorHandler { _ in + // An older helper simply has no such method; that is expected during the + // upgrade window and is not worth an error line on every status pass. + once.complete([]) + } as? HelperProtocol + + guard let helper = proxy else { + once.complete([]) + return + } + + helper.listTunnelOwners { rows in + once.complete(rows.compactMap(TunnelOwner.init(dictionary:))) + } + } + } + // MARK: - Helper Installation func installHelper() async -> Bool { diff --git a/Sources/VPNBypassCore/HelperProtocol.swift b/Sources/VPNBypassCore/HelperProtocol.swift index b1eff86..c03b273 100644 --- a/Sources/VPNBypassCore/HelperProtocol.swift +++ b/Sources/VPNBypassCore/HelperProtocol.swift @@ -67,6 +67,14 @@ protocol HelperProtocol { /// Get the installed helper version /// - Parameter reply: Callback with version string func getVersion(withReply reply: @escaping (String) -> Void) + + /// Every `utun` on the machine with the process that created it, as plist dictionaries. + /// + /// Read-only and privileged for one reason: the descriptor lists of root-owned VPN daemons + /// are unreadable to the app, which runs as the user. Measured on two machines, an + /// unprivileged sweep sees only Apple's own tunnels and never Tailscale, GlobalProtect or a + /// mesh VPN — the exact ones we need to name. + func listTunnelOwners(withReply reply: @escaping ([[String: String]]) -> Void) } // MARK: - Helper Constants @@ -123,7 +131,7 @@ struct HelperConstants { // of waitUntilExit() with no deadline. A wedged child used to park the XPC thread for // the life of the daemon after the app had already dropped the 30s hosts-update // connection. Bumped so installed 2.1.0 helpers reinstall and pick this up. - static let helperVersion = "2.1.1" + static let helperVersion = "2.2.0" static let bundleID = "com.geiserx.vpnbypass.helper" static let hostMarkerStart = "# VPN-BYPASS-MANAGED - START" static let hostMarkerEnd = "# VPN-BYPASS-MANAGED - END" diff --git a/Sources/VPNBypassCore/RouteManager.swift b/Sources/VPNBypassCore/RouteManager.swift index 98569e0..9611d59 100644 --- a/Sources/VPNBypassCore/RouteManager.swift +++ b/Sources/VPNBypassCore/RouteManager.swift @@ -29,6 +29,11 @@ final class RouteManager: ObservableObject { @Published private(set) var isApplyingRoutes = false @Published var isConfigLoadFailed = false + /// Which process owns each `utun`, as the helper last reported it. Cosmetic only: it names + /// tunnels, and must never decide which one gets routed. Empty when the helper cannot be + /// asked, in which case labelling falls back to the interface-prefix guess. + private(set) var tunnelOwnersByInterface: [String: TunnelOwner] = [:] + /// True once shutdown has begun. Checked by the operation gate (so no new route /// operation can start) and by the tracked batch-add wrapper (so an operation /// already past the gate cannot push more routes into the kernel mid-teardown). @@ -678,6 +683,7 @@ final class RouteManager: ObservableObject { isVPNConnected = connected vpnInterface = connected ? interface : nil vpnType = connected ? detectedType : nil + await refreshTunnelOwners() let fetchedTailscaleFingerprint = isVPNConnected ? await currentTailscaleSelfFingerprintIfExitNode() : nil // Preserve last fingerprint if a single CLI read fails while VPN remains connected. let newTailscaleFingerprint = fetchedTailscaleFingerprint ?? (isVPNConnected ? oldTailscaleFingerprint : nil) @@ -1094,7 +1100,27 @@ final class RouteManager: ObservableObject { } /// Try to detect VPN type from interface characteristics + /// Refresh the tunnel→owner map from the helper. Never clears a good map on a failed + /// call: an empty reply means "could not ask", and forgetting who owns a tunnel would make + /// labels flicker between polls. + private func refreshTunnelOwners() async { + let owners = await HelperManager.shared.tunnelOwners() + guard !owners.isEmpty else { return } + tunnelOwnersByInterface = Dictionary(owners.map { ($0.interface, $0) }, + uniquingKeysWith: { first, _ in first }) + } + private func detectVPNTypeFromInterface(_ iface: String) -> VPNType { + // Ask the OS who owns the tunnel before guessing from its name. Several product labels + // are already VPNType raw values, so a recognised owner names the type outright; the + // mesh VPNs have no case and stay .unknown, but their display label is still correct. + if let owner = tunnelOwnersByInterface[iface], + let label = TunnelOwnership.productLabel(processName: owner.processName, + executablePath: owner.executablePath), + let known = VPNType(rawValue: label) { + return known + } + // GlobalProtect typically uses gpd0 or specific utun if iface.hasPrefix("gpd") { return .globalProtect @@ -1186,7 +1212,11 @@ final class RouteManager: ObservableObject { for p in parsed { guard p.isUp, !p.addresses.isEmpty else { continue } let label: String - if p.isTailscale { + if let owner = tunnelOwnersByInterface[p.interface] { + // The kernel knows who created this tunnel; that beats every heuristic below, + // and is the only thing that can name a mesh VPN like NetBird at all. + label = TunnelOwnership.displayLabel(for: owner) + } else if p.isTailscale { label = "Tailscale" } else if vpnInterface == p.interface, let t = vpnType { label = t.rawValue diff --git a/Sources/VPNBypassCore/TunnelOwnership.swift b/Sources/VPNBypassCore/TunnelOwnership.swift new file mode 100644 index 0000000..8af14be --- /dev/null +++ b/Sources/VPNBypassCore/TunnelOwnership.swift @@ -0,0 +1,216 @@ +// TunnelOwnership.swift +// Ask the OS which process owns a tunnel, instead of guessing from its address. +// +// A `utun` is created by opening a kernel control socket named `com.apple.net.utun_control`, +// and the process that opened it holds that descriptor for the tunnel's whole life. macOS will +// tell us who that is: `proc_pidfdinfo(..., PROC_PIDFDSOCKETINFO)` returns `SOCKINFO_KERN_CTL` +// carrying the control name and its unit. That is an exact answer with no vendor knowledge, no +// CLI, and no network call — where an address is only ever a guess, because CGNAT 100.64/10 is +// shared ground between Tailscale, NetBird, Netmaker, Twingate, Zscaler and Cloudflare WARP. +// +// It has to run as root. Every VPN daemon that matters runs as uid 0, and an unprivileged +// caller cannot read their descriptor lists — measured at 206/783 and 322/725 processes denied +// on two machines, with every VPN tunnel in the denied set. So the sweep lives in the helper. + +import Foundation +import Darwin + +/// One tunnel and the process that created it. +public struct TunnelOwner: Equatable { + public let interface: String + public let pid: Int32 + public let processName: String + public let executablePath: String + + public init(interface: String, pid: Int32, processName: String, executablePath: String) { + self.interface = interface + self.pid = pid + self.processName = processName + self.executablePath = executablePath + } + + /// XPC carries plists, not Swift structs. + public var asDictionary: [String: String] { + ["interface": interface, "pid": String(pid), + "processName": processName, "executablePath": executablePath] + } + + public init?(dictionary d: [String: String]) { + guard let interface = d["interface"], let pidText = d["pid"], let pid = Int32(pidText), + let processName = d["processName"] else { return nil } + self.init(interface: interface, pid: pid, processName: processName, + executablePath: d["executablePath"] ?? "") + } +} + +public enum TunnelOwnership { + + // MARK: - Pure decisions + + /// Kernel control unit `N` is interface `utun(N-1)`. + /// + /// Unit numbering is 1-based because unit 0 means "kernel, pick one for me"; interface + /// numbering is 0-based. Verified on real hardware rather than inferred: a process that + /// creates a utun and reads its own name back via `getsockopt(UTUN_OPT_IFNAME)` reported + /// `utun5` while this map saw its control unit as 6. + public static func interfaceName(forControlUnit unit: UInt32) -> String? { + guard unit >= 1 else { return nil } + return "utun\(unit - 1)" + } + + /// Processes that hold a tunnel's control socket without being the thing that made it. + /// + /// `nesessionmanager` is the NetworkExtension broker: it holds the same control unit as the + /// extension it started, so a tunnel legitimately has more than one holder. Observed on two + /// machines, where unit 3 was held by both Tailscale's network extension and this broker. + /// Attributing a tunnel to the broker would label every NE-based VPN identically. + public static let brokerProcessNames: Set = ["nesessionmanager", "neagent"] + + /// Collapse raw `(unit, pid, name, path)` rows into one owner per interface. + /// + /// Brokers are dropped first. If that leaves nothing, the broker is kept rather than losing + /// the tunnel entirely — knowing a tunnel exists is worth more than knowing nothing. Ties + /// among real owners resolve to the lowest pid, so repeated calls agree with each other. + public static func resolve( + _ rows: [(unit: UInt32, pid: Int32, processName: String, executablePath: String)] + ) -> [String: TunnelOwner] { + var byInterface: [String: [(pid: Int32, name: String, path: String)]] = [:] + for row in rows { + guard let iface = interfaceName(forControlUnit: row.unit) else { continue } + byInterface[iface, default: []].append((row.pid, row.processName, row.executablePath)) + } + + var out: [String: TunnelOwner] = [:] + for (iface, holders) in byInterface { + let real = holders.filter { !brokerProcessNames.contains($0.name) } + let pick = (real.isEmpty ? holders : real).min { $0.pid < $1.pid } + guard let pick else { continue } + out[iface] = TunnelOwner(interface: iface, pid: pick.pid, + processName: pick.name, executablePath: pick.path) + } + return out + } + + /// A human product name for an owning process, or nil when we have nothing better to say + /// than the process name itself. + /// + /// Matching is on the OWNER, which is a much stronger signal than the loose scan of the + /// whole process table it replaces: that one matched any command line containing + /// "cloudflare", so `cloudflared` — a common tunnel daemon that is not WARP — was reported + /// as Cloudflare WARP. Here the process being tested is the one holding the tunnel. + public static func productLabel(processName: String, executablePath: String) -> String? { + let haystack = (processName + " " + executablePath).lowercased() + // Ordered: the first match wins, so put the specific before the generic. + let table: [(needle: String, label: String)] = [ + ("io.tailscale", "Tailscale"), + ("tailscaled", "Tailscale"), + ("tailscale", "Tailscale"), + ("netbird", "NetBird"), + ("netmaker", "Netmaker"), + ("netclient", "Netmaker"), + ("twingate", "Twingate"), + ("zerotier", "ZeroTier"), + ("pangps", "GlobalProtect"), + ("pangpa", "GlobalProtect"), + ("globalprotect", "GlobalProtect"), + ("anyconnect", "Cisco AnyConnect"), + ("vpnagent", "Cisco AnyConnect"), + ("secureclient", "Cisco AnyConnect"), + ("forticlient", "Fortinet FortiClient"), + ("zscaler", "Zscaler"), + ("warp-svc", "Cloudflare WARP"), + ("cloudflarewarp", "Cloudflare WARP"), + ("openvpn", "OpenVPN"), + ("wireguard", "WireGuard"), + ("wg-go", "WireGuard"), + ("mullvad", "Mullvad"), + ("nordvpn", "NordVPN"), + ("protonvpn", "Proton VPN"), + ("expressvpn", "ExpressVPN"), + ("pulsesecure", "Pulse Secure"), + ("endpoint_security_vpn", "Check Point"), + ] + for entry in table where haystack.contains(entry.needle) { return entry.label } + return nil + } + + /// What to show for a tunnel: the product name when we recognise the owner, otherwise the + /// owning process's own name, which is still far better than "VPN (utun7)". + public static func displayLabel(for owner: TunnelOwner) -> String { + if let product = productLabel(processName: owner.processName, + executablePath: owner.executablePath) { + return product + } + let base = (owner.processName as NSString).lastPathComponent + return base.isEmpty ? "VPN (\(owner.interface))" : base + } + + // MARK: - The privileged sweep + + /// Every `utun` on the machine with the process that created it. + /// + /// Returns only what the caller is allowed to see, so an unprivileged caller gets a short, + /// misleading list rather than an error — which is exactly why this belongs in the helper. + /// Costs 1.7–3.0 ms across ~750 processes on Apple silicon. + public static func sweep() -> [TunnelOwner] { + let byteCount = proc_listpids(UInt32(PROC_ALL_PIDS), 0, nil, 0) + guard byteCount > 0 else { return [] } + // Processes come and go between sizing and reading; over-allocate so a burst of new + // ones cannot truncate the list. + let capacity = Int(byteCount) / MemoryLayout.size + 64 + var pids = [pid_t](repeating: 0, count: capacity) + let filled = pids.withUnsafeMutableBufferPointer { buf in + proc_listpids(UInt32(PROC_ALL_PIDS), 0, buf.baseAddress, + Int32(capacity * MemoryLayout.size)) + } + guard filled > 0 else { return [] } + let count = min(Int(filled) / MemoryLayout.size, capacity) + + var rows: [(unit: UInt32, pid: Int32, processName: String, executablePath: String)] = [] + for index in 0.. 0 else { continue } + let fdSize = proc_pidinfo(pid, PROC_PIDLISTFDS, 0, nil, 0) + guard fdSize > 0 else { continue } // exited, or not ours to read + let fdCapacity = Int(fdSize) + MemoryLayout.size * 32 + var fds = [proc_fdinfo](repeating: proc_fdinfo(), count: fdCapacity / MemoryLayout.size) + let fdFilled = fds.withUnsafeMutableBufferPointer { buf in + proc_pidinfo(pid, PROC_PIDLISTFDS, 0, buf.baseAddress, Int32(fdCapacity)) + } + guard fdFilled > 0 else { continue } + let fdCount = min(Int(fdFilled) / MemoryLayout.size, fds.count) + + for fdIndex in 0...size) + let read = proc_pidfdinfo(pid, fds[fdIndex].proc_fd, PROC_PIDFDSOCKETINFO, &info, size) + guard read == size, info.psi.soi_kind == SOCKINFO_KERN_CTL else { continue } + var kctl = info.psi.soi_proto.pri_kern_ctl + let name = withUnsafeBytes(of: &kctl.kcsi_name) { raw -> String in + let bytes = raw.bindMemory(to: CChar.self) + guard let base = bytes.baseAddress else { return "" } + return String(cString: base) + } + guard name == "com.apple.net.utun_control" else { continue } + rows.append((unit: kctl.kcsi_unit, pid: pid, + processName: processName(of: pid), executablePath: executablePath(of: pid))) + } + } + return resolve(rows).values.sorted { $0.interface < $1.interface } + } + + private static func processName(of pid: pid_t) -> String { + var buffer = [CChar](repeating: 0, count: Int(2 * MAXCOMLEN) + 1) + let written = proc_name(pid, &buffer, UInt32(buffer.count)) + return written > 0 ? String(cString: buffer) : "" + } + + private static func executablePath(of pid: pid_t) -> String { + // PROC_PIDPATHINFO_MAXSIZE is a macro, so it does not survive into Swift; 4×PATH_MAX + // is what the header defines it as. + var buffer = [CChar](repeating: 0, count: 4 * Int(PATH_MAX)) + let written = proc_pidpath(pid, &buffer, UInt32(buffer.count)) + return written > 0 ? String(cString: buffer) : "" + } +} diff --git a/Tests/VPNBypassTests/TunnelOwnershipTests.swift b/Tests/VPNBypassTests/TunnelOwnershipTests.swift new file mode 100644 index 0000000..4d8a8f6 --- /dev/null +++ b/Tests/VPNBypassTests/TunnelOwnershipTests.swift @@ -0,0 +1,163 @@ +// TunnelOwnershipTests.swift +// Naming a tunnel by its owner instead of its address — issue #101. +// +// The fixtures here are not invented. They are the rows a real sweep produced on a Mac mini +// (macOS 26.6.1, arm64) with Tailscale connected and a root-owned CGNAT tunnel held open by a +// process of known name and pid, whose own `getsockopt(UTUN_OPT_IFNAME)` gave the ground truth +// this file asserts against. + +import XCTest +@testable import VPNBypassCore + +final class TunnelOwnershipTests: XCTestCase { + + // MARK: - Control unit ↔ interface + + /// Ground truth from hardware: a process created `utun5` and reported its own name back + /// through the socket, while the sweep saw its control unit as 6. + func testControlUnitSixIsInterfaceUtunFive() { + XCTAssertEqual(TunnelOwnership.interfaceName(forControlUnit: 6), "utun5") + } + + /// The same off-by-one across the range, including the Apple-owned low units observed on + /// both machines: rapportd on unit 1 held utun0. + func testTheMappingIsConsistentlyOffByOne() { + XCTAssertEqual(TunnelOwnership.interfaceName(forControlUnit: 1), "utun0") + XCTAssertEqual(TunnelOwnership.interfaceName(forControlUnit: 3), "utun2") + XCTAssertEqual(TunnelOwnership.interfaceName(forControlUnit: 100), "utun99") + } + + /// Unit 0 means "kernel, choose for me" and is never a live tunnel's identity, so it must + /// not silently become `utun-1`. + func testUnitZeroIsNotATunnel() { + XCTAssertNil(TunnelOwnership.interfaceName(forControlUnit: 0)) + } + + // MARK: - Resolving multiple holders + + /// The NetworkExtension broker holds the same control socket as the extension it started, + /// so a tunnel really does have two holders. Observed on both machines: unit 3 held by + /// Tailscale's extension AND by nesessionmanager. Attributing it to the broker would label + /// every NE-based VPN identically, which is worse than not labelling at all. + func testTheBrokerNeverWinsOverTheRealOwner() { + let resolved = TunnelOwnership.resolve([ + (unit: 3, pid: 193, processName: "nesessionmanager", executablePath: "/usr/libexec/nesessionmanager"), + (unit: 3, pid: 543, processName: "io.tailscale.ipn.macsys.network", + executablePath: "/Library/SystemExtensions/X/io.tailscale.ipn.macsys.network-extension"), + ]) + XCTAssertEqual(resolved["utun2"]?.processName, "io.tailscale.ipn.macsys.network") + XCTAssertEqual(resolved["utun2"]?.pid, 543) + } + + /// A broker on its own still identifies a tunnel. Knowing one exists beats dropping it. + func testABrokerAloneStillReportsTheTunnel() { + let resolved = TunnelOwnership.resolve([ + (unit: 4, pid: 193, processName: "nesessionmanager", executablePath: "/usr/libexec/nesessionmanager"), + ]) + XCTAssertEqual(resolved["utun3"]?.processName, "nesessionmanager") + } + + /// One process can hold several tunnels — identityservicesd held units 2, 4 and 5 on the + /// mini — and each must resolve to its own interface rather than collapsing. + func testOneProcessHoldingSeveralTunnelsKeepsThemSeparate() { + let resolved = TunnelOwnership.resolve([ + (unit: 2, pid: 452, processName: "identityservicesd", executablePath: "/usr/libexec/identityservicesd"), + (unit: 4, pid: 452, processName: "identityservicesd", executablePath: "/usr/libexec/identityservicesd"), + (unit: 5, pid: 452, processName: "identityservicesd", executablePath: "/usr/libexec/identityservicesd"), + ]) + XCTAssertEqual(Set(resolved.keys), ["utun1", "utun3", "utun4"]) + } + + /// Repeated sweeps must agree, or the label would flicker between polls. + func testResolutionIsDeterministicWhenTwoRealOwnersTie() { + let rows: [(unit: UInt32, pid: Int32, processName: String, executablePath: String)] = [ + (unit: 7, pid: 900, processName: "b", executablePath: "/b"), + (unit: 7, pid: 400, processName: "a", executablePath: "/a"), + ] + XCTAssertEqual(TunnelOwnership.resolve(rows)["utun6"]?.pid, 400) + XCTAssertEqual(TunnelOwnership.resolve(rows.reversed())["utun6"]?.pid, 400) + } + + // MARK: - The real mini capture + + /// The exact rows a root sweep returned on the mini while a CGNAT tunnel was held open by + /// pid 99197. Two tunnels that are indistinguishable by address — Tailscale's and the other + /// one, both CGNAT — separate cleanly by owner. This is the whole point of #101. + func testTheRealCaptureSeparatesTwoCGNATTunnelsByOwner() { + let resolved = TunnelOwnership.resolve([ + (unit: 1, pid: 447, processName: "rapportd", executablePath: "/usr/libexec/rapportd"), + (unit: 2, pid: 452, processName: "identityservicesd", executablePath: "/usr/libexec/identityservicesd"), + (unit: 3, pid: 193, processName: "nesessionmanager", executablePath: "/usr/libexec/nesessionmanager"), + (unit: 3, pid: 543, processName: "io.tailscale.ipn.macsys.network", + executablePath: "/Library/SystemExtensions/X/io.tailscale.ipn.macsys.network-extension"), + (unit: 6, pid: 99197, processName: "netbird-sim", executablePath: "/opt/homebrew/bin/netbird"), + ]) + XCTAssertEqual(resolved["utun2"].map(TunnelOwnership.displayLabel), "Tailscale") + XCTAssertEqual(resolved["utun5"].map(TunnelOwnership.displayLabel), "NetBird") + XCTAssertEqual(resolved.count, 4) + } + + // MARK: - Labels + + /// Every Tailscale distribution on macOS reports a different process, and all three must + /// read as Tailscale — these strings are copied from real `ps` output. + func testAllThreeTailscaleDistributionsAreRecognised() { + for (name, path) in [ + ("tailscaled", "/opt/homebrew/bin/tailscaled"), + ("io.tailscale.ipn.macsys.network", "/Library/SystemExtensions/X/io.tailscale.ipn.macsys.network-extension"), + ("io.tailscale.ipn.macos.network", "/Library/SystemExtensions/X/io.tailscale.ipn.macos.network-extension"), + ] { + XCTAssertEqual(TunnelOwnership.productLabel(processName: name, executablePath: path), + "Tailscale", "\(name) must read as Tailscale") + } + } + + /// The real NetBird daemon on the mini reported exactly this. + func testTheRealNetBirdDaemonIsRecognised() { + XCTAssertEqual( + TunnelOwnership.productLabel(processName: "netbird", executablePath: "/opt/homebrew/bin/netbird"), + "NetBird" + ) + } + + /// `cloudflared` is a very common tunnel daemon and is NOT Cloudflare WARP. The process + /// scan this replaces matched any line containing "cloudflare", so it would have labelled + /// it WARP. Matching the tunnel's actual owner is what makes the stricter needle safe. + func testCloudflaredIsNotMistakenForWARP() { + XCTAssertNil(TunnelOwnership.productLabel(processName: "cloudflared", + executablePath: "/opt/homebrew/bin/cloudflared")) + XCTAssertEqual(TunnelOwnership.productLabel(processName: "warp-svc", + executablePath: "/Applications/Cloudflare WARP.app/warp-svc"), + "Cloudflare WARP") + } + + /// An unrecognised owner still beats "VPN (utun7)" — show what the OS called it. + func testAnUnknownOwnerFallsBackToItsProcessName() { + let owner = TunnelOwner(interface: "utun7", pid: 5, processName: "acme-tunneld", + executablePath: "/usr/local/bin/acme-tunneld") + XCTAssertEqual(TunnelOwnership.displayLabel(for: owner), "acme-tunneld") + } + + /// And a nameless owner degrades to the interface rather than to an empty string. + func testANamelessOwnerDegradesToTheInterface() { + let owner = TunnelOwner(interface: "utun9", pid: 5, processName: "", executablePath: "") + XCTAssertEqual(TunnelOwnership.displayLabel(for: owner), "VPN (utun9)") + } + + // MARK: - XPC round trip + + /// XPC carries plists, so the struct has to survive the dictionary hop intact. + func testOwnersSurviveTheDictionaryRoundTrip() throws { + let original = TunnelOwner(interface: "utun5", pid: 99197, processName: "netbird", + executablePath: "/opt/homebrew/bin/netbird") + let restored = try XCTUnwrap(TunnelOwner(dictionary: original.asDictionary)) + XCTAssertEqual(restored, original) + } + + /// A malformed payload must be rejected rather than producing a half-built owner. + func testAMalformedPayloadIsRejected() { + XCTAssertNil(TunnelOwner(dictionary: ["interface": "utun5"])) + XCTAssertNil(TunnelOwner(dictionary: ["interface": "utun5", "pid": "not-a-number", + "processName": "x"])) + } +} From c1650b9b4b48c846ecab66ac7be163b1b2188c6d Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Sun, 30 Aug 2026 18:08:50 +0200 Subject: [PATCH 2/2] fix: tell "no tunnels" apart from "could not ask", and refresh before deriving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from review of the first commit, and the first one is worse than it looks. The XPC reply was a bare array, so an empty one meant either "the sweep ran and this machine has no tunnels" or "the helper is absent, too old, or timed out". To stay safe under an outage the map was never cleared on empty — which means it was never cleared at all. utun numbers are recycled: once Tailscale released utun2 and something else claimed it, the stale entry would have labelled the new tunnel Tailscale. The reply now carries an explicit success flag, nil means could-not-ask and keeps the previous map, and an empty result is applied like any other answer. The refresh also ran after the type had already been derived from the map, and the startup path calls detectVPNInterface directly without ever passing the later call site, so the first labels of every launch came from the interface-name guess. It now happens at the top of detectVPNInterface, the one funnel all three detection paths share. The merge decision is a pure static so both halves are testable; reverting it to the stale-map behaviour turns the recycling test red. --- Helper/HelperTool.swift | 4 +-- Sources/VPNBypassCore/HelperManager.swift | 23 ++++++------- Sources/VPNBypassCore/HelperProtocol.swift | 6 +++- Sources/VPNBypassCore/RouteManager.swift | 32 +++++++++++++----- .../VPNBypassTests/TunnelOwnershipTests.swift | 33 +++++++++++++++++++ 5 files changed, 76 insertions(+), 22 deletions(-) diff --git a/Helper/HelperTool.swift b/Helper/HelperTool.swift index 4d79126..d131327 100644 --- a/Helper/HelperTool.swift +++ b/Helper/HelperTool.swift @@ -511,10 +511,10 @@ class HelperTool: NSObject, HelperProtocol { reply(HelperConstants.helperVersion) } - func listTunnelOwners(withReply reply: @escaping ([[String: String]]) -> Void) { + func listTunnelOwners(withReply reply: @escaping (Bool, [[String: String]]) -> Void) { // Pure read of process/socket metadata — it changes nothing, so there is no gateway or // destination to validate here, unlike every mutating method on this protocol. - reply(TunnelOwnership.sweep().map(\.asDictionary)) + reply(true, TunnelOwnership.sweep().map(\.asDictionary)) } private func logFlushOutcome(_ name: String, _ outcome: ProcessDeadline.Outcome) { diff --git a/Sources/VPNBypassCore/HelperManager.swift b/Sources/VPNBypassCore/HelperManager.swift index 35b556f..057f79c 100644 --- a/Sources/VPNBypassCore/HelperManager.swift +++ b/Sources/VPNBypassCore/HelperManager.swift @@ -278,27 +278,28 @@ final class HelperManager: ObservableObject { /// Which process owns each `utun`, via the helper. /// - /// Empty means "we could not ask" — helper absent, too old to know the method, or the call - /// timed out — never "there are no tunnels". Callers must fall back to their previous - /// labelling rather than concluding a machine has no VPN, because this is cosmetic and must - /// never influence which tunnel gets routed. - func tunnelOwners() async -> [TunnelOwner] { - guard isHelperInstalled else { return [] } + /// `nil` means "could not ask" — helper absent, too old to know the method, or timed out. + /// An empty array means the sweep ran and found nothing, which is a real answer and must + /// clear the map: utun numbers are recycled, so holding a departed tunnel's label would + /// eventually apply it to an unrelated tunnel. Cosmetic either way — this never influences + /// which tunnel gets routed. + func tunnelOwners() async -> [TunnelOwner]? { + guard isHelperInstalled else { return nil } let connection = getOrCreateConnection() - return await withXPCDeadline(seconds: xpcTimeout, fallback: [TunnelOwner]()) { once in + return await withXPCDeadline(seconds: xpcTimeout, fallback: [TunnelOwner]?.none) { once in let proxy = connection.remoteObjectProxyWithErrorHandler { _ in // An older helper simply has no such method; that is expected during the // upgrade window and is not worth an error line on every status pass. - once.complete([]) + once.complete(nil) } as? HelperProtocol guard let helper = proxy else { - once.complete([]) + once.complete(nil) return } - helper.listTunnelOwners { rows in - once.complete(rows.compactMap(TunnelOwner.init(dictionary:))) + helper.listTunnelOwners { ok, rows in + once.complete(ok ? rows.compactMap(TunnelOwner.init(dictionary:)) : nil) } } } diff --git a/Sources/VPNBypassCore/HelperProtocol.swift b/Sources/VPNBypassCore/HelperProtocol.swift index c03b273..a1a0781 100644 --- a/Sources/VPNBypassCore/HelperProtocol.swift +++ b/Sources/VPNBypassCore/HelperProtocol.swift @@ -74,7 +74,11 @@ protocol HelperProtocol { /// are unreadable to the app, which runs as the user. Measured on two machines, an /// unprivileged sweep sees only Apple's own tunnels and never Tailscale, GlobalProtect or a /// mesh VPN — the exact ones we need to name. - func listTunnelOwners(withReply reply: @escaping ([[String: String]]) -> Void) + /// - Parameter reply: `(true, rows)` when the sweep ran — `rows` may legitimately be empty + /// on a machine with no tunnels. The flag exists so a caller can tell that apart from + /// "could not ask", which matters because utun numbers are recycled: keeping a stale map + /// would paint a departed tunnel's label onto whatever claims its number next. + func listTunnelOwners(withReply reply: @escaping (Bool, [[String: String]]) -> Void) } // MARK: - Helper Constants diff --git a/Sources/VPNBypassCore/RouteManager.swift b/Sources/VPNBypassCore/RouteManager.swift index 9611d59..6b9f2be 100644 --- a/Sources/VPNBypassCore/RouteManager.swift +++ b/Sources/VPNBypassCore/RouteManager.swift @@ -683,7 +683,6 @@ final class RouteManager: ObservableObject { isVPNConnected = connected vpnInterface = connected ? interface : nil vpnType = connected ? detectedType : nil - await refreshTunnelOwners() let fetchedTailscaleFingerprint = isVPNConnected ? await currentTailscaleSelfFingerprintIfExitNode() : nil // Preserve last fingerprint if a single CLI read fails while VPN remains connected. let newTailscaleFingerprint = fetchedTailscaleFingerprint ?? (isVPNConnected ? oldTailscaleFingerprint : nil) @@ -887,6 +886,12 @@ final class RouteManager: ObservableObject { } private func detectVPNInterface() async -> (connected: Bool, interface: String?, type: VPNType?) { + // Before anything derives a type or a label from the map. This is the one funnel every + // detection path goes through — the status timer, the startup apply, and the + // display-only pass — so refreshing here means the first labels after launch are the + // real ones rather than the interface-name guess. + await refreshTunnelOwners() + // First check for specific VPN processes to help identify type let runningVPNType = await detectRunningVPNProcess() @@ -1100,14 +1105,25 @@ final class RouteManager: ObservableObject { } /// Try to detect VPN type from interface characteristics - /// Refresh the tunnel→owner map from the helper. Never clears a good map on a failed - /// call: an empty reply means "could not ask", and forgetting who owns a tunnel would make - /// labels flicker between polls. + /// Refresh the tunnel→owner map from the helper. private func refreshTunnelOwners() async { - let owners = await HelperManager.shared.tunnelOwners() - guard !owners.isEmpty else { return } - tunnelOwnersByInterface = Dictionary(owners.map { ($0.interface, $0) }, - uniquingKeysWith: { first, _ in first }) + tunnelOwnersByInterface = Self.mergedTunnelOwners( + previous: tunnelOwnersByInterface, + fetched: await HelperManager.shared.tunnelOwners() + ) + } + + /// What the map should become after a sweep. + /// + /// `nil` means the helper could not be asked, so the previous map stands — dropping it + /// would make labels flicker between polls. An empty array is a real answer and must be + /// applied: utun numbers are recycled, so keeping a departed tunnel's entry would + /// eventually label an unrelated tunnel with it. + nonisolated static func mergedTunnelOwners( + previous: [String: TunnelOwner], fetched: [TunnelOwner]? + ) -> [String: TunnelOwner] { + guard let fetched else { return previous } + return Dictionary(fetched.map { ($0.interface, $0) }, uniquingKeysWith: { first, _ in first }) } private func detectVPNTypeFromInterface(_ iface: String) -> VPNType { diff --git a/Tests/VPNBypassTests/TunnelOwnershipTests.swift b/Tests/VPNBypassTests/TunnelOwnershipTests.swift index 4d8a8f6..822a2e1 100644 --- a/Tests/VPNBypassTests/TunnelOwnershipTests.swift +++ b/Tests/VPNBypassTests/TunnelOwnershipTests.swift @@ -160,4 +160,37 @@ final class TunnelOwnershipTests: XCTestCase { XCTAssertNil(TunnelOwner(dictionary: ["interface": "utun5", "pid": "not-a-number", "processName": "x"])) } + + // MARK: - Could-not-ask vs genuinely-none + + /// utun numbers are recycled. If Tailscale releases utun2 and a mesh VPN later takes it, + /// a map that was never cleared would label the new tunnel "Tailscale". So a sweep that + /// ran and found nothing MUST clear, even though it looks like an empty answer. + func testAnEmptySweepClearsTheMapBecauseUtunNumbersAreRecycled() { + let stale = ["utun2": TunnelOwner(interface: "utun2", pid: 543, + processName: "io.tailscale.ipn.macsys.network", + executablePath: "/Library/SystemExtensions/X")] + XCTAssertTrue(RouteManager.mergedTunnelOwners(previous: stale, fetched: []).isEmpty, + "a sweep that ran and found nothing is an answer, not a failure") + } + + /// Whereas not being able to ask — no helper, an older helper without the method, a + /// timeout — must leave the map alone, or labels would flicker between polls. + func testAFailedSweepKeepsThePreviousMap() { + let known = ["utun2": TunnelOwner(interface: "utun2", pid: 543, processName: "tailscaled", + executablePath: "/opt/homebrew/bin/tailscaled")] + XCTAssertEqual(RouteManager.mergedTunnelOwners(previous: known, fetched: nil), known) + } + + /// And a successful sweep replaces wholesale rather than merging, so a tunnel that went + /// away leaves no trace behind for a recycled number to inherit. + func testASuccessfulSweepReplacesRatherThanMerges() { + let stale = ["utun2": TunnelOwner(interface: "utun2", pid: 1, processName: "old", + executablePath: "/old")] + let fresh = [TunnelOwner(interface: "utun5", pid: 2, processName: "netbird", + executablePath: "/opt/homebrew/bin/netbird")] + let merged = RouteManager.mergedTunnelOwners(previous: stale, fetched: fresh) + XCTAssertNil(merged["utun2"], "the departed tunnel must not linger") + XCTAssertEqual(merged["utun5"].map(TunnelOwnership.displayLabel), "NetBird") + } }