diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index c18322e0a6..2e4bcb2720 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -12993,7 +12993,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 148.0.0; + version = 149.0.0; }; }; 9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index b6b11e25d1..8da6589602 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "7c235d29fc446436734612e81dd486b7c52aa577", - "version" : "148.0.0" + "revision" : "cd7850dd115f4c896095f410c1049fc32bdf7b16", + "version" : "149.0.0" } }, { diff --git a/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift b/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift index 3632dc549b..494dae251c 100644 --- a/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift +++ b/DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift @@ -31,7 +31,9 @@ public class DataBrokerProtectionPixelsHandler: EventMapping OSLog + private var log: OSLog { + getLog() + } + + init(keychainService: KeychainService = DefaultKeychainService(), + groupNameProvider: GroupNameProviding = Bundle.main, + log: @escaping @autoclosure () -> OSLog = .disabled) { + self.keychainService = keychainService + self.groupNameProvider = groupNameProvider + self.getLog = log + } + + func readData(named: String, serviceName: String) throws -> Data? { + try readOrMigrate(named: named, serviceName: serviceName) + } + func attributesForEntry(named: String, serviceName: String) -> [String: Any] { - return [ - kSecClass: kSecClassGenericPassword, - kSecUseDataProtectionKeychain: true, - kSecAttrSynchronizable: false, - kSecAttrAccessGroup: Bundle.main.appGroupName, - kSecAttrAccount: named - ] as [String: Any] + [ + kSecClass: kSecClassGenericPassword, + kSecUseDataProtectionKeychain: true, + kSecAttrSynchronizable: false, + kSecAttrAccessGroup: groupNameProvider.appGroupName, + kSecAttrAccount: named, + ] as [String: Any] + } +} + +private extension DataBrokerProtectionKeyStoreProvider { + + var afterFirstUnlockAttributeUpdate: [String: Any] { + [kSecAttrAccessible as String: keychainAccessibilityValue] + } + + /// Reads data from the Keychain using the latest query attributes, or if not found, reads using old query attributes and if found, migrates + /// - Parameters: + /// - name: Keychain item name + /// - serviceName: Keychain service name + /// - Returns: Optional Data + func readOrMigrate(named name: String, serviceName: String) throws -> Data? { + + // Try to read keychain data using attributes which include `kSecAttrAccessible` value of `kSecAttrAccessibleAfterFirstUnlock` + let attributes = afterFirstUnlockQueryAttributes(named: name, serviceName: serviceName) + + if let data = try read(serviceName: serviceName, queryAttributes: attributes) { + return data + } else { + + // If we didn't find Keychain data, try using attributes WITHOUT `kSecAttrAccessible` value of `kSecAttrAccessibleAfterFirstUnlock` + let legacyAttributes = whenUnlockedQueryAttributes(named: name, serviceName: serviceName) + + let accessibilityValueString = legacyAttributes[kSecAttrAccessible as String] as? String ?? "[value unavailable]" + os_log("Attempting read and migrate of DBP Keychain data with kSecAttrAccessible value of \(accessibilityValueString)", log: .dataBrokerProtection, type: .debug) + + if let data = try read(serviceName: serviceName, queryAttributes: legacyAttributes) { + // We found Keychain data, so update it's `kSecAttrAccessible` value to `kSecAttrAccessibleAfterFirstUnlock` + try update(serviceName: serviceName, queryAttributes: legacyAttributes, attributeUpdate: afterFirstUnlockAttributeUpdate) + return data + } + } + + return nil + } + + /// Reads a Keychain item + /// - Parameters: + /// - serviceName: Keychain service name + /// - queryAttributes: Attributes used to query the item + /// - Returns: Optional Data + func read(serviceName: String, queryAttributes: [String: Any]) throws -> Data? { + var query = queryAttributes + query[kSecReturnData as String] = true + query[kSecAttrService as String] = serviceName + + var item: CFTypeRef? + + let status = keychainService.itemMatching(query, &item) + + switch status { + + case errSecSuccess: + guard let itemData = item as? Data, + let itemString = String(data: itemData, encoding: .utf8), + let decodedData = Data(base64Encoded: itemString) else { + throw SecureStorageError.keystoreError(status: status) + } + return decodedData + + case errSecItemNotFound: + return nil + + default: + throw SecureStorageError.keystoreReadError(status: status) + } + } + + /// Updates a Keychain item + /// - Parameters: + /// - serviceName: Keychain service name + /// - queryAttributes: Attributes used to query the item + /// - attributeUpdate: Attribute updates + func update(serviceName: String, queryAttributes: [String: Any], attributeUpdate: [String: Any]) throws { + var query = queryAttributes + query[kSecAttrService as String] = serviceName + + let status = keychainService.update(queryAttributes, attributeUpdate) + + guard status == errSecSuccess else { + throw SecureStorageError.keystoreUpdateError(status: status) + } + + let accessibilityValueString = attributeUpdate[kSecAttrAccessible as String] as? String ?? "[value unavailable]" + os_log("Updated DBP Keychain data kSecAttrAccessible value to \(accessibilityValueString)", log: .dataBrokerProtection, type: .debug) + } + + func afterFirstUnlockQueryAttributes(named name: String, serviceName: String) -> [String: Any] { + var attributes = attributesForEntry(named: name, serviceName: serviceName) + attributes[kSecAttrAccessible as String] = keychainAccessibilityValue + return attributes + } + + func whenUnlockedQueryAttributes(named name: String, serviceName: String) -> [String: Any] { + var attributes = attributesForEntry(named: name, serviceName: serviceName) + attributes[kSecAttrAccessible as String] = kSecAttrAccessibleWhenUnlocked + return attributes } } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionKeyStoreProviderTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionKeyStoreProviderTests.swift new file mode 100644 index 0000000000..5af72c98dd --- /dev/null +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionKeyStoreProviderTests.swift @@ -0,0 +1,144 @@ +// +// DataBrokerProtectionKeyStoreProviderTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +import SecureStorage +@testable import DataBrokerProtection + +final class DataBrokerProtectionKeyStoreProviderTests: XCTestCase { + + private let mockGroupNameProvider = MockGroupNameProvider() + + func testWhenReadData_newAttributesAreUsedFirst_andNoFallbackQueryIsPerformedIfDataFound() throws { + + try DataBrokerProtectionKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let mockKeychainService = MockDBPKeychainService() + mockKeychainService.mode = .migratedDataFound + let sut = DataBrokerProtectionKeyStoreProvider(keychainService: mockKeychainService, groupNameProvider: mockGroupNameProvider) + + // When + _ = try sut.readData(named: entry.rawValue, serviceName: sut.keychainServiceName) + + // Then + let secAttrAccessibleQueryValue = mockKeychainService.latestItemMatchingQuery[kSecAttrAccessible as String]! as? String + + XCTAssertEqual(secAttrAccessibleQueryValue, kSecAttrAccessibleAfterFirstUnlock as String) + XCTAssert(mockKeychainService.itemMatchingCallCount == 1) + } + } + + func testWhenReadData_andNoDataFoundWithNewAttributes_thenFallbackQueryIsPerformedWithLegacyAttributes_andUpdateIsPerformed() throws { + + try DataBrokerProtectionKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let mockKeychainService = MockDBPKeychainService() + mockKeychainService.mode = .legacyDataFound + let sut = DataBrokerProtectionKeyStoreProvider(keychainService: mockKeychainService, groupNameProvider: mockGroupNameProvider) + + // When + _ = try sut.readData(named: entry.rawValue, serviceName: sut.keychainServiceName) + + // Then + let secAttrAccessibleQueryValue = mockKeychainService.latestItemMatchingQuery[kSecAttrAccessible as String]! as? String + let secAttrAccessiblUpdateValue = mockKeychainService.latestUpdateAttributes[kSecAttrAccessible as String]! as? String + + XCTAssertEqual(secAttrAccessibleQueryValue, kSecAttrAccessibleWhenUnlocked as String) + XCTAssertEqual(secAttrAccessiblUpdateValue, kSecAttrAccessibleAfterFirstUnlock as String) + XCTAssert(mockKeychainService.itemMatchingCallCount == 2) + XCTAssert(mockKeychainService.updateCallCount == 1) + } + } + + func testWhenReadData_andNoValueIsFound_thenTwoQueriesArePerformed_noUpdateIsAttempted_andNilIsReturned() throws { + + try DataBrokerProtectionKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let mockKeychainService = MockDBPKeychainService() + mockKeychainService.mode = .nothingFound + let sut = DataBrokerProtectionKeyStoreProvider(keychainService: mockKeychainService, groupNameProvider: mockGroupNameProvider) + + // When + let result = try sut.readData(named: entry.rawValue, serviceName: sut.keychainServiceName) + + // Then + let secAttrAccessibleQueryValue = mockKeychainService.latestItemMatchingQuery[kSecAttrAccessible as String]! as? String + + XCTAssertEqual(secAttrAccessibleQueryValue, kSecAttrAccessibleWhenUnlocked as String) + XCTAssert(mockKeychainService.itemMatchingCallCount == 2) + XCTAssert(mockKeychainService.updateCallCount == 0) + XCTAssertNil(result) + } + } + + func testWhenWriteData_correctKeychainAccessibilityValueIsUsed() throws { + + try DataBrokerProtectionKeyStoreProvider.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 mockKeychainService = MockDBPKeychainService() + let sut = DataBrokerProtectionKeyStoreProvider(keychainService: mockKeychainService, groupNameProvider: mockGroupNameProvider) + + // When + _ = try sut.writeData(mockData, named: entry.rawValue, serviceName: sut.keychainServiceName) + + // Then + XCTAssertEqual(mockKeychainService.addCallCount, 1) + XCTAssertEqual(mockKeychainService.latestAddQuery[kSecAttrAccessible as String] as! String, kSecAttrAccessibleAfterFirstUnlock as String) + } + } + + func testWhenKeychainReadErrors_thenKeyStoreReadErrorIsThrown() throws { + + try DataBrokerProtectionKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let mockKeychainService = MockDBPKeychainService() + mockKeychainService.mode = .readError + let expectedError = SecureStorageError.keystoreReadError(status: mockKeychainService.mode.statusCode!) + let sut = DataBrokerProtectionKeyStoreProvider(keychainService: mockKeychainService, groupNameProvider: mockGroupNameProvider) + + // When + XCTAssertThrowsError(try sut.readData(named: entry.rawValue, serviceName: sut.keychainServiceName)) { error in + + // Then + XCTAssertEqual((error as! SecureStorageError), expectedError) + } + } + } + + func testWhenKeychainUpdateErrors_thenKeyStoreUpdateErrorIsThrown() throws { + + try DataBrokerProtectionKeyStoreProvider.EntryName.allCases.forEach { entry in + // Given + let mockKeychainService = MockDBPKeychainService() + mockKeychainService.mode = .updateError + let expectedError = SecureStorageError.keystoreUpdateError(status: mockKeychainService.mode.statusCode!) + let sut = DataBrokerProtectionKeyStoreProvider(keychainService: mockKeychainService, groupNameProvider: mockGroupNameProvider) + + // When + XCTAssertThrowsError(try sut.readData(named: entry.rawValue, serviceName: sut.keychainServiceName)) { error in + + // Then + XCTAssertEqual((error as! SecureStorageError), expectedError) + } + } + } +} diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift index d6bb74149d..fb2361e091 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift @@ -1433,3 +1433,119 @@ final class MockDataProtectionStopAction: DataProtectionStopAction { wasStopCalled = false } } + +public final class MockDBPKeychainService: KeychainService { + + public enum Mode { + case nothingFound + case migratedDataFound + case legacyDataFound + case readError + case updateError + + var statusCode: Int32? { + switch self { + case .readError: + return -25295 + case .updateError: + return -25299 + default: + return nil + } + } + } + + public var latestItemMatchingQuery: [String: Any] = [:] + public var latestUpdateQuery: [String: Any] = [:] + public var latestAddQuery: [String: Any] = [:] + public var latestUpdateAttributes: [String: Any] = [:] + public var addCallCount = 0 + public var itemMatchingCallCount = 0 + public var updateCallCount = 0 + + public var mode: Mode = .nothingFound + + public init() {} + + public func itemMatching(_ query: [String: Any], _ result: UnsafeMutablePointer?) -> OSStatus { + itemMatchingCallCount += 1 + latestItemMatchingQuery = query + + func setResult() { + let originalString = "Mock Keychain data!" + let data = originalString.data(using: .utf8)! + let encodedString = data.base64EncodedString() + let mockResult = encodedString.data(using: .utf8)! as CFData + + if let result = result { + result.pointee = mockResult + } + } + + switch mode { + case .nothingFound: + return errSecItemNotFound + case .migratedDataFound: + setResult() + return errSecSuccess + case .legacyDataFound, .updateError: + if itemMatchingCallCount == 2 { + setResult() + return errSecSuccess + } else { + return errSecItemNotFound + } + case .readError: + return errSecInvalidKeychain + } + } + + public func add(_ query: [String: Any], _ result: UnsafeMutablePointer?) -> OSStatus { + latestAddQuery = query + addCallCount += 1 + return errSecSuccess + } + + public func update(_ query: [String: Any], _ attributesToUpdate: [String: Any]) -> OSStatus { + guard mode != .updateError else { return errSecDuplicateItem } + updateCallCount += 1 + latestUpdateQuery = query + latestUpdateAttributes = attributesToUpdate + return errSecSuccess + } +} + +struct MockGroupNameProvider: GroupNameProviding { + var appGroupName: String { + return "mockGroup" + } +} + +extension SecureStorageError: Equatable { + public static func == (lhs: SecureStorageError, rhs: SecureStorageError) -> Bool { + switch (lhs, rhs) { + case (.initFailed(let cause1), .initFailed(let cause2)): + return cause1.localizedDescription == cause2.localizedDescription + case (.authError(let cause1), .authError(let cause2)): + return cause1.localizedDescription == cause2.localizedDescription + case (.failedToOpenDatabase(let cause1), .failedToOpenDatabase(let cause2)): + return cause1.localizedDescription == cause2.localizedDescription + case (.databaseError(let cause1), .databaseError(let cause2)): + return cause1.localizedDescription == cause2.localizedDescription + case (.keystoreError(let status1), .keystoreError(let status2)): + return status1 == status2 + case (.secError(let status1), .secError(let status2)): + return status1 == status2 + case (.keystoreReadError(let status1), .keystoreReadError(let status2)): + return status1 == status2 + case (.keystoreUpdateError(let status1), .keystoreUpdateError(let status2)): + return status1 == status2 + case (.authRequired, .authRequired), (.invalidPassword, .invalidPassword), + (.noL1Key, .noL1Key), (.noL2Key, .noL2Key), (.duplicateRecord, .duplicateRecord), + (.generalCryptoError, .generalCryptoError), (.encodingFailed, .encodingFailed): + return true + default: + return false + } + } +} diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index 626fd972c0..25b0391e25 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -31,7 +31,7 @@ let package = Package( .library(name: "NetworkProtectionUI", targets: ["NetworkProtectionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "148.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "149.0.0"), .package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.1"), .package(path: "../XPCHelper"), .package(path: "../SwiftUIExtensions"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index 269c7f0143..5cf819c119 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "148.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "149.0.0"), .package(path: "../SwiftUIExtensions") ], targets: [