From 91b54de063eec6a65be6e9fee789fc07ece18507 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Mon, 18 Dec 2023 19:56:02 -0800 Subject: [PATCH 1/3] Add a reason field to the WireGuard invalid state error. --- .../Diagnostics/NetworkProtectionError.swift | 2 +- ...ror+NetworkProtectionErrorConvertible.swift | 4 ++-- .../WireGuardKit/WireGuardAdapter.swift | 18 ++++++++++++------ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift index 508afb3cd..911b94b63 100644 --- a/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift +++ b/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift @@ -61,7 +61,7 @@ public enum NetworkProtectionError: LocalizedError { // Wireguard errors case wireGuardCannotLocateTunnelFileDescriptor - case wireGuardInvalidState + case wireGuardInvalidState(reason: String) case wireGuardDnsResolution case wireGuardSetNetworkSettings(Error) case startWireGuardBackend(Int32) diff --git a/Sources/NetworkProtection/Diagnostics/WireGuardAdapterError+NetworkProtectionErrorConvertible.swift b/Sources/NetworkProtection/Diagnostics/WireGuardAdapterError+NetworkProtectionErrorConvertible.swift index ae8b5486a..1f6e77559 100644 --- a/Sources/NetworkProtection/Diagnostics/WireGuardAdapterError+NetworkProtectionErrorConvertible.swift +++ b/Sources/NetworkProtection/Diagnostics/WireGuardAdapterError+NetworkProtectionErrorConvertible.swift @@ -23,8 +23,8 @@ extension WireGuardAdapterError: NetworkProtectionErrorConvertible { switch self { case .cannotLocateTunnelFileDescriptor: return .wireGuardCannotLocateTunnelFileDescriptor - case .invalidState: - return .wireGuardInvalidState + case .invalidState(let reason): + return .wireGuardInvalidState(reason: reason.rawValue) case .dnsResolution: return .wireGuardDnsResolution case .setNetworkSettings(let error): diff --git a/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift b/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift index 6da060ae1..0b416abd3 100644 --- a/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift +++ b/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift @@ -7,12 +7,18 @@ import NetworkExtension import WireGuard import Common +public enum WireGuardAdapterErrorInvalidStateReason: String { + case alreadyStarted + case alreadyStopped + case updatedTunnelWhileStopped +} + public enum WireGuardAdapterError: Error { /// Failure to locate tunnel file descriptor. case cannotLocateTunnelFileDescriptor - /// Failure to perform an operation in such state. - case invalidState + /// Failure to perform an operation in such state. Includes a reason why the error was returned. + case invalidState(WireGuardAdapterErrorInvalidStateReason) /// Failure to resolve endpoints. case dnsResolution([DNSResolutionError]) @@ -263,7 +269,7 @@ public class WireGuardAdapter { public func start(tunnelConfiguration: TunnelConfiguration, completionHandler: @escaping (WireGuardAdapterError?) -> Void) { workQueue.async { guard case .stopped = self.state else { - completionHandler(.invalidState) + completionHandler(.invalidState(.alreadyStarted)) return } @@ -307,7 +313,7 @@ public class WireGuardAdapter { break case .stopped: - completionHandler(.invalidState) + completionHandler(.invalidState(.alreadyStopped)) return } @@ -323,14 +329,14 @@ public class WireGuardAdapter { /// Update runtime configuration. /// - Parameters: /// - tunnelConfiguration: tunnel configuration. - /// - reassert: wether the connection should reassert or not. + /// - reassert: whether the connection should reassert or not. /// - completionHandler: completion handler. public func update(tunnelConfiguration: TunnelConfiguration, reassert: Bool = true, completionHandler: @escaping (WireGuardAdapterError?) -> Void) { workQueue.async { if case .stopped = self.state { - completionHandler(.invalidState) + completionHandler(.invalidState(.updatedTunnelWhileStopped)) return } From 59a0521d3d5e908eca64a4b27eb4a2a41e8fcb01 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Tue, 19 Dec 2023 00:07:10 -0800 Subject: [PATCH 2/3] =?UTF-8?q?Avoid=20blocking=20server=20list=20updates?= =?UTF-8?q?=20if=20the=20file=20can=E2=80=99t=20be=20removed.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Storage/NetworkProtectionServerListStore.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift b/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift index e3f36833f..024d85ff4 100644 --- a/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift +++ b/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift @@ -170,19 +170,19 @@ public class NetworkProtectionServerListFileSystemStore: NetworkProtectionServer do { return try JSONDecoder().decode([NetworkProtectionServer].self, from: data) } catch { - try removeServerList() + removeServerList() throw NetworkProtectionServerListStoreError.failedToDecodeServerList(error) } } - public func removeServerList() throws { - if FileManager.default.fileExists(atPath: fileURL.relativePath) { - try FileManager.default.removeItem(at: fileURL) + public func removeServerList() { + if FileManager.default.fileExists(atPath: fileURL.path) { + try? FileManager.default.removeItem(at: fileURL) } } private func replaceServerList(with newList: [NetworkProtectionServer]) throws { - try removeServerList() + removeServerList() let serializedJSONData: Data From 36523ecee6e93a78789f69ed196349ca8fbebca3 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Tue, 19 Dec 2023 01:57:12 -0800 Subject: [PATCH 3/3] Fix compiler warnings. --- Sources/NetworkProtection/PacketTunnelProvider.swift | 2 +- .../Storage/NetworkProtectionServerListStore.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index ad85bb378..03ac8b0d5 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -891,7 +891,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { resetRegistrationKey() let serverCache = NetworkProtectionServerListFileSystemStore(errorEvents: nil) - try? serverCache.removeServerList() + serverCache.removeServerList() try? tokenStore.deleteToken() diff --git a/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift b/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift index 024d85ff4..9049e3392 100644 --- a/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift +++ b/Sources/NetworkProtection/Storage/NetworkProtectionServerListStore.swift @@ -163,7 +163,7 @@ public class NetworkProtectionServerListFileSystemStore: NetworkProtectionServer do { data = try Data(contentsOf: fileURL) } catch { - try removeServerList() + removeServerList() throw NetworkProtectionServerListStoreError.failedToReadServerList(error) }