diff --git a/BitwardenResources/Localizations/en.lproj/Localizable.strings b/BitwardenResources/Localizations/en.lproj/Localizable.strings index a22a1d0562..3960ae9a49 100644 --- a/BitwardenResources/Localizations/en.lproj/Localizable.strings +++ b/BitwardenResources/Localizations/en.lproj/Localizable.strings @@ -1287,3 +1287,7 @@ "WarningStartsWithIsAnAdvancedOptionWithIncreasedRiskOfExposingCredentials" = "**Warning:** “Starts with” is an advanced option with increased risk of exposing credentials."; "WarningRegularExpressionIsAnAdvancedOptionWithIncreasedRiskOfExposingCredentials" = "**Warning:** “Regular expression” is an advanced option with increased risk of exposing credentials if used incorrectly."; "DefaultX" = "Default (%1$@)"; +"UpdateYourEncryptionSettings" = "Update your encryption settings"; +"TheNewRecommendedEncryptionSettingsDescriptionLong" = "The new recommended encryption settings will improve your account security. Enter your master password to update now."; +"Updating" = "Updating..."; +"EncryptionSettingsUpdated" = "Encryption settings updated"; diff --git a/BitwardenShared/Core/Auth/Models/Domains/KdfConfig.swift b/BitwardenShared/Core/Auth/Models/Domains/KdfConfig.swift index 7e3d2e36c5..b4abac0617 100644 --- a/BitwardenShared/Core/Auth/Models/Domains/KdfConfig.swift +++ b/BitwardenShared/Core/Auth/Models/Domains/KdfConfig.swift @@ -1,10 +1,16 @@ import BitwardenKit +import BitwardenSdk // MARK: - KdfConfig /// A model for configuring KDF options. /// struct KdfConfig: Codable, Equatable, Hashable { + // MARK: Type Properties + + /// The default `KdfConfig` used for new accounts or when upgrading the KDF config to minimums. + static let defaultKdfConfig = KdfConfig(kdfType: .pbkdf2sha256, iterations: Constants.pbkdf2Iterations) + // MARK: Properties /// The type of KDF used in the request. @@ -40,6 +46,24 @@ struct KdfConfig: Codable, Equatable, Hashable { self.memory = memory self.parallelism = parallelism } + + /// Initializes a `KdfConfig` from the SDK's `Kdf` type. + /// + /// - Parameter kdf: The type of KDF used in the request. + /// + init(kdf: Kdf) { + switch kdf { + case let .argon2id(iterations, memory, parallelism): + self.init( + kdfType: .argon2id, + iterations: Int(iterations), + memory: Int(memory), + parallelism: Int(parallelism) + ) + case let .pbkdf2(iterations): + self.init(kdfType: .pbkdf2sha256, iterations: Int(iterations)) + } + } } // MARK: - KdfConfigProtocol diff --git a/BitwardenShared/Core/Auth/Models/Domains/KdfConfigTests.swift b/BitwardenShared/Core/Auth/Models/Domains/KdfConfigTests.swift new file mode 100644 index 0000000000..176e2f1df5 --- /dev/null +++ b/BitwardenShared/Core/Auth/Models/Domains/KdfConfigTests.swift @@ -0,0 +1,35 @@ +import XCTest + +@testable import BitwardenShared + +class KdfConfigTests: BitwardenTestCase { + // MARK: Tests + + /// `init(kdf:)` initializes `KdfConfig` from `BitwardenSdk.Kdf` when using `argon2id`. + func test_init_kdf_argon2() { + let subject = KdfConfig(kdf: .argon2id(iterations: 3, memory: 64, parallelism: 4)) + XCTAssertEqual( + subject, + KdfConfig( + kdfType: .argon2id, + iterations: 3, + memory: 64, + parallelism: 4 + ) + ) + } + + /// `init(kdf:)` initializes `KdfConfig` from `BitwardenSdk.Kdf` when using `pbkdf2`. + func test_init_kdf_pbkdf2() { + let subject = KdfConfig(kdf: .pbkdf2(iterations: 600_000)) + XCTAssertEqual( + subject, + KdfConfig( + kdfType: .pbkdf2sha256, + iterations: 600_000, + memory: nil, + parallelism: nil + ) + ) + } +} diff --git a/BitwardenShared/Core/Auth/Models/Request/MasterPasswordAuthenticationDataRequestModel.swift b/BitwardenShared/Core/Auth/Models/Request/MasterPasswordAuthenticationDataRequestModel.swift new file mode 100644 index 0000000000..117a9c6ff4 --- /dev/null +++ b/BitwardenShared/Core/Auth/Models/Request/MasterPasswordAuthenticationDataRequestModel.swift @@ -0,0 +1,33 @@ +import BitwardenSdk + +// MARK: - MasterPasswordAuthenticationDataRequestModel + +/// A request model for a user's master password authentication data. +/// +struct MasterPasswordAuthenticationDataRequestModel: Encodable, Equatable { + // MARK: Properties + + /// The KDF settings. + let kdf: KdfConfig + + /// The master password hash. + let masterPasswordAuthenticationHash: String + + /// The salt used to compute the master password hash. + let salt: String +} + +extension MasterPasswordAuthenticationDataRequestModel { + /// Initialize `MasterPasswordAuthenticationDataRequestModel` from `MasterPasswordAuthenticationData`. + /// + /// - Parameter authenticationData: The `MasterPasswordAuthenticationData` used to initialize a + /// `MasterPasswordAuthenticationDataRequestModel`. + /// + init(authenticationData: MasterPasswordAuthenticationData) { + self.init( + kdf: KdfConfig(kdf: authenticationData.kdf), + masterPasswordAuthenticationHash: authenticationData.masterPasswordAuthenticationHash, + salt: authenticationData.salt + ) + } +} diff --git a/BitwardenShared/Core/Auth/Models/Request/MasterPasswordUnlockDataRequestModel.swift b/BitwardenShared/Core/Auth/Models/Request/MasterPasswordUnlockDataRequestModel.swift new file mode 100644 index 0000000000..c1372468ca --- /dev/null +++ b/BitwardenShared/Core/Auth/Models/Request/MasterPasswordUnlockDataRequestModel.swift @@ -0,0 +1,33 @@ +import BitwardenSdk + +// MARK: - MasterPasswordUnlockDataRequestModel + +/// A request model for a user's unlock data. +/// +struct MasterPasswordUnlockDataRequestModel: Encodable, Equatable { + // MARK: Properties + + /// The KDF settings. + let kdf: KdfConfig + + /// The user's master key encrypted with their user key. + let masterKeyWrappedUserKey: String + + /// The salt used to encrypt the user key. + let salt: String +} + +extension MasterPasswordUnlockDataRequestModel { + /// Initialize `MasterPasswordUnlockDataRequestModel` from `MasterPasswordUnlockData`. + /// + /// - Parameter authenticationData: The `MasterPasswordUnlockData` used to initialize a + /// `MasterPasswordUnlockDataRequestModel`. + /// + init(unlockData: MasterPasswordUnlockData) { + self.init( + kdf: KdfConfig(kdf: unlockData.kdf), + masterKeyWrappedUserKey: unlockData.masterKeyWrappedUserKey, + salt: unlockData.salt + ) + } +} diff --git a/BitwardenShared/Core/Auth/Models/Request/UpdateKdfRequestModel.swift b/BitwardenShared/Core/Auth/Models/Request/UpdateKdfRequestModel.swift new file mode 100644 index 0000000000..07f9e05203 --- /dev/null +++ b/BitwardenShared/Core/Auth/Models/Request/UpdateKdfRequestModel.swift @@ -0,0 +1,45 @@ +import BitwardenSdk +import Networking + +// MARK: - UpdateKdfRequestModel + +/// The request body for an update KDF request. +/// +struct UpdateKdfRequestModel: JSONRequestBody, Equatable { + // MARK: Properties + + /// The user's data for authentication. + let authenticationData: MasterPasswordAuthenticationDataRequestModel + + /// The user's key. + let key: String + + /// The hash of the old master password. + let masterPasswordHash: String + + /// The hash of the new master password. + let newMasterPasswordHash: String + + /// The user's data for unlock. + let unlockData: MasterPasswordUnlockDataRequestModel +} + +extension UpdateKdfRequestModel { + /// Initialize `UpdateKdfRequestModel` from `UpdateKdfResponse`. + /// + /// - Parameter response: The `UpdateKdfResponse` used to initialize a `UpdateKdfRequestModel`. + /// + init(response: UpdateKdfResponse) { + self.init( + authenticationData: MasterPasswordAuthenticationDataRequestModel( + authenticationData: response.masterPasswordAuthenticationData + ), + key: response.masterPasswordUnlockData.masterKeyWrappedUserKey, + masterPasswordHash: response.oldMasterPasswordAuthenticationData.masterPasswordAuthenticationHash, + newMasterPasswordHash: response.masterPasswordAuthenticationData.masterPasswordAuthenticationHash, + unlockData: MasterPasswordUnlockDataRequestModel( + unlockData: response.masterPasswordUnlockData + ) + ) + } +} diff --git a/BitwardenShared/Core/Auth/Models/Request/UpdateKdfRequestModelTests.swift b/BitwardenShared/Core/Auth/Models/Request/UpdateKdfRequestModelTests.swift new file mode 100644 index 0000000000..be3703a55e --- /dev/null +++ b/BitwardenShared/Core/Auth/Models/Request/UpdateKdfRequestModelTests.swift @@ -0,0 +1,49 @@ +import BitwardenSdk +import XCTest + +@testable import BitwardenShared + +class UpdateKdfRequestModelTests: BitwardenTestCase { + // MARK: Tests + + /// `init(response:)` initializes `UpdateKdfRequestModel` from a `UpdateKdfResponse`. + func test_init_response() { + let subject = UpdateKdfRequestModel( + response: UpdateKdfResponse( + masterPasswordAuthenticationData: MasterPasswordAuthenticationData( + kdf: .pbkdf2(iterations: 600_000), + salt: "AUTHENTICATION_SALT", + masterPasswordAuthenticationHash: "MASTER_PASSWORD_AUTHENTICATION_HASH" + ), + masterPasswordUnlockData: MasterPasswordUnlockData( + kdf: .argon2id(iterations: 3, memory: 64, parallelism: 4), + masterKeyWrappedUserKey: "MASTER_KEY_WRAPPED_USER_KEY", + salt: "UNLOCK_SALT" + ), + oldMasterPasswordAuthenticationData: MasterPasswordAuthenticationData( + kdf: .pbkdf2(iterations: 100_000), + salt: "OLD_SALT", + masterPasswordAuthenticationHash: "OLD_MASTER_PASSWORD_AUTHENTICATION_HASH" + ) + ) + ) + XCTAssertEqual( + subject, + UpdateKdfRequestModel( + authenticationData: MasterPasswordAuthenticationDataRequestModel( + kdf: KdfConfig(kdfType: .pbkdf2sha256, iterations: 600_000), + masterPasswordAuthenticationHash: "MASTER_PASSWORD_AUTHENTICATION_HASH", + salt: "AUTHENTICATION_SALT" + ), + key: "MASTER_KEY_WRAPPED_USER_KEY", + masterPasswordHash: "OLD_MASTER_PASSWORD_AUTHENTICATION_HASH", + newMasterPasswordHash: "MASTER_PASSWORD_AUTHENTICATION_HASH", + unlockData: MasterPasswordUnlockDataRequestModel( + kdf: KdfConfig(kdfType: .argon2id, iterations: 3, memory: 64, parallelism: 4), + masterKeyWrappedUserKey: "MASTER_KEY_WRAPPED_USER_KEY", + salt: "UNLOCK_SALT" + ) + ) + ) + } +} diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift index e4a86d51f5..9c4708a35d 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepository.swift @@ -420,6 +420,9 @@ class DefaultAuthRepository { /// The service to use system Biometrics for vault unlock. let biometricsRepository: BiometricsRepository + /// The service used to change the user's KDF settings. + private let changeKdfService: ChangeKdfService + /// The service that handles common client functionality such as encryption and decryption. private let clientService: ClientService @@ -468,6 +471,7 @@ class DefaultAuthRepository { /// - appContextHelper: The helper to know about the app context. /// - authService: The service used that handles some of the auth logic. /// - biometricsRepository: The service to use system Biometrics for vault unlock. + /// - changeKdfService: The service used to change the user's KDF settings. /// - clientService: The service that handles common client functionality such as encryption and decryption. /// - configService: The service to get server-specified configuration. /// - environmentService: The service used by the application to manage the environment settings. @@ -488,6 +492,7 @@ class DefaultAuthRepository { appContextHelper: AppContextHelper, authService: AuthService, biometricsRepository: BiometricsRepository, + changeKdfService: ChangeKdfService, clientService: ClientService, configService: ConfigService, environmentService: EnvironmentService, @@ -506,6 +511,7 @@ class DefaultAuthRepository { self.appContextHelper = appContextHelper self.authService = authService self.biometricsRepository = biometricsRepository + self.changeKdfService = changeKdfService self.clientService = clientService self.configService = configService self.environmentService = environmentService @@ -1113,6 +1119,7 @@ extension DefaultAuthRepository: AuthRepository { purpose: .localAuthorization ) try await stateService.setMasterPasswordHash(hashedPassword) + await updateKdfToMinimumsIfNeeded(password: password) case .decryptedKey, .deviceKey, .keyConnector, @@ -1137,6 +1144,20 @@ extension DefaultAuthRepository: AuthRepository { } } + /// Updates the user's KDF settings to the minimums. + /// + /// - Parameter password: The user's master password. + /// + private func updateKdfToMinimumsIfNeeded(password: String) async { + do { + try await changeKdfService.updateKdfToMinimumsIfNeeded(password: password) + } catch { + // If an error occurs, log the error. Don't throw since that would block the vault from + // unlocking. + errorReporter.log(error: error) + } + } + func updateMasterPassword( currentPassword: String, newPassword: String, diff --git a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift index f664c78455..ffc06be470 100644 --- a/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift +++ b/BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift @@ -15,6 +15,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo var appContextHelper: MockAppContextHelper! var authService: MockAuthService! var biometricsRepository: MockBiometricsRepository! + var changeKdfService: MockChangeKdfService! var client: MockHTTPClient! var clientService: MockClientService! var configService: MockConfigService! @@ -96,6 +97,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo accountAPIService = APIService(client: client) authService = MockAuthService() biometricsRepository = MockBiometricsRepository() + changeKdfService = MockChangeKdfService() configService = MockConfigService() environmentService = MockEnvironmentService() errorReporter = MockErrorReporter() @@ -112,6 +114,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo appContextHelper: appContextHelper, authService: authService, biometricsRepository: biometricsRepository, + changeKdfService: changeKdfService, clientService: clientService, configService: configService, environmentService: environmentService, @@ -135,6 +138,7 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo appContextHelper = nil authService = nil biometricsRepository = nil + changeKdfService = nil client = nil clientService = nil configService = nil @@ -2028,6 +2032,86 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo } } + // `unlockVaultWithPassword(_:)` unlocks the vault with the user's password and checks if the + // user's KDF settings need to be updated. + func test_unlockVaultWithPassword_checksForKdfUpdate() async throws { + let account = Account.fixture(profile: .fixture(kdfIterations: 100_000)) + configService.featureFlagsBool[.forceUpdateKdfSettings] = false + changeKdfService.needsKdfUpdateToMinimumsResult = true + stateService.activeAccount = account + stateService.accountEncryptionKeys = [ + "1": AccountEncryptionKeys( + accountKeys: .fixtureFilled(), + encryptedPrivateKey: "PRIVATE_KEY", + encryptedUserKey: "USER_KEY" + ), + ] + + try await subject.unlockVaultWithPassword(password: "password") + + XCTAssertEqual( + clientService.mockCrypto.initializeUserCryptoRequest, + InitUserCryptoRequest( + userId: "1", + kdfParams: .pbkdf2(iterations: UInt32(100_000)), + email: "user@bitwarden.com", + privateKey: "PRIVATE_KEY", + signingKey: "WRAPPED_SIGNING_KEY", + securityState: "SECURITY_STATE", + method: .password(password: "password", userKey: "USER_KEY") + ) + ) + XCTAssertFalse(vaultTimeoutService.isLocked(userId: "1")) + XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction) + XCTAssertEqual(stateService.manuallyLockedAccounts["1"], false) + + XCTAssertTrue(changeKdfService.needsKdfUpdateToMinimumsCalled) + XCTAssertTrue(changeKdfService.updateKdfToMinimumsCalled) + XCTAssertEqual(changeKdfService.updateKdfToMinimumsPassword, "password") + } + + // `unlockVaultWithPassword(_:)` unlocks the vault with the user's password and checks if the + // user's KDF settings need to be updated. If updating the user's KDF fails, an error is logged + // but vault unlock still succeeds. + func test_unlockVaultWithPassword_checksForKdfUpdate_error() async throws { + let account = Account.fixture(profile: .fixture(kdfIterations: 100_000)) + configService.featureFlagsBool[.forceUpdateKdfSettings] = false + changeKdfService.needsKdfUpdateToMinimumsResult = true + changeKdfService.updateKdfToMinimumsResult = .failure(BitwardenTestError.example) + stateService.activeAccount = account + stateService.accountEncryptionKeys = [ + "1": AccountEncryptionKeys( + accountKeys: .fixtureFilled(), + encryptedPrivateKey: "PRIVATE_KEY", + encryptedUserKey: "USER_KEY" + ), + ] + + await assertAsyncDoesNotThrow { + try await subject.unlockVaultWithPassword(password: "password") + } + + XCTAssertEqual( + clientService.mockCrypto.initializeUserCryptoRequest, + InitUserCryptoRequest( + userId: "1", + kdfParams: .pbkdf2(iterations: UInt32(100_000)), + email: "user@bitwarden.com", + privateKey: "PRIVATE_KEY", + signingKey: "WRAPPED_SIGNING_KEY", + securityState: "SECURITY_STATE", + method: .password(password: "password", userKey: "USER_KEY") + ) + ) + XCTAssertFalse(vaultTimeoutService.isLocked(userId: "1")) + XCTAssertTrue(vaultTimeoutService.unlockVaultHadUserInteraction) + XCTAssertEqual(stateService.manuallyLockedAccounts["1"], false) + + XCTAssertTrue(changeKdfService.needsKdfUpdateToMinimumsCalled) + XCTAssertTrue(changeKdfService.updateKdfToMinimumsCalled) + XCTAssertEqual(errorReporter.errors as? [BitwardenTestError], [.example]) + } + /// `logout` throws an error with no accounts. func test_logout_noAccounts() async { stateService.accounts = [] diff --git a/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIService.swift b/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIService.swift index 671016fa8d..e392d3f7fa 100644 --- a/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIService.swift +++ b/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIService.swift @@ -84,6 +84,13 @@ protocol AccountAPIService { /// func setPassword(_ requestModel: SetPasswordRequestModel) async throws + /// Performs the API request to update the user's KDF settings. + /// + /// - Parameter requestModel: The request model containing the details needed to update the + /// user's KDF settings. + /// + func updateKdf(_ requestModel: UpdateKdfRequestModel) async throws + /// Performs the API request to update the user's password. /// /// - Parameter requestModel: The request model used to send the request. @@ -180,6 +187,10 @@ extension APIService: AccountAPIService { try await identityService.send(StartRegistrationRequest(body: requestModel)) } + func updateKdf(_ requestModel: UpdateKdfRequestModel) async throws { + _ = try await apiService.send(UpdateKdfRequest(requestModel: requestModel)) + } + func updatePassword(_ requestModel: UpdatePasswordRequestModel) async throws { _ = try await apiService.send(UpdatePasswordRequest(requestModel: requestModel)) } diff --git a/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIServiceTests.swift b/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIServiceTests.swift index 29cef86332..4a9cee7157 100644 --- a/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIServiceTests.swift +++ b/BitwardenShared/Core/Auth/Services/API/Account/AccountAPIServiceTests.swift @@ -291,6 +291,33 @@ class AccountAPIServiceTests: BitwardenTestCase { // swiftlint:disable:this type XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/accounts/set-password") } + /// `updateKdf()` performs the request to update a user's KDF settings. + func test_updateKdf() async throws { + client.result = .httpSuccess(testData: .emptyResponse) + + let requestModel = UpdateKdfRequestModel( + authenticationData: MasterPasswordAuthenticationDataRequestModel( + kdf: KdfConfig(), + masterPasswordAuthenticationHash: "MASTER_PASSWORD_AUTHENTICATION_HASH", + salt: "AUTHENTICATION_SALT" + ), + key: "key", + masterPasswordHash: "MASTER_PASSWORD_HASH", + newMasterPasswordHash: "NEW_MASTER_PASSWORD_HINT", + unlockData: MasterPasswordUnlockDataRequestModel( + kdf: KdfConfig(), + masterKeyWrappedUserKey: "MASTER_KEY_WRAPPED_USER_KEY", + salt: "UNLOCK_SALT" + ) + ) + try await subject.updateKdf(requestModel) + + XCTAssertEqual(client.requests.count, 1) + XCTAssertNotNil(client.requests[0].body) + XCTAssertEqual(client.requests[0].method, .post) + XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/accounts/kdf") + } + /// `updatePassword()` doesn't throw an error when receiving the empty response. func test_updatePassword() async throws { client.result = .httpSuccess(testData: .emptyResponse) diff --git a/BitwardenShared/Core/Auth/Services/API/Account/Requests/UpdateKdfRequest.swift b/BitwardenShared/Core/Auth/Services/API/Account/Requests/UpdateKdfRequest.swift new file mode 100644 index 0000000000..8d94c62249 --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/API/Account/Requests/UpdateKdfRequest.swift @@ -0,0 +1,23 @@ +import Networking + +// MARK: - UpdateKdfRequest + +/// An API request to update the user's KDF settings. +/// +struct UpdateKdfRequest: Request { + typealias Response = EmptyResponse + + /// The body of the request. + var body: UpdateKdfRequestModel? { + requestModel + } + + /// The HTTP method for this request. + let method: HTTPMethod = .post + + /// The URL path for this request. + let path = "/accounts/kdf" + + /// The request details to include in the body of the request. + let requestModel: UpdateKdfRequestModel +} diff --git a/BitwardenShared/Core/Auth/Services/API/Account/Requests/UpdateKdfRequestTests.swift b/BitwardenShared/Core/Auth/Services/API/Account/Requests/UpdateKdfRequestTests.swift new file mode 100644 index 0000000000..ae461d7b98 --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/API/Account/Requests/UpdateKdfRequestTests.swift @@ -0,0 +1,89 @@ +import XCTest + +@testable import BitwardenShared + +class UpdateKdfRequestTests: BitwardenTestCase { + // MARK: Properties + + var subject: UpdateKdfRequest! + + // MARK: Setup & Teardown + + override func setUp() { + super.setUp() + + subject = UpdateKdfRequest( + requestModel: UpdateKdfRequestModel( + authenticationData: MasterPasswordAuthenticationDataRequestModel( + kdf: KdfConfig( + kdfType: .argon2id, + iterations: 3, + memory: 64, + parallelism: 4 + ), + masterPasswordAuthenticationHash: "MASTER_PASSWORD_AUTHENTICATION_HASH", + salt: "AUTHENTICATION_SALT" + ), + key: "key", + masterPasswordHash: "MASTER_PASSWORD_HASH", + newMasterPasswordHash: "NEW_MASTER_PASSWORD_HINT", + unlockData: MasterPasswordUnlockDataRequestModel( + kdf: KdfConfig(), + masterKeyWrappedUserKey: "MASTER_KEY_WRAPPED_USER_KEY", + salt: "UNLOCK_SALT" + ) + ) + ) + } + + override func tearDown() { + super.tearDown() + + subject = nil + } + + // MARK: Tests + + /// `body` is the JSON encoded request model. + func test_body() throws { + let bodyData = try XCTUnwrap(subject.body?.encode()) + XCTAssertEqual( + bodyData.prettyPrintedJson, + """ + { + "authenticationData" : { + "kdf" : { + "iterations" : 3, + "kdfType" : 1, + "memory" : 64, + "parallelism" : 4 + }, + "masterPasswordAuthenticationHash" : "MASTER_PASSWORD_AUTHENTICATION_HASH", + "salt" : "AUTHENTICATION_SALT" + }, + "key" : "key", + "masterPasswordHash" : "MASTER_PASSWORD_HASH", + "newMasterPasswordHash" : "NEW_MASTER_PASSWORD_HINT", + "unlockData" : { + "kdf" : { + "iterations" : 600000, + "kdfType" : 0 + }, + "masterKeyWrappedUserKey" : "MASTER_KEY_WRAPPED_USER_KEY", + "salt" : "UNLOCK_SALT" + } + } + """ + ) + } + + /// `method` is `.post`. + func test_method() { + XCTAssertEqual(subject.method, .post) + } + + /// `path` is the correct value. + func test_path() { + XCTAssertEqual(subject.path, "/accounts/kdf") + } +} diff --git a/BitwardenShared/Core/Auth/Services/ChangeKdfService.swift b/BitwardenShared/Core/Auth/Services/ChangeKdfService.swift new file mode 100644 index 0000000000..9753e5f695 --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/ChangeKdfService.swift @@ -0,0 +1,151 @@ +import BitwardenKit + +// MARK: - ChangeKdfService + +/// A protocol for a `ChangeKdfService` which handles updating the KDF settings for a user. +/// +protocol ChangeKdfService { + /// Returns whether the user needs to update their KDF settings to the minimum. + /// + /// - Returns: Whether the user needs to update their KDF settings to the minimum. + /// + func needsKdfUpdateToMinimums() async -> Bool + + /// Updates the user's KDF settings to the minimums. + /// + /// - Parameter password: The user's master password. + /// + func updateKdfToMinimums(password: String) async throws +} + +extension ChangeKdfService { + /// Updates the user's KDF settings to the minimums if their current settings are below the minimums. + /// + /// - Parameter password: The user's master password. + /// + func updateKdfToMinimumsIfNeeded(password: String) async throws { + guard await needsKdfUpdateToMinimums() else { return } + try await updateKdfToMinimums(password: password) + } +} + +// MARK: - DefaultChangeKdfService + +/// A default implementation of a `ChangeKdfService`. +/// +class DefaultChangeKdfService: ChangeKdfService { + // MARK: Properties + + /// The services used by the application to make account related API requests. + private let accountAPIService: AccountAPIService + + /// The service that handles common client functionality such as encryption and decryption. + private let clientService: ClientService + + /// The service to get server-specified configuration. + private let configService: ConfigService + + /// The service used by the application to report non-fatal errors. + private let errorReporter: ErrorReporter + + /// The service used by the application for recording temporary debug logs. + private let flightRecorder: FlightRecorder + + /// The service used by the application to manage account state. + private let stateService: StateService + + /// The service used to handle syncing vault data with the API. + private let syncService: SyncService + + // MARK: Initialization + + /// Initialize a `DefaultChangeKdfService`. + /// + /// - Parameters: + /// - accountAPIService: The services used by the application to make account related API requests. + /// - clientService: The service that handles common client functionality such as encryption and decryption. + /// - configService: The service to get server-specified configuration. + /// - errorReporter: The service used by the application to report non-fatal errors. + /// - flightRecorder: The service used by the application for recording temporary debug logs. + /// - stateService: The service used by the application to manage account state. + /// - syncService: The service used to handle syncing vault data with the API. + /// + init( + accountAPIService: AccountAPIService, + clientService: ClientService, + configService: ConfigService, + errorReporter: ErrorReporter, + flightRecorder: FlightRecorder, + stateService: StateService, + syncService: SyncService + ) { + self.accountAPIService = accountAPIService + self.clientService = clientService + self.configService = configService + self.errorReporter = errorReporter + self.flightRecorder = flightRecorder + self.stateService = stateService + self.syncService = syncService + } + + // MARK: Methods + + func needsKdfUpdateToMinimums() async -> Bool { + guard await configService.getFeatureFlag(.forceUpdateKdfSettings) else { return false } + + do { + guard try await accountNeedsKdfUpdate() else { return false } + + // If the account needs a KDF update, sync the user's account and check again before + // proceeding to ensure the local data is up-to-date. + try await syncService.fetchSync(forceSync: false) + return try await accountNeedsKdfUpdate() + } catch { + errorReporter.log(error: error) + return false + } + } + + func updateKdfToMinimums(password: String) async throws { + guard await configService.getFeatureFlag(.forceUpdateKdfSettings) else { return } + + let account = try await stateService.getActiveAccount() + + do { + let kdfConfig = KdfConfig.defaultKdfConfig + let updateKdfResponse = try await clientService.crypto().makeUpdateKdf( + password: password, + kdf: kdfConfig.sdkKdf + ) + try await accountAPIService.updateKdf( + UpdateKdfRequestModel(response: updateKdfResponse) + ) + try await stateService.setAccountKdf(kdfConfig, userId: account.profile.userId) + await flightRecorder.log("[Auth] Upgraded user's KDF to minimums") + } catch { + errorReporter.log(error: BitwardenError.generalError( + type: "Force Update KDF Error", + message: "Unable to update KDF settings (\(account.kdf)", + error: error + )) + throw error + } + } + + // MARK: Private + + /// Returns whether the active account needs to update their KDF to meet the minimum requirements. + /// + /// - Returns: Whether the active account needs a KDF update. + /// + private func accountNeedsKdfUpdate() async throws -> Bool { + let account = try await stateService.getActiveAccount() + guard account.kdf.kdfType == .pbkdf2sha256, + account.kdf.kdfIterations < Constants.minimumPbkdf2IterationsForUpgrade, + try await stateService.getUserHasMasterPassword(userId: account.profile.userId) + else { + return false + } + return true + } +} diff --git a/BitwardenShared/Core/Auth/Services/ChangeKdfServiceTests.swift b/BitwardenShared/Core/Auth/Services/ChangeKdfServiceTests.swift new file mode 100644 index 0000000000..cf8b9aa584 --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/ChangeKdfServiceTests.swift @@ -0,0 +1,242 @@ +import BitwardenKit +import BitwardenKitMocks +import TestHelpers +import XCTest + +@testable import BitwardenShared + +@MainActor +class ChangeKdfServiceTests: BitwardenTestCase { + // MARK: Properties + + var accountAPIService: AccountAPIService! + var client: MockHTTPClient! + var clientService: MockClientService! + var configService: MockConfigService! + var errorReporter: MockErrorReporter! + var flightRecorder: MockFlightRecorder! + var stateService: MockStateService! + var subject: ChangeKdfService! + var syncService: MockSyncService! + + let accountIterationsBelowMin = Account.fixture(profile: .fixture(kdfIterations: 599_999, kdfType: .pbkdf2sha256)) + let accountIterationsAtMin = Account.fixture(profile: .fixture(kdfIterations: 600_000, kdfType: .pbkdf2sha256)) + let accountIterationsAboveMin = Account.fixture(profile: .fixture(kdfIterations: 600_001, kdfType: .pbkdf2sha256)) + + // MARK: Setup & Teardown + + override func setUp() { + super.setUp() + + client = MockHTTPClient() + accountAPIService = APIService(client: client) + clientService = MockClientService() + configService = MockConfigService() + errorReporter = MockErrorReporter() + flightRecorder = MockFlightRecorder() + stateService = MockStateService() + syncService = MockSyncService() + + subject = DefaultChangeKdfService( + accountAPIService: accountAPIService, + clientService: clientService, + configService: configService, + errorReporter: errorReporter, + flightRecorder: flightRecorder, + stateService: stateService, + syncService: syncService + ) + } + + override func tearDown() async throws { + try await super.tearDown() + + accountAPIService = nil + client = nil + clientService = nil + configService = nil + errorReporter = nil + flightRecorder = nil + stateService = nil + subject = nil + syncService = nil + } + + // MARK: Tests + + /// `needsKdfUpdateToMinimums()` returns false if the account needs an update, but after syncing + /// the account has already been updated. + func test_needsKdfUpdateToMinimums_false_afterSync() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsBelowMin + syncService.fetchSyncHandler = { [weak self] in + guard let self else { return } + stateService.activeAccount = accountIterationsAboveMin + } + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertFalse(needsUpdate) + XCTAssertTrue(syncService.didFetchSync) + XCTAssertEqual(syncService.fetchSyncForceSync, false) + } + + /// `needsKdfUpdateToMinimums()` returns false and logs an error if one occurs. + func test_needsKdfUpdateToMinimums_false_error() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertFalse(needsUpdate) + XCTAssertEqual(errorReporter.errors as? [StateServiceError], [.noActiveAccount]) + } + + /// `needsKdfUpdateToMinimums()` returns false if the feature flag is off. + func test_needsKdfUpdateToMinimums_false_featureFlagOff() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = false + stateService.activeAccount = accountIterationsBelowMin + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertFalse(needsUpdate) + } + + /// `needsKdfUpdateToMinimums()` returns false if the account doesn't have a master password. + func test_needsKdfUpdateToMinimums_false_noMasterPassword() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsBelowMin + stateService.userHasMasterPassword["1"] = false + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertFalse(needsUpdate) + } + + /// `needsKdfUpdateToMinimums()` returns false if the account doesn't use PBKDF2. + func test_needsKdfUpdateToMinimums_false_notUsingPbkdf2() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = .fixture(profile: .fixture(kdfIterations: 1, kdfType: .argon2id)) + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertFalse(needsUpdate) + } + + /// `needsKdfUpdateToMinimums()` returns false if the account uses PBKDF2 with iterations above + /// the minimum. + func test_needsKdfUpdateToMinimums_false_iterationsAboveMinimum() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsAboveMin + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertFalse(needsUpdate) + } + + /// `needsKdfUpdateToMinimums()` returns false if the account uses PBKDF2 with iterations at the + /// minimum. + func test_needsKdfUpdateToMinimums_false_iterationsAtMinimum() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsAtMin + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertFalse(needsUpdate) + } + + /// `needsKdfUpdateToMinimums()` returns true if the feature flag is on and the account uses + /// PBKDF2 with iterations below the minimum. + func test_needsKdfUpdateToMinimums_true() async { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsBelowMin + + let needsUpdate = await subject.needsKdfUpdateToMinimums() + XCTAssertTrue(needsUpdate) + XCTAssertTrue(syncService.didFetchSync) + XCTAssertEqual(syncService.fetchSyncForceSync, false) + } + + /// `updateKdfToMinimums(password:)` updates the user's KDF settings. + func test_updateKdfToMinimums() async throws { + client.result = .httpSuccess(testData: .emptyResponse) + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsBelowMin + + try await subject.updateKdfToMinimums(password: "password123!") + + XCTAssertEqual(clientService.mockCrypto.makeUpdateKdfKdf, .pbkdf2(iterations: 600_000)) + XCTAssertEqual(clientService.mockCrypto.makeUpdateKdfPassword, "password123!") + + XCTAssertEqual(client.requests.count, 1) + XCTAssertNotNil(client.requests[0].body) + XCTAssertEqual(client.requests[0].method, .post) + XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/accounts/kdf") + } + + /// `updateKdfToMinimums(password:)` throws an error if there's no active account. + func test_updateKdfToMinimums_error_noActiveAccount() async throws { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + await assertAsyncThrows(error: StateServiceError.noActiveAccount) { + try await subject.updateKdfToMinimums(password: "password123!") + } + } + + /// `updateKdfToMinimums(password:)` logs an error and throws it if updating the KDF fails. + func test_updateKdfToMinimums_error_updateKdfError() async throws { + clientService.mockCrypto.makeUpdateKdfResult = .failure(BitwardenTestError.example) + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsBelowMin + + await assertAsyncThrows(error: BitwardenTestError.example) { + try await subject.updateKdfToMinimums(password: "password123!") + } + + let nsError = try XCTUnwrap(errorReporter.errors.last as? NSError) + XCTAssertEqual(nsError.domain, "General Error: Force Update KDF Error") + XCTAssertEqual( + nsError.userInfo["ErrorMessage"] as? String, + """ + Unable to update KDF settings \ + (KdfConfig(kdfType: BitwardenShared.KdfType.pbkdf2sha256, iterations: 599999, \ + memory: nil, parallelism: nil) + """ + ) + XCTAssertTrue(client.requests.isEmpty) + } + + /// `updateKdfToMinimums(password:)` doesn't updates the user's KDF settings if the feature flag is off. + func test_updateKdfToMinimums_featureFlagOff() async throws { + configService.featureFlagsBool[.forceUpdateKdfSettings] = false + stateService.activeAccount = accountIterationsBelowMin + + try await subject.updateKdfToMinimums(password: "password123!") + + XCTAssertNil(clientService.mockCrypto.makeUpdateKdfKdf) + XCTAssertTrue(client.requests.isEmpty) + } + + /// `updateKdfToMinimumsIfNeeded(password:)` doesn't update the user's KDF settings if the + /// iterations are above the minimum. + func test_updateKdfToMinimumsIfNeeded_iterationsAboveMinimum() async throws { + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsAboveMin + + try await subject.updateKdfToMinimumsIfNeeded(password: "password123!") + + XCTAssertNil(clientService.mockCrypto.makeUpdateKdfKdf) + XCTAssertTrue(client.requests.isEmpty) + } + + /// `updateKdfToMinimumsIfNeeded(password:)` updates the user's KDF settings if the + /// iterations are below the minimum. + func test_updateKdfToMinimumsIfNeeded_iterationsBelowMinimum() async throws { + client.result = .httpSuccess(testData: .emptyResponse) + configService.featureFlagsBool[.forceUpdateKdfSettings] = true + stateService.activeAccount = accountIterationsBelowMin + + try await subject.updateKdfToMinimumsIfNeeded(password: "password123!") + + XCTAssertEqual(clientService.mockCrypto.makeUpdateKdfKdf, .pbkdf2(iterations: 600_000)) + XCTAssertEqual(clientService.mockCrypto.makeUpdateKdfPassword, "password123!") + XCTAssertEqual(stateService.setAccountKdfByUserId["1"], KdfConfig.defaultKdfConfig) + + XCTAssertEqual(client.requests.count, 1) + XCTAssertNotNil(client.requests[0].body) + XCTAssertEqual(client.requests[0].method, .post) + XCTAssertEqual(client.requests[0].url.absoluteString, "https://example.com/api/accounts/kdf") + XCTAssertEqual(flightRecorder.logMessages, ["[Auth] Upgraded user's KDF to minimums"]) + } +} diff --git a/BitwardenShared/Core/Auth/Services/TestHelpers/MockChangeKdfService.swift b/BitwardenShared/Core/Auth/Services/TestHelpers/MockChangeKdfService.swift new file mode 100644 index 0000000000..1266fae0ad --- /dev/null +++ b/BitwardenShared/Core/Auth/Services/TestHelpers/MockChangeKdfService.swift @@ -0,0 +1,21 @@ +@testable import BitwardenShared + +class MockChangeKdfService: ChangeKdfService { + var needsKdfUpdateToMinimumsCalled = false + var needsKdfUpdateToMinimumsResult = false + + var updateKdfToMinimumsCalled = false + var updateKdfToMinimumsPassword: String? + var updateKdfToMinimumsResult: Result = .success(()) + + func needsKdfUpdateToMinimums() async -> Bool { + needsKdfUpdateToMinimumsCalled = true + return needsKdfUpdateToMinimumsResult + } + + func updateKdfToMinimums(password: String) async throws { + updateKdfToMinimumsCalled = true + updateKdfToMinimumsPassword = password + try updateKdfToMinimumsResult.get() + } +} diff --git a/BitwardenShared/Core/Platform/Models/Domain/Account.swift b/BitwardenShared/Core/Platform/Models/Domain/Account.swift index 94d5f7c9ff..7ae71605ed 100644 --- a/BitwardenShared/Core/Platform/Models/Domain/Account.swift +++ b/BitwardenShared/Core/Platform/Models/Domain/Account.swift @@ -107,16 +107,16 @@ extension Account { var hasPremiumPersonally: Bool? /// The number of iterations to use when calculating a password hash. - let kdfIterations: Int? + var kdfIterations: Int? /// The amount of memory to use when calculating a password hash. - let kdfMemory: Int? + var kdfMemory: Int? /// The number of threads to use when calculating a password hash. - let kdfParallelism: Int? + var kdfParallelism: Int? /// The type of KDF algorithm to use. - let kdfType: KdfType? + var kdfType: KdfType? /// The account's name. var name: String? diff --git a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift index 068d135b9d..d1737f7a46 100644 --- a/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift +++ b/BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift @@ -17,6 +17,9 @@ extension FeatureFlag: @retroactive CaseIterable { /// An SDK flag that enables individual cipher encryption. static let enableCipherKeyEncryption = FeatureFlag(rawValue: "enableCipherKeyEncryption") + /// Flag to enable/disable forced KDF updates. + static let forceUpdateKdfSettings = FeatureFlag(rawValue: "pm-18021-force-update-kdf-settings") + /// A feature flag to enable the removal of card item types. static let removeCardPolicy = FeatureFlag( rawValue: "pm-16442-remove-card-item-type-policy" @@ -28,6 +31,7 @@ extension FeatureFlag: @retroactive CaseIterable { .cxpImportMobile, .cipherKeyEncryption, .enableCipherKeyEncryption, + .forceUpdateKdfSettings, .removeCardPolicy, ] } diff --git a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift index 8d95c5fd92..d5eb5674ed 100644 --- a/BitwardenShared/Core/Platform/Services/ServiceContainer.swift +++ b/BitwardenShared/Core/Platform/Services/ServiceContainer.swift @@ -63,6 +63,9 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le /// The service used by the application to manage camera use. let cameraService: CameraService + /// The service used to change the user's KDF settings. + let changeKdfService: ChangeKdfService + /// The service used by the application to handle encryption and decryption tasks. let clientService: ClientService @@ -211,6 +214,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le /// controls for the user. /// - biometricsService: The service used to obtain device biometrics status & data. /// - cameraService: The service used by the application to manage camera use. + /// - changeKdfService: The service used to change the user's KDF settings. /// - clientService: The service used by the application to handle encryption and decryption tasks. /// - configService: The service to get server-specified configuration. /// - environmentService: The service used by the application to manage the environment settings. @@ -271,6 +275,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le biometricsRepository: BiometricsRepository, biometricsService: BiometricsService, cameraService: CameraService, + changeKdfService: ChangeKdfService, clientService: ClientService, configService: ConfigService, environmentService: EnvironmentService, @@ -327,6 +332,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le self.biometricsRepository = biometricsRepository self.biometricsService = biometricsService self.cameraService = cameraService + self.changeKdfService = changeKdfService self.clientService = clientService self.configService = configService self.environmentService = environmentService @@ -628,11 +634,22 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le trustDeviceService: trustDeviceService ) + let changeKdfService = DefaultChangeKdfService( + accountAPIService: apiService, + clientService: clientService, + configService: configService, + errorReporter: errorReporter, + flightRecorder: flightRecorder, + stateService: stateService, + syncService: syncService + ) + let authRepository = DefaultAuthRepository( accountAPIService: apiService, appContextHelper: appContextHelper, authService: authService, biometricsRepository: biometricsRepository, + changeKdfService: changeKdfService, clientService: clientService, configService: configService, environmentService: environmentService, @@ -893,6 +910,7 @@ public class ServiceContainer: Services { // swiftlint:disable:this type_body_le biometricsRepository: biometricsRepository, biometricsService: biometricsService, cameraService: DefaultCameraService(), + changeKdfService: changeKdfService, clientService: clientService, configService: configService, environmentService: environmentService, diff --git a/BitwardenShared/Core/Platform/Services/Services.swift b/BitwardenShared/Core/Platform/Services/Services.swift index 8fad579f91..f1faff3148 100644 --- a/BitwardenShared/Core/Platform/Services/Services.swift +++ b/BitwardenShared/Core/Platform/Services/Services.swift @@ -18,6 +18,7 @@ typealias Services = HasAPIService & HasAutofillCredentialService & HasBiometricsRepository & HasCameraService + & HasChangeKdfService & HasClientService & HasConfigService & HasDeviceAPIService @@ -144,6 +145,13 @@ protocol HasCameraService { var cameraService: CameraService { get } } +/// Protocol for an object that provides a `ChangeKdfService`. +/// +protocol HasChangeKdfService { + /// The service used to change the user's KDF settings. + var changeKdfService: ChangeKdfService { get } +} + /// Protocol for an object that provides a `ClientService`. /// protocol HasClientService { diff --git a/BitwardenShared/Core/Platform/Services/StateService.swift b/BitwardenShared/Core/Platform/Services/StateService.swift index 3dceb365e7..52e7510345 100644 --- a/BitwardenShared/Core/Platform/Services/StateService.swift +++ b/BitwardenShared/Core/Platform/Services/StateService.swift @@ -443,6 +443,14 @@ protocol StateService: AnyObject { /// - value: Whether the user has unlocked their account in the current session. func setAccountHasBeenUnlockedInteractively(userId: String?, value: Bool) async throws + /// Sets the KDF config for an account. + /// + /// - Parameters: + /// - kdfConfig: The account's KDF config. + /// - userId: The user ID of the account to set the KDF config for. + /// + func setAccountKdf(_ kdfConfig: KdfConfig, userId: String) async throws + /// Sets the master password unlock data for an account. /// /// - Parameters: @@ -1149,6 +1157,14 @@ extension StateService { try await setAccountHasBeenUnlockedInteractively(userId: nil, value: value) } + /// Sets the KDF config for the active account. + /// + /// - Parameter kdfConfig: The account's KDF config. + /// + func setAccountKdf(_ kdfConfig: KdfConfig) async throws { + try await setAccountKdf(kdfConfig, userId: getActiveAccountId()) + } + /// Sets the master password unlock data for the active account. /// /// - Parameter masterPasswordUnlock: The account master password unlock data. @@ -1361,6 +1377,9 @@ enum StateServiceError: LocalizedError { /// There are no known accounts. case noAccounts + /// There isn't an account with the specified user ID. + case noAccountForUserId + /// There isn't an active account. case noActiveAccount @@ -1857,6 +1876,15 @@ actor DefaultStateService: StateService, ConfigStateService { // swiftlint:disab && appSettingsStore.pinProtectedUserKey(userId: userId) == nil } + func setAccountKdf(_ kdfConfig: KdfConfig, userId: String) async throws { + try updateAccountProfile(userId: userId) { profile in + profile.kdfType = kdfConfig.kdfType + profile.kdfIterations = kdfConfig.iterations + profile.kdfMemory = kdfConfig.memory + profile.kdfParallelism = kdfConfig.parallelism + } + } + func setAccountEncryptionKeys(_ encryptionKeys: AccountEncryptionKeys, userId: String?) async throws { let userId = try userId ?? getActiveAccountUserId() appSettingsStore.setAccountKeys(encryptionKeys.accountKeys, userId: userId) @@ -2255,6 +2283,21 @@ actor DefaultStateService: StateService, ConfigStateService { // swiftlint:disab return activeUserId } + /// Updates the account's profile. + /// + /// - Parameters: + /// - userId: The user ID of the account to update. + /// - updateProfile: A closure that allows making updates to the account's profile. Any + /// updates made to the profile will be saved when the closure returns. + /// + private func updateAccountProfile(userId: String, updateProfile: (inout Account.AccountProfile) -> Void) throws { + guard var state = appSettingsStore.state else { throw StateServiceError.noAccounts } + guard var profile = state.accounts[userId]?.profile else { throw StateServiceError.noAccountForUserId } + updateProfile(&profile) + state.accounts[userId]?.profile = profile + appSettingsStore.state = state + } + /// Updates the settings badge publisher by determining the settings badge count for the user. /// /// - Parameter userId: The user ID whose settings badge count should be updated. diff --git a/BitwardenShared/Core/Platform/Services/StateServiceTests.swift b/BitwardenShared/Core/Platform/Services/StateServiceTests.swift index a14c7102a5..b813632e00 100644 --- a/BitwardenShared/Core/Platform/Services/StateServiceTests.swift +++ b/BitwardenShared/Core/Platform/Services/StateServiceTests.swift @@ -1930,6 +1930,39 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body } } + /// `setAccountKdf(_:)` sets the KDF config for the user account. + func test_setAccountKdf() async throws { + await subject.addAccount(.fixture(profile: .fixture(kdfType: .argon2id, userId: "1"))) + await subject.addAccount(.fixture(profile: .fixture(userId: "2"))) + + try await subject.setAccountKdf(.defaultKdfConfig, userId: "1") + try await subject.setAccountKdf( + KdfConfig(kdfType: .argon2id, iterations: 1, memory: 2, parallelism: 3), + userId: "2" + ) + + let user1 = try XCTUnwrap(appSettingsStore.state?.accounts["1"]) + XCTAssertEqual(user1.kdf, .defaultKdfConfig) + + let user2 = try XCTUnwrap(appSettingsStore.state?.accounts["2"]) + XCTAssertEqual(user2.kdf, KdfConfig(kdfType: .argon2id, iterations: 1, memory: 2, parallelism: 3)) + } + + /// `setAccountKdf(_:)` throws an error if there's no active account. + func test_setAccountKdf_noActiveAccount() async throws { + await assertAsyncThrows(error: StateServiceError.noActiveAccount) { + try await subject.setAccountKdf(.defaultKdfConfig) + } + } + + /// `setAccountKdf(_:userId:)` throws an error if the account for the user ID doesn't exist. + func test_setAccountKdf_noAccountForUserId() async throws { + await subject.addAccount(.fixture()) + await assertAsyncThrows(error: StateServiceError.noAccountForUserId) { + try await subject.setAccountKdf(.defaultKdfConfig, userId: "-1") + } + } + /// `setAccountMasterPasswordUnlock(_:)` sets the master password unlock data for the user account. func test_setAccountMasterPasswordUnlock() async throws { await subject.addAccount(.fixture(profile: .fixture(userId: "1"))) diff --git a/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift index a5b6c9f596..65907965ee 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/MockStateService.swift @@ -89,6 +89,7 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt var setAccountHasBeenUnlockedInteractivelyHasBeenCalled = false // swiftlint:disable:this identifier_name // swiftlint:disable:next identifier_name var setAccountHasBeenUnlockedInteractivelyResult: Result = .success(()) + var setAccountKdfByUserId = [String: KdfConfig]() var setAccountSetupAutofillCalled = false var setAppRehydrationStateError: Error? var setBiometricAuthenticationEnabledResult: Result = .success(()) @@ -454,6 +455,10 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt try setAccountHasBeenUnlockedInteractivelyResult.get() } + func setAccountKdf(_ kdfConfig: KdfConfig, userId: String) async throws { + setAccountKdfByUserId[userId] = kdfConfig + } + func setAccountMasterPasswordUnlock( _ masterPasswordUnlock: MasterPasswordUnlockResponseModel, userId: String diff --git a/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift b/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift index 15c8a56f76..b9d9716a6e 100644 --- a/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift +++ b/BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift @@ -22,6 +22,7 @@ extension ServiceContainer { biometricsRepository: BiometricsRepository = MockBiometricsRepository(), biometricsService: BiometricsService = MockBiometricsService(), cameraService: CameraService = MockCameraService(), + changeKdfService: ChangeKdfService = MockChangeKdfService(), clientService: ClientService = MockClientService(), configService: ConfigService = MockConfigService(), environmentService: EnvironmentService = MockEnvironmentService(), @@ -83,6 +84,7 @@ extension ServiceContainer { biometricsRepository: biometricsRepository, biometricsService: biometricsService, cameraService: cameraService, + changeKdfService: changeKdfService, clientService: clientService, configService: configService, environmentService: environmentService, diff --git a/BitwardenShared/Core/Platform/Utilities/Constants.swift b/BitwardenShared/Core/Platform/Utilities/Constants.swift index 139a0da8b6..903897b0c9 100644 --- a/BitwardenShared/Core/Platform/Utilities/Constants.swift +++ b/BitwardenShared/Core/Platform/Utilities/Constants.swift @@ -70,6 +70,9 @@ extension Constants { /// A default value for the minimum number of characters required when creating a password. static let minimumPasswordCharacters = 12 + /// The minimum number of PBKDF2 iterations before a forced KDF update is required. + static let minimumPbkdf2IterationsForUpgrade = 600_000 + /// The minimum length when setting a pin. static let minimumPinLength = 4 diff --git a/BitwardenShared/Core/Vault/Services/TestHelpers/MockSyncService.swift b/BitwardenShared/Core/Vault/Services/TestHelpers/MockSyncService.swift index 7d1f2dbdd2..d2999dcd72 100644 --- a/BitwardenShared/Core/Vault/Services/TestHelpers/MockSyncService.swift +++ b/BitwardenShared/Core/Vault/Services/TestHelpers/MockSyncService.swift @@ -6,6 +6,7 @@ class MockSyncService: SyncService { weak var delegate: SyncServiceDelegate? var didFetchSync = false var fetchSyncForceSync: Bool? + var fetchSyncHandler: (() -> Void)? var fetchSyncIsPeriodic: Bool? var fetchSyncResult: Result = .success(()) @@ -34,6 +35,7 @@ class MockSyncService: SyncService { didFetchSync = true fetchSyncForceSync = forceSync fetchSyncIsPeriodic = isPeriodic + fetchSyncHandler?() try fetchSyncResult.get() } diff --git a/BitwardenShared/UI/Auth/CompleteRegistration/CompleteRegistrationProcessor.swift b/BitwardenShared/UI/Auth/CompleteRegistration/CompleteRegistrationProcessor.swift index 47ab024841..55d4f44897 100644 --- a/BitwardenShared/UI/Auth/CompleteRegistration/CompleteRegistrationProcessor.swift +++ b/BitwardenShared/UI/Auth/CompleteRegistration/CompleteRegistrationProcessor.swift @@ -175,7 +175,7 @@ class CompleteRegistrationProcessor: StateProcessor< /// Performs an API request to create the user's account. private func createAccount() async throws { - let kdfConfig = KdfConfig() + let kdfConfig = KdfConfig.defaultKdfConfig let keys = try await services.clientService.auth(isPreAuth: true).makeRegisterKeys( email: state.userEmail, diff --git a/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift b/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift index 8597141450..116c778001 100644 --- a/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift +++ b/BitwardenShared/UI/Platform/Application/Utilities/Alert/Alert/TestHelpers/Alert+TestHelpers.swift @@ -63,6 +63,7 @@ extension Alert { /// - text: The text to set in the alert text field. /// - id: The identifier of the alert text field to set the text for. /// - Throws: Throws an `AlertError` if the alert text field cannot be found. + @MainActor func setText( _ text: String, forTextFieldWithId id: String diff --git a/BitwardenShared/UI/Vault/Extensions/Alert+Vault.swift b/BitwardenShared/UI/Vault/Extensions/Alert+Vault.swift index 5fe071e8a1..1f5cecc85a 100644 --- a/BitwardenShared/UI/Vault/Extensions/Alert+Vault.swift +++ b/BitwardenShared/UI/Vault/Extensions/Alert+Vault.swift @@ -2,6 +2,8 @@ import BitwardenResources import BitwardenSdk import UIKit +// swiftlint:disable file_length + // MARK: - Alert+Vault extension Alert { @@ -393,4 +395,38 @@ extension Alert { ] ) } + + /// An alert notifying the user to update their encryption settings. + /// + /// - Parameter completion: A closure that's executed when the user has entered their password. + /// - Returns: An alert that prompts the user to enter their master password to update their + /// encryption settings. + /// + static func updateEncryptionSettings( + completion: @MainActor @escaping (String) async -> Void + ) -> Alert { + Alert( + title: Localizations.updateYourEncryptionSettings, + message: Localizations.theNewRecommendedEncryptionSettingsDescriptionLong, + alertActions: [ + AlertAction(title: Localizations.cancel, style: .cancel), + AlertAction( + title: Localizations.submit, + style: .default + ) { _, alertTextFields in + guard let password = alertTextFields.first(where: { $0.id == "password" })?.text else { return } + await completion(password) + }, + ], + alertTextFields: [ + AlertTextField( + id: "password", + autocapitalizationType: .none, + autocorrectionType: .no, + isSecureTextEntry: true, + keyboardType: .default + ), + ] + ) + } } diff --git a/BitwardenShared/UI/Vault/Extensions/AlertVaultTests.swift b/BitwardenShared/UI/Vault/Extensions/AlertVaultTests.swift index 4cff097076..0d5cec8a5c 100644 --- a/BitwardenShared/UI/Vault/Extensions/AlertVaultTests.swift +++ b/BitwardenShared/UI/Vault/Extensions/AlertVaultTests.swift @@ -323,4 +323,27 @@ class AlertVaultTests: BitwardenTestCase { // swiftlint:disable:this type_body_l XCTAssertEqual(subject.alertActions.first?.title, Localizations.okGotIt) XCTAssertEqual(subject.alertActions.first?.style, .default) } + + /// `updateEncryptionSettings(_:)` constructs an `Alert` notifying the user to update their + /// encryption settings. + @MainActor + func test_updateEncryptionSettings() async throws { + var enteredPassword: String? + let subject = Alert.updateEncryptionSettings { enteredPassword = $0 } + + XCTAssertEqual(subject.title, Localizations.updateYourEncryptionSettings) + XCTAssertEqual(subject.message, Localizations.theNewRecommendedEncryptionSettingsDescriptionLong) + XCTAssertEqual(subject.alertActions.count, 2) + XCTAssertEqual(subject.alertActions[0].title, Localizations.cancel) + XCTAssertEqual(subject.alertActions[0].style, .cancel) + XCTAssertEqual(subject.alertActions[1].title, Localizations.submit) + XCTAssertEqual(subject.alertActions[1].style, .default) + + try await subject.tapCancel() + XCTAssertNil(enteredPassword) + + try subject.setText("password123!", forTextFieldWithId: "password") + try await subject.tapAction(title: Localizations.submit) + XCTAssertEqual(enteredPassword, "password123!") + } } diff --git a/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift b/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift index 70b4f59adf..7ba4c21211 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift @@ -77,6 +77,7 @@ final class VaultCoordinator: Coordinator, HasStackNavigator { // swiftlint:disa & HasAuthService & HasAutofillCredentialService & HasCameraService + & HasChangeKdfService & HasClientService & HasConfigService & HasEnvironmentService diff --git a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift index d17facf9b5..119ff34660 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift @@ -19,6 +19,7 @@ final class VaultListProcessor: StateProcessor< typealias Services = HasApplication & HasAuthRepository & HasAuthService + & HasChangeKdfService & HasErrorReporter & HasEventService & HasFlightRecorder @@ -207,6 +208,24 @@ extension VaultListProcessor { await loadItemTypesUserCanCreate() } + /// Checks if the user needs to update their KDF settings. + private func checkIfForceKdfUpdateRequired() async { + guard await services.changeKdfService.needsKdfUpdateToMinimums() else { return } + + coordinator.showAlert(.updateEncryptionSettings { password in + self.coordinator.showLoadingOverlay(title: Localizations.updating) + defer { self.coordinator.hideLoadingOverlay() } + + do { + try await self.services.changeKdfService.updateKdfToMinimums(password: password) + self.coordinator.showToast(Localizations.encryptionSettingsUpdated) + } catch { + self.services.errorReporter.log(error: error) + await self.coordinator.showErrorAlert(error: error) + } + }) + } + /// Check if there are any pending login requests for the user to deal with. private func checkPendingLoginRequests() async { do { @@ -342,6 +361,8 @@ extension VaultListProcessor { // will be published by the database, so it needs to be manually updated. state.loadingState = .data([]) } + + await checkIfForceKdfUpdateRequired() } catch URLError.cancelled { // No-op: don't log or alert for cancellation errors. } catch { diff --git a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift index 42191ed788..a62aa64175 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift @@ -17,6 +17,7 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ var application: MockApplication! var authRepository: MockAuthRepository! var authService: MockAuthService! + var changeKdfService: MockChangeKdfService! var coordinator: MockCoordinator! var errorReporter: MockErrorReporter! var flightRecorder: MockFlightRecorder! @@ -43,6 +44,7 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ authRepository = MockAuthRepository() authService = MockAuthService() errorReporter = MockErrorReporter() + changeKdfService = MockChangeKdfService() coordinator = MockCoordinator() errorReporter = MockErrorReporter() flightRecorder = MockFlightRecorder() @@ -59,6 +61,7 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ application: application, authRepository: authRepository, authService: authService, + changeKdfService: changeKdfService, errorReporter: errorReporter, flightRecorder: flightRecorder, notificationService: notificationService, @@ -84,6 +87,7 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ authRepository = nil authService = nil + changeKdfService = nil coordinator = nil errorReporter = nil flightRecorder = nil @@ -224,6 +228,69 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ XCTAssertNil(stateService.loginRequest) } + /// `perform(_:)` with `appeared` checks if the user's KDF settings need to be updated and logs + /// an error and shows an alert if updating the settings fails. + @MainActor + func test_perform_appeared_checkIfForceKdfUpdateRequired_error() async throws { + changeKdfService.needsKdfUpdateToMinimumsResult = true + changeKdfService.updateKdfToMinimumsResult = .failure(BitwardenTestError.example) + stateService.activeAccount = .fixture() + + await subject.perform(.appeared) + + XCTAssertTrue(changeKdfService.needsKdfUpdateToMinimumsCalled) + + let updateEncryptionSettingsAlert = try XCTUnwrap(coordinator.alertShown.first) + XCTAssertEqual(updateEncryptionSettingsAlert, .updateEncryptionSettings { _ in }) + + try updateEncryptionSettingsAlert.setText("password123!", forTextFieldWithId: "password") + try await updateEncryptionSettingsAlert.tapAction(title: Localizations.submit) + + XCTAssertEqual(coordinator.errorAlertsShown as? [BitwardenTestError], [.example]) + XCTAssertEqual(errorReporter.errors as? [BitwardenTestError], [.example]) + } + + /// `perform(_:)` with `appeared` checks if the user's KDF settings need to be updated and does + /// nothing if they already meet the minimums. + @MainActor + func test_perform_appeared_checkIfForceKdfUpdateRequired_false() async { + notificationService.authorizationStatus = .denied + stateService.activeAccount = .fixture() + + await subject.perform(.appeared) + + XCTAssertTrue(changeKdfService.needsKdfUpdateToMinimumsCalled) + XCTAssertFalse(changeKdfService.updateKdfToMinimumsCalled) + XCTAssertTrue(coordinator.alertShown.isEmpty) + } + + /// `perform(_:)` with `appeared` checks if the user's KDF settings need to be updated and + /// shows an alert asking the user for their master password to update the KDF settings. + @MainActor + func test_perform_appeared_checkIfForceKdfUpdateRequired_true() async throws { + changeKdfService.needsKdfUpdateToMinimumsResult = true + stateService.activeAccount = .fixture() + + await subject.perform(.appeared) + + XCTAssertTrue(changeKdfService.needsKdfUpdateToMinimumsCalled) + + let updateEncryptionSettingsAlert = try XCTUnwrap(coordinator.alertShown.first) + XCTAssertEqual(updateEncryptionSettingsAlert, .updateEncryptionSettings { _ in }) + + try await updateEncryptionSettingsAlert.tapCancel() + XCTAssertFalse(changeKdfService.updateKdfToMinimumsCalled) + + try updateEncryptionSettingsAlert.setText("password123!", forTextFieldWithId: "password") + try await updateEncryptionSettingsAlert.tapAction(title: Localizations.submit) + + XCTAssertTrue(changeKdfService.updateKdfToMinimumsCalled) + XCTAssertEqual(changeKdfService.updateKdfToMinimumsPassword, "password123!") + XCTAssertFalse(coordinator.isLoadingOverlayShowing) + XCTAssertEqual(coordinator.loadingOverlaysShown, [LoadingOverlayState(title: Localizations.updating)]) + XCTAssertEqual(coordinator.toastsShown, [Toast(title: Localizations.encryptionSettingsUpdated)]) + } + /// `perform(_:)` with `appeared` does not register the device for notifications /// if the user has denied notifications func test_perform_appeared_notificationRegistration_denied() async throws {