From 34d5577ae4f05e40e01982d4671ee1a5374f8393 Mon Sep 17 00:00:00 2001 From: Artur Guseinov Date: Tue, 31 Oct 2023 03:17:36 +0800 Subject: [PATCH 1/4] Keychain migration --- .../Keychain/KeychainStorage.swift | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift b/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift index c8c3ee083..4573172d3 100644 --- a/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift +++ b/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift @@ -55,7 +55,8 @@ public final class KeychainStorage: KeychainStorageProtocol { case errSecSuccess: return item as? Data case errSecItemNotFound: - return nil + // TODO: Replace with nil once migration period ends + return try tryMigrateAttrAccessible(key: key) default: throw KeychainError(status) } @@ -100,11 +101,46 @@ public final class KeychainStorage: KeychainStorageProtocol { private func buildBaseServiceQuery(for key: String) -> [CFString: Any] { return [ kSecClass: kSecClassGenericPassword, - kSecAttrAccessible: kSecAttrAccessibleWhenUnlockedThisDeviceOnly, + kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, kSecAttrIsInvisible: true, kSecUseDataProtectionKeychain: true, kSecAttrService: service, kSecAttrAccount: key ] } + + private func tryMigrateAttrAccessible(key: String) throws -> Data? { + var query = buildBaseServiceQuery(for: key) + query[kSecReturnData] = true + query[kSecAttrAccessible] = kSecAttrAccessibleWhenUnlockedThisDeviceOnly + + var item: CFTypeRef? + let status = secItem.copyMatching(query as CFDictionary, &item) + + switch status { + case errSecSuccess: // Migration needed + guard let data = item as? Data else { return nil } + + // Fetching old value + var deleteQuery = buildBaseServiceQuery(for: key) + deleteQuery[kSecAttrAccessible] = kSecAttrAccessibleWhenUnlockedThisDeviceOnly + + // Deleting old value + let status = secItem.delete(deleteQuery as CFDictionary) + + guard status == errSecSuccess || status == errSecItemNotFound else { + throw KeychainError(status) + } + + // Replacing with new value + try add(data: data, forKey: key) + + // Continue `readData` execution + return item as? Data + case errSecItemNotFound: + return nil + default: + throw KeychainError(status) + } + } } From 8ad09bbeb94b61466c9bb3e3ac55ea687313f3fc Mon Sep 17 00:00:00 2001 From: Artur Guseinov Date: Tue, 31 Oct 2023 21:42:10 +0800 Subject: [PATCH 2/4] Update instead of replacing --- .../Keychain/KeychainError.swift | 10 ++--- .../Keychain/KeychainStorage.swift | 44 ++++++------------- .../ClientAuth/ClientIdStorage.swift | 3 ++ 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/Sources/WalletConnectKMS/Keychain/KeychainError.swift b/Sources/WalletConnectKMS/Keychain/KeychainError.swift index 78c16dcf7..fc3626e94 100644 --- a/Sources/WalletConnectKMS/Keychain/KeychainError.swift +++ b/Sources/WalletConnectKMS/Keychain/KeychainError.swift @@ -1,22 +1,22 @@ import Foundation // TODO: Integrate with WalletConnectError -struct KeychainError: Error, LocalizedError { +public struct KeychainError: Error, LocalizedError { - let status: OSStatus + public let status: OSStatus - init(_ status: OSStatus) { + public init(_ status: OSStatus) { self.status = status } - var errorDescription: String? { + public var errorDescription: String? { return "OSStatus: \(status), message: \(status.message)" } } extension KeychainError: CustomStringConvertible { - var description: String { + public var description: String { status.message } } diff --git a/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift b/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift index 4573172d3..f52f4c828 100644 --- a/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift +++ b/Sources/WalletConnectKMS/Keychain/KeychainStorage.swift @@ -55,8 +55,7 @@ public final class KeychainStorage: KeychainStorageProtocol { case errSecSuccess: return item as? Data case errSecItemNotFound: - // TODO: Replace with nil once migration period ends - return try tryMigrateAttrAccessible(key: key) + return tryMigrateAttrAccessible(key: key) // TODO: Replace with nil once migration period ends default: throw KeychainError(status) } @@ -109,38 +108,23 @@ public final class KeychainStorage: KeychainStorageProtocol { ] } - private func tryMigrateAttrAccessible(key: String) throws -> Data? { - var query = buildBaseServiceQuery(for: key) - query[kSecReturnData] = true - query[kSecAttrAccessible] = kSecAttrAccessibleWhenUnlockedThisDeviceOnly - - var item: CFTypeRef? - let status = secItem.copyMatching(query as CFDictionary, &item) + private func tryMigrateAttrAccessible(key: String) -> Data? { + var updateQuery = buildBaseServiceQuery(for: key) + updateQuery[kSecAttrAccessible] = kSecAttrAccessibleWhenUnlockedThisDeviceOnly - switch status { - case errSecSuccess: // Migration needed - guard let data = item as? Data else { return nil } - - // Fetching old value - var deleteQuery = buildBaseServiceQuery(for: key) - deleteQuery[kSecAttrAccessible] = kSecAttrAccessibleWhenUnlockedThisDeviceOnly + let attributes = [kSecAttrAccessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly] + let status = secItem.update(updateQuery as CFDictionary, attributes as CFDictionary) - // Deleting old value - let status = secItem.delete(deleteQuery as CFDictionary) + guard status == errSecSuccess else { + return nil + } - guard status == errSecSuccess || status == errSecItemNotFound else { - throw KeychainError(status) - } + var readQuery = buildBaseServiceQuery(for: key) + readQuery[kSecReturnData] = true - // Replacing with new value - try add(data: data, forKey: key) + var item: CFTypeRef? + _ = secItem.copyMatching(readQuery as CFDictionary, &item) - // Continue `readData` execution - return item as? Data - case errSecItemNotFound: - return nil - default: - throw KeychainError(status) - } + return item as? Data } } diff --git a/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift b/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift index 272451b96..05125a486 100644 --- a/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift +++ b/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift @@ -26,6 +26,9 @@ public struct ClientIdStorage: ClientIdStoring { let publicPart = try getPublicPart() return try getPrivatePart(for: publicPart) } catch { + guard let error = error as? KeychainError, error.status == errSecItemNotFound else { + throw error + } let privateKey = SigningPrivateKey() try setPrivatePart(privateKey) setPublicPart(privateKey.publicKey) From 9dd5dd0356507e8fd1c769dc0b492481c1d08208 Mon Sep 17 00:00:00 2001 From: Artur Guseinov Date: Wed, 1 Nov 2023 00:52:13 +0800 Subject: [PATCH 3/4] Keychain error refactor --- .../Crypto/KeyManagementService.swift | 2 +- .../Keychain/KeychainError.swift | 23 +++++++++++++------ .../ClientAuth/ClientIdStorage.swift | 11 +++++---- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/Sources/WalletConnectKMS/Crypto/KeyManagementService.swift b/Sources/WalletConnectKMS/Crypto/KeyManagementService.swift index 53a271b14..975a39251 100644 --- a/Sources/WalletConnectKMS/Crypto/KeyManagementService.swift +++ b/Sources/WalletConnectKMS/Crypto/KeyManagementService.swift @@ -93,7 +93,7 @@ public class KeyManagementService: KeyManagementServiceProtocol { public func getPrivateKey(for publicKey: AgreementPublicKey) throws -> AgreementPrivateKey? { do { return try keychain.read(key: publicKey.hexRepresentation) as AgreementPrivateKey - } catch let error where (error as? KeychainError)?.status == errSecItemNotFound { + } catch KeychainError.itemNotFound { return nil } catch { throw error diff --git a/Sources/WalletConnectKMS/Keychain/KeychainError.swift b/Sources/WalletConnectKMS/Keychain/KeychainError.swift index fc3626e94..9b947819f 100644 --- a/Sources/WalletConnectKMS/Keychain/KeychainError.swift +++ b/Sources/WalletConnectKMS/Keychain/KeychainError.swift @@ -1,23 +1,32 @@ import Foundation -// TODO: Integrate with WalletConnectError -public struct KeychainError: Error, LocalizedError { - - public let status: OSStatus +public enum KeychainError: Error, LocalizedError { + case itemNotFound + case other(OSStatus) public init(_ status: OSStatus) { - self.status = status + switch status { + case errSecItemNotFound: + self = .itemNotFound + default: + self = .other(status) + } } public var errorDescription: String? { - return "OSStatus: \(status), message: \(status.message)" + switch self { + case .itemNotFound: + return "Keychain item not found" + case .other(let status): + return "OSStatus: \(status), message: \(status.message)" + } } } extension KeychainError: CustomStringConvertible { public var description: String { - status.message + return errorDescription ?? "" } } diff --git a/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift b/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift index 05125a486..e07333198 100644 --- a/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift +++ b/Sources/WalletConnectRelay/ClientAuth/ClientIdStorage.swift @@ -25,14 +25,13 @@ public struct ClientIdStorage: ClientIdStoring { do { let publicPart = try getPublicPart() return try getPrivatePart(for: publicPart) - } catch { - guard let error = error as? KeychainError, error.status == errSecItemNotFound else { - throw error - } + } catch Errors.privatePartNotFound, Errors.publicPartNotFound { let privateKey = SigningPrivateKey() try setPrivatePart(privateKey) setPublicPart(privateKey.publicKey) return privateKey + } catch { + throw error } } @@ -79,8 +78,10 @@ private extension ClientIdStorage { func getPrivatePart(for publicPart: SigningPublicKey) throws -> SigningPrivateKey { do { return try keychain.read(key: publicPart.storageId) - } catch { + } catch KeychainError.itemNotFound { throw Errors.privatePartNotFound + } catch { + throw error } } From 2e7b778bdab041a019535760972395c4952af22b Mon Sep 17 00:00:00 2001 From: Artur Guseinov Date: Wed, 1 Nov 2023 00:58:40 +0800 Subject: [PATCH 4/4] Status for KeychainError --- Sources/WalletConnectKMS/Keychain/KeychainError.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Sources/WalletConnectKMS/Keychain/KeychainError.swift b/Sources/WalletConnectKMS/Keychain/KeychainError.swift index 9b947819f..de1865aeb 100644 --- a/Sources/WalletConnectKMS/Keychain/KeychainError.swift +++ b/Sources/WalletConnectKMS/Keychain/KeychainError.swift @@ -13,6 +13,15 @@ public enum KeychainError: Error, LocalizedError { } } + public var status: OSStatus { + switch self { + case .itemNotFound: + return errSecItemNotFound + case .other(let status): + return status + } + } + public var errorDescription: String? { switch self { case .itemNotFound: