From 171981de9f827ff8093f52586be4324433321742 Mon Sep 17 00:00:00 2001 From: Max Goedjen Date: Mon, 11 Dec 2023 02:13:08 -0800 Subject: [PATCH] Turn on strict concurrency (#497) * WIP * Add concurrency warnings. --- Sources/Packages/Package.swift | 17 +++++++---- Sources/Packages/Sources/Brief/Updater.swift | 2 +- .../Sources/SecretAgentKit/Agent.swift | 2 +- .../Sources/SecretAgentKit/Sendability.swift | 11 ++++++++ .../SecretAgentKit/SocketController.swift | 12 ++++---- .../SecretKit/Erasers/AnySecretStore.swift | 2 +- .../OpenSSH/OpenSSHCertificateHandler.swift | 2 +- .../SecretKit/OpenSSH/OpenSSHReader.swift | 2 +- .../PublicKeyStandinFileController.swift | 2 +- .../Sources/SecretKit/SecretStoreList.swift | 2 +- .../SecureEnclaveStore.swift | 8 +++--- .../SmartCardSecretKit/SmartCardStore.swift | 28 +++++++++++-------- .../Tests/SecretAgentKitTests/StubStore.swift | 2 +- Sources/Secretive.xcodeproj/project.pbxproj | 8 +++++- 14 files changed, 63 insertions(+), 37 deletions(-) create mode 100644 Sources/Packages/Sources/SecretAgentKit/Sendability.swift diff --git a/Sources/Packages/Package.swift b/Sources/Packages/Package.swift index c5bc4c0..061f7ad 100644 --- a/Sources/Packages/Package.swift +++ b/Sources/Packages/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version:5.5 +// swift-tools-version:5.9 // The swift-tools-version declares the minimum version of Swift required to build this package. import PackageDescription @@ -33,23 +33,28 @@ let package = Package( targets: [ .target( name: "SecretKit", - dependencies: [] + dependencies: [], + swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])] ), .testTarget( name: "SecretKitTests", - dependencies: ["SecretKit", "SecureEnclaveSecretKit", "SmartCardSecretKit"] + dependencies: ["SecretKit", "SecureEnclaveSecretKit", "SmartCardSecretKit"], + swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])] ), .target( name: "SecureEnclaveSecretKit", - dependencies: ["SecretKit"] + dependencies: ["SecretKit"], + swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])] ), .target( name: "SmartCardSecretKit", - dependencies: ["SecretKit"] + dependencies: ["SecretKit"], + swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])] ), .target( name: "SecretAgentKit", - dependencies: ["SecretKit", "SecretAgentKitHeaders"] + dependencies: ["SecretKit", "SecretAgentKitHeaders"], + swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])] ), .systemLibrary( name: "SecretAgentKitHeaders" diff --git a/Sources/Packages/Sources/Brief/Updater.swift b/Sources/Packages/Sources/Brief/Updater.swift index c71e538..6c88d82 100644 --- a/Sources/Packages/Sources/Brief/Updater.swift +++ b/Sources/Packages/Sources/Brief/Updater.swift @@ -2,7 +2,7 @@ import Foundation import Combine /// A concrete implementation of ``UpdaterProtocol`` which considers the current release and OS version. -public class Updater: ObservableObject, UpdaterProtocol { +public final class Updater: ObservableObject, UpdaterProtocol { @Published public var update: Release? public let testBuild: Bool diff --git a/Sources/Packages/Sources/SecretAgentKit/Agent.swift b/Sources/Packages/Sources/SecretAgentKit/Agent.swift index abee4dd..ed1654b 100644 --- a/Sources/Packages/Sources/SecretAgentKit/Agent.swift +++ b/Sources/Packages/Sources/SecretAgentKit/Agent.swift @@ -5,7 +5,7 @@ import SecretKit import AppKit /// The `Agent` is an implementation of an SSH agent. It manages coordination and access between a socket, traces requests, notifies witnesses and passes requests to stores. -public class Agent { +public final class Agent { private let storeList: SecretStoreList private let witness: SigningWitness? diff --git a/Sources/Packages/Sources/SecretAgentKit/Sendability.swift b/Sources/Packages/Sources/SecretAgentKit/Sendability.swift new file mode 100644 index 0000000..5338464 --- /dev/null +++ b/Sources/Packages/Sources/SecretAgentKit/Sendability.swift @@ -0,0 +1,11 @@ +import Foundation + +struct UncheckedSendable: @unchecked Sendable { + + let value: T + + init(_ value: T) { + self.value = value + } + +} diff --git a/Sources/Packages/Sources/SecretAgentKit/SocketController.swift b/Sources/Packages/Sources/SecretAgentKit/SocketController.swift index 43243db..a51951f 100644 --- a/Sources/Packages/Sources/SecretAgentKit/SocketController.swift +++ b/Sources/Packages/Sources/SecretAgentKit/SocketController.swift @@ -2,7 +2,7 @@ import Foundation import OSLog /// A controller that manages socket configuration and request dispatching. -public class SocketController { +public final class SocketController { /// The active FileHandle. private var fileHandle: FileHandle? @@ -10,7 +10,7 @@ public class SocketController { private var port: SocketPort? /// A handler that will be notified when a new read/write handle is available. /// False if no data could be read - public var handler: ((FileHandleReader, FileHandleWriter) async -> Bool)? + public var handler: (@Sendable (FileHandleReader, FileHandleWriter) async -> Bool)? /// Logger. private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "SocketController") @@ -69,7 +69,7 @@ public class SocketController { @objc func handleConnectionAccept(notification: Notification) { logger.debug("Socket controller accepted connection") guard let new = notification.userInfo?[NSFileHandleNotificationFileHandleItem] as? FileHandle else { return } - Task { + Task { [handler, fileHandle] in _ = await handler?(new, new) await new.waitForDataInBackgroundAndNotifyOnMainActor() await fileHandle?.acceptConnectionInBackgroundAndNotifyOnMainActor() @@ -82,12 +82,12 @@ public class SocketController { logger.debug("Socket controller has new data available") guard let new = notification.object as? FileHandle else { return } logger.debug("Socket controller received new file handle") - Task { + Task { [handler, logger = UncheckedSendable(logger)] in if((await handler?(new, new)) == true) { - logger.debug("Socket controller handled data, wait for more data") + logger.value.debug("Socket controller handled data, wait for more data") await new.waitForDataInBackgroundAndNotifyOnMainActor() } else { - logger.debug("Socket controller called with empty data, socked closed") + logger.value.debug("Socket controller called with empty data, socked closed") } } } diff --git a/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift b/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift index f70000b..bf5a74d 100644 --- a/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift +++ b/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift @@ -71,7 +71,7 @@ public class AnySecretStore: SecretStore { } -public class AnySecretStoreModifiable: AnySecretStore, SecretStoreModifiable { +public final class AnySecretStoreModifiable: AnySecretStore, SecretStoreModifiable { private let _create: (String, Bool) throws -> Void private let _delete: (AnySecret) throws -> Void diff --git a/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHCertificateHandler.swift b/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHCertificateHandler.swift index 12ecefa..5066545 100644 --- a/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHCertificateHandler.swift +++ b/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHCertificateHandler.swift @@ -2,7 +2,7 @@ import Foundation import OSLog /// Manages storage and lookup for OpenSSH certificates. -public class OpenSSHCertificateHandler { +public final class OpenSSHCertificateHandler { private let publicKeyFileStoreController = PublicKeyFileStoreController(homeDirectory: NSHomeDirectory()) private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "OpenSSHCertificateHandler") diff --git a/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHReader.swift b/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHReader.swift index 027b63d..6b7bc08 100644 --- a/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHReader.swift +++ b/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHReader.swift @@ -1,7 +1,7 @@ import Foundation /// Reads OpenSSH protocol data. -public class OpenSSHReader { +public final class OpenSSHReader { var remaining: Data diff --git a/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift b/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift index 7ec0a62..7c3f8da 100644 --- a/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift +++ b/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift @@ -2,7 +2,7 @@ import Foundation import OSLog /// Controller responsible for writing public keys to disk, so that they're easily accessible by scripts. -public class PublicKeyFileStoreController { +public final class PublicKeyFileStoreController { private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "PublicKeyFileStoreController") private let directory: String diff --git a/Sources/Packages/Sources/SecretKit/SecretStoreList.swift b/Sources/Packages/Sources/SecretKit/SecretStoreList.swift index 1345d56..eb8456f 100644 --- a/Sources/Packages/Sources/SecretKit/SecretStoreList.swift +++ b/Sources/Packages/Sources/SecretKit/SecretStoreList.swift @@ -2,7 +2,7 @@ import Foundation import Combine /// A "Store Store," which holds a list of type-erased stores. -public class SecretStoreList: ObservableObject { +public final class SecretStoreList: ObservableObject { /// The Stores managed by the SecretStoreList. @Published public var stores: [AnySecretStore] = [] diff --git a/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift b/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift index 5a853c4..75935c1 100644 --- a/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift +++ b/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift @@ -8,7 +8,7 @@ import SecretKit extension SecureEnclave { /// An implementation of Store backed by the Secure Enclave. - public class Store: SecretStoreModifiable { + public final class Store: SecretStoreModifiable { public var isAvailable: Bool { // For some reason, as of build time, CryptoKit.SecureEnclave.isAvailable always returns false @@ -24,8 +24,8 @@ extension SecureEnclave { /// Initializes a Store. public init() { - DistributedNotificationCenter.default().addObserver(forName: .secretStoreUpdated, object: nil, queue: .main) { _ in - self.reloadSecretsInternal(notifyAgent: false) + DistributedNotificationCenter.default().addObserver(forName: .secretStoreUpdated, object: nil, queue: .main) { [reload = reloadSecretsInternal(notifyAgent:)] _ in + reload(false) } loadSecrets() } @@ -211,7 +211,7 @@ extension SecureEnclave.Store { /// Reloads all secrets from the store. /// - Parameter notifyAgent: A boolean indicating whether a distributed notification should be posted, notifying other processes (ie, the SecretAgent) to reload their stores as well. - private func reloadSecretsInternal(notifyAgent: Bool = true) { + @Sendable private func reloadSecretsInternal(notifyAgent: Bool = true) { let before = secrets secrets.removeAll() loadSecrets() diff --git a/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift b/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift index 6d999ac..dd52d22 100644 --- a/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift +++ b/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift @@ -8,7 +8,7 @@ import SecretKit extension SmartCard { /// An implementation of Store backed by a Smart Card. - public class Store: SecretStore { + public final class Store: SecretStore { @Published public var isAvailable: Bool = false public let id = UUID() @@ -20,12 +20,14 @@ extension SmartCard { /// Initializes a Store. public init() { tokenID = watcher.nonSecureEnclaveTokens.first - watcher.setInsertionHandler { string in + watcher.setInsertionHandler { [reload = reloadSecretsInternal] string in guard self.tokenID == nil else { return } guard !string.contains("setoken") else { return } self.tokenID = string - self.reloadSecrets() + DispatchQueue.main.async { + reload() + } self.watcher.addRemovalHandler(self.smartcardRemoved, forTokenID: string) } if let tokenID = tokenID { @@ -106,15 +108,7 @@ extension SmartCard { /// Reloads all secrets from the store. public func reloadSecrets() { - DispatchQueue.main.async { - self.isAvailable = self.tokenID != nil - let before = self.secrets - self.secrets.removeAll() - self.loadSecrets() - if self.secrets != before { - NotificationCenter.default.post(name: .secretStoreReloaded, object: self) - } - } + reloadSecretsInternal() } } @@ -123,6 +117,16 @@ extension SmartCard { extension SmartCard.Store { + @Sendable private func reloadSecretsInternal() { + self.isAvailable = self.tokenID != nil + let before = self.secrets + self.secrets.removeAll() + self.loadSecrets() + if self.secrets != before { + NotificationCenter.default.post(name: .secretStoreReloaded, object: self) + } + } + /// Resets the token ID and reloads secrets. /// - Parameter tokenID: The ID of the token that was removed. private func smartcardRemoved(for tokenID: String? = nil) { diff --git a/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift b/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift index f155706..f990f97 100644 --- a/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift +++ b/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift @@ -6,7 +6,7 @@ struct Stub {} extension Stub { - public class Store: SecretStore { + public final class Store: SecretStore { public let isAvailable = true public let id = UUID() diff --git a/Sources/Secretive.xcodeproj/project.pbxproj b/Sources/Secretive.xcodeproj/project.pbxproj index fe78151..0eb5b4d 100644 --- a/Sources/Secretive.xcodeproj/project.pbxproj +++ b/Sources/Secretive.xcodeproj/project.pbxproj @@ -3,7 +3,7 @@ archiveVersion = 1; classes = { }; - objectVersion = 52; + objectVersion = 54; objects = { /* Begin PBXBuildFile section */ @@ -691,6 +691,7 @@ PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = ""; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; }; name = Debug; @@ -718,6 +719,7 @@ PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = "Secretive - Host"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; }; name = Release; @@ -848,6 +850,7 @@ MARKETING_VERSION = 1; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host; PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; }; name = Test; @@ -891,6 +894,7 @@ MARKETING_VERSION = 1; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent; PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; }; name = Test; @@ -914,6 +918,7 @@ MARKETING_VERSION = 1; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent; PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; }; name = Debug; @@ -939,6 +944,7 @@ PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = "Secretive - Secret Agent"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_VERSION = 5.0; }; name = Release;