diff --git a/Tests/VPNBypassTests/ControlSurfaceTests.swift b/Tests/VPNBypassTests/ControlSurfaceTests.swift index 611035d..aba7a47 100644 --- a/Tests/VPNBypassTests/ControlSurfaceTests.swift +++ b/Tests/VPNBypassTests/ControlSurfaceTests.swift @@ -35,15 +35,18 @@ final class ControlSurfaceTests: XCTestCase { func testRouteSetRepointsLiveListenerKeepingStablePort() async { let id = UUID() + // A port from TestPorts, not a fixed one: a fixed port fails this test whenever + // something else holds it, because the listener then falls back to a random port. + let stablePort = TestPorts.nextListenPort().rawValue var cfg = RouteManager.shared.config cfg.multiRouteEnabled = true cfg.routes = [Route(id: id, name: "oxy", egress: .proxyHTTP, - proxyHost: "127.0.0.1", proxyPort: 8001, localListenPort: 18099)] + proxyHost: "127.0.0.1", proxyPort: 8001, localListenPort: Int(stablePort))] RouteManager.shared.config = cfg await RouteManager.shared.reconcileProxyListeners() let started = await waitForPort(id) - XCTAssertEqual(started, 18099, "listener up on its stable port") + XCTAssertEqual(started, stablePort, "listener up on its stable port") // Re-point the upstream port (the canonical "switch the exit IP" command). let resp = await ControlSurface.handle(ControlRequest(cmd: "route.set", @@ -52,7 +55,7 @@ final class ControlSurfaceTests: XCTestCase { XCTAssertEqual(resp.result?.routes?.first?.proxyPort, 8002) XCTAssertEqual(RouteManager.shared.config.routes.first?.proxyPort, 8002, "change persisted to config") let afterRepoint = await waitForPort(id) - XCTAssertEqual(afterRepoint, 18099, + XCTAssertEqual(afterRepoint, stablePort, "listener re-pointed in place — stable local port survives (HTTPS_PROXY keeps working)") } diff --git a/Tests/VPNBypassTests/DocScreenshotsTests.swift b/Tests/VPNBypassTests/DocScreenshotsTests.swift index 85a9b28..e600eeb 100644 --- a/Tests/VPNBypassTests/DocScreenshotsTests.swift +++ b/Tests/VPNBypassTests/DocScreenshotsTests.swift @@ -203,7 +203,8 @@ final class DocScreenshotsTests: XCTestCase { startedListener = true let deadline = Date().addingTimeInterval(5) while !up && Date() < deadline { RunLoop.main.run(until: Date().addingTimeInterval(0.05)) } - XCTAssertNotNil(ProxyListenerManager.shared.port(for: proxy.id), "office-proxy's listener is up") + XCTAssertEqual(ProxyListenerManager.shared.port(for: proxy.id), 18168, + "office-proxy's listener is up on the port the docs show") } private func route(_ destination: String, via gateway: String, for source: String) -> RouteManager.ActiveRoute { diff --git a/Tests/VPNBypassTests/LiveProxyEgressTests.swift b/Tests/VPNBypassTests/LiveProxyEgressTests.swift index 5661d22..ccb5e93 100644 --- a/Tests/VPNBypassTests/LiveProxyEgressTests.swift +++ b/Tests/VPNBypassTests/LiveProxyEgressTests.swift @@ -18,7 +18,7 @@ final class LiveProxyEgressTests: XCTestCase { throw XCTSkip("no upstream proxy URL in env") } - let forwarder = ProxyForwarder(listenPort: 0, upstream: upstream) + let forwarder = ProxyForwarder(listenPort: TestPorts.nextListenPort().rawValue, upstream: upstream) try forwarder.start() defer { forwarder.stop() } guard let port = forwarder.boundPort else { return XCTFail("forwarder did not bind a port") } @@ -88,9 +88,10 @@ final class LiveProxyEgressTests: XCTestCase { let up = Self.parseUpstream(proxyURL, iface: nil) else { throw XCTSkip("no upstream") } let routeId = UUID() + let stablePort = TestPorts.nextListenPort().rawValue let route = Route(id: routeId, name: "oxy-live", egress: .proxyHTTP, proxyHost: up.host, proxyPort: Int(up.port), - proxyUser: up.username, proxyPass: up.password, localListenPort: 18443) + proxyUser: up.username, proxyPass: up.password, localListenPort: Int(stablePort)) var cfg = RouteManager.shared.config cfg.multiRouteEnabled = true cfg.routes = [route] @@ -103,7 +104,7 @@ final class LiveProxyEgressTests: XCTestCase { let port = ProxyListenerManager.shared.port(for: routeId) print("APP-RECONCILE listener port: \(port.map(String.init) ?? "nil")") - XCTAssertEqual(port, 18443, "stable per-route port honored") + XCTAssertEqual(port, stablePort, "stable per-route port honored") guard let port else { return } let exitIP = try await Self.fetchExitIP(throughLoopbackPort: port) @@ -128,9 +129,10 @@ final class LiveProxyEgressTests: XCTestCase { XCTAssertTrue(ProxyListenerManager.isTailnetHost(peerHost), "peer must be a 100.64/10 tailnet IP") let routeId = UUID() + let stablePort = TestPorts.nextListenPort().rawValue let route = Route(id: routeId, name: "ts-live", egress: .tailscaleExit, proxyHost: peerHost, proxyPort: peerPort, - tailscaleExitNode: "peer", localListenPort: 18944) + tailscaleExitNode: "peer", localListenPort: Int(stablePort)) var cfg = RouteManager.shared.config cfg.multiRouteEnabled = true cfg.routes = [route] @@ -147,7 +149,7 @@ final class LiveProxyEgressTests: XCTestCase { let port = ProxyListenerManager.shared.port(for: routeId) print("TS-RECONCILE listener port: \(port.map(String.init) ?? "nil")") - XCTAssertEqual(port, 18944, "stable per-route port honored") + XCTAssertEqual(port, stablePort, "stable per-route port honored") guard let port else { return } let exitIP = try await Self.fetchExitIP(throughLoopbackPort: port) diff --git a/Tests/VPNBypassTests/LoopbackPeerAuthTests.swift b/Tests/VPNBypassTests/LoopbackPeerAuthTests.swift index 4793852..55b63c3 100644 --- a/Tests/VPNBypassTests/LoopbackPeerAuthTests.swift +++ b/Tests/VPNBypassTests/LoopbackPeerAuthTests.swift @@ -20,6 +20,11 @@ final class LoopbackPeerAuthTests: XCTestCase { "a different uid must be rejected") } + // The two listen-socket tests below keep port .any on purpose: no client ever dials + // these listeners, so there is no connect() to collide with a TIME_WAIT, and the kernel + // hands out a port no socket holds. The wildcard one could not use TestPorts anyway: + // TestPorts checks 127.0.0.1 only, and a 0.0.0.0 bind also fails when a socket on any + // other address holds the port. func testUidLookupFindsOwnListeningPort() throws { let parameters = NWParameters.tcp parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: .any) @@ -96,8 +101,8 @@ final class LoopbackPeerAuthTests: XCTestCase { private func acceptOneLoopbackConnection() throws -> (uid: uid_t?, clientPort: UInt16, listenerPort: UInt16) { let parameters = NWParameters.tcp // Not port 0: a listener in the ephemeral range can collide with an earlier test's - // TIME_WAIT and leave the client stuck on EADDRINUSE. See ProxyForwarderTests.nextListenPort. - parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: ProxyForwarderTests.nextListenPort()) + // TIME_WAIT and leave the client stuck on EADDRINUSE. See TestPorts.nextListenPort. + parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: TestPorts.nextListenPort()) let listener = try NWListener(using: parameters) let queue = DispatchQueue(label: "test.loopback.peer.accept") let listening = expectation(description: "listener ready") diff --git a/Tests/VPNBypassTests/ProxyForwarderTests.swift b/Tests/VPNBypassTests/ProxyForwarderTests.swift index 6bf195e..da5f23e 100644 --- a/Tests/VPNBypassTests/ProxyForwarderTests.swift +++ b/Tests/VPNBypassTests/ProxyForwarderTests.swift @@ -8,61 +8,11 @@ import Network // (b) bytes relay end-to-end through the tunnel. Everything is driven off // XCTestExpectations (no sleeps) so it is deterministic. // -// Every listener here (forwarders and mock upstreams) binds a port from -// nextListenPort(), never port 0. See that function for why. +// Every listener a client dials here (forwarders and mock upstreams) binds a port from +// TestPorts.nextListenPort(), never port 0. See that function for why. +// testPortZeroReportsTheAssignedPort binds port 0 on purpose and nothing dials it. final class ProxyForwarderTests: XCTestCase { - // MARK: - Listener ports - - /// A loopback port for one listener in this suite: below the ephemeral range, never - /// handed out twice in a run, and free right now. - /// - /// Port 0 made these tests flaky. A port 0 listener gets its port from the ephemeral - /// range (49152-65535), the same range client sockets take their source ports from. - /// Every test leaves a TIME_WAIT on its 127.0.0.1 listener/client port pair for 30 s. - /// Once in a while a later test drew the same pair again. In every case caught it was - /// reversed: its listener on an earlier client's port, its client on that earlier - /// listener's port. - /// connect() then fails with EADDRINUSE, NWConnection waits in `.waiting` without - /// retrying, and the test runs out its 5 s timeout. A fresh connection does not get - /// out of it: on macOS it was handed the same source port again. - /// - /// Production never mixes the two ranges (route listeners use 18000-18999), and - /// neither does this suite now: ports come from 20000-48999, each one once per run, - /// and each is bound first without SO_REUSEADDR, which also refuses a port that still - /// has a TIME_WAIT on it. - static func nextListenPort() -> NWEndpoint.Port { - listenPortLock.lock(); defer { listenPortLock.unlock() } - for _ in 0..= listenPortFirst + listenPortSpan - 1 ? listenPortFirst : listenPortCursor + 1 - if canBindLoopback(port: candidate) { return NWEndpoint.Port(rawValue: candidate)! } - } - fatalError("no free loopback port in \(listenPortFirst)..<\(listenPortFirst + listenPortSpan)") - } - - static let listenPortFirst: UInt16 = 20_000 - static let listenPortSpan: UInt16 = 29_000 // 20000...48999 - private static let listenPortLock = NSLock() - private static var listenPortCursor: UInt16 = listenPortFirst + UInt16.random(in: 0.. Bool { - let fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) - guard fd >= 0 else { return false } - defer { close(fd) } - var addr = sockaddr_in() - addr.sin_len = UInt8(MemoryLayout.size) - addr.sin_family = sa_family_t(AF_INET) - addr.sin_port = port.bigEndian - addr.sin_addr.s_addr = inet_addr("127.0.0.1") - return withUnsafePointer(to: &addr) { - $0.withMemoryRebound(to: sockaddr.self, capacity: 1) { - Darwin.bind(fd, $0, socklen_t(MemoryLayout.size)) == 0 - } - } - } - private let mockQueue = DispatchQueue(label: "test.proxy.mock-upstream") private let clientQueue = DispatchQueue(label: "test.proxy.client") @@ -128,7 +78,7 @@ final class ProxyForwarderTests: XCTestCase { // 2. Forwarder chaining through the mock, injecting Basic u:p. let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: mockPort, username: "u", password: "p", boundInterface: nil) ) try forwarder.start() @@ -168,7 +118,7 @@ final class ProxyForwarderTests: XCTestCase { var seen = Set() for _ in 0..<3 { let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: 1, username: "", password: "", boundInterface: nil) ) try forwarder.start() @@ -197,7 +147,7 @@ final class ProxyForwarderTests: XCTestCase { /// The route's stable port has to be bindable again, or turning a route off and on /// moves it to a random port and every app pointed at the old one loses it. func testStopReleasesThePortWhenTheCallerDropsTheForwarderAtOnce() throws { - let port = Self.nextListenPort().rawValue + let port = TestPorts.nextListenPort().rawValue let upstream = ProxyForwarder.Upstream(host: "127.0.0.1", port: 1, username: "", password: "", boundInterface: nil) var first: ProxyForwarder? = ProxyForwarder(listenPort: port, upstream: upstream) try first?.start() @@ -223,7 +173,7 @@ final class ProxyForwarderTests: XCTestCase { /// `200 Connection established`, then echoes any subsequent bytes back. private func startMockUpstream(gotConnect: XCTestExpectation, ready: @escaping (UInt16) -> Void) throws { let parameters = NWParameters.tcp - parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: Self.nextListenPort()) + parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: TestPorts.nextListenPort()) let listener = try NWListener(using: parameters) self.mockListener = listener @@ -337,7 +287,7 @@ final class ProxyForwarderTests: XCTestCase { XCTAssertNotEqual(mockPort, 0, "mock SOCKS5 should report its bound port") let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: mockPort, username: user, password: pass, boundInterface: nil, isSOCKS5: true) ) try forwarder.start() @@ -370,7 +320,7 @@ final class ProxyForwarderTests: XCTestCase { // Upstream (port 1) is never dialed — isValidAuthority rejects first, so this is safe. let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: 1, username: "", password: "", boundInterface: nil) ) try forwarder.start() @@ -415,7 +365,7 @@ final class ProxyForwarderTests: XCTestCase { XCTAssertNotEqual(portA, portB, "the two mock upstreams must be distinct") let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: portA, username: "u", password: "p", boundInterface: nil) ) try forwarder.start() @@ -461,7 +411,7 @@ final class ProxyForwarderTests: XCTestCase { private func startMockSOCKS5(expectAuth: Bool, expectedUser: String, expectedPass: String, gotConnect: XCTestExpectation, ready: @escaping (UInt16) -> Void) throws { let parameters = NWParameters.tcp - parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: Self.nextListenPort()) + parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: TestPorts.nextListenPort()) let listener = try NWListener(using: parameters) self.mockListener = listener listener.stateUpdateHandler = { [weak self] state in @@ -570,7 +520,7 @@ final class ProxyForwarderTests: XCTestCase { storeConnection: @escaping (NWConnection) -> Void, gotConnect: XCTestExpectation, ready: @escaping (UInt16) -> Void) throws { let parameters = NWParameters.tcp - parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: Self.nextListenPort()) + parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: TestPorts.nextListenPort()) let listener = try NWListener(using: parameters) storeListener(listener) listener.stateUpdateHandler = { state in @@ -672,7 +622,7 @@ final class ProxyForwarderTests: XCTestCase { private func assertLocalAuth(clientAuthLine: String?, expect: String) throws { let got = expectation(description: "client received the expected refusal") let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: 1, username: "up", password: "pw", boundInterface: nil), localSecret: Self.testSecret ) @@ -717,7 +667,7 @@ final class ProxyForwarderTests: XCTestCase { wait(for: [mockReady], timeout: 5.0) let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: mockPort, username: "up", password: "pw", boundInterface: nil), localSecret: Self.testSecret ) @@ -758,7 +708,7 @@ final class ProxyForwarderTests: XCTestCase { wait(for: [mockReady], timeout: 5.0) let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: mockPort, username: "up", password: "pw", boundInterface: nil), localSecret: Self.testSecret ) @@ -857,7 +807,7 @@ final class ProxyForwarderTests: XCTestCase { XCTAssertNotEqual(mockPort, 0) let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: mockPort, username: "", password: "", boundInterface: nil) ) try forwarder.start() @@ -900,7 +850,7 @@ final class ProxyForwarderTests: XCTestCase { XCTAssertNotEqual(mockPort, 0) let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: mockPort, username: "", password: "", boundInterface: nil) ) try forwarder.start() @@ -925,7 +875,7 @@ final class ProxyForwarderTests: XCTestCase { private func assertMalformedConnectAuthorityReturns400(authority: String) throws { let got400 = expectation(description: "client received 400 Bad Request for authority '\(authority)'") let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: 1, username: "", password: "", boundInterface: nil) ) try forwarder.start() @@ -965,7 +915,7 @@ final class ProxyForwarderTests: XCTestCase { XCTAssertNotEqual(mockPort, 0) let forwarder = ProxyForwarder( - listenPort: Self.nextListenPort().rawValue, + listenPort: TestPorts.nextListenPort().rawValue, upstream: .init(host: "127.0.0.1", port: mockPort, username: "", password: "", boundInterface: nil) ) try forwarder.start() @@ -990,7 +940,7 @@ final class ProxyForwarderTests: XCTestCase { /// forwarder dialed upstream with. private func startCapturingHTTPMock(onHead: @escaping (String) -> Void, ready: @escaping (UInt16) -> Void) throws { let parameters = NWParameters.tcp - parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: Self.nextListenPort()) + parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: "127.0.0.1", port: TestPorts.nextListenPort()) let listener = try NWListener(using: parameters) self.mockListener = listener listener.stateUpdateHandler = { [weak self] state in diff --git a/Tests/VPNBypassTests/ProxyListenerManagerTests.swift b/Tests/VPNBypassTests/ProxyListenerManagerTests.swift index 94d72dd..b2e44c2 100644 --- a/Tests/VPNBypassTests/ProxyListenerManagerTests.swift +++ b/Tests/VPNBypassTests/ProxyListenerManagerTests.swift @@ -166,18 +166,19 @@ final class ProxyListenerManagerTests: XCTestCase { // and an identical reconcile must be a no-op — both on a stable per-route port. let manager = ProxyListenerManager() let id = UUID() - let r1 = Route(id: id, name: "p", egress: .proxyHTTP, proxyHost: "127.0.0.1", proxyPort: 9, localListenPort: 18077) + let port = TestPorts.nextListenPort().rawValue + let r1 = Route(id: id, name: "p", egress: .proxyHTTP, proxyHost: "127.0.0.1", proxyPort: 9, localListenPort: Int(port)) let e1 = expectation(description: "start") manager.reconcile(routes: [r1], boundInterface: nil) { e1.fulfill() } wait(for: [e1], timeout: 5) - XCTAssertEqual(manager.port(for: id), 18077) + XCTAssertEqual(manager.port(for: id), port) - let r2 = Route(id: id, name: "p", egress: .proxyHTTP, proxyHost: "127.0.0.1", proxyPort: 10, localListenPort: 18077) + let r2 = Route(id: id, name: "p", egress: .proxyHTTP, proxyHost: "127.0.0.1", proxyPort: 10, localListenPort: Int(port)) let e2 = expectation(description: "restart") manager.reconcile(routes: [r2], boundInterface: nil) { e2.fulfill() } wait(for: [e2], timeout: 5) - XCTAssertEqual(manager.port(for: id), 18077, "still served on its stable port after an in-place edit") + XCTAssertEqual(manager.port(for: id), port, "still served on its stable port after an in-place edit") manager.stopAll() } @@ -188,7 +189,7 @@ final class ProxyListenerManagerTests: XCTestCase { // manager dropped the forwarder straight after it, so the old listener kept the // port and the route came back on a random one. let manager = ProxyListenerManager() - let port = ProxyForwarderTests.nextListenPort().rawValue + let port = TestPorts.nextListenPort().rawValue let route = Route(name: "p", egress: .proxyHTTP, proxyHost: "127.0.0.1", proxyPort: 9, localListenPort: Int(port)) let on = expectation(description: "on") diff --git a/Tests/VPNBypassTests/TestPorts.swift b/Tests/VPNBypassTests/TestPorts.swift new file mode 100644 index 0000000..23170d8 --- /dev/null +++ b/Tests/VPNBypassTests/TestPorts.swift @@ -0,0 +1,61 @@ +// TestPorts.swift +// Loopback ports for every test that opens a TCP listener a client then dials. + +import Darwin +import Foundation +import Network + +enum TestPorts { + + /// A loopback port for one listener: below the ephemeral range, never handed out twice + /// in a run, and free right now. + /// + /// Port 0 made the proxy tests flaky. A port 0 listener gets its port from the ephemeral + /// range (49152-65535), the same range client sockets take their source ports from. + /// Every test leaves a TIME_WAIT on its 127.0.0.1 listener/client port pair for 30 s. + /// Once in a while a later test drew the same pair again. In every case caught it was + /// reversed: its listener on an earlier client's port, its client on that earlier + /// listener's port. + /// connect() then fails with EADDRINUSE, NWConnection waits in `.waiting` without + /// retrying, and the test runs out its 5 s timeout. A fresh connection does not get + /// out of it: on macOS it was handed the same source port again. + /// + /// A fixed port fails the other way: the test breaks whenever anything else on the + /// machine holds it, and route listeners fall back to port 0 when their port is taken. + /// + /// Production never mixes the two ranges (route listeners use 18000-18999), and the + /// tests do not either: ports come from 20000-48999, each one once per run, and each is + /// bound first without SO_REUSEADDR, which also refuses a port that still has a + /// TIME_WAIT on it. + static func nextListenPort() -> NWEndpoint.Port { + lock.lock(); defer { lock.unlock() } + for _ in 0..= first + span - 1 ? first : cursor + 1 + if canBindLoopback(port: candidate) { return NWEndpoint.Port(rawValue: candidate)! } + } + fatalError("no free loopback port in \(first)..<\(first + span)") + } + + static let first: UInt16 = 20_000 + static let span: UInt16 = 29_000 // 20000...48999 + private static let lock = NSLock() + private static var cursor: UInt16 = first + UInt16.random(in: 0.. Bool { + let fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) + guard fd >= 0 else { return false } + defer { close(fd) } + var addr = sockaddr_in() + addr.sin_len = UInt8(MemoryLayout.size) + addr.sin_family = sa_family_t(AF_INET) + addr.sin_port = port.bigEndian + addr.sin_addr.s_addr = inet_addr("127.0.0.1") + return withUnsafePointer(to: &addr) { + $0.withMemoryRebound(to: sockaddr.self, capacity: 1) { + Darwin.bind(fd, $0, socklen_t(MemoryLayout.size)) == 0 + } + } + } +} diff --git a/Tests/VPNBypassTests/TestPortsTests.swift b/Tests/VPNBypassTests/TestPortsTests.swift new file mode 100644 index 0000000..9f0e20a --- /dev/null +++ b/Tests/VPNBypassTests/TestPortsTests.swift @@ -0,0 +1,100 @@ +// TestPortsTests.swift +// TestPorts is what keeps the socket tests off ports they can collide on, so its two +// refusals are checked here: a port a listener holds, and a port a closed connection +// left in TIME_WAIT, the case that hung the proxy tests. + +import Darwin +import XCTest + +final class TestPortsTests: XCTestCase { + + func testRefusesAPortAListenerHolds() throws { + let port = TestPorts.nextListenPort().rawValue + let fd = try listenOnLoopback(port: port) + XCTAssertFalse(TestPorts.canBindLoopback(port: port), "port \(port) is held by a listener") + close(fd) + XCTAssertTrue(TestPorts.canBindLoopback(port: port), "port \(port) is free once the listener closes") + } + + /// The listener side closes first, so the TIME_WAIT sits on the listener's port, as it + /// did after every proxy test. A listener later bound there and dialled from the + /// earlier client's port is the pair that failed connect() with EADDRINUSE. + func testRefusesAPortLeftInTimeWait() throws { + let port = TestPorts.nextListenPort().rawValue + let listener = try listenOnLoopback(port: port) + // Each step throws on failure, so accept() never blocks waiting for a client + // that did not connect. + let client = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) + guard client >= 0 else { + let err = errno + close(listener) + throw SocketError(call: "socket", errno: err) + } + var addr = loopback(port: port) + let connected = withUnsafePointer(to: &addr) { + $0.withMemoryRebound(to: sockaddr.self, capacity: 1) { + Darwin.connect(client, $0, socklen_t(MemoryLayout.size)) + } + } + guard connected == 0 else { + let err = errno + close(client); close(listener) + throw SocketError(call: "connect to \(port)", errno: err) + } + let accepted = Darwin.accept(listener, nil, nil) + guard accepted >= 0 else { + let err = errno + close(client); close(listener) + throw SocketError(call: "accept on \(port)", errno: err) + } + close(accepted) + close(listener) + close(client) + + XCTAssertFalse(TestPorts.canBindLoopback(port: port), + "port \(port) still has a TIME_WAIT and must not be handed out") + } + + func testHandsOutPortsBelowTheEphemeralRangeOnceEach() { + var ephemeralFirst: Int32 = 0 + var size = MemoryLayout.size + XCTAssertEqual(sysctlbyname("net.inet.ip.portrange.first", &ephemeralFirst, &size, nil, 0), 0) + var seen = Set() + for _ in 0..<200 { + let port = TestPorts.nextListenPort().rawValue + XCTAssertTrue((20_000...48_999).contains(port), "port \(port) outside 20000-48999") + XCTAssertLessThan(Int32(port), ephemeralFirst, "port \(port) shares the client source-port range") + XCTAssertTrue(seen.insert(port).inserted, "port \(port) handed out twice") + } + } + + // MARK: - Plain BSD sockets, so nothing here depends on Network.framework timing + + private func loopback(port: UInt16) -> sockaddr_in { + var addr = sockaddr_in() + addr.sin_len = UInt8(MemoryLayout.size) + addr.sin_family = sa_family_t(AF_INET) + addr.sin_port = port.bigEndian + addr.sin_addr.s_addr = inet_addr("127.0.0.1") + return addr + } + + private func listenOnLoopback(port: UInt16) throws -> Int32 { + let fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) + guard fd >= 0 else { throw SocketError(call: "socket", errno: errno) } + var addr = loopback(port: port) + let bound = withUnsafePointer(to: &addr) { + $0.withMemoryRebound(to: sockaddr.self, capacity: 1) { + Darwin.bind(fd, $0, socklen_t(MemoryLayout.size)) + } + } + guard bound == 0, Darwin.listen(fd, 1) == 0 else { + let err = errno + close(fd) + throw SocketError(call: "bind/listen on \(port)", errno: err) + } + return fd + } + + private struct SocketError: Error { let call: String; let errno: Int32 } +} diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7d93d72..c04d10b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Routes are restored when macOS drops them after a network change.** With two connections to the same network — Wi-Fi and Ethernet, say — macOS can switch which one traffic actually uses, and it takes that interface's routes with it. Nothing the app watched changed: the connection stayed up, the gateway stayed the same, and both interfaces were still there, so it never noticed its routes were gone and left them missing. With a VPN or Tailscale exit node running, that traffic then went through the tunnel instead of around it. The app now checks the real routing table rather than its own record of what it installed, and puts back anything that vanished. - **A stuck DNS-cache flush can no longer hang the privileged helper.** After writing `/etc/hosts`, the helper waited forever for `dscacheutil -flushcache` and `killall -HUP mDNSResponder`. If either child wedged, that helper thread stayed blocked for the life of the daemon even after the app gave up at 30 seconds. Those two processes now have a 3-second deadline, then SIGTERM and SIGKILL if they do not exit. - **With two VPNs running, the app could pick the wrong one.** Several tunnels being up at once is ordinary — a corporate VPN alongside Tailscale — but the app took the first one it happened to see, which had nothing to do with which tunnel was carrying traffic. Because Tailscale usually appears earlier in that list, it could win, and in VPN Only mode the app would then install its rules to escape the wrong tunnel and send the other one's traffic straight out. It now prefers the tunnel actually carrying the default route, never picks Tailscale, keeps its choice stable while several remain valid, and breaks any remaining tie the same way every time. +- **No test that dials a listener sits on a port that can collide or be taken.** Two tests were still exposed after the proxy test fix. The live egress test started its forwarder on port 0, which puts the listener in the range client sockets draw their source ports from, and a client that later lands on an old pair fails to connect with EADDRINUSE. The control socket test that re-points a route expected the fixed port 18099, and three others expected 18077, 18443 and 18944. Each failed whenever something on the Mac held its port, because the listener then fell back to a random one. Every test that opens a listener a client dials now takes its port from one shared helper, `TestPorts.nextListenPort()`, which hands out each port in 20000-48999 once per run and only after binding it first. New tests check that it refuses a port a listener holds and a port a closed connection left in TIME_WAIT. ### Changed - **"Route" means one thing on screen, and the Bypass list is no longer called Custom Domains.** "Route" named three things: the kernel entries the app installs ("62 routes" in the dropdown and on the Status page), the ways out in Custom mode (the Routes page, "1/1 active"), and the ways out that have rules (Routes In Use). A user reading "62 routes" next to "1/1 active" was counting two different things under one word. "Route" now means only a way out in Custom mode: Direct, a VPN, a proxy or a Tailscale peer. Every count of kernel entries reads as addresses: "Telegram, 12 addresses", "62 addresses routed 23 s ago, none failed", "Checked 10 of 62 addresses", the Addresses fact and the Addresses section on the Status page, the NOTHING ROUTED pill, the notifications and the "Last change" line after a `routes.clear`. One address now reads in the singular ("1 address stays routed for when it reconnects."), where "1 route" used to land in a plural sentence. The Domains page is titled Bypass list or VPN Only list, after its mode, instead of Custom Domains or VPN Only Domains. The command names Refresh Routes, Verify Routes and Remove All Routes… keep their names, and so do the socket verbs (`routes.active`, `routes.clear`) and the log lines. The new text is in English, Spanish and French. diff --git a/docs/development.md b/docs/development.md index f44475a..b880097 100644 --- a/docs/development.md +++ b/docs/development.md @@ -29,6 +29,10 @@ pngquant --quality 95-100 --speed 1 --force --ext .png /tmp/shots/*.png Run it on a Mac with a 2x display, since the images are 2x. Each window draws as the key window of the active app, in the dark appearance, so the traffic lights and switches are in colour even when the test runs over ssh. A test fails if the close button comes out grey. Copy the files over the old ones and read each one before you commit it. Re-render after a change to any view the images show. +## Tests that open a port + +A test that starts a TCP listener and connects a client to it takes the port from `TestPorts.nextListenPort()` in [`Tests/VPNBypassTests/TestPorts.swift`](https://github.com/GeiserX/VPN-Bypass/blob/main/Tests/VPNBypassTests/TestPorts.swift), never port 0 and never a fixed number. Port 0 picks from the range client sockets take their source ports from, so a listener can land on an old client's port and the next connect fails with EADDRINUSE. A fixed port fails whenever another process holds it. The helper hands out each port in 20000-48999 once per run, below that range and outside the 18000-18999 range the app's route listeners use, and binds it first to check it is free. A listener nothing connects to can stay on port 0, as `LoopbackPeerAuthTests` and `testPortZeroReportsTheAssignedPort` do. + ## Contributing Contributions are welcome! Here's how you can help: