diff --git a/swift/apple/Firezone.xcodeproj/project.pbxproj b/swift/apple/Firezone.xcodeproj/project.pbxproj index e632b80ee..587b8b827 100644 --- a/swift/apple/Firezone.xcodeproj/project.pbxproj +++ b/swift/apple/Firezone.xcodeproj/project.pbxproj @@ -595,7 +595,7 @@ SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; WATCHOS_DEPLOYMENT_TARGET = ""; @@ -637,7 +637,7 @@ SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; VALIDATE_PRODUCT = YES; @@ -681,7 +681,7 @@ SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_INCLUDE_PATHS = "$(PROJECT_DIR)/FirezoneNetworkExtension/Connlib/Generated"; SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; }; name = Debug; }; @@ -723,7 +723,7 @@ SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_INCLUDE_PATHS = "$(PROJECT_DIR)/FirezoneNetworkExtension/Connlib/Generated"; SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; }; name = Release; }; @@ -789,9 +789,11 @@ MTL_ENABLE_DEBUG_INFO = INCLUDE_SOURCE; MTL_FAST_MATH = YES; ONLY_ACTIVE_ARCH = YES; + OTHER_SWIFT_FLAGS = "-enable-upcoming-feature ExistentialAny"; SUPPORTED_PLATFORMS = "macosx iphoneos"; SWIFT_ACTIVE_COMPILATION_CONDITIONS = DEBUG; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_TREAT_WARNINGS_AS_ERRORS = YES; }; name = Debug; @@ -851,9 +853,11 @@ MACOSX_DEPLOYMENT_TARGET = 12.4; MTL_ENABLE_DEBUG_INFO = NO; MTL_FAST_MATH = YES; + OTHER_SWIFT_FLAGS = "-enable-upcoming-feature ExistentialAny"; SUPPORTED_PLATFORMS = "macosx iphoneos"; SWIFT_COMPILATION_MODE = wholemodule; SWIFT_OPTIMIZATION_LEVEL = "-O"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_TREAT_WARNINGS_AS_ERRORS = YES; }; name = Release; @@ -899,7 +903,7 @@ SWIFT_INSTALL_OBJC_HEADER = NO; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; WATCHOS_DEPLOYMENT_TARGET = ""; @@ -948,7 +952,7 @@ SWIFT_INSTALL_MODULE = NO; SWIFT_INSTALL_OBJC_HEADER = NO; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; WATCHOS_DEPLOYMENT_TARGET = ""; diff --git a/swift/apple/Firezone/Info.plist b/swift/apple/Firezone/Info.plist index 3bee9dcf5..f5dca407c 100644 --- a/swift/apple/Firezone/Info.plist +++ b/swift/apple/Firezone/Info.plist @@ -47,5 +47,12 @@ GitSha $(GIT_SHA) + LSEnvironment + + SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL + 1 + SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE + nocrash + diff --git a/swift/apple/FirezoneKit/Package.swift b/swift/apple/FirezoneKit/Package.swift index 3716696b0..39ff025d1 100644 --- a/swift/apple/FirezoneKit/Package.swift +++ b/swift/apple/FirezoneKit/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version: 5.7 +// swift-tools-version: 6.0 // The swift-tools-version declares the minimum version of Swift required to build this package. import PackageDescription diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift index ba22f3c46..f2a9c99ff 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift @@ -12,13 +12,13 @@ import Foundation #endif public class DeviceMetadata { + @MainActor public static func getDeviceName() -> String { // Returns a generic device name on iOS 16 and higher // See https://github.com/firezone/firezone/issues/3034 #if os(iOS) return UIDevice.current.name #else - // Use hostname return ProcessInfo.processInfo.hostName #endif } @@ -33,6 +33,7 @@ public class DeviceMetadata { } #if os(iOS) + @MainActor public static func getIdentifierForVendor() -> String? { return UIDevice.current.identifierForVendor?.uuidString } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift index 801eca058..14885fa85 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift @@ -6,12 +6,11 @@ import CryptoKit import Foundation -import NetworkExtension +@preconcurrency import NetworkExtension // TODO: Use a more abstract IPC protocol to make this less terse -// TODO: Consider making this an actor to guarantee strict ordering -class IPCClient { +actor IPCClient { enum Error: Swift.Error { case decodeIPCDataFailed case noIPCData @@ -31,7 +30,7 @@ class IPCClient { // IPC only makes sense if there's a valid session. Session in this case refers to the `connection` field of // the NETunnelProviderManager instance. - let session: NETunnelProviderSession + nonisolated let session: NETunnelProviderSession // Track the "version" of the resource list so we can more efficiently // retrieve it from the Provider @@ -46,8 +45,8 @@ class IPCClient { } // Encoder used to send messages to the tunnel - let encoder = PropertyListEncoder() - let decoder = PropertyListDecoder() + nonisolated let encoder = PropertyListEncoder() + nonisolated let decoder = PropertyListDecoder() // Auto-connect @MainActor @@ -78,7 +77,7 @@ class IPCClient { try stop() } - func stop() throws { + nonisolated func stop() throws { try session().stopTunnel() } @@ -98,9 +97,8 @@ class IPCClient { } #endif - @MainActor func setConfiguration(_ configuration: Configuration) async throws { - let tunnelConfiguration = configuration.toTunnelConfiguration() + let tunnelConfiguration = await configuration.toTunnelConfiguration() let message = ProviderMessage.setConfiguration(tunnelConfiguration) if sessionStatus() != .connected { @@ -111,37 +109,38 @@ class IPCClient { } func fetchResources() async throws -> ResourceList { - return try await withCheckedThrowingContinuation { continuation in + // Capture current hash before entering continuation + let currentHash = resourceListHash + + // Get data from the provider - continuation returns just the data + let data = try await withCheckedThrowingContinuation { continuation in do { // Request list of resources from the provider. We send the hash of the resource list we already have. // If it differs, we'll get the full list in the callback. If not, we'll get nil. try session([.connected]).sendProviderMessage( - encoder.encode(ProviderMessage.getResourceList(resourceListHash)) + encoder.encode(ProviderMessage.getResourceList(currentHash)) ) { data in - guard let data = data - else { - // No data returned; Resources haven't changed - continuation.resume(returning: self.resourcesListCache) - - return - } - - // Save hash to compare against - self.resourceListHash = Data(SHA256.hash(data: data)) - - do { - let decoded = try self.decoder.decode([Resource].self, from: data) - self.resourcesListCache = ResourceList.loaded(decoded) - - continuation.resume(returning: self.resourcesListCache) - } catch { - continuation.resume(throwing: error) - } + continuation.resume(returning: data) } } catch { continuation.resume(throwing: error) } } + + // Back on the actor - safe to access and mutate state directly + guard let data = data else { + // No data returned; Resources haven't changed + return resourcesListCache + } + + // Save hash to compare against + resourceListHash = Data(SHA256.hash(data: data)) + + // Decode and cache the new resource list + let decoded = try decoder.decode([Resource].self, from: data) + resourcesListCache = ResourceList.loaded(decoded) + + return resourcesListCache } func clearLogs() async throws { @@ -176,7 +175,7 @@ class IPCClient { // Call this with a closure that will append each chunk to a buffer // of some sort, like a file. The completed buffer is a valid Apple Archive // in AAR format. - func exportLogs( + nonisolated func exportLogs( appender: @escaping (LogChunk) -> Void, errorHandler: @escaping (Error) -> Void ) { @@ -220,8 +219,9 @@ class IPCClient { // Subscribe to system notifications about our VPN status changing // and let our handler know about them. - func subscribeToVPNStatusUpdates(handler: @escaping @MainActor (NEVPNStatus) async throws -> Void) - { + nonisolated func subscribeToVPNStatusUpdates( + handler: @escaping @MainActor (NEVPNStatus) async throws -> Void + ) { Task { for await notification in NotificationCenter.default.notifications( named: .NEVPNStatusDidChange) @@ -232,9 +232,8 @@ class IPCClient { } if session.status == .disconnected { - // Reset resource list - resourceListHash = Data() - resourcesListCache = ResourceList.loading + // Reset resource list on disconnect + await self.resetResourceList() } do { try await handler(session.status) } catch { Log.error(error) } @@ -242,11 +241,17 @@ class IPCClient { } } - func sessionStatus() -> NEVPNStatus { + private func resetResourceList() { + resourceListHash = Data() + resourcesListCache = ResourceList.loading + } + + nonisolated func sessionStatus() -> NEVPNStatus { return session.status } - private func session(_ requiredStatuses: Set = []) throws -> NETunnelProviderSession + nonisolated private func session(_ requiredStatuses: Set = []) throws + -> NETunnelProviderSession { if requiredStatuses.isEmpty || requiredStatuses.contains(session.status) { return session diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift index 80462c16f..9bbd7ec16 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift @@ -8,25 +8,29 @@ import Foundation import OSLog public final class Log { - private static var logger = + private static let logger: Logger = { switch Bundle.main.bundleIdentifier { case "dev.firezone.firezone": - Logger(subsystem: "dev.firezone.firezone", category: "app") + return Logger(subsystem: "dev.firezone.firezone", category: "app") case "dev.firezone.firezone.network-extension": - Logger(subsystem: "dev.firezone.firezone", category: "tunnel") + return Logger(subsystem: "dev.firezone.firezone", category: "tunnel") default: fatalError("Unknown bundle id: \(Bundle.main.bundleIdentifier!)") } + }() - private static var logWriter = + private static let logWriter: LogWriter? = { + let folderURL: URL? switch Bundle.main.bundleIdentifier { case "dev.firezone.firezone": - LogWriter(folderURL: SharedAccess.appLogFolderURL, logger: logger) + folderURL = SharedAccess.appLogFolderURL case "dev.firezone.firezone.network-extension": - LogWriter(folderURL: SharedAccess.tunnelLogFolderURL, logger: logger) + folderURL = SharedAccess.tunnelLogFolderURL default: fatalError("Unknown bundle id: \(Bundle.main.bundleIdentifier!)") } + return LogWriter(folderURL: folderURL, logger: logger) + }() public static func log(_ message: String) { debug(message) @@ -67,11 +71,6 @@ public final class Log { let fileManager = FileManager.default var totalSize: Int64 = 0 - func sizeOfFile(at url: URL, with resourceValues: URLResourceValues) -> Int64 { - guard resourceValues.isRegularFile == true else { return 0 } - return Int64(resourceValues.totalFileAllocatedSize ?? resourceValues.totalFileSize ?? 0) - } - // Tally size of each log file in parallel await withTaskGroup(of: Int64.self) { taskGroup in fileManager.forEachFileUnder( @@ -82,8 +81,12 @@ public final class Log { .isRegularFileKey, ] ) { url, resourceValues in - taskGroup.addTask { - return sizeOfFile(at: url, with: resourceValues) + // Extract non-Sendable values before passing to @Sendable closure + guard resourceValues.isRegularFile == true else { return } + let size = Int64(resourceValues.totalFileAllocatedSize ?? resourceValues.totalFileSize ?? 0) + + taskGroup.addTask { @Sendable in + return size } } @@ -118,7 +121,9 @@ public final class Log { } } -private final class LogWriter { +/// Thread-safe: All mutable state access is serialised through workQueue. +/// Log writes are queued asynchronously to avoid blocking the caller. +private final class LogWriter: @unchecked Sendable { enum Severity: String, Codable { case trace = "TRACE" case debug = "DEBUG" @@ -228,14 +233,12 @@ private final class LogWriter { severity: severity, message: message) - guard var jsonData = try? jsonEncoder.encode(logEntry) + guard let jsonData = try? jsonEncoder.encode(logEntry) + Data("\n".utf8) else { logger.error("Could not encode log message to JSON!") return } - jsonData.append(Data("\n".utf8)) - workQueue.async { [weak self] in guard let self = self else { return } guard let handle = self.ensureFileExists() else { return } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift index 06b53ba5d..e907ceb96 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift @@ -154,7 +154,9 @@ import System #endif extension LogExporter { - private static let fileManager = FileManager.default + /// Thread-safe: FileManager.default is documented as thread-safe by Apple. + /// Reference: https://developer.apple.com/documentation/foundation/filemanager + private nonisolated(unsafe) static let fileManager = FileManager.default static func now() -> String { let dateFormatter = ISO8601DateFormatter() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift index f8b98a0a6..c25d9bf0c 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift @@ -6,30 +6,50 @@ import Sentry +/// Actor that manages telemetry state with thread-safe access. +actor TelemetryState { + private var firezoneId: String? + private var accountSlug: String? + + func setFirezoneId(_ id: String?) { + firezoneId = id + updateUser() + } + + func setAccountSlug(_ slug: String?) { + accountSlug = slug + updateUser() + } + + private func updateUser() { + guard let firezoneId, let accountSlug else { + return + } + + SentrySDK.configureScope { configuration in + // Matches the format we use in rust/telemetry/lib.rs + let user = User(userId: firezoneId) + user.data = ["account_slug": accountSlug] + + configuration.setUser(user) + } + } +} + public enum Telemetry { // We can only create a new User object after Sentry is started; not retrieve // the existing one. So we need to collect these fields from various codepaths // during initialization / sign in so we can build a new User object any time // one of these is updated. - private static var _firezoneId: String? - private static var _accountSlug: String? - public static var firezoneId: String? { - get { - return self._firezoneId - } - set { - self._firezoneId = newValue - updateUser(id: self._firezoneId, slug: self._accountSlug) - } + + private static let state = TelemetryState() + + public static func setFirezoneId(_ id: String?) async { + await state.setFirezoneId(id) } - public static var accountSlug: String? { - get { - return self._accountSlug - } - set { - self._accountSlug = newValue - updateUser(id: self._firezoneId, slug: self._accountSlug) - } + + public static func setAccountSlug(_ slug: String?) async { + await state.setAccountSlug(slug) } public static func start() { @@ -68,22 +88,6 @@ public enum Telemetry { SentrySDK.capture(error: err) } - private static func updateUser(id: String?, slug: String?) { - guard let id, - let slug - else { - return - } - - SentrySDK.configureScope { configuration in - // Matches the format we use in rust/telemetry/lib.rs - let user = User(userId: id) - user.data = ["account_slug": slug] - - configuration.setUser(user) - } - } - private static func distributionType() -> String { // Apps from the app store have a receipt file if BundleHelper.isAppStore() { diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift index 4b6ea0b46..4bfcaee49 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift @@ -21,7 +21,9 @@ enum VPNConfigurationManagerError: Error { } } -public class VPNConfigurationManager { +/// Thread-safe: Immutable wrapper around NETunnelProviderManager. +/// Only contains a 'let' property. NETunnelProviderManager handles its own synchronisation. +public class VPNConfigurationManager: @unchecked Sendable { // Persists our tunnel settings let manager: NETunnelProviderManager diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift index 3e47ba129..cc00edd3a 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift @@ -108,7 +108,11 @@ public class Configuration: ObservableObject { static let supportURL = "supportURL" } - private var defaults: UserDefaults + // nonisolated(unsafe) is required because: + // 1. UserDefaults is thread-safe + // 2. Used only for NotificationCenter observer registration (in init/deinit) + // 3. deinit is nonisolated and needs access to remove the observer + private nonisolated(unsafe) var defaults: UserDefaults init(userDefaults: UserDefaults = UserDefaults.standard) { defaults = userDefaults @@ -176,7 +180,7 @@ public class Configuration: ObservableObject { } // Configuration does not conform to Decodable, so introduce a simpler type here to encode for IPC -public struct TunnelConfiguration: Codable { +public struct TunnelConfiguration: Codable, Sendable { public let apiURL: String public let accountSlug: String public let logFilter: String diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift index f39bd73ef..c6d0864c6 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift @@ -9,11 +9,11 @@ import Foundation class StatusSymbol { - static var enabled: String = "<->" - static var disabled: String = "—" + static let enabled: String = "<->" + static let disabled: String = "—" } -public enum ResourceList { +public enum ResourceList: Sendable { case loading case loaded([Resource]) @@ -27,7 +27,7 @@ public enum ResourceList { } } -public struct Resource: Codable, Identifiable, Equatable { +public struct Resource: Codable, Identifiable, Equatable, Sendable { public let id: String public var name: String public var address: String? @@ -59,7 +59,7 @@ public struct Resource: Codable, Identifiable, Equatable { } } -public enum ResourceStatus: String, Codable { +public enum ResourceStatus: String, Codable, Sendable { case offline = "Offline" case online = "Online" case unknown = "Unknown" @@ -91,7 +91,7 @@ public enum ResourceStatus: String, Codable { } } -public enum ResourceType: String, Codable { +public enum ResourceType: String, Codable, Sendable { case dns case cidr case ip diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift index 94c7205ad..f31c32c38 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift @@ -22,6 +22,7 @@ public enum NotificationIndentifier: String { case dismissNotificationAction } +@MainActor public class SessionNotification: NSObject { public var signInHandler = {} private let notificationCenter = UNUserNotificationCenter.current() @@ -69,7 +70,7 @@ public class SessionNotification: NSObject { #if os(iOS) // In iOS, use User Notifications. // This gets called from the tunnel side. - public static func showSignedOutNotificationiOS() { + nonisolated public static func showSignedOutNotificationiOS() { UNUserNotificationCenter.current().getNotificationSettings { notificationSettings in if notificationSettings.authorizationStatus == .authorized { Log.log( @@ -127,7 +128,7 @@ public class SessionNotification: NSObject { #if os(iOS) extension SessionNotification: UNUserNotificationCenterDelegate { - public func userNotificationCenter( + nonisolated public func userNotificationCenter( _ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void @@ -139,12 +140,12 @@ public class SessionNotification: NSObject { actionId == NotificationIndentifier.signInNotificationAction.rawValue { // User clicked on 'Sign In' in the notification - signInHandler() + Task { @MainActor in + signInHandler() + } } - DispatchQueue.main.async { - completionHandler() - } + completionHandler() } } #endif diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift index 84d6a41bc..f67fd9944 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift @@ -7,7 +7,7 @@ import Foundation -public struct Site: Codable, Identifiable, Equatable { +public struct Site: Codable, Identifiable, Equatable, Sendable { public let id: String public var name: String diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift index 5d4497ac9..7dd91d430 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift @@ -15,7 +15,9 @@ public struct Token: CustomStringConvertible { private static let label = "Firezone token" #endif - private static let query: [CFString: Any] = [ + /// Thread-safe: Immutable dictionary initialised at compile time. + /// CFString keys and constant string values are both Sendable. + private nonisolated(unsafe) static let query: [CFString: Any] = [ kSecAttrLabel: "Firezone token", kSecAttrAccount: "1", kSecAttrService: BundleHelper.appGroupId, diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift index 93ad842d2..d09cd975c 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift @@ -13,7 +13,6 @@ import Foundation @MainActor struct WebAuthSession { private static let scheme = "firezone-fd0020211111" - static let anchor = PresentationAnchor() static func signIn(store: Store, configuration: Configuration? = nil) async throws { let configuration = configuration ?? Configuration.shared @@ -27,8 +26,33 @@ struct WebAuthSession { throw AuthClientError.invalidAuthURL } - let authResponse: AuthResponse? = try await withCheckedThrowingContinuation { continuation in - let session = ASWebAuthenticationSession(url: url, callbackURLScheme: scheme) { + // Create anchor on MainActor, then pass to concurrent function + let anchor = PresentationAnchor() + + // Call @concurrent function to avoid MainActor inference on callback closure + let authResponse = try await performAuthentication( + url: url, + callbackScheme: scheme, + authClient: authClient, + anchor: anchor + ) + + if let authResponse { + try await store.signIn(authResponse: authResponse) + } + } + + // @concurrent runs on global executor, preventing closure from inheriting MainActor isolation + @concurrent private static func performAuthentication( + url: URL, + callbackScheme: String, + authClient: AuthClient, + anchor: PresentationAnchor + ) async throws -> AuthResponse? { + // Anchor passed as parameter, keeping strong reference + + return try await withCheckedThrowingContinuation { continuation in + let session = ASWebAuthenticationSession(url: url, callbackURLScheme: callbackScheme) { returnedURL, error in do { if let error = error as? ASWebAuthenticationSessionError, @@ -55,18 +79,20 @@ struct WebAuthSession { // load cookies session.prefersEphemeralWebBrowserSession = false - // Start auth - session.start() - } - - if let authResponse { - try await store.signIn(authResponse: authResponse) + // Start auth - must be called on MainActor + // Use Task to asynchronously hop to MainActor from concurrent context + // (cannot use await MainActor.run - withCheckedThrowingContinuation requires sync closure) + Task { @MainActor in + session.start() + } } } } // Required shim to use as "presentationAnchor" for the Webview. Why Apple? -final class PresentationAnchor: NSObject, ASWebAuthenticationPresentationContextProviding { +final class PresentationAnchor: NSObject, ASWebAuthenticationPresentationContextProviding, + Sendable +{ @MainActor func presentationAnchor(for _: ASWebAuthenticationSession) -> ASPresentationAnchor { ASPresentationAnchor() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift index 81b3ed114..56359143a 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift @@ -42,6 +42,9 @@ public final class Store: ObservableObject { private var vpnConfigurationManager: VPNConfigurationManager? private var cancellables: Set = [] + // Cached IPCClient instance - one per Store instance + private var cachedIPCClient: IPCClient? + // Track which session expired alerts have been shown to prevent duplicates private var shownAlertIds: Set @@ -107,7 +110,8 @@ public final class Store: ObservableObject { #endif private func setupTunnelObservers() async throws { - let vpnStatusChangeHandler: (NEVPNStatus) async throws -> Void = { [weak self] status in + let vpnStatusChangeHandler: @Sendable (NEVPNStatus) async throws -> Void = { + [weak self] status in try await self?.handleVPNStatusChange(newVPNStatus: status) } try ipcClient().subscribeToVPNStatusUpdates(handler: vpnStatusChangeHandler) @@ -200,16 +204,27 @@ public final class Store: ObservableObject { // Create a new VPN configuration in system settings. self.vpnConfigurationManager = try await VPNConfigurationManager() + // Invalidate cached IPCClient since we have a new configuration + cachedIPCClient = nil + try await setupTunnelObservers() } func ipcClient() throws -> IPCClient { + // Return cached instance if it exists + if let cachedIPCClient = cachedIPCClient { + return cachedIPCClient + } + + // Create new instance and cache it guard let session = try manager().session() else { throw VPNConfigurationManagerError.managerNotInitialized } - return IPCClient(session: session) + let client = IPCClient(session: session) + cachedIPCClient = client + return client } func manager() throws -> VPNConfigurationManager { @@ -248,7 +263,7 @@ public final class Store: ObservableObject { UserDefaults.standard.set(actorName, forKey: "actorName") configuration.accountSlug = accountSlug - Telemetry.accountSlug = accountSlug + await Telemetry.setAccountSlug(accountSlug) try await manager().enable() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift index 98bc7fab0..2eae95242 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift @@ -23,7 +23,11 @@ } } - private var timer: Timer? + // nonisolated(unsafe) is required because: + // 1. Timer.invalidate() is thread-safe and can be called from any thread + // 2. Only accessed from MainActor-isolated methods (init, start/stop) and nonisolated deinit + // 3. deinit is nonisolated and needs access to invalidate the timer + private nonisolated(unsafe) var timer: Timer? private let notificationAdapter: NotificationAdapter = NotificationAdapter() private let versionCheckUrl: URL private let marketingVersion: SemanticVersion diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift index a9b1336f6..5f9ba57eb 100644 --- a/swift/apple/FirezoneNetworkExtension/Adapter.swift +++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift @@ -12,6 +12,24 @@ import Foundation import NetworkExtension import OSLog +/// Thread-safe wrapper for mutable state using NSLock. +/// Provides similar API to OSAllocatedUnfairLock but compatible with iOS 15+. +/// We can't use OSAllocatedUnfairLock as it requires iOS 16+. +final class LockedState: @unchecked Sendable { + private let lock = NSLock() + private var _value: Value + + init(initialState: Value) { + _value = initialState + } + + func withLock(_ body: (inout Value) -> Result) -> Result { + lock.lock() + defer { lock.unlock() } + return body(&_value) + } +} + enum AdapterError: Error { /// Failure to perform an operation in such state. case invalidSession(Session?) @@ -106,7 +124,7 @@ class Adapter: @unchecked Sendable { /// - https://github.com/firezone/firezone/issues/3343 /// - https://github.com/firezone/firezone/issues/3235 /// - https://github.com/firezone/firezone/issues/3175 - private lazy var pathUpdateHandler: (Network.NWPath) -> Void = { [weak self] path in + private lazy var pathUpdateHandler: @Sendable (Network.NWPath) -> Void = { [weak self] path in guard let self else { return } if path.status == .unsatisfied { @@ -184,19 +202,39 @@ class Adapter: @unchecked Sendable { func start() throws { Log.log("Adapter.start: Starting session for account: \(accountSlug)") - // Get device metadata - let deviceName = DeviceMetadata.getDeviceName() + // Get device metadata - synchronously get values from MainActor + #if os(iOS) + let deviceMetadata = LockedState<(String, String?)>(initialState: ("", nil)) + #else + let deviceMetadata = LockedState(initialState: "") + #endif + let semaphore = DispatchSemaphore(value: 0) + + Task { @MainActor in + let name = DeviceMetadata.getDeviceName() + #if os(iOS) + let identifier = DeviceMetadata.getIdentifierForVendor() + deviceMetadata.withLock { $0 = (name, identifier) } + #else + deviceMetadata.withLock { $0 = name } + #endif + semaphore.signal() + } + semaphore.wait() + let osVersion = DeviceMetadata.getOSVersion() let logDir = SharedAccess.connlibLogFolderURL?.path ?? "/tmp/firezone" #if os(iOS) + let (deviceName, identifierForVendor) = deviceMetadata.withLock { $0 } let deviceInfo = DeviceInfo( firebaseInstallationId: nil, deviceUuid: nil, deviceSerial: nil, - identifierForVendor: DeviceMetadata.getIdentifierForVendor() + identifierForVendor: identifierForVendor ) #else + let deviceName = deviceMetadata.withLock { $0 } let deviceInfo = DeviceInfo( firebaseInstallationId: nil, deviceUuid: getDeviceUuid(), @@ -291,22 +329,28 @@ class Adapter: @unchecked Sendable { /// Get the current set of resources in the completionHandler, only returning /// them if the resource list has changed. func getResourcesIfVersionDifferentFrom( - hash: Data, completionHandler: @escaping (Data?) -> Void + hash: Data, completionHandler: @escaping @Sendable (Data?) -> Void ) { - // This is async to avoid blocking the main UI thread - workQueue.async { [weak self] in - guard let self = self else { return } + Task { [weak self] in + guard let self = self else { + completionHandler(nil) + return + } // Convert uniffi resources to FirezoneKit resources and encode with PropertyList guard let uniffiResources = self.resources - else { return completionHandler(nil) } + else { + completionHandler(nil) + return + } let firezoneResources = uniffiResources.map { self.convertResource($0) } guard let encoded = try? PropertyListEncoder().encode(firezoneResources) else { Log.log("Failed to encode resources as PropertyList") - return completionHandler(nil) + completionHandler(nil) + return } if hash == Data(SHA256.hash(data: encoded)) { @@ -557,7 +601,11 @@ class Adapter: @unchecked Sendable { return BindResolvers.getServers() } - var resolvers: [String] = [] + // Use a class box to safely capture result across @Sendable closure boundary + final class ResolversBox: @unchecked Sendable { + var value: [String] = [] + } + let resolversBox = ResolversBox() // The caller is in an async context, so it's ok to block this thread here. let semaphore = DispatchSemaphore(value: 0) @@ -570,7 +618,7 @@ class Adapter: @unchecked Sendable { guard let networkSettings = self.networkSettings else { return } // Only now can we get the system resolvers - resolvers = BindResolvers.getServers() + resolversBox.value = BindResolvers.getServers() // Restore connlib's DNS resolvers networkSettings.clearDummyMatchDomain() @@ -578,7 +626,7 @@ class Adapter: @unchecked Sendable { } semaphore.wait() - return resolvers + return resolversBox.value } } #endif diff --git a/swift/apple/FirezoneNetworkExtension/BindResolvers.swift b/swift/apple/FirezoneNetworkExtension/BindResolvers.swift index 3470e3223..cabbfe915 100644 --- a/swift/apple/FirezoneNetworkExtension/BindResolvers.swift +++ b/swift/apple/FirezoneNetworkExtension/BindResolvers.swift @@ -55,6 +55,14 @@ enum BindResolvers { NI_NUMERICHOST) } } - return String(cString: hostBuffer) + // Truncate null termination and decode as UTF-8 + // Convert CChar (Int8) to UInt8 for String(decoding:) + if let nullIndex = hostBuffer.firstIndex(of: 0) { + let bytes = hostBuffer[.. AppGroupIdentifier $(APP_GROUP_ID) + LSEnvironment + + SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL + 1 + SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE + nocrash + diff --git a/swift/apple/FirezoneNetworkExtension/Info.macOS.plist b/swift/apple/FirezoneNetworkExtension/Info.macOS.plist index b582aa148..02517a23d 100644 --- a/swift/apple/FirezoneNetworkExtension/Info.macOS.plist +++ b/swift/apple/FirezoneNetworkExtension/Info.macOS.plist @@ -26,6 +26,13 @@ $(APP_GROUP_ID) NSHumanReadableCopyright Copyright © 2025 Firezone, Inc. All rights reserved. + LSEnvironment + + SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL + 1 + SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE + nocrash + NetworkExtension NEMachServiceName diff --git a/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift b/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift index 819090fd9..e51fd36ec 100644 --- a/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift +++ b/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift @@ -53,7 +53,7 @@ class NetworkSettings { self.matchDomains.append(contentsOf: self.searchDomains) } - func apply(completionHandler: (() -> Void)? = nil) { + func apply(completionHandler: (@Sendable () -> Void)? = nil) { // We don't really know the connlib gateway IP address at this point, but just using 127.0.0.1 is okay // because the OS doesn't really need this IP address. // NEPacketTunnelNetworkSettings taking in tunnelRemoteAddress is probably a bad abstraction caused by diff --git a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift index 856409e84..61155e3ff 100644 --- a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift +++ b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift @@ -54,7 +54,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { override func startTunnel( options: [String: NSObject]?, - completionHandler: @escaping (Error?) -> Void + completionHandler: @escaping @Sendable (Error?) -> Void ) { // Dummy start to get the extension running on macOS after upgrade if options?["dryRun"] as? Bool == true { @@ -114,7 +114,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // Configure telemetry Telemetry.setEnvironmentOrClose(apiURL) - Telemetry.accountSlug = accountSlug + Task { await Telemetry.setAccountSlug(accountSlug) } let enabled = legacyConfiguration?["internetResourceEnabled"] let internetResourceEnabled = @@ -152,7 +152,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // When called by the system, we call Adapter.stop() from here. // When initiated by connlib, we've already called stop() there. override func stopTunnel( - with reason: NEProviderStopReason, completionHandler: @escaping () -> Void + with reason: NEProviderStopReason, completionHandler: @escaping @Sendable () -> Void ) { Log.log("stopTunnel: Reason: \(reason)") @@ -164,8 +164,9 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // It would be helpful to be able to encapsulate Errors here. To do that // we need to update ProviderMessage to encode/decode Result to and from Data. // TODO: Move to a more abstract IPC protocol - @MainActor - override func handleAppMessage(_ message: Data, completionHandler: ((Data?) -> Void)? = nil) { + override func handleAppMessage( + _ message: Data, completionHandler: (@Sendable (Data?) -> Void)? = nil + ) { do { let providerMessage = try PropertyListDecoder().decode(ProviderMessage.self, from: message) @@ -221,7 +222,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { return Token(passedToken) ?? keychainToken } - func clearLogs(_ completionHandler: ((Data?) -> Void)? = nil) { + func clearLogs(_ completionHandler: (@Sendable (Data?) -> Void)? = nil) { do { try Log.clear(in: SharedAccess.logFolderURL) } catch { @@ -231,7 +232,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { completionHandler?(nil) } - func getLogFolderSize(_ completionHandler: ((Data?) -> Void)? = nil) { + func getLogFolderSize(_ completionHandler: (@Sendable (Data?) -> Void)? = nil) { guard let logFolderURL = SharedAccess.logFolderURL else { completionHandler?(nil) @@ -239,17 +240,18 @@ class PacketTunnelProvider: NEPacketTunnelProvider { return } - getLogFolderSizeTask = Task { + let task = Task { let size = await Log.size(of: logFolderURL) let data = withUnsafeBytes(of: size) { Data($0) } - // Ensure completionHandler is called on the same actor as handleAppMessage and isn't cancelled by deinit - if getLogFolderSizeTask?.isCancelled ?? true { return } - await MainActor.run { completionHandler?(data) } + // Call completion handler with data if not cancelled + guard !Task.isCancelled else { return } + completionHandler?(data) } + getLogFolderSizeTask = task } - func exportLogs(_ completionHandler: @escaping (Data?) -> Void) { + func exportLogs(_ completionHandler: @escaping @Sendable (Data?) -> Void) { func sendChunk(_ tunnelLogArchive: TunnelLogArchive) { do { let (chunk, done) = try tunnelLogArchive.readChunk()