diff --git a/Sources/SecureStorage/SecureStorageError.swift b/Sources/SecureStorage/SecureStorageError.swift index 1d482cf06..390e111b4 100644 --- a/Sources/SecureStorage/SecureStorageError.swift +++ b/Sources/SecureStorage/SecureStorageError.swift @@ -44,6 +44,8 @@ public enum SecureStorageError: Error { case secError(status: Int32) case generalCryptoError case encodingFailed + case keystoreReadError(status: Int32) + case keystoreUpdateError(status: Int32) } extension SecureStorageError: CustomNSError { @@ -66,6 +68,8 @@ extension SecureStorageError: CustomNSError { case .secError: return 11 case .generalCryptoError: return 12 case .encodingFailed: return 13 + case .keystoreReadError: return 14 + case .keystoreUpdateError: return 15 } } @@ -87,6 +91,10 @@ extension SecureStorageError: CustomNSError { } case .keystoreError(status: let code): errorUserInfo["NSUnderlyingError"] = NSError(domain: "keystoreError", code: Int(code), userInfo: nil) + case .keystoreReadError(status: let code): + errorUserInfo["NSUnderlyingError"] = NSError(domain: "keystoreReadError", code: Int(code), userInfo: nil) + case .keystoreUpdateError(status: let code): + errorUserInfo["NSUnderlyingError"] = NSError(domain: "keystoreUpdateError", code: Int(code), userInfo: nil) case .secError(status: let code): errorUserInfo["NSUnderlyingError"] = NSError(domain: "secError", code: Int(code), userInfo: nil) case .authRequired, .invalidPassword, .noL1Key, .noL2Key, .duplicateRecord, .generalCryptoError, .encodingFailed: diff --git a/Sources/SecureStorage/SecureStorageKeyStoreProvider.swift b/Sources/SecureStorage/SecureStorageKeyStoreProvider.swift index f86c330c8..b8c318a8e 100644 --- a/Sources/SecureStorage/SecureStorageKeyStoreProvider.swift +++ b/Sources/SecureStorage/SecureStorageKeyStoreProvider.swift @@ -23,6 +23,7 @@ public protocol KeychainService { func itemMatching(_ query: [String: Any], _ result: UnsafeMutablePointer?) -> OSStatus func add(_ attributes: [String: Any], _ result: UnsafeMutablePointer?) -> OSStatus func delete(_ query: [String: Any]) -> OSStatus + func update(_ query: [String: Any], _ attributesToUpdate: [String: Any]) -> OSStatus } public extension KeychainService { @@ -37,6 +38,10 @@ public extension KeychainService { func delete(_ query: [String: Any]) -> OSStatus { SecItemDelete(query as CFDictionary) } + + func update(_ query: [String: Any], _ attributesToUpdate: [String: Any]) -> OSStatus { + SecItemUpdate(query as CFDictionary, attributesToUpdate as CFDictionary) + } } public struct DefaultKeychainService: KeychainService { @@ -56,6 +61,7 @@ public protocol SecureStorageKeyStoreProvider { var l1KeyEntryName: String { get } var l2KeyEntryName: String { get } var keychainServiceName: String { get } + var keychainAccessibilityValue: String { get } func storeGeneratedPassword(_ password: Data) throws func generatedPassword() throws -> Data? @@ -79,6 +85,10 @@ public extension SecureStorageKeyStoreProvider { DefaultKeychainService() } + var keychainAccessibilityValue: String { + kSecAttrAccessibleWhenUnlocked as String + } + func generatedPassword() throws -> Data? { return try readData(named: generatedPasswordEntryName, serviceName: keychainServiceName) } @@ -144,7 +154,7 @@ public extension SecureStorageKeyStoreProvider { var query = attributesForEntry(named: name, serviceName: serviceName) query[kSecAttrService as String] = serviceName - query[kSecAttrAccessible as String] = kSecAttrAccessibleWhenUnlocked + query[kSecAttrAccessible as String] = keychainAccessibilityValue query[kSecValueData as String] = base64Data let status = keychainService.add(query, nil) diff --git a/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift b/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift index ab609054c..89bce30c3 100644 --- a/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift +++ b/Tests/BrowserServicesKitTests/SecureVault/AutofillKeyStoreProviderTests.swift @@ -128,4 +128,23 @@ final class AutofillKeyStoreProviderTests: XCTestCase { XCTAssertEqual(keychainService.latestAddQuery[kSecAttrService as String] as! String, AutofillKeyStoreProvider.Constants.v3ServiceName) } } + + func testWhenWriteData_correctKeychainAccessibilityValueIsUsed() throws { + try AutofillKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let originalString = "Mock Keychain data!" + let data = originalString.data(using: .utf8)! + let encodedString = data.base64EncodedString() + let mockData = encodedString.data(using: .utf8)! + let keychainService = MockKeychainService() + let sut = AutofillKeyStoreProvider(keychainService: keychainService) + + // When + _ = try sut.writeData(mockData, named: entry.keychainIdentifier, serviceName: sut.keychainServiceName) + + // Then + XCTAssertEqual(keychainService.addCallCount, 1) + XCTAssertEqual(keychainService.latestAddQuery[kSecAttrAccessible as String] as! String, kSecAttrAccessibleWhenUnlocked as String) + } + } }