diff --git a/Sources/VPNBypassCore/ClassicRouteCompiler.swift b/Sources/VPNBypassCore/ClassicRouteCompiler.swift index 9ef5f29..4850758 100644 --- a/Sources/VPNBypassCore/ClassicRouteCompiler.swift +++ b/Sources/VPNBypassCore/ClassicRouteCompiler.swift @@ -19,6 +19,51 @@ import Foundation enum ClassicRouteCompiler { + /// The bypass-all routes VPN Only installs: four /2s covering all of IPv4 via the local + /// gateway. NOT the classic 0.0.0.0/1 + 128.0.0.0/1 pair, deliberately: wg-quick and + /// OpenVPN's redirect-gateway def1 capture traffic by installing that exact pair + /// themselves, so claiming it did not add routes — it REPOINTED the VPN's own routes to + /// the local gateway (the helper converges an existing destination in place) and teardown + /// then DELETED them out from under the VPN (#103: VPN Only under WireGuard inverted, and + /// WireGuard could not connect while the app ran, both sides fighting over the same two + /// kernel entries). The /2 quartet is additive: longest-prefix beats the VPN's /1s without + /// touching them, listed destinations still win over /2 with their /32s and CIDRs, and + /// removing the quartet hands traffic straight back to the tunnel. + static let bypassAllCatchAlls: [String] = [ + "0.0.0.0/2", "64.0.0.0/2", "128.0.0.0/2", "192.0.0.0/2", + ] + + /// The quartet minus every quarter an inverse CIDR claims (equals or covers). One kernel + /// entry exists per destination, and a broader listed CIDR must keep the whole space it + /// names on the VPN — installing a more-specific local /2 inside it would silently carve + /// that traffic back out of the tunnel. Shared by the compiler and the DNS refresh planner + /// so their ownership views can never diverge. + static func unclaimedCatchAlls(inverseCIDRs: [String]) -> [String] { + bypassAllCatchAlls.filter { quarter in + !inverseCIDRs.contains { covers($0, quarter) } + } + } + + /// True when `outer` (a well-formed IPv4 CIDR) contains the whole of `inner`. + static func covers(_ outer: String, _ inner: String) -> Bool { + guard let o = RouteCIDR.parse(outer), let i = RouteCIDR.parse(inner), + o.prefixLength <= i.prefixLength, + let oAddr = ipv4Value(o.network), let iAddr = ipv4Value(i.network) else { return false } + let mask: UInt32 = o.prefixLength == 0 ? 0 : UInt32.max << (32 - UInt32(o.prefixLength)) + return (oAddr & mask) == (iAddr & mask) + } + + private static func ipv4Value(_ dotted: String) -> UInt32? { + let parts = dotted.split(separator: ".").compactMap { UInt32($0) } + guard parts.count == 4, parts.allSatisfy({ $0 <= 255 }) else { return nil } + return (parts[0] << 24) | (parts[1] << 16) | (parts[2] << 8) | parts[3] + } + + /// The ownership `source` every bypass-all catch-all is recorded under. Stale-route + /// classification keys off destination AND this source, so a user's own route that merely + /// shares a catch-all destination string is never mistaken for one of ours. + static let catchAllSource = "VPN Only catch-all" + /// A kernel route to install (matches the caller's `routesToAdd` tuple, as a testable value). struct Route: Equatable, Hashable { let destination: String @@ -69,8 +114,9 @@ enum ClassicRouteCompiler { var allSourceEntries: [SourceEntry] = [] var seenSourceDests: Set = [] // dedup (source, destination) ownership pairs - // VPN Only: catch-all through the local gateway (0.0.0.0/1 + 128.0.0.0/1 cover all IPv4 - // with higher specificity than the default route), then inverse CIDRs through the VPN. + // VPN Only: bypass-all through the local gateway (the /2 quartet covers all IPv4 with + // higher specificity than both a default route AND a /1-pair full tunnel — see + // bypassAllCatchAlls), then inverse CIDRs through the VPN. // // INSTALL ORDER IS LEAK-CRITICAL. The helper writes this array strictly in order, so the // catch-alls are deferred to the very END rather than emitted here. Installing them first @@ -85,15 +131,6 @@ enum ClassicRouteCompiler { // Same principle, opposite end: the catch-alls are the last thing on and the first thing off. var deferredCatchAlls: [Route] = [] if isInverse { - deferredCatchAlls.append(Route(destination: "0.0.0.0/1", gateway: localGateway, isNetwork: true, source: "VPN Only catch-all")) - deferredCatchAlls.append(Route(destination: "128.0.0.0/1", gateway: localGateway, isNetwork: true, source: "VPN Only catch-all")) - seenDestinations.insert("0.0.0.0/1") - seenDestinations.insert("128.0.0.0/1") - allSourceEntries.append(SourceEntry(destination: "0.0.0.0/1", gateway: localGateway, source: "VPN Only catch-all")) - allSourceEntries.append(SourceEntry(destination: "128.0.0.0/1", gateway: localGateway, source: "VPN Only catch-all")) - seenSourceDests.insert("VPN Only catch-all|0.0.0.0/1") - seenSourceDests.insert("VPN Only catch-all|128.0.0.0/1") - for cidr in inverseCIDRs { if !seenDestinations.contains(cidr) { seenDestinations.insert(cidr) @@ -105,6 +142,17 @@ enum ClassicRouteCompiler { allSourceEntries.append(SourceEntry(destination: cidr, gateway: routeGateway, source: cidr)) } } + + // AFTER the inverse CIDRs, and only the quarters no listed CIDR claims: an exact + // /2 owns its kernel entry outright, and a broader CIDR (a /1) must keep every + // quarter inside it on the VPN — a more-specific local /2 would silently carve + // that traffic back out of the tunnel. Unclaimed quarters still go local. + for catchAll in unclaimedCatchAlls(inverseCIDRs: inverseCIDRs) where !seenDestinations.contains(catchAll) { + deferredCatchAlls.append(Route(destination: catchAll, gateway: localGateway, isNetwork: true, source: catchAllSource)) + seenDestinations.insert(catchAll) + allSourceEntries.append(SourceEntry(destination: catchAll, gateway: localGateway, source: catchAllSource)) + seenSourceDests.insert("\(catchAllSource)|\(catchAll)") + } } // Resolved domain / service-domain IPs (host routes) through the route gateway. diff --git a/Sources/VPNBypassCore/CommandRouter.swift b/Sources/VPNBypassCore/CommandRouter.swift index b90b00e..cc37fa8 100644 --- a/Sources/VPNBypassCore/CommandRouter.swift +++ b/Sources/VPNBypassCore/CommandRouter.swift @@ -496,8 +496,8 @@ enum CommandRouter { /// Validates a rule MATCH pattern (RuleResolver matches traffic IPs against it), NOT a /// kernel route destination. `/0` is intentionally allowed here — it means "match any /// IPv4" (a catch-all rule) — whereas RouteManager.isValidCIDR rejects `/0` for route - /// *destinations* because it collides with the VPN-Only `0.0.0.0/1`+`128.0.0.0/1` - /// catch-all. Different purposes; the `/0` difference is deliberate, not drift. + /// *destinations* because it collides with the VPN-Only bypass-all + /// catch-alls. Different purposes; the `/0` difference is deliberate, not drift. private static func isValidCIDR(_ s: String) -> Bool { let parts = s.split(separator: "/", omittingEmptySubsequences: false) guard parts.count == 2, let bits = Int(parts[1]), (0...32).contains(bits) else { return false } diff --git a/Sources/VPNBypassCore/DNSRefreshPlanner.swift b/Sources/VPNBypassCore/DNSRefreshPlanner.swift index 6071283..dc130da 100644 --- a/Sources/VPNBypassCore/DNSRefreshPlanner.swift +++ b/Sources/VPNBypassCore/DNSRefreshPlanner.swift @@ -71,7 +71,7 @@ enum DNSRefreshPlanner { /// - cachedDomainIPs: the disk-cache snapshot, consulted only for the DNS-failed fallback. /// - existingDestinations: kernel destinations already present (never re-added). /// - existingSourceDests: ownership pairs already tracked (never re-recorded). - /// - isInverse: `true` for VPN Only (seeds the two catch-alls), `false` for Bypass. + /// - isInverse: `true` for VPN Only (seeds the bypass-all catch-alls), `false` for Bypass. /// - routeGateway: the gateway every refreshed host route + ownership row rides. /// - inverseCIDRs: enabled inverse CIDR entries (VPN Only). Seeded into `expectedEntries` as /// `(cidr, cidr)`, never DNS-resolved or added here — the caller repairs their kernel route. @@ -93,8 +93,13 @@ enum DNSRefreshPlanner { // Preserve catch-all routes in VPN Only mode (they aren't DNS-resolved), then seed the // static inverse CIDR entries. Both are expected but never added by this planner. if isInverse { - expectedEntries.insert(SourceDest(source: "VPN Only catch-all", destination: "0.0.0.0/1")) - expectedEntries.insert(SourceDest(source: "VPN Only catch-all", destination: "128.0.0.0/1")) + // Mirror the compiler's claim rules exactly: a quarter an inverse CIDR owns is + // never installed as a catch-all, so expecting it here would record a second + // ownership row for the same destination and make the CIDR's later removal skip + // the kernel delete ("another owner still wants it"). + for catchAll in ClassicRouteCompiler.unclaimedCatchAlls(inverseCIDRs: inverseCIDRs) { + expectedEntries.insert(SourceDest(source: ClassicRouteCompiler.catchAllSource, destination: catchAll)) + } for cidr in inverseCIDRs { // CIDR entries: preserve as static routes, no DNS resolution. expectedEntries.insert(SourceDest(source: cidr, destination: cidr)) diff --git a/Sources/VPNBypassCore/RouteCompiler.swift b/Sources/VPNBypassCore/RouteCompiler.swift index 1167b20..40faa5a 100644 --- a/Sources/VPNBypassCore/RouteCompiler.swift +++ b/Sources/VPNBypassCore/RouteCompiler.swift @@ -189,7 +189,16 @@ enum RouteCompiler { /// trips its route monitor and tears the tunnel down (the original incident that /// motivates the whole project). Custom mode is per-rule and should never produce /// these, but the guard is the custom-engine analog of refuseVPNOnlyUnderGlobalProtect. - static let catchAllDestinations: Set = ["0.0.0.0/0", "0.0.0.0/1", "128.0.0.0/1"] + /// The set carries everything this app has EVER installed as a bypass-all: the /2 quartet + /// VPN Only installs today, the 0.0.0.0/1 + 128.0.0.0/1 pair every build up to 4.8.0 + /// installed (teardown and strand-sweeps must still recognise what an older build left + /// behind), and the custom 0.0.0.0/0. Cleanup ordering, unstranding and stale-route + /// recognition key off this set; the structural-shadow predicate below deliberately + /// does not. + static let catchAllDestinations: Set = [ + "0.0.0.0/0", "0.0.0.0/1", "128.0.0.0/1", + "0.0.0.0/2", "64.0.0.0/2", "128.0.0.0/2", "192.0.0.0/2", + ] /// A destination that structurally shadows a full-tunnel VPN's default route. /// The canonical trio, PLUS any CIDR with prefix length <= 1 (a /0 or /1 covers @@ -198,7 +207,11 @@ enum RouteCompiler { /// not a replacement, so it doesn't trip GP's route monitor — that's the user's /// explicit choice, not a teardown vector. Covers IPv4 and IPv6 (::/0) alike. static func isCatchAll(_ destination: String) -> Bool { - if catchAllDestinations.contains(destination) { return true } + // Deliberately NOT keyed off catchAllDestinations: that set now also carries the /2 + // quartet VPN Only installs (and must clean up), and a /2 is additive, not a shadow — + // a user's explicit /2 rule under GlobalProtect stays allowed, exactly as before. + // For every input this is byte-identical to the old set-plus-prefix check: the old + // set's members all had prefix <= 1 themselves. let parts = destination.split(separator: "/") if parts.count == 2, let prefix = Int(parts[1]), prefix <= 1 { return true } return false diff --git a/Sources/VPNBypassCore/RouteKernel.swift b/Sources/VPNBypassCore/RouteKernel.swift index 835aa84..1aadb59 100644 --- a/Sources/VPNBypassCore/RouteKernel.swift +++ b/Sources/VPNBypassCore/RouteKernel.swift @@ -320,6 +320,34 @@ public enum RouteKernel { return value } + /// The tunnel that owns the 0.0.0.0/1 + 128.0.0.0/1 full-tunnel pair, if any. + /// + /// wg-quick and OpenVPN's redirect-gateway def1 capture traffic by installing that pair and + /// leaving `default` on the physical link, so `route get default` — selection's usual ground + /// truth — keeps naming the physical interface while every packet actually enters the tunnel. + /// By longest-prefix match the /1 owner IS the traffic carrier. Our own routes are excluded + /// by their RTF_PROTO1 mark (and are /2s besides). Only interface-gatewayed (AF_LINK) rows + /// are attributable — an OpenVPN-style /1 via an AF_INET next hop carries no interface index + /// in the dump, and returning nil there falls back to today's behaviour. + public static func slashOneTunnelOwnerIndex(_ table: [KernelRoute]) -> UInt16? { + for route in table where !route.isOurs && route.prefix == 1 { + guard route.destination == 0 || route.destination == 0x8000_0000 else { continue } + if let index = route.gatewayInterfaceIndex { return index } + } + return nil + } + + /// `slashOneTunnelOwnerIndex` resolved to a name, kept only when it names a tunnel-class + /// interface — a /1 pinned to a physical link is not a VPN and must not steer selection. + public static func slashOneTunnelOwner(_ table: [KernelRoute]) -> String? { + guard let index = slashOneTunnelOwnerIndex(table) else { return nil } + var buffer = [CChar](repeating: 0, count: Int(IFNAMSIZ) + 1) + guard if_indextoname(UInt32(index), &buffer) != nil else { return nil } + let name = String(cString: buffer) + let tunnelPrefixes = ["utun", "tun", "tap", "ppp", "ipsec"] + return tunnelPrefixes.contains(where: { name.hasPrefix($0) }) ? name : nil + } + static func dotted(_ ip: UInt32) -> String { "\((ip >> 24) & 0xff).\((ip >> 16) & 0xff).\((ip >> 8) & 0xff).\(ip & 0xff)" } diff --git a/Sources/VPNBypassCore/RouteManager.swift b/Sources/VPNBypassCore/RouteManager.swift index 6b9f2be..2932a81 100644 --- a/Sources/VPNBypassCore/RouteManager.swift +++ b/Sources/VPNBypassCore/RouteManager.swift @@ -1018,7 +1018,7 @@ final class RouteManager: ObservableObject { )) } - let defaultRouteInterface = await currentDefaultRouteInterface() + let defaultRouteInterface = await currentTrafficCarrierInterface() // Resolve the user's pin, if any. utun indices renumber across reconnects, so when the // pinned NAME is gone but a product label was stored, re-resolve by label against the @@ -1060,7 +1060,7 @@ final class RouteManager: ObservableObject { let all = candidates.map(\.interface).joined(separator: ", ") await MainActor.run { log(.info, "Multiple tunnels up (\(all)) — using \(selected)" + - (defaultRouteInterface == selected ? " (carries the default route)" : "")) + (defaultRouteInterface == selected ? " (carries the traffic)" : "")) } } let type = hintType ?? detectVPNTypeFromInterface(selected) @@ -1080,6 +1080,7 @@ final class RouteManager: ObservableObject { /// Which interface the default route currently exits through, or nil if unreadable. /// This is the only direct evidence of which tunnel is actually carrying traffic. + /// The plain `route get default` answer — what the coexistence diagnostics card shows. func currentDefaultRouteInterface() async -> String? { guard let result = await runProcessAsync("/sbin/route", arguments: ["-n", "get", "default"], timeout: 5.0) else { return nil @@ -1087,6 +1088,25 @@ final class RouteManager: ObservableObject { return VPNInterfaceSelector.parseDefaultRouteInterface(result.output) } + /// The interface actually carrying the traffic — selection's ground truth. + /// + /// A wg-quick / OpenVPN-def1 style tunnel owns 0.0.0.0/1 + 128.0.0.0/1 and leaves + /// `default` on the physical link, so the default route's interface names the wrong + /// carrier: by longest-prefix the /1 owner is where the traffic actually goes. Consult + /// the kernel table for such an owner first (our own catch-alls are RTF_PROTO1-marked + /// and excluded), then fall back to the default route. Without this, selection rule 1 + /// never fires for a /1-style VPN, and hysteresis pins whichever tunnel happened to + /// connect first — observed in #103 as NetBird staying "the VPN" while the user's + /// WireGuard carried the traffic. Kept separate from currentDefaultRouteInterface so + /// the diagnostics card keeps reporting the real default route. + func currentTrafficCarrierInterface() async -> String? { + if let table = RouteKernel.currentTable(), + let owner = RouteKernel.slashOneTunnelOwner(table) { + return owner + } + return await currentDefaultRouteInterface() + } + /// Check if interface name suggests it's a VPN interface private func isVPNInterface(_ iface: String) -> Bool { // Common VPN interface prefixes @@ -1576,7 +1596,7 @@ final class RouteManager: ObservableObject { log(.warning, "Config failed to load — enforcing nothing, and removing anything this app left behind") // The latch blocks APPLY, never TEARDOWN. This method is the only caller of the // no-VPN startup sweep (:1535), which is what heals a killed previous run's - // leftovers — VPN Only's 0.0.0.0/1 + 128.0.0.0/1 catch-alls included. Returning + // leftovers — VPN Only's bypass-all catch-alls included. Returning // early here left those in the kernel with nothing able to remove them, so an // unreadable config.json turned a recoverable state into a broken network. if HelperManager.shared.isHelperInstalled { @@ -1713,7 +1733,7 @@ final class RouteManager: ObservableObject { await applyAllRoutesInternal(sendNotification: true, forceReassert: true) } - /// VPN Only mode installs 0.0.0.0/1 + 128.0.0.0/1 catch-all routes that + /// VPN Only mode installs bypass-all catch-all routes that /// structurally defeat a full-tunnel VPN. Under GlobalProtect that trips its /// route monitor and tears the tunnel down (the original incident), so EVERY /// route-applying path must refuse it. Returns true (and logs) when the apply @@ -1735,7 +1755,11 @@ final class RouteManager: ObservableObject { catchAlls: Set ) -> [String] { activeRoutes.compactMap { route in - if catchAlls.contains(route.destination) { return route.destination } + // Destination alone is not ownership: a user's Bypass range may spell exactly + // "0.0.0.0/2". Only OUR catch-alls (matched by their recorded source) are stale + // by fiat; the user's route is judged by its gateway like everything else. + if catchAlls.contains(route.destination), + route.source == ClassicRouteCompiler.catchAllSource { return route.destination } // Interface-scoped routes name a tunnel that is going away. if route.gateway.hasPrefix("iface:") { return route.destination } // Anything not egressing via the local gateway is VPN-bound. @@ -1748,7 +1772,9 @@ final class RouteManager: ObservableObject { /// remove what is already installed rather than only declining to add more. private var installedCatchAllDestinations: [String] { let catchAlls = Set(RouteCompiler.catchAllDestinations) - return activeRoutes.map { $0.destination }.filter { catchAlls.contains($0) } + return activeRoutes + .filter { catchAlls.contains($0.destination) && $0.source == ClassicRouteCompiler.catchAllSource } + .map { $0.destination } } private func refuseVPNOnlyUnderGlobalProtect() -> Bool { @@ -2260,7 +2286,7 @@ final class RouteManager: ObservableObject { var failedDests: Set = [] if !destinations.isEmpty { if HelperManager.shared.isHelperInstalled { - // Remove the catch-all routes (0.0.0.0/1 + 128.0.0.0/1, or a custom 0.0.0.0/0) + // Remove the catch-all routes (the bypass-all set, or a custom 0.0.0.0/0) // FIRST, in their own fast batch. If a time-capped quit cuts teardown short, // the full-tunnel-defeating catch-alls are already gone rather than stranded // (leaving the machine forcing all traffic at a now-dead gateway). @@ -2345,9 +2371,9 @@ final class RouteManager: ObservableObject { let isInverse = config.routingMode == .vpnOnly - // Refuse VPN Only under GlobalProtect on this startup fast-path too — it - // installs the same 0.0.0.0/1 + 128.0.0.0/1 catch-all and would tear down - // the GP tunnel on the common cached-launch path. + // Refuse VPN Only under GlobalProtect on this startup fast-path too — its + // bypass-all catch-alls route traffic around the corporate tunnel on the + // common cached-launch path. if refuseVPNOnlyUnderGlobalProtect() { return false } // Custom mode: compile from cached IPs (no live DNS) for instant startup. diff --git a/Sources/VPNBypassCore/VPNBypassApp.swift b/Sources/VPNBypassCore/VPNBypassApp.swift index 2db8b1b..a940283 100644 --- a/Sources/VPNBypassCore/VPNBypassApp.swift +++ b/Sources/VPNBypassCore/VPNBypassApp.swift @@ -142,7 +142,7 @@ class AppDelegate: NSObject, NSApplicationDelegate { /// sends (`pkill -x VPNBypass`) on every upgrade. Because `activeRoutes` lives only in memory /// and nothing reconciles against the kernel, an upgrade previously left the entire route set /// installed but *untracked*: the replacement binary started with an empty model, so no later - /// cleanup could ever remove them. In VPN Only that strands the `0.0.0.0/1`+`128.0.0.0/1` + /// cleanup could ever remove them. In VPN Only that strands the bypass-all /// catch-alls pointing at the local gateway — every subsequent connection silently leaves the /// tunnel while the app reports itself healthy. That is the most likely mechanism behind the /// "app is on but my public IP is my real one" reports. diff --git a/Tests/VPNBypassTests/ClassicRouteCompilerTests.swift b/Tests/VPNBypassTests/ClassicRouteCompilerTests.swift index 5ef0832..8dde96c 100644 --- a/Tests/VPNBypassTests/ClassicRouteCompilerTests.swift +++ b/Tests/VPNBypassTests/ClassicRouteCompilerTests.swift @@ -43,7 +43,7 @@ final class ClassicRouteCompilerTests: XCTestCase { inverseCIDRs: [], resolvedGroups: [], serviceRanges: []) XCTAssertTrue(b.routesToAdd.isEmpty) XCTAssertTrue(b.allSourceEntries.isEmpty) - XCTAssertFalse(b.routesToAdd.contains { $0.destination == "0.0.0.0/1" }) + XCTAssertFalse(b.routesToAdd.contains { RouteCompiler.catchAllDestinations.contains($0.destination) }) } func testBypassSameIPFromTwoSourcesIsOneRouteButTwoOwnershipEntries() { @@ -116,12 +116,16 @@ final class ClassicRouteCompilerTests: XCTestCase { serviceRanges: []) XCTAssertEqual(b.routesToAdd, [ route("3.3.3.3", vpn, false, "x.com"), // domain IP rides the VPN gateway — installed FIRST - route("0.0.0.0/1", local, true, "VPN Only catch-all"), - route("128.0.0.0/1", local, true, "VPN Only catch-all"), + route("0.0.0.0/2", local, true, "VPN Only catch-all"), + route("64.0.0.0/2", local, true, "VPN Only catch-all"), + route("128.0.0.0/2", local, true, "VPN Only catch-all"), + route("192.0.0.0/2", local, true, "VPN Only catch-all"), ]) XCTAssertEqual(b.allSourceEntries, [ - entry("0.0.0.0/1", local, "VPN Only catch-all"), - entry("128.0.0.0/1", local, "VPN Only catch-all"), + entry("0.0.0.0/2", local, "VPN Only catch-all"), + entry("64.0.0.0/2", local, "VPN Only catch-all"), + entry("128.0.0.0/2", local, "VPN Only catch-all"), + entry("192.0.0.0/2", local, "VPN Only catch-all"), entry("3.3.3.3", vpn, "x.com"), ]) } @@ -131,8 +135,10 @@ final class ClassicRouteCompilerTests: XCTestCase { inverseCIDRs: ["172.16.0.0/12"], resolvedGroups: [], serviceRanges: []) XCTAssertEqual(b.routesToAdd, [ route("172.16.0.0/12", vpn, true, "172.16.0.0/12"), - route("0.0.0.0/1", local, true, "VPN Only catch-all"), - route("128.0.0.0/1", local, true, "VPN Only catch-all"), + route("0.0.0.0/2", local, true, "VPN Only catch-all"), + route("64.0.0.0/2", local, true, "VPN Only catch-all"), + route("128.0.0.0/2", local, true, "VPN Only catch-all"), + route("192.0.0.0/2", local, true, "VPN Only catch-all"), ]) } @@ -143,7 +149,7 @@ final class ClassicRouteCompilerTests: XCTestCase { inverseCIDRs: ["172.16.0.0/12", "10.1.0.0/16"], resolvedGroups: [C.ResolvedGroup(source: "a.com", ips: ["4.4.4.4", "5.5.5.5"])], serviceRanges: []) - let catchAlls: Set = ["0.0.0.0/1", "128.0.0.0/1"] + let catchAlls = Set(C.bypassAllCatchAlls) let firstCatchAll = b.routesToAdd.firstIndex { catchAlls.contains($0.destination) } let lastProtected = b.routesToAdd.lastIndex { !catchAlls.contains($0.destination) } XCTAssertNotNil(firstCatchAll, "VPN Only must install catch-alls") @@ -154,6 +160,52 @@ final class ClassicRouteCompilerTests: XCTestCase { } } + /// #104 review: a listed inverse CIDR that is exactly one of the quarters must keep its + /// VPN route — that quarter is simply not claimed for the local gateway (one kernel entry + /// exists per destination, and the user's explicit choice wins it). + func testExactQuarterInverseCIDRKeepsItsVPNRoute() { + let b = C.build(isInverse: true, localGateway: local, routeGateway: vpn, + inverseCIDRs: ["0.0.0.0/2"], resolvedGroups: [], serviceRanges: []) + XCTAssertEqual(b.routesToAdd, [ + route("0.0.0.0/2", vpn, true, "0.0.0.0/2"), + route("64.0.0.0/2", local, true, "VPN Only catch-all"), + route("128.0.0.0/2", local, true, "VPN Only catch-all"), + route("192.0.0.0/2", local, true, "VPN Only catch-all"), + ]) + XCTAssertEqual(b.allSourceEntries.filter { $0.destination == "0.0.0.0/2" }, + [entry("0.0.0.0/2", vpn, "0.0.0.0/2")]) + } + + /// #104 review: a listed /1 covers TWO quarters — both must stay on the VPN. Installing a + /// more-specific local /2 inside the user's /1 would silently carve traffic back out. + func testBroadInverseCIDRClaimsEveryQuarterItCovers() { + let b = C.build(isInverse: true, localGateway: local, routeGateway: vpn, + inverseCIDRs: ["0.0.0.0/1"], resolvedGroups: [], serviceRanges: []) + XCTAssertEqual(b.routesToAdd, [ + route("0.0.0.0/1", vpn, true, "0.0.0.0/1"), + route("128.0.0.0/2", local, true, "VPN Only catch-all"), + route("192.0.0.0/2", local, true, "VPN Only catch-all"), + ]) + } + + /// A CIDR narrower than /2 claims nothing — the quarter still carries the REST of its + /// space to the local gateway, and the narrower CIDR wins inside itself by prefix length. + func testNarrowInverseCIDRClaimsNoQuarter() { + let b = C.build(isInverse: true, localGateway: local, routeGateway: vpn, + inverseCIDRs: ["10.0.0.0/8"], resolvedGroups: [], serviceRanges: []) + XCTAssertEqual(b.routesToAdd.filter { $0.source == C.catchAllSource }.count, 4) + } + + func testCoversMatrix() { + XCTAssertTrue(C.covers("0.0.0.0/1", "0.0.0.0/2")) + XCTAssertTrue(C.covers("0.0.0.0/1", "64.0.0.0/2")) + XCTAssertFalse(C.covers("0.0.0.0/1", "128.0.0.0/2")) + XCTAssertTrue(C.covers("128.0.0.0/1", "192.0.0.0/2")) + XCTAssertTrue(C.covers("64.0.0.0/2", "64.0.0.0/2")) + XCTAssertFalse(C.covers("0.0.0.0/3", "0.0.0.0/2")) + XCTAssertFalse(C.covers("garbage", "0.0.0.0/2")) + } + func testVPNOnlyDuplicateInverseCIDRDeduped() { let b = C.build(isInverse: true, localGateway: local, routeGateway: vpn, inverseCIDRs: ["172.16.0.0/12", "172.16.0.0/12"], @@ -170,8 +222,10 @@ final class ClassicRouteCompilerTests: XCTestCase { inverseCIDRs: [], resolvedGroups: [], serviceRanges: [(source: "svc", range: "10.0.0.0/8")]) XCTAssertEqual(b.routesToAdd, [ - route("0.0.0.0/1", local, true, "VPN Only catch-all"), - route("128.0.0.0/1", local, true, "VPN Only catch-all"), + route("0.0.0.0/2", local, true, "VPN Only catch-all"), + route("64.0.0.0/2", local, true, "VPN Only catch-all"), + route("128.0.0.0/2", local, true, "VPN Only catch-all"), + route("192.0.0.0/2", local, true, "VPN Only catch-all"), ]) XCTAssertFalse(b.routesToAdd.contains { $0.destination == "10.0.0.0/8" }) } @@ -180,10 +234,12 @@ final class ClassicRouteCompilerTests: XCTestCase { let b = C.build(isInverse: true, localGateway: local, routeGateway: vpn, inverseCIDRs: [], resolvedGroups: [], serviceRanges: []) XCTAssertEqual(b.routesToAdd, [ - route("0.0.0.0/1", local, true, "VPN Only catch-all"), - route("128.0.0.0/1", local, true, "VPN Only catch-all"), + route("0.0.0.0/2", local, true, "VPN Only catch-all"), + route("64.0.0.0/2", local, true, "VPN Only catch-all"), + route("128.0.0.0/2", local, true, "VPN Only catch-all"), + route("192.0.0.0/2", local, true, "VPN Only catch-all"), ]) - XCTAssertEqual(b.allSourceEntries.count, 2) + XCTAssertEqual(b.allSourceEntries.count, C.bypassAllCatchAlls.count) } // MARK: - Invariants that guard against a leak diff --git a/Tests/VPNBypassTests/DNSRefreshPlannerTests.swift b/Tests/VPNBypassTests/DNSRefreshPlannerTests.swift index dca4687..ff4506c 100644 --- a/Tests/VPNBypassTests/DNSRefreshPlannerTests.swift +++ b/Tests/VPNBypassTests/DNSRefreshPlannerTests.swift @@ -115,7 +115,7 @@ final class DNSRefreshPlannerTests: XCTestCase { // MARK: - VPN Only (inverse) mode - /// Inverse mode injects the two catch-alls as expected (never added), seeds each CIDR as a + /// Inverse mode injects the bypass-all catch-alls as expected (never added), seeds each CIDR as a /// static expected entry (never added), and adds only the resolved domain IP via the VPN gateway. func testInverseSeedsCatchAllsAndCIDRsWhichAreExpectedNotAdded() { let plan = P.plan( @@ -133,8 +133,10 @@ final class DNSRefreshPlannerTests: XCTestCase { XCTAssertEqual(plan.candidateActiveEntries, [ce("3.3.3.3", vpn, "x.com")]) // Catch-alls + CIDR + resolved IP are all expected. XCTAssertEqual(plan.expectedEntries, [ - sd("VPN Only catch-all", "0.0.0.0/1"), - sd("VPN Only catch-all", "128.0.0.0/1"), + sd("VPN Only catch-all", "0.0.0.0/2"), + sd("VPN Only catch-all", "64.0.0.0/2"), + sd("VPN Only catch-all", "128.0.0.0/2"), + sd("VPN Only catch-all", "192.0.0.0/2"), sd("172.16.0.0/12", "172.16.0.0/12"), sd("x.com", "3.3.3.3"), ]) @@ -142,6 +144,24 @@ final class DNSRefreshPlannerTests: XCTestCase { XCTAssertFalse(plan.routesToAdd.contains { $0.destination == "172.16.0.0/12" }) } + /// #104 review: a quarter an inverse CIDR claims is never installed as a catch-all, so it + /// must not be EXPECTED as one either — a second ownership row for the same destination + /// would make the CIDR's later removal skip the kernel delete. + func testClaimedQuarterIsNotExpectedAsCatchAll() { + let plan = P.plan( + domainsToResolve: [], resolvedDomainIPs: [:], cachedDomainIPs: [:], + existingDestinations: [], existingSourceDests: [], + isInverse: true, routeGateway: vpn, + inverseCIDRs: ["0.0.0.0/2"] + ) + XCTAssertEqual(plan.expectedEntries, [ + sd("VPN Only catch-all", "64.0.0.0/2"), + sd("VPN Only catch-all", "128.0.0.0/2"), + sd("VPN Only catch-all", "192.0.0.0/2"), + sd("0.0.0.0/2", "0.0.0.0/2"), + ]) + } + /// Inverse mode with no resolvable domains: catch-alls + every CIDR are expected, nothing is /// added, and there are no ownership candidates. func testInverseEmptyDomainsIsCatchAllsAndCIDRsOnly() { @@ -158,8 +178,10 @@ final class DNSRefreshPlannerTests: XCTestCase { XCTAssertTrue(plan.routesToAdd.isEmpty) XCTAssertTrue(plan.candidateActiveEntries.isEmpty) XCTAssertEqual(plan.expectedEntries, [ - sd("VPN Only catch-all", "0.0.0.0/1"), - sd("VPN Only catch-all", "128.0.0.0/1"), + sd("VPN Only catch-all", "0.0.0.0/2"), + sd("VPN Only catch-all", "64.0.0.0/2"), + sd("VPN Only catch-all", "128.0.0.0/2"), + sd("VPN Only catch-all", "192.0.0.0/2"), sd("172.16.0.0/12", "172.16.0.0/12"), sd("10.0.0.0/8", "10.0.0.0/8"), ]) @@ -201,7 +223,7 @@ final class DNSRefreshPlannerTests: XCTestCase { /// The VPN-Only leak path: an inverse domain whose DNS fails falls back to its cached IPs. /// Those cached IPs must be expected (reconcile-protected) but NEVER added to the kernel and - /// NEVER become ownership candidates — and the two catch-alls are still seeded. + /// NEVER become ownership candidates — and the four /2 catch-alls are still seeded. func testInverseDNSFailedFallsBackToCacheExpectedButNotAdded() { let plan = P.plan( domainsToResolve: [(domain: "v.com", source: "v.com")], @@ -219,8 +241,10 @@ final class DNSRefreshPlannerTests: XCTestCase { XCTAssertFalse(plan.routesToAdd.contains { $0.destination == "7.7.7.7" || $0.destination == "6.6.6.6" }) // ...but they ARE expected, alongside the always-seeded catch-alls. XCTAssertEqual(plan.expectedEntries, [ - sd("VPN Only catch-all", "0.0.0.0/1"), - sd("VPN Only catch-all", "128.0.0.0/1"), + sd("VPN Only catch-all", "0.0.0.0/2"), + sd("VPN Only catch-all", "64.0.0.0/2"), + sd("VPN Only catch-all", "128.0.0.0/2"), + sd("VPN Only catch-all", "192.0.0.0/2"), sd("v.com", "7.7.7.7"), sd("v.com", "6.6.6.6"), ]) } diff --git a/Tests/VPNBypassTests/FullTunnelSlashOneTests.swift b/Tests/VPNBypassTests/FullTunnelSlashOneTests.swift new file mode 100644 index 0000000..9f66609 --- /dev/null +++ b/Tests/VPNBypassTests/FullTunnelSlashOneTests.swift @@ -0,0 +1,100 @@ +// FullTunnelSlashOneTests.swift +// #103: a wg-quick / OpenVPN-def1 style VPN owns 0.0.0.0/1 + 128.0.0.0/1 and leaves `default` +// on the physical link. These pin the pure decisions the fix rests on: the /1 owner is +// selection's ground truth, and the structural-shadow predicate stays byte-identical so a +// user's explicit /2 rule remains allowed under GlobalProtect. + +import XCTest +import Darwin +@testable import VPNBypassCore + +final class FullTunnelSlashOneTests: XCTestCase { + + private func kr(_ destination: UInt32, prefix: Int?, ifIndex: UInt16? = nil, + gateway: UInt32? = nil, flags: Int32 = 0) -> RouteKernel.KernelRoute { + RouteKernel.KernelRoute(destination: destination, prefix: prefix, + gatewayAddress: gateway, gatewayInterfaceIndex: ifIndex, + flags: flags) + } + + // MARK: - Who owns the /1 pair + + /// The wg-quick shape: default still on the physical gateway, the /1 pair bound to the + /// tunnel by interface. The owner is the tunnel's index. + func testWireGuardStyleSlashOnePairIsAttributedToItsInterface() { + let table = [ + kr(0, prefix: 0, gateway: 0xC0A8_0A01), // default via 192.168.10.1 + kr(0, prefix: 1, ifIndex: 24), // 0.0.0.0/1 -interface utunX + kr(0x8000_0000, prefix: 1, ifIndex: 24), // 128.0.0.0/1 -interface utunX + ] + XCTAssertEqual(RouteKernel.slashOneTunnelOwnerIndex(table), 24) + } + + /// Our own VPN Only catch-alls are RTF_PROTO1-tagged; they must never masquerade as a VPN. + func testOurOwnRoutesNeverCountAsTheSlashOneOwner() { + let table = [ + kr(0, prefix: 1, ifIndex: 24, flags: RTF_PROTO1), + kr(0x8000_0000, prefix: 1, ifIndex: 24, flags: RTF_PROTO1), + ] + XCTAssertNil(RouteKernel.slashOneTunnelOwnerIndex(table)) + } + + /// An OpenVPN-style /1 with an AF_INET next hop carries no interface index in the dump — + /// nil here means selection falls back to the default-route interface, today's behaviour. + func testAddressGatewayedSlashOneIsNotAttributable() { + let table = [kr(0, prefix: 1, gateway: 0x0A08_0001)] + XCTAssertNil(RouteKernel.slashOneTunnelOwnerIndex(table)) + } + + /// A /1 covering only half the space plus unrelated routes is not treated as an owner + /// signature by accident of some other prefix: only prefix == 1 on the two halves counts. + func testTableWithoutSlashOnesGivesNoOwner() { + let table = [ + kr(0, prefix: 0, gateway: 0xC0A8_0A01), + kr(0x0A00_0000, prefix: 8, ifIndex: 24), + kr(0x8000_0000, prefix: 2, ifIndex: 24), // a /2 — ours or anyone's — is not a /1 + ] + XCTAssertNil(RouteKernel.slashOneTunnelOwnerIndex(table)) + } + + /// One attributable half is enough — OpenVPN and WireGuard both install the pair, but a + /// mid-transition table can momentarily hold only one of them. + func testASingleAttributableHalfIsEnough() { + let table = [kr(0x8000_0000, prefix: 1, ifIndex: 7)] + XCTAssertEqual(RouteKernel.slashOneTunnelOwnerIndex(table), 7) + } + + // MARK: - Structural-shadow predicate unchanged (custom-engine GP guard) + + /// /2 is additive, not a shadow: a user's explicit /2 rule stays allowed under GP even + /// though the /2 quartet now lives in catchAllDestinations for cleanup recognition. + func testSlashTwoIsNotStructurallyCatchAll() { + for destination in ClassicRouteCompiler.bypassAllCatchAlls { + XCTAssertFalse(RouteCompiler.isCatchAll(destination), + "\(destination) must stay allowed as an explicit custom rule") + } + } + + func testSlashZeroAndSlashOneRemainStructurallyCatchAll() { + XCTAssertTrue(RouteCompiler.isCatchAll("0.0.0.0/0")) + XCTAssertTrue(RouteCompiler.isCatchAll("0.0.0.0/1")) + XCTAssertTrue(RouteCompiler.isCatchAll("128.0.0.0/1")) + XCTAssertTrue(RouteCompiler.isCatchAll("::/0")) + XCTAssertFalse(RouteCompiler.isCatchAll("10.0.0.0/8")) + XCTAssertFalse(RouteCompiler.isCatchAll("1.2.3.4")) + } + + /// The cleanup-recognition set must carry BOTH generations: the quartet installed now and + /// the /1 pair a build up to 4.8.0 may have stranded. + func testRecognitionSetCoversBothGenerations() { + for destination in ClassicRouteCompiler.bypassAllCatchAlls + ["0.0.0.0/1", "128.0.0.0/1", "0.0.0.0/0"] { + XCTAssertTrue(RouteCompiler.catchAllDestinations.contains(destination), destination) + } + } + + /// The quartet actually covers all of IPv4 — four /2s, disjoint, starting at each quarter. + func testQuartetCoversTheWholeAddressSpace() { + XCTAssertEqual(ClassicRouteCompiler.bypassAllCatchAlls, + ["0.0.0.0/2", "64.0.0.0/2", "128.0.0.0/2", "192.0.0.0/2"]) + } +} diff --git a/Tests/VPNBypassTests/VPNBoundRouteTests.swift b/Tests/VPNBypassTests/VPNBoundRouteTests.swift index 0bd4322..5030ad6 100644 --- a/Tests/VPNBypassTests/VPNBoundRouteTests.swift +++ b/Tests/VPNBypassTests/VPNBoundRouteTests.swift @@ -53,7 +53,8 @@ final class VPNBoundRouteTests: XCTestCase { } /// VPN-Only catch-alls are always torn down, even though they egress the LOCAL gateway and so - /// would otherwise look "still valid" by the gateway test alone. + /// would otherwise look "still valid" by the gateway test alone. The /1 pair here is what + /// builds up to 4.8.0 installed — it stays recognised so an update cleans an old strand. func testCatchAllsAreAlwaysStaleEvenViaLocalGateway() { let routes = [route("0.0.0.0/1", local, "VPN Only catch-all"), route("128.0.0.0/1", local, "VPN Only catch-all"), @@ -61,6 +62,20 @@ final class VPNBoundRouteTests: XCTestCase { XCTAssertEqual(Set(stale(routes)), ["0.0.0.0/1", "128.0.0.0/1"]) } + /// #104 review: a user's own route that merely spells a catch-all destination is NOT ours + /// to tear down — classification is destination AND source. + func testUserRouteSharingACatchAllDestinationSurvives() { + let routes = [route("0.0.0.0/2", local, "my-range"), route("7.7.7.7", local)] + XCTAssertTrue(stale(routes).isEmpty) + } + + /// The /2 quartet VPN Only installs since #103 is recognised the same way. + func testQuartetCatchAllsAreStale() { + let routes = ClassicRouteCompiler.bypassAllCatchAlls.map { route($0, local, "VPN Only catch-all") } + + [route("7.7.7.7", local)] + XCTAssertEqual(Set(stale(routes)), Set(ClassicRouteCompiler.bypassAllCatchAlls)) + } + /// With no known local gateway we cannot prove any route is still valid, so everything is /// treated as stale. Fails toward removing (churn) rather than toward leaving a dead route. func testUnknownLocalGatewayTreatsEverythingAsStale() {