Keep the credential key with SwiftKeychain (0.15.0, breaking)

CHANGELOG.md

@@ -6,6 +6,28 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Breaking — the Keychain types are SwiftKeychain's (0.15.0)
+- **`KeychainAccess`, `SystemKeychain` and `CredentialKeyError` are removed.**
+  `CredentialStoreKey` now keeps its key through
+  [SwiftKeychain](https://github.com/jpurnell/SwiftKeychain) 0.1.1, and takes SwiftKeychain's
+  `KeychainAccess` (`SystemKeychain`, `InMemoryKeychain`, or your own). MCPClient does not
+  re-export them: code that names them depends on SwiftKeychain and imports it. See
+  "Migrating to 0.15.0" in the migration guide for each old name's replacement.
+- **`CredentialStoreKey.loadOrCreate()` and `delete()` throw `KeychainError`**, typed.
+  `CredentialKeyError.keychain(OSStatus)` becomes `.locked` (`errSecInteractionNotAllowed`),
+  `.missingEntitlement`, or `.status(Int32)`; `.malformedKey` becomes `.malformed(String)`.
+  `MCPOAuthSession.persistent()` throws them too.
+- **`CredentialStoreKey` and `MCPOAuthSession.persistent()` are macOS, iOS and visionOS
+  only.** They were compiled wherever `Security` could be imported, which included tvOS and
+  watchOS; SwiftKeychain does not support those, so there they are now compiled out as on
+  Linux.
+- **Stored credentials are not affected.** The key is read from the item every earlier version
+  wrote: service `com.swiftmcpclient.credential-store`, account `credential-encryption-key`,
+  after first unlock on this device only, not synchronized, no access group, and on macOS the
+  login keychain (`SystemKeychain(dataProtection: false)`), not the data-protection keychain.
+  `CredentialStoreKey.item` and the default `keychain` are public, and tests pin every field
+  of both.
+
 ### Security
 - **A redirect could send a request — custom headers, session id and body — to any origin,
   and from `https` to `http` (CWE-200, CWE-522, CWE-319).** `StreamableHTTPTransport` and
@@ -125,6 +147,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   a couple of short words, and a key in the host name.
 
 ### Changed
+- **Two processes starting at once now agree on the key.** A new key is added, never stored
+  over another; the process that loses the race reads the winner's key
+  (`KeychainKey.loadOrCreate`). Before, each stored its own and the second replaced the first,
+  stranding anything the first had already sealed.
+- The hand-written `SecItem` code is gone. `Package.resolved` gains one package,
+  `swiftkeychain`, with no traits enabled; it is linked on macOS, iOS and visionOS only.
 - **Cross-origin redirects are no longer followed.** A deployment that answered the configured
   URL with a redirect to another host or port — or from `http://` to `https://` — worked
   before and fails now with `MCPError.redirectRejected`. Configure the transport with the URL
@@ -235,6 +263,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   server adds.
 
 ### Tests
+- `CredentialStoreKeyTests` run against SwiftKeychain's `InMemoryKeychain` instead of a fake
+  of their own, through the public API rather than `@testable`. New: the item's identity is
+  pinned field by field against literal strings; the default store is
+  `SystemKeychain(dataProtection: false)`; a key seeded at the old identity is returned
+  unchanged; a locked keychain throws `.locked` and writes nothing.
 - `TransportRedirectWireTests` — two loopback servers, every request each HTTP transport makes
   × `301`/`302`/`303`/`307`/`308`, asserting on what the *second* server received; the same
   with an `authorization:` provider; four spellings of "elsewhere"; an HTTPS first server

Package.swift

@@ -19,6 +19,12 @@ var targets: [Target] = [
             // does, so the dependency is declared rather than borrowed.
             .product(name: "NIOSSL", package: "swift-nio-ssl"),
             .product(name: "WebSocketKit", package: "websocket-kit"),
+            // The Keychain, for the one key that seals the credential files. Apple-only:
+            // there is no Keychain on Linux, and SwiftKeychain imports Security. tvOS and
+            // watchOS are left out because SwiftKeychain does not support them.
+            .product(
+                name: "SwiftKeychain", package: "SwiftKeychain",
+                condition: .when(platforms: [.macOS, .iOS, .visionOS])),
         ],
         // Declared rather than left implicit. SwiftPM does not claim this catalog on a
         // plain build and warns that it is unhandled; the obvious quietening — `exclude` —
@@ -51,6 +57,10 @@ var targets: [Target] = [
             .product(name: "X509", package: "swift-certificates"),
             .product(name: "SwiftASN1", package: "swift-asn1"),
             .product(name: "Crypto", package: "swift-crypto"),
+            // `InMemoryKeychain`, so the credential key's tests never touch the real Keychain.
+            .product(
+                name: "SwiftKeychain", package: "SwiftKeychain",
+                condition: .when(platforms: [.macOS, .iOS, .visionOS])),
         ]
     ),
 ]
@@ -142,6 +152,10 @@ let package = Package(
         .package(url: "https://github.com/apple/swift-asn1.git", from: "1.6.0"),
         .package(url: "https://github.com/vapor/websocket-kit.git", from: "2.15.0"),
         .package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.4.3"),
+        // Where the credential store's key lives, in place of SecItem code written here. No
+        // traits: the core store and `KeychainKey` are all this needs, and SwiftPM resolves
+        // nothing a disabled trait names.
+        .package(url: "https://github.com/jpurnell/SwiftKeychain.git", from: "0.1.1"),
     ],
     targets: targets
 )

README.md

@@ -19,6 +19,13 @@ A Swift 6 client library for the [Model Context Protocol (MCP)](https://modelcon
 - macOS 14+ / iOS 17+ / tvOS 17+ / watchOS 10+
 - Linux (Ubuntu 22.04+) — built and tested in CI
 
+`MCPOAuthSession.persistent()` and `CredentialStoreKey`, which keep the key that seals stored
+credentials in the Keychain, are available on macOS, iOS and visionOS. They use
+[SwiftKeychain](https://github.com/jpurnell/SwiftKeychain) and take its `KeychainAccess`, so
+code that passes its own store (an `InMemoryKeychain` in tests, say) imports `SwiftKeychain`.
+On Linux, tvOS and watchOS, build the session from `EncryptedFileClientStorage` and
+`EncryptedFileRegistrationStore` with a key from that platform's own secret store.
+
 ## Installation
 
 Add to your `Package.swift`:
@@ -303,7 +310,8 @@ version, and a Developer ID signature.
 The signature is not decoration. `MCPOAuthSession` seals its credentials with a key from the
 Keychain, and the Keychain decides what may read that key by the program's code identity. An
 unsigned build has a new identity every time it is compiled, so every rebuild would lose the
-saved OAuth tokens. A signed one keeps them.
+saved OAuth tokens. A signed one keeps them. The key is in the login keychain, not the
+data-protection keychain, which an unsigned build cannot write to at all.
 
 The first Developer ID signing on a machine raises a system dialog asking to use the key, so
 run that command from a terminal you are sitting at and choose **Always Allow**. It is

Sources/MCPClient/CredentialStoreKey.swift

@@ -1,136 +1,7 @@
-#if canImport(Security)
+#if canImport(SwiftKeychain)
 import Foundation
 import Crypto
-import Security
-
-/// Why the credential store's key could not be obtained.
-public enum CredentialKeyError: Error, Equatable, Sendable {
-
-    /// The Keychain refused the read, the write, or the delete.
-    ///
-    /// Carries the `OSStatus`, because "Keychain failed" is unactionable and the status is
-    /// the only thing distinguishing a locked keychain from a missing entitlement.
-    case keychain(OSStatus)
-
-    /// The stored item was not a key of the expected size.
-    ///
-    /// Refused rather than replaced. Replacing it would mint a new key and quietly make every
-    /// stored credential undecryptable, which reaches the user as an unexplained request to
-    /// sign in again.
-    case malformedKey
-}
-
-/// Somewhere small secrets are kept.
-///
-/// A protocol so the logic above it can be tested. The real Keychain prompts, behaves
-/// differently under an unsigned test bundle, and leaves items on the machine that ran the
-/// tests — none of which exercises the behaviour that matters here, which is what happens on
-/// the *second* launch and when a read is refused.
-public protocol KeychainAccess: Sendable {
-
-    /// The data stored for a service and account, if any.
-    func data(service: String, account: String) throws -> Data?
-
-    /// Stores data, replacing anything already there.
-    func store(_ data: Data, service: String, account: String) throws
-
-    /// Removes stored data. Removing what is not there is not an error.
-    func delete(service: String, account: String) throws
-}
-
-/// The system Keychain.
-public struct SystemKeychain: KeychainAccess {
-
-    /// Creates an accessor.
-    public init() {}
-
-    /// Reads an item.
-    ///
-    /// - Parameters:
-    ///   - service: The service the item is filed under.
-    ///   - account: The account name.
-    /// - Returns: The data, or `nil` if there is no such item.
-    /// - Throws: ``CredentialKeyError/keychain(_:)``.
-    public func data(service: String, account: String) throws -> Data? {
-        let query: [String: Any] = [
-            kSecClass as String: kSecClassGenericPassword,
-            kSecAttrService as String: service,
-            kSecAttrAccount as String: account,
-            kSecReturnData as String: true,
-            kSecMatchLimit as String: kSecMatchLimitOne
-        ]
-
-        var item: CFTypeRef?
-        let status = SecItemCopyMatching(query as CFDictionary, &item)
-
-        switch status {
-        case errSecSuccess:
-            return item as? Data
-        case errSecItemNotFound:
-            // First run, not a failure.
-            return nil
-        default:
-            throw CredentialKeyError.keychain(status)
-        }
-    }
-
-    /// Stores an item, replacing any existing one.
-    ///
-    /// - Parameters:
-    ///   - data: What to store.
-    ///   - service: The service to file it under.
-    ///   - account: The account name.
-    /// - Throws: ``CredentialKeyError/keychain(_:)``.
-    public func store(_ data: Data, service: String, account: String) throws {
-        let attributes: [String: Any] = [
-            kSecClass as String: kSecClassGenericPassword,
-            kSecAttrService as String: service,
-            kSecAttrAccount as String: account,
-            kSecValueData as String: data,
-            // Never leaves this device, and is unavailable until the device has been unlocked
-            // once. A credential key that syncs is a credential key on every device the user
-            // owns, including the ones they stopped using.
-            kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
-        ]
-
-        var status = SecItemAdd(attributes as CFDictionary, nil)
-
-        if status == errSecDuplicateItem {
-            // Replace rather than fail. An add that collides means something is already
-            // filed here, and the caller asked for this value to be the one stored.
-            let query: [String: Any] = [
-                kSecClass as String: kSecClassGenericPassword,
-                kSecAttrService as String: service,
-                kSecAttrAccount as String: account
-            ]
-            status = SecItemUpdate(
-                query as CFDictionary,
-                [kSecValueData as String: data] as CFDictionary)
-        }
-
-        guard status == errSecSuccess else {
-            throw CredentialKeyError.keychain(status)
-        }
-    }
-
-    /// Removes an item.
-    ///
-    /// - Parameters:
-    ///   - service: The service the item is filed under.
-    ///   - account: The account name.
-    /// - Throws: ``CredentialKeyError/keychain(_:)``.
-    public func delete(service: String, account: String) throws {
-        let query: [String: Any] = [
-            kSecClass as String: kSecClassGenericPassword,
-            kSecAttrService as String: service,
-            kSecAttrAccount as String: account
-        ]
-        let status = SecItemDelete(query as CFDictionary)
-        guard status == errSecSuccess || status == errSecItemNotFound else {
-            throw CredentialKeyError.keychain(status)
-        }
-    }
-}
+import SwiftKeychain
 
 /// The key that `EncryptedFileClientStorage` seals credentials with, kept in the Keychain.
 ///
@@ -142,23 +13,36 @@ public struct SystemKeychain: KeychainAccess {
 /// connection and a prompt surface that grows with them; putting the key beside the file
 /// protects nothing at all. One key in the Keychain, one encrypted file next to the app's
 /// other state.
-public struct CredentialStoreKey {
-
-    /// The service the key is filed under.
-    static let service = "com.swiftmcpclient.credential-store"
-
-    /// The account name for the key item.
-    static let account = "credential-encryption-key"
-
-    /// A 256-bit key, in bytes.
-    private static let keyByteCount = 32
+///
+/// The Keychain itself is SwiftKeychain's: pass any `KeychainAccess`, such as its
+/// `InMemoryKeychain` in tests. Errors are SwiftKeychain's `KeychainError`.
+public struct CredentialStoreKey: Sendable {
 
-    private let keychain: any KeychainAccess
+    /// The Keychain item the key is kept in.
+    ///
+    /// Fixed, and the same item every earlier version of this package wrote: a key filed
+    /// anywhere else is a key that is not found, and a key that is not found is replaced by a
+    /// new one that cannot open any credential sealed before it. Kept on this device, readable
+    /// after the first unlock, never synchronized, in the application's own access group.
+    public static let item = KeychainItem(
+        service: "com.swiftmcpclient.credential-store",
+        account: "credential-encryption-key",
+        accessibility: .afterFirstUnlockThisDeviceOnly,
+        synchronizable: false,
+        accessGroup: nil)
+
+    /// Where the key is kept.
+    public let keychain: any KeychainAccess
 
     /// Creates an accessor for the credential key.
     ///
-    /// - Parameter keychain: Where the key is kept. Defaults to the system Keychain.
-    public init(keychain: any KeychainAccess = SystemKeychain()) {
+    /// The default is the system Keychain without the data-protection keychain, which on
+    /// macOS means the login keychain. That is where every earlier version stored the key, and
+    /// where an unsigned command-line build can store one at all. On iOS there is only one
+    /// keychain and the choice is ignored.
+    ///
+    /// - Parameter keychain: Where the key is kept.
+    public init(keychain: any KeychainAccess = SystemKeychain(dataProtection: false)) {
         self.keychain = keychain
     }
 
@@ -169,37 +53,26 @@ public struct CredentialStoreKey {
     /// stored credential undecryptable and sends the user back to a sign-in screen with no
     /// explanation.
     ///
+    /// A stored item of the wrong size is refused rather than replaced, a refused read
+    /// (`KeychainError.locked`, for one) is thrown rather than read as "no key yet", and a new
+    /// key is only added, never stored over another: if a second process created one first,
+    /// that one is returned.
+    ///
     /// - Returns: The key.
-    /// - Throws: ``CredentialKeyError``.
-    public func loadOrCreate() throws -> SymmetricKey {
-        if let stored = try keychain.data(service: Self.service, account: Self.account) {
-            guard stored.count == Self.keyByteCount else {
-                // Not replaced. See `malformedKey`.
-                throw CredentialKeyError.malformedKey
-            }
-            return SymmetricKey(data: stored)
-        }
-
-        let created = SymmetricKey(size: .bits256)
-        // `Data.init` passed directly rather than wrapped in a closure: a closure whose result
-        // mentions the borrowed buffer cannot be told apart from one that lets it escape, and
-        // the initialiser already takes a sequence of bytes.
-        let bytes = created.withUnsafeBytes(Data.init)
-
-        // Stored before it is returned. Handing back a key that was never written would work
-        // for this launch and be gone by the next one, taking every credential with it.
-        try keychain.store(bytes, service: Self.service, account: Self.account)
-        return created
+    /// - Throws: `KeychainError`.
+    public func loadOrCreate() throws(KeychainError) -> SymmetricKey {
+        SymmetricKey(data: try KeychainKey.loadOrCreate(Self.item, using: keychain))
     }
 
     /// Removes the stored key.
     ///
     /// The credential file becomes undecryptable, which is the intended effect: this is how a
-    /// caller discards every stored credential at once without reaching each one.
+    /// caller discards every stored credential at once without reaching each one. Removing a
+    /// key that is not there is not an error.
     ///
-    /// - Throws: ``CredentialKeyError/keychain(_:)``.
-    public func delete() throws {
-        try keychain.delete(service: Self.service, account: Self.account)
+    /// - Throws: `KeychainError`.
+    public func delete() throws(KeychainError) {
+        try keychain.delete(Self.item)
     }
 }
 #endif