Skip to content

Commit

Permalink
DBP: Enable DBP Operations to Run When Device is Locked (#2797)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1206488453854252/1206891169830209/f
Tech Design URL:
https://app.asana.com/0/1206488453854252/1207313481959835/f

**Description**:
This PR changes the accessibility attribute of DBP Keychain items to
enable them to be accessed when the device is locked. It also migrates
Keychain values for existing DBP users.
  • Loading branch information
aataraxiaa authored Jun 3, 2024
1 parent 2e0956a commit 9984e50
Show file tree
Hide file tree
Showing 13 changed files with 426 additions and 19 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" */ = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
},
{
Expand Down
4 changes: 3 additions & 1 deletion DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ public class DataBrokerProtectionPixelsHandler: EventMapping<DataBrokerProtectio
PixelKit.fire(DebugEvent(event, error: error))
case .generalError(let error, _),
.secureVaultInitError(let error),
.secureVaultError(let error):
.secureVaultError(let error),
.secureVaultKeyStoreReadError(let error),
.secureVaultKeyStoreUpdateError(let error):
PixelKit.fire(DebugEvent(event, error: error))
case .ipcServerProfileSavedXPCError(error: let error),
.ipcServerImmediateScansFinishedWithError(error: let error),
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/DataImport/DataImport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ struct LoginImporterError: DataImportError {
.databaseError(let error):
return error

case .keystoreError(let status):
case .keystoreError(let status), .keystoreReadError(let status), .keystoreUpdateError(let status):
return NSError(domain: "KeyStoreError", code: Int(status))

case .secError(let status):
Expand Down Expand Up @@ -678,7 +678,7 @@ struct LoginImporterError: DataImportError {
.databaseError:
return .keychainError

case .keystoreError, .secError:
case .keystoreError, .secError, .keystoreReadError, .keystoreUpdateError:
return .keychainError

case .authRequired,
Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/DataBrokerProtection/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let package = Package(
targets: ["DataBrokerProtection"])
],
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"),
.package(path: "../XPCHelper"),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@

import Foundation

extension Bundle {
protocol GroupNameProviding {
var appGroupName: String { get }
}

extension Bundle: GroupNameProviding {

static let dbpAppGroupName = "DBP_APP_GROUP"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ final class DataBrokerProtectionSecureVaultErrorReporter: SecureVaultReporting {

func secureVaultError(_ error: SecureStorageError) {
switch error {
case .initFailed(let cause as SecureStorageError):
switch cause {
case .keystoreReadError:
pixelHandler.fire(.secureVaultKeyStoreReadError(error: cause))
case .keystoreUpdateError:
pixelHandler.fire(.secureVaultKeyStoreUpdateError(error: cause))
default:
pixelHandler.fire(.secureVaultInitError(error: error))
}
case .initFailed, .failedToOpenDatabase:
pixelHandler.fire(.secureVaultInitError(error: error))
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ public enum DataBrokerProtectionPixels {
case error(error: DataBrokerProtectionError, dataBroker: String)
case generalError(error: Error, functionOccurredIn: String)
case secureVaultInitError(error: Error)
case secureVaultKeyStoreReadError(error: Error)
case secureVaultKeyStoreUpdateError(error: Error)
case secureVaultError(error: Error)
case parentChildMatches(parent: String, child: String, value: Int)

Expand Down Expand Up @@ -203,6 +205,8 @@ extension DataBrokerProtectionPixels: PixelKitEvent {
case .error: return "m_mac_data_broker_error"
case .generalError: return "m_mac_data_broker_error"
case .secureVaultInitError: return "m_mac_dbp_secure_vault_init_error"
case .secureVaultKeyStoreReadError: return "m_mac_dbp_secure_vault_keystore_read_error"
case .secureVaultKeyStoreUpdateError: return "m_mac_dbp_secure_vault_keystore_update_error"
case .secureVaultError: return "m_mac_dbp_secure_vault_error"

case .backgroundAgentStarted: return "m_mac_dbp_background-agent_started"
Expand Down Expand Up @@ -375,6 +379,8 @@ extension DataBrokerProtectionPixels: PixelKitEvent {
.entitlementCheckInvalid,
.entitlementCheckError,
.secureVaultInitError,
.secureVaultKeyStoreReadError,
.secureVaultKeyStoreUpdateError,
.secureVaultError:
return [:]
case .ipcServerProfileSavedCalledByApp,
Expand Down Expand Up @@ -432,7 +438,9 @@ public class DataBrokerProtectionPixelsHandler: EventMapping<DataBrokerProtectio
case .generalError(let error, _):
PixelKit.fire(DebugEvent(event, error: error))
case .secureVaultInitError(let error),
.secureVaultError(let error):
.secureVaultError(let error),
.secureVaultKeyStoreReadError(let error),
.secureVaultKeyStoreUpdateError(let error):
PixelKit.fire(DebugEvent(event, error: error))
case .ipcServerProfileSavedXPCError(error: let error),
.ipcServerImmediateScansFinishedWithError(error: let error),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// limitations under the License.
//

import Common
import Foundation
import BrowserServicesKit
import SecureStorage
Expand All @@ -28,7 +29,7 @@ final class DataBrokerProtectionKeyStoreProvider: SecureStorageKeyStoreProvider

// DO NOT CHANGE except if you want to deliberately invalidate all users's vaults.
// The keys have a uid to deter casual hacker from easily seeing which keychain entry is related to what.
private enum EntryName: String {
enum EntryName: String, CaseIterable {
case generatedPassword = "5FCE971A-DE67-4649-9A42-5E6DAB026E72"
case l1Key = "9CA59EDC-5CE8-4F53-AAC6-286A7378F384"
case l2Key = "E544DC56-1D72-4C5D-9738-FDFA6602C47E"
Expand All @@ -50,13 +51,136 @@ final class DataBrokerProtectionKeyStoreProvider: SecureStorageKeyStoreProvider
return Constants.defaultServiceName
}

var keychainAccessibilityValue: String {
kSecAttrAccessibleAfterFirstUnlock as String
}

let keychainService: KeychainService
private let groupNameProvider: GroupNameProviding
private let getLog: () -> 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
}
}
Loading

0 comments on commit 9984e50

Please sign in to comment.