Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable DBP Keychain Item Accessibility Migration #827

Merged
merged 5 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Sources/SecureStorage/SecureStorageError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}

Expand All @@ -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:
Expand Down
12 changes: 11 additions & 1 deletion Sources/SecureStorage/SecureStorageKeyStoreProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public protocol KeychainService {
func itemMatching(_ query: [String: Any], _ result: UnsafeMutablePointer<CFTypeRef?>?) -> OSStatus
func add(_ attributes: [String: Any], _ result: UnsafeMutablePointer<CFTypeRef?>?) -> OSStatus
func delete(_ query: [String: Any]) -> OSStatus
func update(_ query: [String: Any], _ attributesToUpdate: [String: Any]) -> OSStatus
}

public extension KeychainService {
Expand All @@ -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 {
Expand All @@ -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?
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Loading