1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 08:13:42 +00:00

[PM-21033/PM-22863] User Encryption v2 (#14942)

* Add new encrypt service functions

* Undo changes

* Cleanup

* Fix build

* Fix comments

* Switch encrypt service to use SDK functions

* Move remaining functions to PureCrypto

* Tests

* Increase test coverage

* Split up userkey rotation v2 and add tests

* Fix eslint

* Fix type errors

* Fix tests

* Implement signing keys

* Fix sdk init

* Remove key rotation v2 flag

* Fix parsing when user does not have signing keys

* Clear up trusted key naming

* Split up getNewAccountKeys

* Add trim and lowercase

* Replace user.email with masterKeySalt

* Add wasTrustDenied to verifyTrust in key rotation service

* Move testable userkey rotation service code to testable class

* Fix build

* Add comments

* Undo changes

* Fix incorrect behavior on aborting key rotation and fix import

* Fix tests

* Make members of userkey rotation service protected

* Fix type error

* Cleanup and add injectable annotation

* Fix tests

* Update apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

* Remove v1 rotation request

* Add upgrade to user encryption v2

* Fix types

* Update sdk method calls

* Update request models for new server api for rotation

* Fix build

* Update userkey rotation for new server API

* Update crypto client call for new sdk changes

* Fix rotation with signing keys

* Cargo lock

* Fix userkey rotation service

* Fix types

* Undo changes to feature flag service

* Fix linting

* [PM-22863] Account security state (#15309)

* Add account security state

* Update key rotation

* Rename

* Fix build

* Cleanup

* Further cleanup

* Tests

* Increase test coverage

* Add test

* Increase test coverage

* Fix builds and update sdk

* Fix build

* Fix tests

* Reset changes to encrypt service

* Cleanup

* Add comment

* Cleanup

* Cleanup

* Rename model

* Cleanup

* Fix build

* Clean up

* Fix types

* Cleanup

* Cleanup

* Cleanup

* Add test

* Simplify request model

* Rename and add comments

* Fix tests

* Update responses to use less strict typing

* Fix response parsing for v1 users

* Update libs/common/src/key-management/keys/response/private-keys.response.ts

Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com>

* Update libs/common/src/key-management/keys/response/private-keys.response.ts

Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com>

* Fix build

* Fix build

* Fix build

* Undo change

* Fix attachments not encrypting for v2 users

---------

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>
Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com>
This commit is contained in:
Bernd Schoolmann
2025-10-10 23:04:47 +02:00
committed by GitHub
parent 89eb60135f
commit cc8bd71775
36 changed files with 1693 additions and 327 deletions

View File

@@ -1,4 +1,5 @@
import { EncryptedString } from "../../../key-management/crypto/models/enc-string";
import { WrappedSigningKey } from "../../../key-management/types";
import { UserKey } from "../../../types/key";
import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key";
import { CRYPTO_DISK, CRYPTO_MEMORY, UserKeyDefinition } from "../../state";
@@ -25,3 +26,12 @@ export const USER_KEY = new UserKeyDefinition<UserKey>(CRYPTO_MEMORY, "userKey",
deserializer: (obj) => SymmetricCryptoKey.fromJSON(obj) as UserKey,
clearOn: ["logout", "lock"],
});
export const USER_KEY_ENCRYPTED_SIGNING_KEY = new UserKeyDefinition<WrappedSigningKey>(
CRYPTO_DISK,
"userSigningKey",
{
deserializer: (obj) => obj,
clearOn: ["logout"],
},
);

View File

@@ -1,7 +1,7 @@
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom, of } from "rxjs";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { KdfConfigService, KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management";
@@ -18,6 +18,7 @@ import { AccountInfo } from "../../../auth/abstractions/account.service";
import { EncryptedString } from "../../../key-management/crypto/models/enc-string";
import { UserId } from "../../../types/guid";
import { UserKey } from "../../../types/key";
import { ConfigService } from "../../abstractions/config/config.service";
import { Environment, EnvironmentService } from "../../abstractions/environment.service";
import { PlatformUtilsService } from "../../abstractions/platform-utils.service";
import { SdkClientFactory } from "../../abstractions/sdk/sdk-client-factory";
@@ -43,6 +44,7 @@ describe("DefaultSdkService", () => {
let platformUtilsService!: MockProxy<PlatformUtilsService>;
let kdfConfigService!: MockProxy<KdfConfigService>;
let keyService!: MockProxy<KeyService>;
let securityStateService!: MockProxy<SecurityStateService>;
let configService!: MockProxy<ConfigService>;
let service!: DefaultSdkService;
let accountService!: FakeAccountService;
@@ -57,6 +59,7 @@ describe("DefaultSdkService", () => {
platformUtilsService = mock<PlatformUtilsService>();
kdfConfigService = mock<KdfConfigService>();
keyService = mock<KeyService>();
securityStateService = mock<SecurityStateService>();
apiService = mock<ApiService>();
const mockUserId = Utils.newGuid() as UserId;
accountService = mockAccountServiceWith(mockUserId);
@@ -75,6 +78,7 @@ describe("DefaultSdkService", () => {
accountService,
kdfConfigService,
keyService,
securityStateService,
apiService,
fakeStateProvider,
configService,
@@ -100,6 +104,8 @@ describe("DefaultSdkService", () => {
.calledWith(userId)
.mockReturnValue(of("private-key" as EncryptedString));
keyService.encryptedOrgKeys$.calledWith(userId).mockReturnValue(of({}));
keyService.userSigningKey$.calledWith(userId).mockReturnValue(of(null));
securityStateService.accountSecurityState$.calledWith(userId).mockReturnValue(of(null));
});
describe("given no client override has been set for the user", () => {

View File

@@ -31,6 +31,8 @@ import { ApiService } from "../../../abstractions/api.service";
import { AccountInfo, AccountService } from "../../../auth/abstractions/account.service";
import { DeviceType } from "../../../enums/device-type.enum";
import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string";
import { SecurityStateService } from "../../../key-management/security-state/abstractions/security-state.service";
import { SignedSecurityState, WrappedSigningKey } from "../../../key-management/types";
import { OrganizationId, UserId } from "../../../types/guid";
import { UserKey } from "../../../types/key";
import { Environment, EnvironmentService } from "../../abstractions/environment.service";
@@ -98,6 +100,7 @@ export class DefaultSdkService implements SdkService {
private accountService: AccountService,
private kdfConfigService: KdfConfigService,
private keyService: KeyService,
private securityStateService: SecurityStateService,
private apiService: ApiService,
private stateProvider: StateProvider,
private configService: ConfigService,
@@ -160,10 +163,14 @@ export class DefaultSdkService implements SdkService {
const privateKey$ = this.keyService
.userEncryptedPrivateKey$(userId)
.pipe(distinctUntilChanged());
const signingKey$ = this.keyService.userSigningKey$(userId).pipe(distinctUntilChanged());
const userKey$ = this.keyService.userKey$(userId).pipe(distinctUntilChanged());
const orgKeys$ = this.keyService.encryptedOrgKeys$(userId).pipe(
distinctUntilChanged(compareValues), // The upstream observable emits different objects with the same values
);
const securityState$ = this.securityStateService
.accountSecurityState$(userId)
.pipe(distinctUntilChanged(compareValues));
const client$ = combineLatest([
this.environmentService.getEnvironment$(userId),
@@ -171,51 +178,57 @@ export class DefaultSdkService implements SdkService {
kdfParams$,
privateKey$,
userKey$,
signingKey$,
orgKeys$,
securityState$,
SdkLoadService.Ready, // Makes sure we wait (once) for the SDK to be loaded
]).pipe(
// switchMap is required to allow the clean-up logic to be executed when `combineLatest` emits a new value.
switchMap(([env, account, kdfParams, privateKey, userKey, orgKeys]) => {
// Create our own observable to be able to implement clean-up logic
return new Observable<Rc<BitwardenClient>>((subscriber) => {
const createAndInitializeClient = async () => {
if (env == null || kdfParams == null || privateKey == null || userKey == null) {
return undefined;
}
switchMap(
([env, account, kdfParams, privateKey, userKey, signingKey, orgKeys, securityState]) => {
// Create our own observable to be able to implement clean-up logic
return new Observable<Rc<BitwardenClient>>((subscriber) => {
const createAndInitializeClient = async () => {
if (env == null || kdfParams == null || privateKey == null || userKey == null) {
return undefined;
}
const settings = this.toSettings(env);
const client = await this.sdkClientFactory.createSdkClient(
new JsTokenProvider(this.apiService, userId),
settings,
);
const settings = this.toSettings(env);
const client = await this.sdkClientFactory.createSdkClient(
new JsTokenProvider(this.apiService, userId),
settings,
);
await this.initializeClient(
userId,
client,
account,
kdfParams,
privateKey,
userKey,
orgKeys,
);
await this.initializeClient(
userId,
client,
account,
kdfParams,
privateKey,
userKey,
signingKey,
securityState,
orgKeys,
);
return client;
};
return client;
};
let client: Rc<BitwardenClient> | undefined;
createAndInitializeClient()
.then((c) => {
client = c === undefined ? undefined : new Rc(c);
let client: Rc<BitwardenClient> | undefined;
createAndInitializeClient()
.then((c) => {
client = c === undefined ? undefined : new Rc(c);
subscriber.next(client);
})
.catch((e) => {
subscriber.error(e);
});
subscriber.next(client);
})
.catch((e) => {
subscriber.error(e);
});
return () => client?.markForDisposal();
});
}),
return () => client?.markForDisposal();
});
},
),
tap({ finalize: () => this.sdkClientCache.delete(userId) }),
shareReplay({ refCount: true, bufferSize: 1 }),
);
@@ -231,6 +244,8 @@ export class DefaultSdkService implements SdkService {
kdfParams: KdfConfig,
privateKey: EncryptedString,
userKey: UserKey,
signingKey: WrappedSigningKey | null,
securityState: SignedSecurityState | null,
orgKeys: Record<OrganizationId, EncString>,
) {
await client.crypto().initialize_user_crypto({
@@ -248,8 +263,8 @@ export class DefaultSdkService implements SdkService {
},
},
privateKey,
signingKey: undefined,
securityState: undefined,
signingKey: signingKey || undefined,
securityState: securityState || undefined,
});
// We initialize the org crypto even if the org_keys are

View File

@@ -11,6 +11,8 @@ import {
UserDecryptionOptions,
UserDecryptionOptionsServiceAbstraction,
} from "@bitwarden/auth/common";
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { KeyService, PBKDF2KdfConfig } from "@bitwarden/key-management";
@@ -72,6 +74,7 @@ describe("DefaultSyncService", () => {
let tokenService: MockProxy<TokenService>;
let authService: MockProxy<AuthService>;
let stateProvider: MockProxy<StateProvider>;
let securityStateService: MockProxy<SecurityStateService>;
let sut: DefaultSyncService;
@@ -101,6 +104,7 @@ describe("DefaultSyncService", () => {
tokenService = mock();
authService = mock();
stateProvider = mock();
securityStateService = mock();
sut = new DefaultSyncService(
masterPasswordAbstraction,
@@ -127,6 +131,7 @@ describe("DefaultSyncService", () => {
tokenService,
authService,
stateProvider,
securityStateService,
);
});
@@ -155,6 +160,142 @@ describe("DefaultSyncService", () => {
stateProvider.getUser.mockReturnValue(mock());
});
it("sets the correct keys for a V1 user with old response model", async () => {
const v1Profile = {
id: user1,
key: "encryptedUserKey",
privateKey: "privateKey",
providers: [] as any[],
organizations: [] as any[],
providerOrganizations: [] as any[],
avatarColor: "#fff",
securityStamp: "stamp",
emailVerified: true,
verifyDevices: false,
premiumPersonally: false,
premiumFromOrganization: false,
usesKeyConnector: false,
};
apiService.getSync.mockResolvedValue(
new SyncResponse({
profile: v1Profile,
folders: [],
collections: [],
ciphers: [],
sends: [],
domains: [],
policies: [],
}),
);
await sut.fullSync(true);
expect(masterPasswordAbstraction.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
new EncString("encryptedUserKey"),
user1,
);
expect(keyService.setPrivateKey).toHaveBeenCalledWith("privateKey", user1);
expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1);
expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1);
});
it("sets the correct keys for a V1 user", async () => {
const v1Profile = {
id: user1,
key: "encryptedUserKey",
privateKey: "privateKey",
providers: [] as any[],
organizations: [] as any[],
providerOrganizations: [] as any[],
avatarColor: "#fff",
securityStamp: "stamp",
emailVerified: true,
verifyDevices: false,
premiumPersonally: false,
premiumFromOrganization: false,
usesKeyConnector: false,
accountKeys: {
publicKeyEncryptionKeyPair: {
wrappedPrivateKey: "wrappedPrivateKey",
publicKey: "publicKey",
},
},
};
apiService.getSync.mockResolvedValue(
new SyncResponse({
profile: v1Profile,
folders: [],
collections: [],
ciphers: [],
sends: [],
domains: [],
policies: [],
}),
);
await sut.fullSync(true);
expect(masterPasswordAbstraction.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
new EncString("encryptedUserKey"),
user1,
);
expect(keyService.setPrivateKey).toHaveBeenCalledWith("wrappedPrivateKey", user1);
expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1);
expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1);
});
it("sets the correct keys for a V2 user", async () => {
const v2Profile = {
id: user1,
key: "encryptedUserKey",
providers: [] as unknown[],
organizations: [] as unknown[],
providerOrganizations: [] as unknown[],
avatarColor: "#fff",
securityStamp: "stamp",
emailVerified: true,
verifyDevices: false,
premiumPersonally: false,
premiumFromOrganization: false,
usesKeyConnector: false,
privateKey: "wrappedPrivateKey",
accountKeys: {
publicKeyEncryptionKeyPair: {
wrappedPrivateKey: "wrappedPrivateKey",
publicKey: "publicKey",
signedPublicKey: "signedPublicKey",
},
signatureKeyPair: {
wrappedSigningKey: "wrappedSigningKey",
verifyingKey: "verifyingKey",
},
securityState: {
securityState: "securityState",
},
},
};
apiService.getSync.mockResolvedValue(
new SyncResponse({
profile: v2Profile,
folders: [],
collections: [],
ciphers: [],
sends: [],
domains: [],
policies: [],
}),
);
await sut.fullSync(true);
expect(masterPasswordAbstraction.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
new EncString("encryptedUserKey"),
user1,
);
expect(keyService.setPrivateKey).toHaveBeenCalledWith("wrappedPrivateKey", user1);
expect(keyService.setUserSigningKey).toHaveBeenCalledWith("wrappedSigningKey", user1);
expect(securityStateService.setAccountSecurityState).toHaveBeenCalledWith(
"securityState",
user1,
);
expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1);
expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1);
});
it("does a token refresh when option missing from options", async () => {
await sut.fullSync(true, { allowThrowOnError: false });

View File

@@ -10,6 +10,7 @@ import {
CollectionService,
} from "@bitwarden/admin-console/common";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service";
// eslint-disable-next-line no-restricted-imports
import { KeyService } from "@bitwarden/key-management";
@@ -98,6 +99,7 @@ export class DefaultSyncService extends CoreSyncService {
tokenService: TokenService,
authService: AuthService,
stateProvider: StateProvider,
private securityStateService: SecurityStateService,
) {
super(
tokenService,
@@ -233,13 +235,34 @@ export class DefaultSyncService extends CoreSyncService {
if (response?.key) {
await this.masterPasswordService.setMasterKeyEncryptedUserKey(response.key, response.id);
}
await this.keyService.setPrivateKey(response.privateKey, response.id);
// Cleanup: Only the first branch should be kept after the server always returns accountKeys https://bitwarden.atlassian.net/browse/PM-21768
if (response.accountKeys != null) {
await this.keyService.setPrivateKey(
response.accountKeys.publicKeyEncryptionKeyPair.wrappedPrivateKey,
response.id,
);
if (response.accountKeys.signatureKeyPair !== null) {
// User is V2 user
await this.keyService.setUserSigningKey(
response.accountKeys.signatureKeyPair.wrappedSigningKey,
response.id,
);
await this.securityStateService.setAccountSecurityState(
response.accountKeys.securityState.securityState,
response.id,
);
}
} else {
await this.keyService.setPrivateKey(response.privateKey, response.id);
}
await this.keyService.setProviderKeys(response.providers, response.id);
await this.keyService.setOrgKeys(
response.organizations,
response.providerOrganizations,
response.id,
);
await this.avatarService.setSyncAvatarColor(response.id, response.avatarColor);
await this.tokenService.setSecurityStamp(response.securityStamp, response.id);
await this.accountService.setAccountEmailVerified(response.id, response.emailVerified);