1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-21 16:18:28 +01:00

[PM-7919] Add more tde logging (#9035)

* adds additional logging to TDE service

* remove base catch swallowing errors

* add dependency to cli

* fix comment
This commit is contained in:
Jake Fink 2024-05-06 11:15:33 -04:00 committed by GitHub
parent b223e62c06
commit 09ff12fc02
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 40 additions and 22 deletions

View File

@ -34,6 +34,7 @@ import {
KeyGenerationServiceInitOptions, KeyGenerationServiceInitOptions,
keyGenerationServiceFactory, keyGenerationServiceFactory,
} from "../../../platform/background/service-factories/key-generation-service.factory"; } from "../../../platform/background/service-factories/key-generation-service.factory";
import { logServiceFactory } from "../../../platform/background/service-factories/log-service.factory";
import { import {
PlatformUtilsServiceInitOptions, PlatformUtilsServiceInitOptions,
platformUtilsServiceFactory, platformUtilsServiceFactory,
@ -88,6 +89,7 @@ export function deviceTrustServiceFactory(
await stateProviderFactory(cache, opts), await stateProviderFactory(cache, opts),
await secureStorageServiceFactory(cache, opts), await secureStorageServiceFactory(cache, opts),
await userDecryptionOptionsServiceFactory(cache, opts), await userDecryptionOptionsServiceFactory(cache, opts),
await logServiceFactory(cache, opts),
), ),
); );
} }

View File

@ -631,6 +631,7 @@ export default class MainBackground {
this.stateProvider, this.stateProvider,
this.secureStorageService, this.secureStorageService,
this.userDecryptionOptionsService, this.userDecryptionOptionsService,
this.logService,
); );
this.devicesService = new DevicesServiceImplementation(this.devicesApiService); this.devicesService = new DevicesServiceImplementation(this.devicesApiService);

View File

@ -486,6 +486,7 @@ export class Main {
this.stateProvider, this.stateProvider,
this.secureStorageService, this.secureStorageService,
this.userDecryptionOptionsService, this.userDecryptionOptionsService,
this.logService,
); );
this.authRequestService = new AuthRequestService( this.authRequestService = new AuthRequestService(

View File

@ -272,6 +272,7 @@ export class BaseLoginDecryptionOptionsComponent implements OnInit, OnDestroy {
// this.loading to support clients without async-actions-support // this.loading to support clients without async-actions-support
this.loading = true; this.loading = true;
// errors must be caught in child components to prevent navigation
try { try {
const { publicKey, privateKey } = await this.cryptoService.initAccount(); const { publicKey, privateKey } = await this.cryptoService.initAccount();
const keysRequest = new KeysRequest(publicKey, privateKey.encryptedString); const keysRequest = new KeysRequest(publicKey, privateKey.encryptedString);
@ -288,8 +289,6 @@ export class BaseLoginDecryptionOptionsComponent implements OnInit, OnDestroy {
if (this.rememberDeviceForm.value.rememberDevice) { if (this.rememberDeviceForm.value.rememberDevice) {
await this.deviceTrustService.trustDevice(this.activeAccountId); await this.deviceTrustService.trustDevice(this.activeAccountId);
} }
} catch (error) {
this.validationService.showError(error);
} finally { } finally {
this.loading = false; this.loading = false;
} }

View File

@ -967,6 +967,7 @@ const safeProviders: SafeProvider[] = [
StateProvider, StateProvider,
SECURE_STORAGE, SECURE_STORAGE,
UserDecryptionOptionsServiceAbstraction, UserDecryptionOptionsServiceAbstraction,
LogService,
], ],
}), }),
safeProvider({ safeProvider({

View File

@ -241,7 +241,7 @@ export class SsoLoginStrategy extends LoginStrategy {
if (userDecryptionOptions?.trustedDeviceOption) { if (userDecryptionOptions?.trustedDeviceOption) {
await this.trySetUserKeyWithApprovedAdminRequestIfExists(userId); await this.trySetUserKeyWithApprovedAdminRequestIfExists(userId);
const hasUserKey = await this.cryptoService.hasUserKey(); const hasUserKey = await this.cryptoService.hasUserKey(userId);
// Only try to set user key with device key if admin approval request was not successful // Only try to set user key with device key if admin approval request was not successful
if (!hasUserKey) { if (!hasUserKey) {

View File

@ -8,6 +8,7 @@ import { CryptoService } from "../../platform/abstractions/crypto.service";
import { EncryptService } from "../../platform/abstractions/encrypt.service"; import { EncryptService } from "../../platform/abstractions/encrypt.service";
import { I18nService } from "../../platform/abstractions/i18n.service"; import { I18nService } from "../../platform/abstractions/i18n.service";
import { KeyGenerationService } from "../../platform/abstractions/key-generation.service"; import { KeyGenerationService } from "../../platform/abstractions/key-generation.service";
import { LogService } from "../../platform/abstractions/log.service";
import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service";
import { AbstractStorageService } from "../../platform/abstractions/storage.service"; import { AbstractStorageService } from "../../platform/abstractions/storage.service";
import { StorageLocation } from "../../platform/enums"; import { StorageLocation } from "../../platform/enums";
@ -61,6 +62,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
private stateProvider: StateProvider, private stateProvider: StateProvider,
private secureStorageService: AbstractStorageService, private secureStorageService: AbstractStorageService,
private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
private logService: LogService,
) { ) {
this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe( this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe(
map((options) => options?.trustedDeviceOption != null ?? false), map((options) => options?.trustedDeviceOption != null ?? false),
@ -110,7 +112,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
} }
// Attempt to get user key // Attempt to get user key
const userKey: UserKey = await this.cryptoService.getUserKey(); const userKey: UserKey = await this.cryptoService.getUserKey(userId);
// If user key is not found, throw error // If user key is not found, throw error
if (!userKey) { if (!userKey) {
@ -223,19 +225,23 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
throw new Error("UserId is required. Cannot get device key."); throw new Error("UserId is required. Cannot get device key.");
} }
if (this.platformSupportsSecureStorage) { try {
const deviceKeyB64 = await this.secureStorageService.get< if (this.platformSupportsSecureStorage) {
ReturnType<SymmetricCryptoKey["toJSON"]> const deviceKeyB64 = await this.secureStorageService.get<
>(`${userId}${this.deviceKeySecureStorageKey}`, this.getSecureStorageOptions(userId)); ReturnType<SymmetricCryptoKey["toJSON"]>
>(`${userId}${this.deviceKeySecureStorageKey}`, this.getSecureStorageOptions(userId));
const deviceKey = SymmetricCryptoKey.fromJSON(deviceKeyB64) as DeviceKey; const deviceKey = SymmetricCryptoKey.fromJSON(deviceKeyB64) as DeviceKey;
return deviceKey;
}
const deviceKey = await firstValueFrom(this.stateProvider.getUserState$(DEVICE_KEY, userId));
return deviceKey; return deviceKey;
} catch (e) {
this.logService.error("Failed to get device key", e);
} }
const deviceKey = await firstValueFrom(this.stateProvider.getUserState$(DEVICE_KEY, userId));
return deviceKey;
} }
private async setDeviceKey(userId: UserId, deviceKey: DeviceKey | null): Promise<void> { private async setDeviceKey(userId: UserId, deviceKey: DeviceKey | null): Promise<void> {
@ -243,16 +249,20 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
throw new Error("UserId is required. Cannot set device key."); throw new Error("UserId is required. Cannot set device key.");
} }
if (this.platformSupportsSecureStorage) { try {
await this.secureStorageService.save<DeviceKey>( if (this.platformSupportsSecureStorage) {
`${userId}${this.deviceKeySecureStorageKey}`, await this.secureStorageService.save<DeviceKey>(
deviceKey, `${userId}${this.deviceKeySecureStorageKey}`,
this.getSecureStorageOptions(userId), deviceKey,
); this.getSecureStorageOptions(userId),
return; );
} return;
}
await this.stateProvider.setUserState(DEVICE_KEY, deviceKey?.toJSON(), userId); await this.stateProvider.setUserState(DEVICE_KEY, deviceKey?.toJSON(), userId);
} catch (e) {
this.logService.error("Failed to set device key", e);
}
} }
private async makeDeviceKey(): Promise<DeviceKey> { private async makeDeviceKey(): Promise<DeviceKey> {
@ -293,6 +303,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
return new SymmetricCryptoKey(userKey) as UserKey; return new SymmetricCryptoKey(userKey) as UserKey;
} catch (e) { } catch (e) {
// If either decryption effort fails, we want to remove the device key // If either decryption effort fails, we want to remove the device key
this.logService.error("Failed to decrypt using device key. Removing device key.");
await this.setDeviceKey(userId, null); await this.setDeviceKey(userId, null);
return null; return null;

View File

@ -14,6 +14,7 @@ import { CryptoService } from "../../platform/abstractions/crypto.service";
import { EncryptService } from "../../platform/abstractions/encrypt.service"; import { EncryptService } from "../../platform/abstractions/encrypt.service";
import { I18nService } from "../../platform/abstractions/i18n.service"; import { I18nService } from "../../platform/abstractions/i18n.service";
import { KeyGenerationService } from "../../platform/abstractions/key-generation.service"; import { KeyGenerationService } from "../../platform/abstractions/key-generation.service";
import { LogService } from "../../platform/abstractions/log.service";
import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service";
import { AbstractStorageService } from "../../platform/abstractions/storage.service"; import { AbstractStorageService } from "../../platform/abstractions/storage.service";
import { StorageLocation } from "../../platform/enums"; import { StorageLocation } from "../../platform/enums";
@ -48,6 +49,7 @@ describe("deviceTrustService", () => {
const i18nService = mock<I18nService>(); const i18nService = mock<I18nService>();
const platformUtilsService = mock<PlatformUtilsService>(); const platformUtilsService = mock<PlatformUtilsService>();
const secureStorageService = mock<AbstractStorageService>(); const secureStorageService = mock<AbstractStorageService>();
const logService = mock<LogService>();
const userDecryptionOptionsService = mock<UserDecryptionOptionsServiceAbstraction>(); const userDecryptionOptionsService = mock<UserDecryptionOptionsServiceAbstraction>();
const decryptionOptions = new BehaviorSubject<UserDecryptionOptions>(null); const decryptionOptions = new BehaviorSubject<UserDecryptionOptions>(null);
@ -726,6 +728,7 @@ describe("deviceTrustService", () => {
stateProvider, stateProvider,
secureStorageService, secureStorageService,
userDecryptionOptionsService, userDecryptionOptionsService,
logService,
); );
} }
}); });