From 4843038cbd3b2f84d3bf551223f7c73598c8117e Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Tue, 18 Mar 2025 16:19:14 -0400 Subject: [PATCH] Innovation/opaque registration integration into password strategy (#13884) * WIP on registration and all todos * Fix naming after merge * Initial draft of register method * Override processTokenResponse * remove premature todo * Password Login Strategy - (1) adjust comment (2) fix import * ChangePassword - update logic to use default argon config * Password Login Strategy - (1) Wire up saving MP to cache (2) Add null checking to registration * DefaultOpaqueKeyExchangeSvc - (1) Update naming (2) Add null param error handling --------- Co-authored-by: Thomas Rittson --- .../settings/change-password.component.ts | 23 +++---- .../src/services/jslib-services.module.ts | 2 + .../login-strategies/base-login.strategy.ts | 4 +- .../password-login.strategy.ts | 60 +++++++++++++++++++ .../login-strategy.service.ts | 8 ++- .../default-opaque-key-exchange.service.ts | 30 +++++++--- .../opaque/models/cipher-configuration.ts | 1 + 7 files changed, 105 insertions(+), 23 deletions(-) diff --git a/apps/web/src/app/auth/settings/change-password.component.ts b/apps/web/src/app/auth/settings/change-password.component.ts index 9db1c438a6a..c853dbdf95b 100644 --- a/apps/web/src/app/auth/settings/change-password.component.ts +++ b/apps/web/src/app/auth/settings/change-password.component.ts @@ -24,7 +24,7 @@ import { MasterKey, UserKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { DialogService, ToastService } from "@bitwarden/components"; -import { KdfConfigService, KeyService } from "@bitwarden/key-management"; +import { Argon2KdfConfig, KdfConfigService, KdfType, KeyService } from "@bitwarden/key-management"; import { UserKeyRotationService } from "../../key-management/key-rotation/user-key-rotation.service"; @@ -222,24 +222,27 @@ export class ChangePasswordComponent return this.updateKey(); }); } else { + // PBKDF2 is not recommended for opaque, so force use of Argon2 with default params if the user is using PBKDF2. + const userConfiguredKdf = await this.kdfConfigService.getKdfConfig(); + const opaqueKdf = + userConfiguredKdf.kdfType === KdfType.Argon2id + ? userConfiguredKdf + : new Argon2KdfConfig(); + const sessionId = await this.opaqueKeyExchangeService.register( this.masterPassword, newUserKey[0], - { - memory: 256 * 1024, - iterations: 3, - parallelism: 4, - }, + opaqueKdf, ); request.opaqueSessionId = sessionId; this.formPromise = this.masterPasswordApiService.postPassword(request); } // TODO: remove this test code - await this.opaqueKeyExchangeService.register(this.masterPassword, newUserKey[0], { - algorithm: "argon2id", - parameters: { memory: 256 * 1024, iterations: 3, parallelism: 4 }, - }); + const userConfiguredKdf = await this.kdfConfigService.getKdfConfig(); + const opaqueKdf = + userConfiguredKdf.kdfType === KdfType.Argon2id ? userConfiguredKdf : new Argon2KdfConfig(); + await this.opaqueKeyExchangeService.register(this.masterPassword, newUserKey[0], opaqueKdf); this.toastService.showToast({ variant: "success", diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 0744373a6e3..4ba234bed0c 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -477,6 +477,8 @@ const safeProviders: SafeProvider[] = [ KdfConfigService, TaskSchedulerService, PrePasswordLoginApiService, + ConfigService, + OpaqueKeyExchangeService, ], }), safeProvider({ diff --git a/libs/auth/src/common/login-strategies/base-login.strategy.ts b/libs/auth/src/common/login-strategies/base-login.strategy.ts index 46b9c1e6631..ba32929643d 100644 --- a/libs/auth/src/common/login-strategies/base-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/base-login.strategy.ts @@ -98,7 +98,7 @@ export abstract class BaseLoginStrategy { protected userDecryptionOptionsService: InternalUserDecryptionOptionsServiceAbstraction, protected billingAccountProfileStateService: BillingAccountProfileStateService, protected vaultTimeoutSettingsService: VaultTimeoutSettingsService, - protected KdfConfigService: KdfConfigService, + protected kdfConfigService: KdfConfigService, protected environmentService: EnvironmentService, ) {} @@ -246,7 +246,7 @@ export abstract class BaseLoginStrategy { tokenResponse.refreshToken, // Note: CLI login via API key sends undefined for refresh token. ); - await this.KdfConfigService.setKdfConfig( + await this.kdfConfigService.setKdfConfig( userId as UserId, tokenResponse.kdf === KdfType.PBKDF2_SHA256 ? new PBKDF2KdfConfig(tokenResponse.kdfIterations) diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index a33d3c04e4d..a04f7f9a99b 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -13,11 +13,15 @@ import { IdentityCaptchaResponse } from "@bitwarden/common/auth/models/response/ import { IdentityDeviceVerificationResponse } from "@bitwarden/common/auth/models/response/identity-device-verification.response"; import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/identity-token.response"; import { IdentityTwoFactorResponse } from "@bitwarden/common/auth/models/response/identity-two-factor.response"; +import { OpaqueKeyExchangeService } from "@bitwarden/common/auth/opaque/opaque-key-exchange.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { HashPurpose } from "@bitwarden/common/platform/enums"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey } from "@bitwarden/common/types/key"; +import { Argon2KdfConfig, KdfType } from "@bitwarden/key-management"; import { PasswordHashLoginCredentials } from "../models/domain/login-credentials"; import { CacheData } from "../services/login-strategies/login-strategy.state"; @@ -32,6 +36,12 @@ export class PasswordLoginStrategyData implements LoginStrategyData { userEnteredEmail: string; /** If 2fa is required, token is returned to bypass captcha */ captchaBypassToken?: string; + + // TODO: we need to get a security audit as to whether or not this is safe to do. It is only used in memory + // for the duration of the login process, but it is still a security risk - especially in 2FA and new device verification + /** The user's master password */ + masterPassword: string; + /** The local version of the user's master key hash */ localMasterKeyHash: string; /** The user's master key */ @@ -65,6 +75,8 @@ export class PasswordLoginStrategy extends BaseLoginStrategy { data: PasswordLoginStrategyData, private passwordStrengthService: PasswordStrengthServiceAbstraction, private policyService: PolicyService, + private configService: ConfigService, + private opaqueKeyExchangeService: OpaqueKeyExchangeService, ...sharedDeps: ConstructorParameters ) { super(...sharedDeps); @@ -82,6 +94,7 @@ export class PasswordLoginStrategy extends BaseLoginStrategy { const data = new PasswordLoginStrategyData(); + data.masterPassword = masterPassword; data.masterKey = await this.makePrePasswordLoginMasterKey(masterPassword, email, kdfConfig); data.userEnteredEmail = email; @@ -138,6 +151,23 @@ export class PasswordLoginStrategy extends BaseLoginStrategy { authResult.forcePasswordReset = ForceSetPasswordReason.WeakMasterPassword; } } + + return authResult; + } + + protected override async processTokenResponse( + response: IdentityTokenResponse, + ): Promise { + const authResult = await super.processTokenResponse(response); + + const opaqueKeyExchangeFeatureFlagEnabled = await this.configService.getFeatureFlag( + FeatureFlag.OpaqueKeyExchange, + ); + if (opaqueKeyExchangeFeatureFlagEnabled) { + // Register the user for opaque password key exchange + await this.registerUserForOpaqueKeyExchange(authResult.userId); + } + return authResult; } @@ -246,4 +276,34 @@ export class PasswordLoginStrategy extends BaseLoginStrategy { const [authResult] = await this.startLogIn(); return authResult; } + + /** + * Registers password using users with the OPAQUE key exchange protocol which is a more secure password + * authN protocol which prevents the server from ever knowing anything about the user's password. + */ + private async registerUserForOpaqueKeyExchange(userId: UserId) { + const masterPassword = this.cache.value?.masterPassword; + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); + + // PBKDF2 is not recommended for opaque, so force use of Argon2 with default params if the user is using PBKDF2. + const userConfiguredKdf = await this.kdfConfigService.getKdfConfig(); + + if (!masterPassword || !userKey || !userConfiguredKdf) { + this.logService.error( + `Unable to register user for OPAQUE key exchange due to missing data. MasterPassword exists: ${!!masterPassword}; UserKey exists ${!!userKey}; KdfConfig exists: ${!!userConfiguredKdf}`, + ); + return; + } + + const opaqueKdf = + userConfiguredKdf.kdfType === KdfType.Argon2id ? userConfiguredKdf : new Argon2KdfConfig(); + + try { + await this.opaqueKeyExchangeService.register(masterPassword, userKey, opaqueKdf); + } catch (error) { + // If this process fails for any reason, we don't want to stop the login process + // so just log the error and continue. + this.logService.error(`Failed to register user for OPAQUE key exchange: ${error}`); + } + } } diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts index 1a0ba5b4ad3..9c92e96cbb6 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts @@ -18,6 +18,7 @@ import { AuthenticationType } from "@bitwarden/common/auth/enums/authentication- import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; import { TokenTwoFactorRequest } from "@bitwarden/common/auth/models/request/identity-token/token-two-factor.request"; import { PrePasswordLoginRequest } from "@bitwarden/common/auth/models/request/pre-password-login.request"; +import { OpaqueKeyExchangeService } from "@bitwarden/common/auth/opaque/opaque-key-exchange.service"; import { PrePasswordLoginApiService } from "@bitwarden/common/auth/services/pre-password-login-api.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; @@ -27,6 +28,7 @@ import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key- import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -131,6 +133,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { protected kdfConfigService: KdfConfigService, protected taskSchedulerService: TaskSchedulerService, protected prePasswordLoginApiService: PrePasswordLoginApiService, + protected configService: ConfigService, + protected opaqueKeyExchangeService: OpaqueKeyExchangeService, ) { this.currentAuthnTypeState = this.stateProvider.get(CURRENT_LOGIN_STRATEGY_KEY); this.loginStrategyCacheState = this.stateProvider.get(CACHE_KEY); @@ -226,8 +230,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { if (credentials.type === AuthenticationType.Password) { const preLoginRequest = new PrePasswordLoginRequest(credentials.email); - // TODO: OPAQUE: we have to save off whether or not to enroll the user in OPAQUE based on the - // response from the pre-password-login request and execute the enroll in the password login strategy const preLoginResponse = await this.prePasswordLoginApiService.postPrePasswordLogin(preLoginRequest); ownedCredentials = credentials.toSpecificLoginCredentials(preLoginResponse); @@ -396,6 +398,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { data?.password ?? new PasswordLoginStrategyData(), this.passwordStrengthService, this.policyService, + this.configService, + this.opaqueKeyExchangeService, ...sharedDeps, ); case AuthenticationType.Sso: diff --git a/libs/common/src/auth/opaque/default-opaque-key-exchange.service.ts b/libs/common/src/auth/opaque/default-opaque-key-exchange.service.ts index 4fc8a0949e3..2badddadaf5 100644 --- a/libs/common/src/auth/opaque/default-opaque-key-exchange.service.ts +++ b/libs/common/src/auth/opaque/default-opaque-key-exchange.service.ts @@ -25,25 +25,31 @@ export class DefaultOpaqueKeyExchangeService implements OpaqueKeyExchangeService async register( masterPassword: string, userKey: UserKey, - ksfParameters: Argon2IdParameters, + keyStretchingFuncArgon2Params: Argon2IdParameters, // TODO: eval if we can use KdfConfig existing type ): Promise { - const config = new CipherConfiguration(ksfParameters); + if (!masterPassword || !userKey || !keyStretchingFuncArgon2Params) { + throw new Error( + `Unable to register user with missing parameters. masterPassword exists: ${!!masterPassword}, userKey exists: ${!!userKey}, keyStretchingFuncArgon2Params exists: ${!!keyStretchingFuncArgon2Params}`, + ); + } + + const cipherConfig = new CipherConfiguration(keyStretchingFuncArgon2Params); const cryptoClient = (await firstValueFrom(this.sdkService.client$)).crypto(); const registrationStart = cryptoClient.opaque_register_start( masterPassword, - config.toSdkConfig(), + cipherConfig.toSdkConfig(), ); const registrationStartResponse = await this.opaqueKeyExchangeApiService.registrationStart( new RegistrationStartRequest( Utils.fromBufferToB64(registrationStart.registration_request), - config, + cipherConfig, ), ); const registrationFinish = cryptoClient.opaque_register_finish( masterPassword, - config.toSdkConfig(), + cipherConfig.toSdkConfig(), Utils.fromB64ToArray(registrationStartResponse.registrationResponse), registrationStart.state, ); @@ -75,19 +81,25 @@ export class DefaultOpaqueKeyExchangeService implements OpaqueKeyExchangeService async login( email: string, masterPassword: string, - ksfConfig: Argon2IdParameters, + keyStretchingFuncArgon2Params: Argon2IdParameters, ): Promise { - const config = new CipherConfiguration(ksfConfig); + if (!email || !masterPassword || !keyStretchingFuncArgon2Params) { + throw new Error( + `Unable to log in user with missing parameters. email exists: ${!!email}; masterPassword exists: ${!!masterPassword}; keyStretchingFuncArgon2Params exists: ${!!keyStretchingFuncArgon2Params}`, + ); + } + + const cipherConfig = new CipherConfiguration(keyStretchingFuncArgon2Params); const cryptoClient = (await firstValueFrom(this.sdkService.client$)).crypto(); - const loginStart = cryptoClient.opaque_login_start(masterPassword, config.toSdkConfig()); + const loginStart = cryptoClient.opaque_login_start(masterPassword, cipherConfig.toSdkConfig()); const loginStartResponse = await this.opaqueKeyExchangeApiService.loginStart( new LoginStartRequest(email, Utils.fromBufferToB64(loginStart.credential_request)), ); const loginFinish = cryptoClient.opaque_login_finish( masterPassword, - config.toSdkConfig(), + cipherConfig.toSdkConfig(), Utils.fromB64ToArray(loginStartResponse.credentialResponse), loginStart.state, ); diff --git a/libs/common/src/auth/opaque/models/cipher-configuration.ts b/libs/common/src/auth/opaque/models/cipher-configuration.ts index d5a6731db32..d41298e2c72 100644 --- a/libs/common/src/auth/opaque/models/cipher-configuration.ts +++ b/libs/common/src/auth/opaque/models/cipher-configuration.ts @@ -38,6 +38,7 @@ export class CipherConfiguration { } } +// TODO: eval if we can just use KdfConfig existing type export type Argon2IdParameters = { // Memory in KiB memory: number;