From 4c26ab5a9e25cf538c399413bb31c647a126659e Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Wed, 24 Jul 2024 10:25:57 -0400 Subject: [PATCH] [PM-10059] alert server if device trust is lost (#10235) * alert server if device trust is lost * add test * add tests for extra errors * fix build --------- Co-authored-by: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> --- .../browser/src/background/main.background.ts | 1 + apps/cli/src/service-container.ts | 1 + .../src/services/jslib-services.module.ts | 1 + .../sso-login.strategy.spec.ts | 21 ++++++++++++++ .../login-strategies/sso-login.strategy.ts | 10 +++++-- .../device-trust.service.abstraction.ts | 5 ++++ .../devices-api.service.abstraction.ts | 7 +++++ .../device-trust.service.implementation.ts | 21 ++++++++++++++ .../services/device-trust.service.spec.ts | 29 +++++++++++++++++++ .../devices-api.service.implementation.ts | 14 +++++++++ libs/common/src/enums/feature-flag.enum.ts | 2 ++ 11 files changed, 109 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 2fb8aa203b..86400e4d17 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -697,6 +697,7 @@ export default class MainBackground { this.secureStorageService, this.userDecryptionOptionsService, this.logService, + this.configService, ); this.devicesService = new DevicesServiceImplementation(this.devicesApiService); diff --git a/apps/cli/src/service-container.ts b/apps/cli/src/service-container.ts index cfe310318f..cc53302732 100644 --- a/apps/cli/src/service-container.ts +++ b/apps/cli/src/service-container.ts @@ -534,6 +534,7 @@ export class ServiceContainer { this.secureStorageService, this.userDecryptionOptionsService, this.logService, + this.configService, ); this.authRequestService = new AuthRequestService( diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 1b534b6f77..56b2e956cd 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1050,6 +1050,7 @@ const safeProviders: SafeProvider[] = [ SECURE_STORAGE, UserDecryptionOptionsServiceAbstraction, LogService, + ConfigService, ], }), safeProvider({ diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index 492772081d..885cad014c 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -312,6 +312,27 @@ describe("SsoLoginStrategy", () => { expect(cryptoService.setUserKey).not.toHaveBeenCalled(); }); + it("logs when a device key is found but no decryption keys were recieved in token response", async () => { + // Arrange + const userDecryptionOpts = userDecryptionOptsServerResponseWithTdeOption; + userDecryptionOpts.TrustedDeviceOption.EncryptedPrivateKey = null; + userDecryptionOpts.TrustedDeviceOption.EncryptedUserKey = null; + + const idTokenResponse: IdentityTokenResponse = identityTokenResponseFactory( + null, + userDecryptionOpts, + ); + + apiService.postIdentityToken.mockResolvedValue(idTokenResponse); + deviceTrustService.getDeviceKey.mockResolvedValue(mockDeviceKey); + + // Act + await ssoLoginStrategy.logIn(credentials); + + // Assert + expect(deviceTrustService.recordDeviceTrustLoss).toHaveBeenCalledTimes(1); + }); + describe("AdminAuthRequest", () => { let tokenResponse: IdentityTokenResponse; diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index 2ba0f682b5..5c979c5559 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -296,16 +296,20 @@ export class SsoLoginStrategy extends LoginStrategy { if (!deviceKey || !encDevicePrivateKey || !encUserKey) { if (!deviceKey) { - await this.logService.warning("Unable to set user key due to missing device key."); + this.logService.warning("Unable to set user key due to missing device key."); + } else if (!encDevicePrivateKey || !encUserKey) { + // Tell the server that we have a device key, but received no decryption keys + await this.deviceTrustService.recordDeviceTrustLoss(); } if (!encDevicePrivateKey) { - await this.logService.warning( + this.logService.warning( "Unable to set user key due to missing encrypted device private key.", ); } if (!encUserKey) { - await this.logService.warning("Unable to set user key due to missing encrypted user key."); + this.logService.warning("Unable to set user key due to missing encrypted user key."); } + return; } diff --git a/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts b/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts index 123f710338..4c52945554 100644 --- a/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts @@ -32,4 +32,9 @@ export abstract class DeviceTrustServiceAbstraction { newUserKey: UserKey, masterPasswordHash: string, ) => Promise; + /** + * Notifies the server that the device has a device key, but didn't receive any associated decryption keys. + * Note: For debugging purposes only. + */ + recordDeviceTrustLoss: () => Promise; } diff --git a/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts b/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts index 7ab929a2c2..8c3d0c3015 100644 --- a/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts @@ -27,4 +27,11 @@ export abstract class DevicesApiServiceAbstraction { deviceIdentifier: string, secretVerificationRequest: SecretVerificationRequest, ) => Promise; + + /** + * Notifies the server that the device has a device key, but didn't receive any associated decryption keys. + * Note: For debugging purposes only. + * @param deviceIdentifier - current device identifier + */ + postDeviceTrustLoss: (deviceIdentifier: string) => Promise; } diff --git a/libs/common/src/auth/services/device-trust.service.implementation.ts b/libs/common/src/auth/services/device-trust.service.implementation.ts index b02ff11448..cc80ea888c 100644 --- a/libs/common/src/auth/services/device-trust.service.implementation.ts +++ b/libs/common/src/auth/services/device-trust.service.implementation.ts @@ -2,7 +2,9 @@ import { firstValueFrom, map, Observable } from "rxjs"; import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { FeatureFlag } from "../../enums/feature-flag.enum"; import { AppIdService } from "../../platform/abstractions/app-id.service"; +import { ConfigService } from "../../platform/abstractions/config/config.service"; import { CryptoFunctionService } from "../../platform/abstractions/crypto-function.service"; import { CryptoService } from "../../platform/abstractions/crypto.service"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; @@ -68,6 +70,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { private secureStorageService: AbstractStorageService, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private logService: LogService, + private configService: ConfigService, ) { this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe( map((options) => options?.trustedDeviceOption != null ?? false), @@ -287,6 +290,16 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { throw new Error("UserId is required. Cannot decrypt user key with device key."); } + if (!encryptedDevicePrivateKey) { + throw new Error( + "Encrypted device private key is required. Cannot decrypt user key with device key.", + ); + } + + if (!encryptedUserKey) { + throw new Error("Encrypted user key is required. Cannot decrypt user key with device key."); + } + if (!deviceKey) { // User doesn't have a device key anymore so device is untrusted return null; @@ -315,6 +328,14 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { } } + async recordDeviceTrustLoss(): Promise { + if (!(await this.configService.getFeatureFlag(FeatureFlag.DeviceTrustLogging))) { + return; + } + const deviceIdentifier = await this.appIdService.getAppId(); + await this.devicesApiService.postDeviceTrustLoss(deviceIdentifier); + } + private getSecureStorageOptions(userId: UserId): StorageOptions { return { storageLocation: StorageLocation.Disk, diff --git a/libs/common/src/auth/services/device-trust.service.spec.ts b/libs/common/src/auth/services/device-trust.service.spec.ts index 7cc4de8b2d..7afd38ec0a 100644 --- a/libs/common/src/auth/services/device-trust.service.spec.ts +++ b/libs/common/src/auth/services/device-trust.service.spec.ts @@ -9,6 +9,7 @@ import { FakeActiveUserState } from "../../../spec/fake-state"; import { FakeStateProvider } from "../../../spec/fake-state-provider"; import { DeviceType } from "../../enums"; import { AppIdService } from "../../platform/abstractions/app-id.service"; +import { ConfigService } from "../../platform/abstractions/config/config.service"; import { CryptoFunctionService } from "../../platform/abstractions/crypto-function.service"; import { CryptoService } from "../../platform/abstractions/crypto.service"; import { EncryptService } from "../../platform/abstractions/encrypt.service"; @@ -50,6 +51,7 @@ describe("deviceTrustService", () => { const platformUtilsService = mock(); const secureStorageService = mock(); const logService = mock(); + const configService = mock(); const userDecryptionOptionsService = mock(); const decryptionOptions = new BehaviorSubject(null); @@ -533,6 +535,32 @@ describe("deviceTrustService", () => { ).rejects.toThrow("UserId is required. Cannot decrypt user key with device key."); }); + it("throws an error when a nullish encrypted device private key is passed in", async () => { + await expect( + deviceTrustService.decryptUserKeyWithDeviceKey( + mockUserId, + null, + mockEncryptedUserKey, + mockDeviceKey, + ), + ).rejects.toThrow( + "Encrypted device private key is required. Cannot decrypt user key with device key.", + ); + }); + + it("throws an error when a nullish encrypted user key is passed in", async () => { + await expect( + deviceTrustService.decryptUserKeyWithDeviceKey( + mockUserId, + mockEncryptedDevicePrivateKey, + null, + mockDeviceKey, + ), + ).rejects.toThrow( + "Encrypted user key is required. Cannot decrypt user key with device key.", + ); + }); + it("returns null when device key isn't provided", async () => { const result = await deviceTrustService.decryptUserKeyWithDeviceKey( mockUserId, @@ -731,6 +759,7 @@ describe("deviceTrustService", () => { secureStorageService, userDecryptionOptionsService, logService, + configService, ); } }); diff --git a/libs/common/src/auth/services/devices-api.service.implementation.ts b/libs/common/src/auth/services/devices-api.service.implementation.ts index 5ecd287458..fbef5c782e 100644 --- a/libs/common/src/auth/services/devices-api.service.implementation.ts +++ b/libs/common/src/auth/services/devices-api.service.implementation.ts @@ -101,4 +101,18 @@ export class DevicesApiServiceImplementation implements DevicesApiServiceAbstrac ); return new ProtectedDeviceResponse(result); } + + async postDeviceTrustLoss(deviceIdentifier: string): Promise { + await this.apiService.send( + "POST", + "/devices/lost-trust", + null, + true, + false, + null, + (headers) => { + headers.set("Device-Identifier", deviceIdentifier); + }, + ); + } } diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 7e88af236f..b5a617bc7e 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -26,6 +26,7 @@ export enum FeatureFlag { ProviderClientVaultPrivacyBanner = "ac-2833-provider-client-vault-privacy-banner", VaultBulkManagementAction = "vault-bulk-management-action", AC2828_ProviderPortalMembersPage = "AC-2828_provider-portal-members-page", + DeviceTrustLogging = "pm-8285-device-trust-logging", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -62,6 +63,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.ProviderClientVaultPrivacyBanner]: FALSE, [FeatureFlag.VaultBulkManagementAction]: FALSE, [FeatureFlag.AC2828_ProviderPortalMembersPage]: FALSE, + [FeatureFlag.DeviceTrustLogging]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue;