Turn on strict concurrency (#497)

* WIP

* Add concurrency warnings.
This commit is contained in:
Max Goedjen 2023-12-11 02:13:08 -08:00 committed by GitHub
parent cf58630065
commit 171981de9f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 63 additions and 37 deletions

View File

@ -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. // The swift-tools-version declares the minimum version of Swift required to build this package.
import PackageDescription import PackageDescription
@ -33,23 +33,28 @@ let package = Package(
targets: [ targets: [
.target( .target(
name: "SecretKit", name: "SecretKit",
dependencies: [] dependencies: [],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])]
), ),
.testTarget( .testTarget(
name: "SecretKitTests", name: "SecretKitTests",
dependencies: ["SecretKit", "SecureEnclaveSecretKit", "SmartCardSecretKit"] dependencies: ["SecretKit", "SecureEnclaveSecretKit", "SmartCardSecretKit"],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])]
), ),
.target( .target(
name: "SecureEnclaveSecretKit", name: "SecureEnclaveSecretKit",
dependencies: ["SecretKit"] dependencies: ["SecretKit"],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])]
), ),
.target( .target(
name: "SmartCardSecretKit", name: "SmartCardSecretKit",
dependencies: ["SecretKit"] dependencies: ["SecretKit"],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])]
), ),
.target( .target(
name: "SecretAgentKit", name: "SecretAgentKit",
dependencies: ["SecretKit", "SecretAgentKitHeaders"] dependencies: ["SecretKit", "SecretAgentKitHeaders"],
swiftSettings: [.enableExperimentalFeature("StrictConcurrency"), .unsafeFlags(["-warnings-as-errors"])]
), ),
.systemLibrary( .systemLibrary(
name: "SecretAgentKitHeaders" name: "SecretAgentKitHeaders"

View File

@ -2,7 +2,7 @@ import Foundation
import Combine import Combine
/// A concrete implementation of ``UpdaterProtocol`` which considers the current release and OS version. /// 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? @Published public var update: Release?
public let testBuild: Bool public let testBuild: Bool

View File

@ -5,7 +5,7 @@ import SecretKit
import AppKit 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. /// 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 storeList: SecretStoreList
private let witness: SigningWitness? private let witness: SigningWitness?

View File

@ -0,0 +1,11 @@
import Foundation
struct UncheckedSendable<T>: @unchecked Sendable {
let value: T
init(_ value: T) {
self.value = value
}
}

View File

@ -2,7 +2,7 @@ import Foundation
import OSLog import OSLog
/// A controller that manages socket configuration and request dispatching. /// A controller that manages socket configuration and request dispatching.
public class SocketController { public final class SocketController {
/// The active FileHandle. /// The active FileHandle.
private var fileHandle: FileHandle? private var fileHandle: FileHandle?
@ -10,7 +10,7 @@ public class SocketController {
private var port: SocketPort? private var port: SocketPort?
/// A handler that will be notified when a new read/write handle is available. /// A handler that will be notified when a new read/write handle is available.
/// False if no data could be read /// False if no data could be read
public var handler: ((FileHandleReader, FileHandleWriter) async -> Bool)? public var handler: (@Sendable (FileHandleReader, FileHandleWriter) async -> Bool)?
/// Logger. /// Logger.
private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "SocketController") private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "SocketController")
@ -69,7 +69,7 @@ public class SocketController {
@objc func handleConnectionAccept(notification: Notification) { @objc func handleConnectionAccept(notification: Notification) {
logger.debug("Socket controller accepted connection") logger.debug("Socket controller accepted connection")
guard let new = notification.userInfo?[NSFileHandleNotificationFileHandleItem] as? FileHandle else { return } guard let new = notification.userInfo?[NSFileHandleNotificationFileHandleItem] as? FileHandle else { return }
Task { Task { [handler, fileHandle] in
_ = await handler?(new, new) _ = await handler?(new, new)
await new.waitForDataInBackgroundAndNotifyOnMainActor() await new.waitForDataInBackgroundAndNotifyOnMainActor()
await fileHandle?.acceptConnectionInBackgroundAndNotifyOnMainActor() await fileHandle?.acceptConnectionInBackgroundAndNotifyOnMainActor()
@ -82,12 +82,12 @@ public class SocketController {
logger.debug("Socket controller has new data available") logger.debug("Socket controller has new data available")
guard let new = notification.object as? FileHandle else { return } guard let new = notification.object as? FileHandle else { return }
logger.debug("Socket controller received new file handle") logger.debug("Socket controller received new file handle")
Task { Task { [handler, logger = UncheckedSendable(logger)] in
if((await handler?(new, new)) == true) { 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() await new.waitForDataInBackgroundAndNotifyOnMainActor()
} else { } else {
logger.debug("Socket controller called with empty data, socked closed") logger.value.debug("Socket controller called with empty data, socked closed")
} }
} }
} }

View File

@ -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 _create: (String, Bool) throws -> Void
private let _delete: (AnySecret) throws -> Void private let _delete: (AnySecret) throws -> Void

View File

@ -2,7 +2,7 @@ import Foundation
import OSLog import OSLog
/// Manages storage and lookup for OpenSSH certificates. /// Manages storage and lookup for OpenSSH certificates.
public class OpenSSHCertificateHandler { public final class OpenSSHCertificateHandler {
private let publicKeyFileStoreController = PublicKeyFileStoreController(homeDirectory: NSHomeDirectory()) private let publicKeyFileStoreController = PublicKeyFileStoreController(homeDirectory: NSHomeDirectory())
private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "OpenSSHCertificateHandler") private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "OpenSSHCertificateHandler")

View File

@ -1,7 +1,7 @@
import Foundation import Foundation
/// Reads OpenSSH protocol data. /// Reads OpenSSH protocol data.
public class OpenSSHReader { public final class OpenSSHReader {
var remaining: Data var remaining: Data

View File

@ -2,7 +2,7 @@ import Foundation
import OSLog import OSLog
/// Controller responsible for writing public keys to disk, so that they're easily accessible by scripts. /// 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 logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent", category: "PublicKeyFileStoreController")
private let directory: String private let directory: String

View File

@ -2,7 +2,7 @@ import Foundation
import Combine import Combine
/// A "Store Store," which holds a list of type-erased stores. /// 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. /// The Stores managed by the SecretStoreList.
@Published public var stores: [AnySecretStore] = [] @Published public var stores: [AnySecretStore] = []

View File

@ -8,7 +8,7 @@ import SecretKit
extension SecureEnclave { extension SecureEnclave {
/// An implementation of Store backed by the Secure Enclave. /// An implementation of Store backed by the Secure Enclave.
public class Store: SecretStoreModifiable { public final class Store: SecretStoreModifiable {
public var isAvailable: Bool { public var isAvailable: Bool {
// For some reason, as of build time, CryptoKit.SecureEnclave.isAvailable always returns false // For some reason, as of build time, CryptoKit.SecureEnclave.isAvailable always returns false
@ -24,8 +24,8 @@ extension SecureEnclave {
/// Initializes a Store. /// Initializes a Store.
public init() { public init() {
DistributedNotificationCenter.default().addObserver(forName: .secretStoreUpdated, object: nil, queue: .main) { _ in DistributedNotificationCenter.default().addObserver(forName: .secretStoreUpdated, object: nil, queue: .main) { [reload = reloadSecretsInternal(notifyAgent:)] _ in
self.reloadSecretsInternal(notifyAgent: false) reload(false)
} }
loadSecrets() loadSecrets()
} }
@ -211,7 +211,7 @@ extension SecureEnclave.Store {
/// Reloads all secrets from the 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. /// - 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 let before = secrets
secrets.removeAll() secrets.removeAll()
loadSecrets() loadSecrets()

View File

@ -8,7 +8,7 @@ import SecretKit
extension SmartCard { extension SmartCard {
/// An implementation of Store backed by a Smart Card. /// An implementation of Store backed by a Smart Card.
public class Store: SecretStore { public final class Store: SecretStore {
@Published public var isAvailable: Bool = false @Published public var isAvailable: Bool = false
public let id = UUID() public let id = UUID()
@ -20,12 +20,14 @@ extension SmartCard {
/// Initializes a Store. /// Initializes a Store.
public init() { public init() {
tokenID = watcher.nonSecureEnclaveTokens.first tokenID = watcher.nonSecureEnclaveTokens.first
watcher.setInsertionHandler { string in watcher.setInsertionHandler { [reload = reloadSecretsInternal] string in
guard self.tokenID == nil else { return } guard self.tokenID == nil else { return }
guard !string.contains("setoken") else { return } guard !string.contains("setoken") else { return }
self.tokenID = string self.tokenID = string
self.reloadSecrets() DispatchQueue.main.async {
reload()
}
self.watcher.addRemovalHandler(self.smartcardRemoved, forTokenID: string) self.watcher.addRemovalHandler(self.smartcardRemoved, forTokenID: string)
} }
if let tokenID = tokenID { if let tokenID = tokenID {
@ -106,7 +108,16 @@ extension SmartCard {
/// Reloads all secrets from the store. /// Reloads all secrets from the store.
public func reloadSecrets() { public func reloadSecrets() {
DispatchQueue.main.async { reloadSecretsInternal()
}
}
}
extension SmartCard.Store {
@Sendable private func reloadSecretsInternal() {
self.isAvailable = self.tokenID != nil self.isAvailable = self.tokenID != nil
let before = self.secrets let before = self.secrets
self.secrets.removeAll() self.secrets.removeAll()
@ -115,13 +126,6 @@ extension SmartCard {
NotificationCenter.default.post(name: .secretStoreReloaded, object: self) NotificationCenter.default.post(name: .secretStoreReloaded, object: self)
} }
} }
}
}
}
extension SmartCard.Store {
/// Resets the token ID and reloads secrets. /// Resets the token ID and reloads secrets.
/// - Parameter tokenID: The ID of the token that was removed. /// - Parameter tokenID: The ID of the token that was removed.

View File

@ -6,7 +6,7 @@ struct Stub {}
extension Stub { extension Stub {
public class Store: SecretStore { public final class Store: SecretStore {
public let isAvailable = true public let isAvailable = true
public let id = UUID() public let id = UUID()

View File

@ -3,7 +3,7 @@
archiveVersion = 1; archiveVersion = 1;
classes = { classes = {
}; };
objectVersion = 52; objectVersion = 54;
objects = { objects = {
/* Begin PBXBuildFile section */ /* Begin PBXBuildFile section */
@ -691,6 +691,7 @@
PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host;
PRODUCT_NAME = "$(TARGET_NAME)"; PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = ""; PROVISIONING_PROFILE_SPECIFIER = "";
SWIFT_STRICT_CONCURRENCY = complete;
SWIFT_VERSION = 5.0; SWIFT_VERSION = 5.0;
}; };
name = Debug; name = Debug;
@ -718,6 +719,7 @@
PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host;
PRODUCT_NAME = "$(TARGET_NAME)"; PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "Secretive - Host"; PROVISIONING_PROFILE_SPECIFIER = "Secretive - Host";
SWIFT_STRICT_CONCURRENCY = complete;
SWIFT_VERSION = 5.0; SWIFT_VERSION = 5.0;
}; };
name = Release; name = Release;
@ -848,6 +850,7 @@
MARKETING_VERSION = 1; MARKETING_VERSION = 1;
PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.Host;
PRODUCT_NAME = "$(TARGET_NAME)"; PRODUCT_NAME = "$(TARGET_NAME)";
SWIFT_STRICT_CONCURRENCY = complete;
SWIFT_VERSION = 5.0; SWIFT_VERSION = 5.0;
}; };
name = Test; name = Test;
@ -891,6 +894,7 @@
MARKETING_VERSION = 1; MARKETING_VERSION = 1;
PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent;
PRODUCT_NAME = "$(TARGET_NAME)"; PRODUCT_NAME = "$(TARGET_NAME)";
SWIFT_STRICT_CONCURRENCY = complete;
SWIFT_VERSION = 5.0; SWIFT_VERSION = 5.0;
}; };
name = Test; name = Test;
@ -914,6 +918,7 @@
MARKETING_VERSION = 1; MARKETING_VERSION = 1;
PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent;
PRODUCT_NAME = "$(TARGET_NAME)"; PRODUCT_NAME = "$(TARGET_NAME)";
SWIFT_STRICT_CONCURRENCY = complete;
SWIFT_VERSION = 5.0; SWIFT_VERSION = 5.0;
}; };
name = Debug; name = Debug;
@ -939,6 +944,7 @@
PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent; PRODUCT_BUNDLE_IDENTIFIER = com.maxgoedjen.Secretive.SecretAgent;
PRODUCT_NAME = "$(TARGET_NAME)"; PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "Secretive - Secret Agent"; PROVISIONING_PROFILE_SPECIFIER = "Secretive - Secret Agent";
SWIFT_STRICT_CONCURRENCY = complete;
SWIFT_VERSION = 5.0; SWIFT_VERSION = 5.0;
}; };
name = Release; name = Release;