mirror of
https://github.com/bitwarden/browser
synced 2025-12-06 00:13:28 +00:00
[PM-2199] Implement userkey rotation for all TDE devices (#13576)
* Implement key rotation v2 * Pass through masterpassword hint * Properly split old and new code * Mark legacy rotation as deprecated * Throw when data is null * Cleanup * Add tests * Fix build * Update libs/key-management/src/key.service.spec.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update apps/web/src/app/auth/settings/change-password.component.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Add documentation * Centralize loading logic * Add proof-of-concept for tde rotation * Fix build * Only include trusted devices in rotation request * Undo featureflag change * Fix tests * Prettier format * Fix build * Undo changes to migrate legacy component * Address feedback & add tests --------- Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>
This commit is contained in:
@@ -2,7 +2,6 @@
|
||||
// @ts-strict-ignore
|
||||
import { ListResponse } from "../../models/response/list.response";
|
||||
import { DeviceResponse } from "../abstractions/devices/responses/device.response";
|
||||
import { SecretVerificationRequest } from "../models/request/secret-verification.request";
|
||||
import { UpdateDevicesTrustRequest } from "../models/request/update-devices-trust.request";
|
||||
import { ProtectedDeviceResponse } from "../models/response/protected-device.response";
|
||||
|
||||
@@ -25,10 +24,7 @@ export abstract class DevicesApiServiceAbstraction {
|
||||
deviceIdentifier: string,
|
||||
) => Promise<void>;
|
||||
|
||||
getDeviceKeys: (
|
||||
deviceIdentifier: string,
|
||||
secretVerificationRequest: SecretVerificationRequest,
|
||||
) => Promise<ProtectedDeviceResponse>;
|
||||
getDeviceKeys: (deviceIdentifier: string) => Promise<ProtectedDeviceResponse>;
|
||||
|
||||
/**
|
||||
* Notifies the server that the device has a device key, but didn't receive any associated decryption keys.
|
||||
|
||||
@@ -13,5 +13,5 @@ export class DeviceKeysUpdateRequest {
|
||||
}
|
||||
|
||||
export class OtherDeviceKeysUpdateRequest extends DeviceKeysUpdateRequest {
|
||||
id: string;
|
||||
deviceId: string;
|
||||
}
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
// @ts-strict-ignore
|
||||
import { Jsonify } from "type-fest";
|
||||
|
||||
import { RotateableKeySet } from "@bitwarden/auth/common";
|
||||
|
||||
import { DeviceType } from "../../../enums";
|
||||
import { BaseResponse } from "../../../models/response/base.response";
|
||||
import { EncString } from "../../../platform/models/domain/enc-string";
|
||||
@@ -38,4 +40,12 @@ export class ProtectedDeviceResponse extends BaseResponse {
|
||||
* This enabled a user to rotate the keys for all of their devices.
|
||||
*/
|
||||
encryptedPublicKey: EncString;
|
||||
|
||||
getRotateableKeyset(): RotateableKeySet {
|
||||
return new RotateableKeySet(this.encryptedUserKey, this.encryptedPublicKey);
|
||||
}
|
||||
|
||||
isTrusted(): boolean {
|
||||
return this.encryptedUserKey != null && this.encryptedPublicKey != null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,7 +5,6 @@ import { ListResponse } from "../../models/response/list.response";
|
||||
import { Utils } from "../../platform/misc/utils";
|
||||
import { DeviceResponse } from "../abstractions/devices/responses/device.response";
|
||||
import { DevicesApiServiceAbstraction } from "../abstractions/devices-api.service.abstraction";
|
||||
import { SecretVerificationRequest } from "../models/request/secret-verification.request";
|
||||
import { UpdateDevicesTrustRequest } from "../models/request/update-devices-trust.request";
|
||||
import { ProtectedDeviceResponse } from "../models/response/protected-device.response";
|
||||
|
||||
@@ -90,14 +89,11 @@ export class DevicesApiServiceImplementation implements DevicesApiServiceAbstrac
|
||||
);
|
||||
}
|
||||
|
||||
async getDeviceKeys(
|
||||
deviceIdentifier: string,
|
||||
secretVerificationRequest: SecretVerificationRequest,
|
||||
): Promise<ProtectedDeviceResponse> {
|
||||
async getDeviceKeys(deviceIdentifier: string): Promise<ProtectedDeviceResponse> {
|
||||
const result = await this.apiService.send(
|
||||
"POST",
|
||||
`/devices/${deviceIdentifier}/retrieve-keys`,
|
||||
secretVerificationRequest,
|
||||
null,
|
||||
true,
|
||||
true,
|
||||
);
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
// @ts-strict-ignore
|
||||
import { Observable } from "rxjs";
|
||||
|
||||
import { DeviceKeysUpdateRequest } from "@bitwarden/common/auth/models/request/update-devices-trust.request";
|
||||
|
||||
import { DeviceResponse } from "../../../auth/abstractions/devices/responses/device.response";
|
||||
import { EncString } from "../../../platform/models/domain/enc-string";
|
||||
import { UserId } from "../../../types/guid";
|
||||
@@ -55,4 +57,9 @@ export abstract class DeviceTrustServiceAbstraction {
|
||||
* Note: For debugging purposes only.
|
||||
*/
|
||||
recordDeviceTrustLoss: () => Promise<void>;
|
||||
getRotatedData: (
|
||||
oldUserKey: UserKey,
|
||||
newUserKey: UserKey,
|
||||
userId: UserId,
|
||||
) => Promise<DeviceKeysUpdateRequest[]>;
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
// @ts-strict-ignore
|
||||
import { firstValueFrom, map, Observable, Subject } from "rxjs";
|
||||
|
||||
import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common";
|
||||
import { RotateableKeySet, UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common";
|
||||
import { KeyService } from "@bitwarden/key-management";
|
||||
|
||||
import { DeviceResponse } from "../../../auth/abstractions/devices/responses/device.response";
|
||||
@@ -10,6 +10,7 @@ import { DevicesApiServiceAbstraction } from "../../../auth/abstractions/devices
|
||||
import { SecretVerificationRequest } from "../../../auth/models/request/secret-verification.request";
|
||||
import {
|
||||
DeviceKeysUpdateRequest,
|
||||
OtherDeviceKeysUpdateRequest,
|
||||
UpdateDevicesTrustRequest,
|
||||
} from "../../../auth/models/request/update-devices-trust.request";
|
||||
import { AppIdService } from "../../../platform/abstractions/app-id.service";
|
||||
@@ -187,6 +188,51 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
||||
return deviceResponse;
|
||||
}
|
||||
|
||||
async getRotatedData(
|
||||
oldUserKey: UserKey,
|
||||
newUserKey: UserKey,
|
||||
userId: UserId,
|
||||
): Promise<DeviceKeysUpdateRequest[]> {
|
||||
if (!userId) {
|
||||
throw new Error("UserId is required. Cannot get rotated data.");
|
||||
}
|
||||
if (!oldUserKey) {
|
||||
throw new Error("Old user key is required. Cannot get rotated data.");
|
||||
}
|
||||
if (!newUserKey) {
|
||||
throw new Error("New user key is required. Cannot get rotated data.");
|
||||
}
|
||||
|
||||
const devices = await this.devicesApiService.getDevices();
|
||||
return await Promise.all(
|
||||
devices.data
|
||||
.filter((device) => device.isTrusted)
|
||||
.map(async (device) => {
|
||||
const deviceWithKeys = await this.devicesApiService.getDeviceKeys(device.identifier);
|
||||
const publicKey = await this.encryptService.decryptToBytes(
|
||||
deviceWithKeys.encryptedPublicKey,
|
||||
oldUserKey,
|
||||
);
|
||||
const newEncryptedPublicKey = await this.encryptService.encrypt(publicKey, newUserKey);
|
||||
const newEncryptedUserKey = await this.encryptService.rsaEncrypt(
|
||||
newUserKey.key,
|
||||
publicKey,
|
||||
);
|
||||
|
||||
const newRotateableKeySet = new RotateableKeySet(
|
||||
newEncryptedUserKey,
|
||||
newEncryptedPublicKey,
|
||||
);
|
||||
|
||||
const request = new OtherDeviceKeysUpdateRequest();
|
||||
request.encryptedPublicKey = newRotateableKeySet.encryptedPublicKey.encryptedString;
|
||||
request.encryptedUserKey = newRotateableKeySet.encryptedUserKey.encryptedString;
|
||||
request.deviceId = device.id;
|
||||
return request;
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
async rotateDevicesTrust(
|
||||
userId: UserId,
|
||||
newUserKey: UserKey,
|
||||
@@ -216,10 +262,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
||||
secretVerificationRequest.masterPasswordHash = masterPasswordHash;
|
||||
|
||||
// Get the keys that are used in rotating a devices keys from the server
|
||||
const currentDeviceKeys = await this.devicesApiService.getDeviceKeys(
|
||||
deviceIdentifier,
|
||||
secretVerificationRequest,
|
||||
);
|
||||
const currentDeviceKeys = await this.devicesApiService.getDeviceKeys(deviceIdentifier);
|
||||
|
||||
// Decrypt the existing device public key with the old user key
|
||||
const decryptedDevicePublicKey = await this.encryptService.decryptToBytes(
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
UserDecryptionOptionsServiceAbstraction,
|
||||
UserDecryptionOptions,
|
||||
} from "@bitwarden/auth/common";
|
||||
import { ListResponse } from "@bitwarden/common/models/response/list.response";
|
||||
import { KeyService } from "@bitwarden/key-management";
|
||||
|
||||
import { FakeAccountService, mockAccountServiceWith } from "../../../../spec/fake-account-service";
|
||||
@@ -655,6 +656,86 @@ describe("deviceTrustService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("getRotatedData", () => {
|
||||
let fakeNewUserKey: UserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey;
|
||||
let fakeOldUserKey: UserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey;
|
||||
const userId: UserId = Utils.newGuid() as UserId;
|
||||
|
||||
it("throws an error when a null user id is passed in", async () => {
|
||||
await expect(
|
||||
deviceTrustService.getRotatedData(fakeOldUserKey, fakeNewUserKey, null),
|
||||
).rejects.toThrow("UserId is required. Cannot get rotated data.");
|
||||
});
|
||||
|
||||
it("throws an error when a null old user key is passed in", async () => {
|
||||
await expect(
|
||||
deviceTrustService.getRotatedData(null, fakeNewUserKey, userId),
|
||||
).rejects.toThrow("Old user key is required. Cannot get rotated data.");
|
||||
});
|
||||
|
||||
it("throws an error when a null new user key is passed in", async () => {
|
||||
await expect(
|
||||
deviceTrustService.getRotatedData(fakeOldUserKey, null, userId),
|
||||
).rejects.toThrow("New user key is required. Cannot get rotated data.");
|
||||
});
|
||||
|
||||
it("returns the expected data when all required parameters are provided", async () => {
|
||||
const deviceResponse = {
|
||||
id: "",
|
||||
userId: "",
|
||||
name: "",
|
||||
identifier: "",
|
||||
type: DeviceType.Android,
|
||||
creationDate: "",
|
||||
revisionDate: "",
|
||||
isTrusted: true,
|
||||
};
|
||||
devicesApiService.getDevices.mockResolvedValue(
|
||||
new ListResponse(
|
||||
{
|
||||
data: [deviceResponse],
|
||||
},
|
||||
DeviceResponse,
|
||||
),
|
||||
);
|
||||
encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(64));
|
||||
encryptService.encrypt.mockResolvedValue(new EncString("test_encrypted_data"));
|
||||
encryptService.rsaEncrypt.mockResolvedValue(new EncString("test_encrypted_data"));
|
||||
|
||||
const protectedDeviceResponse = new ProtectedDeviceResponse({
|
||||
id: "",
|
||||
creationDate: "",
|
||||
identifier: "test_device_identifier",
|
||||
name: "Firefox",
|
||||
type: DeviceType.FirefoxBrowser,
|
||||
encryptedPublicKey: "",
|
||||
encryptedUserKey: "",
|
||||
});
|
||||
devicesApiService.getDeviceKeys.mockResolvedValue(protectedDeviceResponse);
|
||||
const fakeOldUserKeyData = new Uint8Array(64);
|
||||
fakeOldUserKeyData.fill(5, 0, 1);
|
||||
fakeOldUserKey = new SymmetricCryptoKey(fakeOldUserKeyData) as UserKey;
|
||||
|
||||
const fakeNewUserKeyData = new Uint8Array(64);
|
||||
fakeNewUserKeyData.fill(1, 0, 1);
|
||||
fakeNewUserKey = new SymmetricCryptoKey(fakeNewUserKeyData) as UserKey;
|
||||
|
||||
const result = await deviceTrustService.getRotatedData(
|
||||
fakeOldUserKey,
|
||||
fakeNewUserKey,
|
||||
userId,
|
||||
);
|
||||
|
||||
expect(result).toEqual([
|
||||
{
|
||||
deviceId: "",
|
||||
encryptedUserKey: "test_encrypted_data",
|
||||
encryptedPublicKey: "test_encrypted_data",
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("rotateDevicesTrust", () => {
|
||||
let fakeNewUserKey: UserKey = null;
|
||||
|
||||
@@ -708,11 +789,8 @@ describe("deviceTrustService", () => {
|
||||
|
||||
appIdService.getAppId.mockResolvedValue("test_device_identifier");
|
||||
|
||||
devicesApiService.getDeviceKeys.mockImplementation((deviceIdentifier, secretRequest) => {
|
||||
if (
|
||||
deviceIdentifier !== "test_device_identifier" ||
|
||||
secretRequest.masterPasswordHash !== "my_password_hash"
|
||||
) {
|
||||
devicesApiService.getDeviceKeys.mockImplementation((deviceIdentifier) => {
|
||||
if (deviceIdentifier !== "test_device_identifier") {
|
||||
return Promise.resolve(null);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user