From dee9d4e90af2dda070cbe72cc76cf4a375197012 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Mon, 15 Jul 2024 11:13:20 +0200 Subject: [PATCH 1/2] Add IAN-based TunnelPinger refactoring the pinger protocol accordingly --- ios/MullvadVPN.xcodeproj/project.pbxproj | 6 +- .../xcshareddata/swiftpm/Package.resolved | 2 +- .../PacketTunnelProvider.swift | 4 +- .../WireGuardAdapter/WgAdapter.swift | 4 + .../WireGuardAdapterError+Localization.swift | 8 ++ ios/PacketTunnelCore/Pinger/Pinger.swift | 17 +++- .../Pinger/PingerProtocol.swift | 9 +- .../Pinger/TunnelPinger.swift | 95 +++++++++++++++++++ .../TunnelMonitor/TunnelMonitor.swift | 12 +-- .../Mocks/PingerMock.swift | 10 +- 10 files changed, 148 insertions(+), 19 deletions(-) create mode 100644 ios/PacketTunnelCore/Pinger/TunnelPinger.swift diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index 1441486f87c3..f939a417806e 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -49,6 +49,7 @@ 44B3C43D2C00CBBD0079782C /* PacketTunnelActorReducerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44B3C43C2C00CBBC0079782C /* PacketTunnelActorReducerTests.swift */; }; 44BB5F972BE527F4002520EB /* TunnelState+UI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44BB5F962BE527F4002520EB /* TunnelState+UI.swift */; }; 44BB5F982BE527F4002520EB /* TunnelState+UI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44BB5F962BE527F4002520EB /* TunnelState+UI.swift */; }; + 44C18DE32C74DF93009BE3E1 /* TunnelPinger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 449275432C3C3029000526DE /* TunnelPinger.swift */; }; 44DD7D242B6CFFD70005F67F /* StartTunnelOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44DD7D232B6CFFD70005F67F /* StartTunnelOperationTests.swift */; }; 44DD7D272B6D18FB0005F67F /* MockTunnelInteractor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44DD7D262B6D18FB0005F67F /* MockTunnelInteractor.swift */; }; 44DD7D292B7113CA0005F67F /* MockTunnel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44DD7D282B7113CA0005F67F /* MockTunnel.swift */; }; @@ -1371,6 +1372,7 @@ 06FAE67B28F83CA50033DD93 /* REST.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = REST.swift; sourceTree = ""; }; 06FAE67D28F83CA50033DD93 /* RESTTransport.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RESTTransport.swift; sourceTree = ""; }; 449275412C3570CA000526DE /* ICMP.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ICMP.swift; sourceTree = ""; }; + 449275432C3C3029000526DE /* TunnelPinger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelPinger.swift; sourceTree = ""; }; 449872E02B7BBC5400094DDC /* TunnelSettingsUpdate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelSettingsUpdate.swift; sourceTree = ""; }; 449872E32B7CB96300094DDC /* TunnelSettingsUpdateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelSettingsUpdateTests.swift; sourceTree = ""; }; 449EB9FC2B95F8AD00DFA4EB /* DeviceMock.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DeviceMock.swift; sourceTree = ""; }; @@ -3347,6 +3349,7 @@ 5838318A27C40A3900000571 /* Pinger.swift */, 58799A352A84FC9F007BE51F /* PingerProtocol.swift */, 449275412C3570CA000526DE /* ICMP.swift */, + 449275432C3C3029000526DE /* TunnelPinger.swift */, ); path = Pinger; sourceTree = ""; @@ -5938,6 +5941,7 @@ F0570CD12C4FB8E1007BDF2D /* EphemeralPeerExchangingPipeline.swift in Sources */, F0570CD22C4FB8E1007BDF2D /* SingleHopEphemeralPeerExchanger.swift in Sources */, F0570CD42C4FB8E1007BDF2D /* MultiHopEphemeralPeerExchanger.swift in Sources */, + 44C18DE32C74DF93009BE3E1 /* TunnelPinger.swift in Sources */, 58C7A45B2A8640030060C66F /* PacketTunnelPathObserver.swift in Sources */, 580D6B8E2AB33BBF00B2D6E0 /* BlockedStateErrorMapper.swift in Sources */, 06AC116228F94C450037AF9A /* ApplicationConfiguration.swift in Sources */, @@ -9206,7 +9210,7 @@ repositoryURL = "https://github.com/mullvad/wireguard-apple.git"; requirement = { kind = revision; - revision = 143776f946e1da2566aa3e830f95b3e75f914f35; + revision = 82ae19d03fcaa83b9636e560ce9bea8fec9dc96f; }; }; /* End XCRemoteSwiftPackageReference section */ diff --git a/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 6212e4cc1c6f..4ac54b324d72 100644 --- a/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -14,7 +14,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/mullvad/wireguard-apple.git", "state" : { - "revision" : "143776f946e1da2566aa3e830f95b3e75f914f35" + "revision" : "82ae19d03fcaa83b9636e560ce9bea8fec9dc96f" } } ], diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift index 5062076688b3..d76bf3b81ee5 100644 --- a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift +++ b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift @@ -59,9 +59,11 @@ class PacketTunnelProvider: NEPacketTunnelProvider { adapter = WgAdapter(packetTunnelProvider: self) + let pinger = TunnelPinger(pingProvider: adapter.icmpPingProvider, replyQueue: internalQueue) + let tunnelMonitor = TunnelMonitor( eventQueue: internalQueue, - pinger: Pinger(replyQueue: internalQueue), + pinger: pinger, tunnelDeviceInfo: adapter, timings: TunnelMonitorTimings() ) diff --git a/ios/PacketTunnel/WireGuardAdapter/WgAdapter.swift b/ios/PacketTunnel/WireGuardAdapter/WgAdapter.swift index cf52553a5ee8..dd2d562c2e57 100644 --- a/ios/PacketTunnel/WireGuardAdapter/WgAdapter.swift +++ b/ios/PacketTunnel/WireGuardAdapter/WgAdapter.swift @@ -100,6 +100,10 @@ struct WgAdapter: TunnelAdapterProtocol { let isUsingSameIP = (hasIPv4SameAddress || hasIPv6SameAddress) ? "" : "NOT " logger.debug("Same IP is \(isUsingSameIP)being used") } + + public var icmpPingProvider: ICMPPingProvider { + adapter + } } extension WgAdapter: TunnelDeviceInfoProtocol { diff --git a/ios/PacketTunnel/WireGuardAdapter/WireGuardAdapterError+Localization.swift b/ios/PacketTunnel/WireGuardAdapter/WireGuardAdapterError+Localization.swift index 69806809d491..4ad6ffce0ea3 100644 --- a/ios/PacketTunnel/WireGuardAdapter/WireGuardAdapterError+Localization.swift +++ b/ios/PacketTunnel/WireGuardAdapter/WireGuardAdapterError+Localization.swift @@ -37,6 +37,14 @@ extension WireGuardAdapterError: LocalizedError { return "Failure to start WireGuard backend (error code: \(code))." case .noInterfaceIp: return "Interface has no IP address specified." + case .noSuchTunnel: + return "No such WireGuard tunnel" + case .noTunnelVirtualInterface: + return "Tunnel has no virtual (IAN) interface" + case .icmpSocketNotOpen: + return "ICMP socket not open" + case let .internalError(code): + return "Internal error \(code)" } } } diff --git a/ios/PacketTunnelCore/Pinger/Pinger.swift b/ios/PacketTunnelCore/Pinger/Pinger.swift index 68a06a702816..69ae1ab5af35 100644 --- a/ios/PacketTunnelCore/Pinger/Pinger.swift +++ b/ios/PacketTunnelCore/Pinger/Pinger.swift @@ -9,6 +9,8 @@ import Foundation import Network +// This is the legacy Pinger using native TCP/IP networking. + /// ICMP client. public final class Pinger: PingerProtocol { // Socket read buffer size. @@ -22,6 +24,7 @@ public final class Pinger: PingerProtocol { private var readBuffer = [UInt8](repeating: 0, count: bufferSize) private let stateLock = NSRecursiveLock() private let replyQueue: DispatchQueue + private var destAddress: IPv4Address? public var onReply: ((PingerReply) -> Void)? { get { @@ -49,12 +52,14 @@ public final class Pinger: PingerProtocol { /// Open socket and optionally bind it to the given interface. /// Automatically closes the previously opened socket when called multiple times in a row. - public func openSocket(bindTo interfaceName: String?) throws { + public func openSocket(bindTo interfaceName: String?, destAddress: IPv4Address) throws { stateLock.lock() defer { stateLock.unlock() } closeSocket() + self.destAddress = destAddress + var context = CFSocketContext() context.info = Unmanaged.passUnretained(self).toOpaque() @@ -109,7 +114,7 @@ public final class Pinger: PingerProtocol { /// Send ping packet to the given address. /// Returns `PingerSendResult` on success, otherwise throws a `Pinger.Error`. - public func send(to address: IPv4Address) throws -> PingerSendResult { + public func send() throws -> PingerSendResult { stateLock.lock() defer { stateLock.unlock() } @@ -117,10 +122,14 @@ public final class Pinger: PingerProtocol { throw Error.closedSocket } + guard let destAddress else { + throw Error.parseIPAddress + } + var sa = sockaddr_in() sa.sin_len = UInt8(MemoryLayout.size(ofValue: sa)) sa.sin_family = sa_family_t(AF_INET) - sa.sin_addr = address.rawValue.withUnsafeBytes { buffer in + sa.sin_addr = destAddress.rawValue.withUnsafeBytes { buffer in buffer.bindMemory(to: in_addr.self).baseAddress!.pointee } @@ -149,7 +158,7 @@ public final class Pinger: PingerProtocol { throw Error.sendPacket(errno) } - return PingerSendResult(sequenceNumber: sequenceNumber, bytesSent: UInt(bytesSent)) + return PingerSendResult(sequenceNumber: sequenceNumber) } private func nextSequenceNumber() -> UInt16 { diff --git a/ios/PacketTunnelCore/Pinger/PingerProtocol.swift b/ios/PacketTunnelCore/Pinger/PingerProtocol.swift index 2f1c9d544cbf..67c64c14482e 100644 --- a/ios/PacketTunnelCore/Pinger/PingerProtocol.swift +++ b/ios/PacketTunnelCore/Pinger/PingerProtocol.swift @@ -23,15 +23,16 @@ public struct PingerSendResult { /// Sequence id. public var sequenceNumber: UInt16 - /// How many bytes were sent. - public var bytesSent: UInt + public init(sequenceNumber: UInt16) { + self.sequenceNumber = sequenceNumber + } } /// A type capable of sending and receving ICMP traffic. public protocol PingerProtocol { var onReply: ((PingerReply) -> Void)? { get set } - func openSocket(bindTo interfaceName: String?) throws + func openSocket(bindTo interfaceName: String?, destAddress: IPv4Address) throws func closeSocket() - func send(to address: IPv4Address) throws -> PingerSendResult + func send() throws -> PingerSendResult } diff --git a/ios/PacketTunnelCore/Pinger/TunnelPinger.swift b/ios/PacketTunnelCore/Pinger/TunnelPinger.swift new file mode 100644 index 000000000000..d1f7973b5e88 --- /dev/null +++ b/ios/PacketTunnelCore/Pinger/TunnelPinger.swift @@ -0,0 +1,95 @@ +// +// TunnelPinger.swift +// PacketTunnelCore +// +// Created by Andrew Bulhak on 2024-07-08. +// Copyright © 2024 Mullvad VPN AB. All rights reserved. +// + +import Foundation +import MullvadLogging +import Network +import PacketTunnelCore +import WireGuardKit + +public final class TunnelPinger: PingerProtocol { + private var sequenceNumber: UInt16 = 0 + private let stateLock = NSRecursiveLock() + private let pingQueue: DispatchQueue + private let replyQueue: DispatchQueue + private var destAddress: IPv4Address? + private var _onReply: ((PingerReply) -> Void)? + public var onReply: ((PingerReply) -> Void)? { + get { + stateLock.withLock { + return _onReply + } + } + set { + stateLock.withLock { + _onReply = newValue + } + } + } + + var socketHandle: Int32? + + var pingProvider: ICMPPingProvider + + private let logger: Logger + + init(pingProvider: ICMPPingProvider, replyQueue: DispatchQueue) { + self.pingProvider = pingProvider + self.replyQueue = replyQueue + self.pingQueue = DispatchQueue(label: "PacketTunnel.icmp") + self.logger = Logger(label: "TunnelPinger") + } + + deinit { + pingProvider.closeICMP() + } + + public func openSocket(bindTo interfaceName: String?, destAddress: IPv4Address) throws { + try pingProvider.openICMP(address: destAddress) + self.destAddress = destAddress + } + + public func closeSocket() { + pingProvider.closeICMP() + self.destAddress = nil + } + + public func send() throws -> PingerSendResult { + let sequenceNumber = nextSequenceNumber() + logger.debug("*** sending ping \(sequenceNumber)") + + pingQueue.async { [weak self] in + guard let self, let destAddress else { return } + let reply: PingerReply + do { + try pingProvider.sendICMPPing(seqNumber: sequenceNumber) + reply = .success(destAddress, sequenceNumber) + } catch { + reply = .parseError(error) + } + self.logger.debug("--- Pinger reply: \(reply)") + + replyQueue.async { [weak self] in + guard let self else { return } + // NOTE: we cheat here by returning the destination address we were passed, rather than parsing it from the packet on the other side of the FFI boundary. + self.onReply?(reply) + } + } + + return PingerSendResult(sequenceNumber: UInt16(sequenceNumber)) + } + + private func nextSequenceNumber() -> UInt16 { + stateLock.lock() + let (nextValue, _) = sequenceNumber.addingReportingOverflow(1) + sequenceNumber = nextValue + stateLock.unlock() + + return nextValue + } +} diff --git a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift index 40adf180ce08..b72386219135 100644 --- a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift +++ b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift @@ -222,7 +222,7 @@ public final class TunnelMonitor: TunnelMonitorProtocol { state.isHeartbeatSuspended = false state.timeoutReference = now } - sendPing(to: probeAddress, now: now) + sendPing(now: now) } } @@ -252,9 +252,9 @@ public final class TunnelMonitor: TunnelMonitorProtocol { sendConnectionLostEvent() } - private func sendPing(to receiver: IPv4Address, now: Date) { + private func sendPing(now: Date) { do { - let sendResult = try pinger.send(to: receiver) + let sendResult = try pinger.send() state.updatePingStats(sendResult: sendResult, now: now) logger.trace("Send ping icmp_seq=\(sendResult.sequenceNumber).") @@ -298,12 +298,12 @@ public final class TunnelMonitor: TunnelMonitorProtocol { private func startMonitoring() { do { - guard let interfaceName = tunnelDeviceInfo.interfaceName else { - logger.debug("Failed to obtain utun interface name.") + guard let interfaceName = tunnelDeviceInfo.interfaceName, let probeAddress else { + logger.debug("Failed to obtain utun interface name or probe address.") return } - try pinger.openSocket(bindTo: interfaceName) + try pinger.openSocket(bindTo: interfaceName, destAddress: probeAddress) state.connectionState = .connecting startConnectivityCheckTimer() diff --git a/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift b/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift index 9ed50fcd3ca6..ab80bee9144b 100644 --- a/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift +++ b/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift @@ -34,8 +34,9 @@ class PingerMock: PingerProtocol { self.decideOutcome = decideOutcome } - func openSocket(bindTo interfaceName: String?) throws { + func openSocket(bindTo interfaceName: String?, destAddress: IPv4Address) throws { stateLock.withLock { + state.destAddress = destAddress state.isSocketOpen = true } } @@ -46,11 +47,15 @@ class PingerMock: PingerProtocol { } } - func send(to address: IPv4Address) throws -> PingerSendResult { + func send() throws -> PingerSendResult { // Used for simulation. In reality can be any number. // But for realism it is: IPv4 header (20 bytes) + ICMP header (8 bytes) let icmpPacketSize: UInt = 28 + guard let address = state.destAddress else { + fatalError("Address somehow not set when sending ping") + } + let nextSequenceId = try stateLock.withLock { guard state.isSocketOpen else { throw POSIXError(.ENOTCONN) } @@ -91,6 +96,7 @@ class PingerMock: PingerProtocol { var sequenceId: UInt16 = 0 var isSocketOpen = false var onReply: ((PingerReply) -> Void)? + var destAddress: IPv4Address? = nil mutating func incrementSequenceId() -> UInt16 { sequenceId += 1 From e90f94db7b378fff8f2511ec9de82858c1e51eb7 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Wed, 18 Sep 2024 15:25:21 +0200 Subject: [PATCH 2/2] Remove unused variable --- ios/PacketTunnelCore/Pinger/TunnelPinger.swift | 8 +++----- ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift | 2 +- ios/PacketTunnelCoreTests/Mocks/PingerMock.swift | 4 ++-- ios/PacketTunnelCoreTests/PingerTests.swift | 4 ++-- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/ios/PacketTunnelCore/Pinger/TunnelPinger.swift b/ios/PacketTunnelCore/Pinger/TunnelPinger.swift index d1f7973b5e88..f011fc9a01b8 100644 --- a/ios/PacketTunnelCore/Pinger/TunnelPinger.swift +++ b/ios/PacketTunnelCore/Pinger/TunnelPinger.swift @@ -32,9 +32,7 @@ public final class TunnelPinger: PingerProtocol { } } - var socketHandle: Int32? - - var pingProvider: ICMPPingProvider + private var pingProvider: ICMPPingProvider private let logger: Logger @@ -44,7 +42,7 @@ public final class TunnelPinger: PingerProtocol { self.pingQueue = DispatchQueue(label: "PacketTunnel.icmp") self.logger = Logger(label: "TunnelPinger") } - + deinit { pingProvider.closeICMP() } @@ -68,6 +66,7 @@ public final class TunnelPinger: PingerProtocol { let reply: PingerReply do { try pingProvider.sendICMPPing(seqNumber: sequenceNumber) + // NOTE: we cheat here by returning the destination address we were passed, rather than parsing it from the packet on the other side of the FFI boundary. reply = .success(destAddress, sequenceNumber) } catch { reply = .parseError(error) @@ -76,7 +75,6 @@ public final class TunnelPinger: PingerProtocol { replyQueue.async { [weak self] in guard let self else { return } - // NOTE: we cheat here by returning the destination address we were passed, rather than parsing it from the packet on the other side of the FFI boundary. self.onReply?(reply) } } diff --git a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift index b72386219135..e2b3dbd17bfc 100644 --- a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift +++ b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift @@ -178,7 +178,7 @@ public final class TunnelMonitor: TunnelMonitorProtocol { nslock.lock() defer { nslock.unlock() } - guard let probeAddress, let newStats = getStats(), + guard let newStats = getStats(), state.connectionState == .connecting || state.connectionState == .connected else { return } diff --git a/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift b/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift index ab80bee9144b..0dd16f6f65b3 100644 --- a/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift +++ b/ios/PacketTunnelCoreTests/Mocks/PingerMock.swift @@ -86,7 +86,7 @@ class PingerMock: PingerProtocol { networkStatsReporting.reportBytesSent(UInt64(icmpPacketSize)) - return PingerSendResult(sequenceNumber: nextSequenceId, bytesSent: icmpPacketSize) + return PingerSendResult(sequenceNumber: nextSequenceId) } // MARK: - Types @@ -96,7 +96,7 @@ class PingerMock: PingerProtocol { var sequenceId: UInt16 = 0 var isSocketOpen = false var onReply: ((PingerReply) -> Void)? - var destAddress: IPv4Address? = nil + var destAddress: IPv4Address? mutating func incrementSequenceId() -> UInt16 { sequenceId += 1 diff --git a/ios/PacketTunnelCoreTests/PingerTests.swift b/ios/PacketTunnelCoreTests/PingerTests.swift index 12fe12ddc5b8..e849b0fda242 100644 --- a/ios/PacketTunnelCoreTests/PingerTests.swift +++ b/ios/PacketTunnelCoreTests/PingerTests.swift @@ -25,8 +25,8 @@ final class PingerTests: XCTestCase { } } - try pinger.openSocket(bindTo: "lo0") - sendResult = try pinger.send(to: .loopback) + try pinger.openSocket(bindTo: "lo0", destAddress: .loopback) + sendResult = try pinger.send() waitForExpectations(timeout: .UnitTest.timeout) }