From 2a1f65adab802d113e3bf0f30467515a51e96670 Mon Sep 17 00:00:00 2001 From: Mariano Belinky Date: Tue, 3 Mar 2026 14:26:45 +0100 Subject: [PATCH] fix(iOS): make keychain writes rollback-safe and dedupe store helpers --- apps/ios/Sources/Gateway/KeychainStore.swift | 40 +--------- .../OpenClawKit/GatewayTLSPinning.swift | 43 ++--------- .../GenericPasswordKeychainStore.swift | 77 +++++++++++++++++++ 3 files changed, 88 insertions(+), 72 deletions(-) create mode 100644 apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift diff --git a/apps/ios/Sources/Gateway/KeychainStore.swift b/apps/ios/Sources/Gateway/KeychainStore.swift index 4d1a000986e..c4f1871eedb 100644 --- a/apps/ios/Sources/Gateway/KeychainStore.swift +++ b/apps/ios/Sources/Gateway/KeychainStore.swift @@ -1,48 +1,16 @@ import Foundation -import Security +import OpenClawKit enum KeychainStore { static func loadString(service: String, account: String) -> String? { - 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) - guard status == errSecSuccess, let data = item as? Data else { return nil } - return String(data: data, encoding: .utf8) + GenericPasswordKeychainStore.loadString(service: service, account: account) } static func saveString(_ value: String, service: String, account: String) -> Bool { - // Delete-then-add ensures kSecAttrAccessible is always applied. - // SecItemUpdate cannot change the accessibility level of an existing item, - // so a stale item created with a weaker policy would retain it on update. - let data = Data(value.utf8) - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service, - kSecAttrAccount as String: account, - ] - - SecItemDelete(query as CFDictionary) - - var insert = query - insert[kSecValueData as String] = data - insert[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly - return SecItemAdd(insert as CFDictionary, nil) == errSecSuccess + GenericPasswordKeychainStore.saveString(value, service: service, account: account) } static func delete(service: String, account: String) -> Bool { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: service, - kSecAttrAccount as String: account, - ] - let status = SecItemDelete(query as CFDictionary) - return status == errSecSuccess || status == errSecItemNotFound + GenericPasswordKeychainStore.delete(service: service, account: account) } } diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayTLSPinning.swift b/apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayTLSPinning.swift index 981f43047ab..ca88ae8fb7e 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayTLSPinning.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayTLSPinning.swift @@ -25,13 +25,14 @@ public enum GatewayTLSStore { public static func loadFingerprint(stableID: String) -> String? { self.migrateFromUserDefaultsIfNeeded(stableID: stableID) - let raw = self.keychainLoad(account: stableID)?.trimmingCharacters(in: .whitespacesAndNewlines) + let raw = GenericPasswordKeychainStore.loadString(service: self.keychainService, account: stableID)? + .trimmingCharacters(in: .whitespacesAndNewlines) if raw?.isEmpty == false { return raw } return nil } public static func saveFingerprint(_ value: String, stableID: String) { - self.keychainSave(value, account: stableID) + _ = GenericPasswordKeychainStore.saveString(value, service: self.keychainService, account: stableID) } // MARK: - Migration @@ -45,43 +46,13 @@ public enum GatewayTLSStore { .trimmingCharacters(in: .whitespacesAndNewlines), !existing.isEmpty else { return } - if self.keychainLoad(account: stableID) == nil { - guard self.keychainSave(existing, account: stableID) else { return } + if GenericPasswordKeychainStore.loadString(service: self.keychainService, account: stableID) == nil { + guard GenericPasswordKeychainStore.saveString(existing, service: self.keychainService, account: stableID) else { + return + } } defaults.removeObject(forKey: legacyKey) } - - // MARK: - Self-contained Keychain helpers (OpenClawKit can't import iOS KeychainStore) - - private static func keychainLoad(account: String) -> String? { - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: self.keychainService, - kSecAttrAccount as String: account, - kSecReturnData as String: true, - kSecMatchLimit as String: kSecMatchLimitOne, - ] - var item: CFTypeRef? - let status = SecItemCopyMatching(query as CFDictionary, &item) - guard status == errSecSuccess, let data = item as? Data else { return nil } - return String(data: data, encoding: .utf8) - } - - @discardableResult - private static func keychainSave(_ value: String, account: String) -> Bool { - let data = Data(value.utf8) - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: self.keychainService, - kSecAttrAccount as String: account, - ] - // Delete-then-add to enforce accessibility attribute. - SecItemDelete(query as CFDictionary) - var insert = query - insert[kSecValueData as String] = data - insert[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly - return SecItemAdd(insert as CFDictionary, nil) == errSecSuccess - } } public final class GatewayTLSPinningSession: NSObject, WebSocketSessioning, URLSessionDelegate, @unchecked Sendable { diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift b/apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift new file mode 100644 index 00000000000..01603f7848b --- /dev/null +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/GenericPasswordKeychainStore.swift @@ -0,0 +1,77 @@ +import Foundation +import Security + +public enum GenericPasswordKeychainStore { + public static func loadString(service: String, account: String) -> String? { + guard let data = self.loadData(service: service, account: account) else { return nil } + return String(data: data, encoding: .utf8) + } + + @discardableResult + public static func saveString( + _ value: String, + service: String, + account: String, + accessible: CFString = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly + ) -> Bool { + self.saveData(Data(value.utf8), service: service, account: account, accessible: accessible) + } + + @discardableResult + public static func delete(service: String, account: String) -> Bool { + let query = self.baseQuery(service: service, account: account) + let status = SecItemDelete(query as CFDictionary) + return status == errSecSuccess || status == errSecItemNotFound + } + + private static func loadData(service: String, account: String) -> Data? { + var query = self.baseQuery(service: service, account: account) + query[kSecReturnData as String] = true + query[kSecMatchLimit as String] = kSecMatchLimitOne + + var item: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &item) + guard status == errSecSuccess, let data = item as? Data else { return nil } + return data + } + + @discardableResult + private static func saveData( + _ data: Data, + service: String, + account: String, + accessible: CFString + ) -> Bool { + let query = self.baseQuery(service: service, account: account) + let previousData = self.loadData(service: service, account: account) + + let deleteStatus = SecItemDelete(query as CFDictionary) + guard deleteStatus == errSecSuccess || deleteStatus == errSecItemNotFound else { + return false + } + + var insert = query + insert[kSecValueData as String] = data + insert[kSecAttrAccessible as String] = accessible + if SecItemAdd(insert as CFDictionary, nil) == errSecSuccess { + return true + } + + // Best-effort rollback: preserve prior value if replacement fails. + guard let previousData else { return false } + var rollback = query + rollback[kSecValueData as String] = previousData + rollback[kSecAttrAccessible as String] = accessible + _ = SecItemDelete(query as CFDictionary) + _ = SecItemAdd(rollback as CFDictionary, nil) + return false + } + + private static func baseQuery(service: String, account: String) -> [String: Any] { + [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + ] + } +}