diff --git a/OpenASO.xcodeproj/project.pbxproj b/OpenASO.xcodeproj/project.pbxproj index 7ed7e0b..a4abff8 100644 --- a/OpenASO.xcodeproj/project.pbxproj +++ b/OpenASO.xcodeproj/project.pbxproj @@ -1611,7 +1611,7 @@ CODE_SIGN_IDENTITY = "Apple Development"; CODE_SIGN_STYLE = Automatic; COMBINE_HIDPI_IMAGES = YES; - CURRENT_PROJECT_VERSION = 9; + CURRENT_PROJECT_VERSION = 10; DEAD_CODE_STRIPPING = YES; DEVELOPMENT_TEAM = 85BF4D5D6B; GENERATE_INFOPLIST_FILE = NO; @@ -1630,7 +1630,7 @@ "@executable_path/../Frameworks", ); MACOSX_DEPLOYMENT_TARGET = 15.0; - MARKETING_VERSION = 0.4.3; + MARKETING_VERSION = 0.4.4; POSTHOG_HOST = ""; POSTHOG_PROJECT_TOKEN = ""; PRODUCT_BUNDLE_IDENTIFIER = com.thirdtech.openaso.dev; @@ -1840,7 +1840,7 @@ CODE_SIGN_IDENTITY = "Apple Development"; CODE_SIGN_STYLE = Automatic; COMBINE_HIDPI_IMAGES = YES; - CURRENT_PROJECT_VERSION = 9; + CURRENT_PROJECT_VERSION = 10; DEAD_CODE_STRIPPING = YES; DEVELOPMENT_TEAM = 85BF4D5D6B; ENABLE_HARDENED_RUNTIME = YES; @@ -1860,7 +1860,7 @@ "@executable_path/../Frameworks", ); MACOSX_DEPLOYMENT_TARGET = 15.0; - MARKETING_VERSION = 0.4.3; + MARKETING_VERSION = 0.4.4; POSTHOG_HOST = ""; POSTHOG_PROJECT_TOKEN = ""; PRODUCT_BUNDLE_IDENTIFIER = com.thirdtech.openaso; diff --git a/OpenASO/Services/Credentials/KeychainService.swift b/OpenASO/Services/Credentials/KeychainService.swift index 6c1340e..7db36a2 100644 --- a/OpenASO/Services/Credentials/KeychainService.swift +++ b/OpenASO/Services/Credentials/KeychainService.swift @@ -41,22 +41,40 @@ extension KeychainService { struct SystemKeychainService: KeychainService { typealias CopyMatching = (CFDictionary, UnsafeMutablePointer?) -> OSStatus + typealias Update = (CFDictionary, CFDictionary) -> OSStatus + typealias Add = (CFDictionary, UnsafeMutablePointer?) -> OSStatus + typealias Delete = (CFDictionary) -> OSStatus typealias ReadFailureReporter = (KeychainReadFailure) -> Void private static let logger = Logger(subsystem: OpenASOLog.subsystem, category: "keychain") private let copyMatching: CopyMatching + private let update: Update + private let add: Add + private let delete: Delete private let reportReadFailure: ReadFailureReporter init( copyMatching: @escaping CopyMatching = { query, result in SecItemCopyMatching(query, result) }, + update: @escaping Update = { query, attributes in + SecItemUpdate(query, attributes) + }, + add: @escaping Add = { query, result in + SecItemAdd(query, result) + }, + delete: @escaping Delete = { query in + SecItemDelete(query) + }, reportReadFailure: @escaping ReadFailureReporter = { failure in SystemKeychainService.logReadFailure(failure) } ) { self.copyMatching = copyMatching + self.update = update + self.add = add + self.delete = delete self.reportReadFailure = reportReadFailure } @@ -80,17 +98,10 @@ struct SystemKeychainService: KeychainService { usesDataProtectionKeychain: false ) ) - #if DEBUG - // A debug build that had to use the legacy writer cannot prove that `save` migrated the - // item to the protected store, because `save` may itself have taken the debug fallback. - // Keep the source item so credentials and sessions survive the next development launch. - reportFailureIfNeeded(legacyResult) - return legacyResult - #else if case .success(let data) = legacyResult, - (try? save(data, service: service, account: account)) != nil + (try? saveToDataProtectionKeychain(data, service: service, account: account)) == true { - SecItemDelete(keychainQuery( + _ = delete(keychainQuery( service: service, account: account, usesDataProtectionKeychain: false @@ -98,7 +109,6 @@ struct SystemKeychainService: KeychainService { } reportFailureIfNeeded(legacyResult) return legacyResult - #endif } private func readData(query baseQuery: [String: Any]) -> KeychainReadResult { @@ -133,21 +143,33 @@ struct SystemKeychainService: KeychainService { } func save(_ data: Data, service: String, account: String) throws { + guard try saveToDataProtectionKeychain(data, service: service, account: account) else { + try saveToLegacyKeychain(data, service: service, account: account) + return + } + } + + /// Returns `false` when this signed process cannot use the Data Protection Keychain and the + /// caller should use the encrypted login Keychain instead. + private func saveToDataProtectionKeychain( + _ data: Data, + service: String, + account: String + ) throws -> Bool { let query = keychainQuery( service: service, account: account, usesDataProtectionKeychain: true ) let attributes: [String: Any] = [kSecValueData as String: data] - let status = SecItemUpdate(query as CFDictionary, attributes as CFDictionary) + let status = update(query as CFDictionary, attributes as CFDictionary) if status == errSecSuccess { - return + return true } if Self.shouldUseLegacyWriteFallback(for: status) { - try saveToLegacyKeychain(data, service: service, account: account) - return + return false } guard status == errSecItemNotFound else { @@ -158,25 +180,21 @@ struct SystemKeychainService: KeychainService { addQuery[kSecValueData as String] = data addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock - let addStatus = SecItemAdd(addQuery as CFDictionary, nil) + let addStatus = add(addQuery as CFDictionary, nil) if Self.shouldUseLegacyWriteFallback(for: addStatus) { - try saveToLegacyKeychain(data, service: service, account: account) - return + return false } guard addStatus == errSecSuccess else { throw OpenASOError.providerUnavailable("Could not save item to Keychain.") } + return true } - /// Local ad-hoc/debug builds can lack the application identifier entitlement required by the - /// Data Protection Keychain. Keep Release on the protected store while allowing development - /// builds to use the encrypted macOS login Keychain that `readData` already migrates from. + /// Directly distributed and local builds can lack the application identifier entitlement + /// required by the Data Protection Keychain. In that case, use the encrypted macOS login + /// Keychain that `readData` already supports. nonisolated static func shouldUseLegacyWriteFallback(for status: OSStatus) -> Bool { - #if DEBUG status == errSecMissingEntitlement - #else - false - #endif } private func saveToLegacyKeychain(_ data: Data, service: String, account: String) throws { @@ -186,7 +204,7 @@ struct SystemKeychainService: KeychainService { usesDataProtectionKeychain: false ) let attributes: [String: Any] = [kSecValueData as String: data] - let updateStatus = SecItemUpdate(query as CFDictionary, attributes as CFDictionary) + let updateStatus = update(query as CFDictionary, attributes as CFDictionary) if updateStatus == errSecSuccess { return } @@ -198,18 +216,18 @@ struct SystemKeychainService: KeychainService { var addQuery = query addQuery[kSecValueData as String] = data addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock - guard SecItemAdd(addQuery as CFDictionary, nil) == errSecSuccess else { + guard add(addQuery as CFDictionary, nil) == errSecSuccess else { throw OpenASOError.providerUnavailable("Could not save item to Keychain.") } } func delete(service: String, account: String) { - SecItemDelete(keychainQuery( + _ = delete(keychainQuery( service: service, account: account, usesDataProtectionKeychain: true ) as CFDictionary) - SecItemDelete(keychainQuery( + _ = delete(keychainQuery( service: service, account: account, usesDataProtectionKeychain: false diff --git a/OpenASOTests/AppServicesDependencyTests.swift b/OpenASOTests/AppServicesDependencyTests.swift index 5013fa0..2479324 100644 --- a/OpenASOTests/AppServicesDependencyTests.swift +++ b/OpenASOTests/AppServicesDependencyTests.swift @@ -818,14 +818,62 @@ struct AppServicesDependencyTests { } @Test - func debugBuildFallsBackToLoginKeychainOnlyForMissingEntitlement() { - #if DEBUG + func systemKeychainFallsBackToLoginKeychainOnlyForMissingEntitlement() { #expect(SystemKeychainService.shouldUseLegacyWriteFallback(for: errSecMissingEntitlement)) - #endif #expect(!SystemKeychainService.shouldUseLegacyWriteFallback(for: errSecAuthFailed)) #expect(!SystemKeychainService.shouldUseLegacyWriteFallback(for: errSecInteractionNotAllowed)) } + @Test + func systemKeychainUsesLoginKeychainWhenDataProtectionEntitlementIsMissing() throws { + var updatedQueries: [[String: Any]] = [] + var addedQueries: [[String: Any]] = [] + let keychain = SystemKeychainService( + update: { query, _ in + let query = query as NSDictionary as! [String: Any] + updatedQueries.append(query) + return query[kSecUseDataProtectionKeychain as String] as? Bool == true + ? errSecMissingEntitlement + : errSecItemNotFound + }, + add: { query, _ in + addedQueries.append(query as NSDictionary as! [String: Any]) + return errSecSuccess + } + ) + + try keychain.save(Data("secret".utf8), service: "service", account: "account") + + #expect(updatedQueries.count == 2) + #expect(updatedQueries.first?[kSecUseDataProtectionKeychain as String] as? Bool == true) + #expect(updatedQueries.last?[kSecUseDataProtectionKeychain as String] == nil) + #expect(addedQueries.count == 1) + #expect(addedQueries.first?[kSecUseDataProtectionKeychain as String] == nil) + } + + @Test + func systemKeychainPreservesLegacyItemWhenProtectedMigrationLacksEntitlement() { + let data = Data("secret".utf8) + var readCount = 0 + var deletedQueries: [[String: Any]] = [] + let keychain = SystemKeychainService( + copyMatching: { _, result in + readCount += 1 + guard readCount == 2 else { return errSecItemNotFound } + result?.pointee = data as CFData + return errSecSuccess + }, + update: { _, _ in errSecMissingEntitlement }, + delete: { query in + deletedQueries.append(query as NSDictionary as! [String: Any]) + return errSecSuccess + } + ) + + #expect(keychain.readData(service: "service", account: "account") == .success(data)) + #expect(deletedQueries.isEmpty) + } + @Test func systemKeychainTreatsMissingItemAsExpectedAbsence() { var reportedFailures: [KeychainReadFailure] = []