1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-25 00:53:22 +00:00

[PM-29208] Remove individual cryptographic-key states & migrate key service (#18164)

* Remove inividual user key states and migrate to account cryptographic state

* Fix browser

* Fix tests

* Clean up migration

* Remove key-pair creation from login strategy

* Add clearing for the account cryptographic state

* Add migration

* Cleanup

* Fix linting
This commit is contained in:
Bernd Schoolmann
2026-02-09 12:39:55 +01:00
committed by jaasen-livefront
parent d718cf6cda
commit 89c9200552
43 changed files with 562 additions and 597 deletions

View File

@@ -19,4 +19,9 @@ export abstract class AccountCryptographicStateService {
accountCryptographicState: WrappedAccountCryptographicState,
userId: UserId,
): Promise<void>;
/**
* Clears the account cryptographic state.
*/
abstract clearAccountCryptographicState(userId: UserId): Promise<void>;
}

View File

@@ -32,4 +32,8 @@ export class DefaultAccountCryptographicStateService implements AccountCryptogra
userId,
);
}
async clearAccountCryptographicState(userId: UserId): Promise<void> {
await this.stateProvider.setUserState(ACCOUNT_CRYPTOGRAPHIC_STATE, null, userId);
}
}

View File

@@ -524,14 +524,6 @@ describe("KeyConnectorService", () => {
},
mockUserId,
);
expect(keyService.setPrivateKey).toHaveBeenCalledWith(mockPrivateKey, mockUserId);
expect(keyService.setUserSigningKey).toHaveBeenCalledWith(mockSigningKey, mockUserId);
expect(securityStateService.setAccountSecurityState).toHaveBeenCalledWith(
mockSecurityState,
mockUserId,
);
expect(keyService.setSignedPublicKey).toHaveBeenCalledWith(mockSignedPublicKey, mockUserId);
expect(await firstValueFrom(conversionState.state$)).toBeNull();
});
@@ -557,10 +549,6 @@ describe("KeyConnectorService", () => {
expect(
accountCryptographicStateService.setAccountCryptographicState,
).not.toHaveBeenCalled();
expect(keyService.setPrivateKey).not.toHaveBeenCalled();
expect(keyService.setUserSigningKey).not.toHaveBeenCalled();
expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled();
expect(keyService.setSignedPublicKey).not.toHaveBeenCalled();
});
it("should throw error when account cryptographic state is not V2", async () => {
@@ -595,10 +583,6 @@ describe("KeyConnectorService", () => {
expect(
accountCryptographicStateService.setAccountCryptographicState,
).not.toHaveBeenCalled();
expect(keyService.setPrivateKey).not.toHaveBeenCalled();
expect(keyService.setUserSigningKey).not.toHaveBeenCalled();
expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled();
expect(keyService.setSignedPublicKey).not.toHaveBeenCalled();
});
it("should throw error when post_keys_for_key_connector_registration fails", async () => {
@@ -625,10 +609,6 @@ describe("KeyConnectorService", () => {
expect(
accountCryptographicStateService.setAccountCryptographicState,
).not.toHaveBeenCalled();
expect(keyService.setPrivateKey).not.toHaveBeenCalled();
expect(keyService.setUserSigningKey).not.toHaveBeenCalled();
expect(securityStateService.setAccountSecurityState).not.toHaveBeenCalled();
expect(keyService.setSignedPublicKey).not.toHaveBeenCalled();
});
});

View File

@@ -38,7 +38,6 @@ import { KeyGenerationService } from "../../crypto";
import { EncString } from "../../crypto/models/enc-string";
import { InternalMasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction";
import { SecurityStateService } from "../../security-state/abstractions/security-state.service";
import { SignedPublicKey, SignedSecurityState, WrappedSigningKey } from "../../types";
import { KeyConnectorService as KeyConnectorServiceAbstraction } from "../abstractions/key-connector.service";
import { KeyConnectorDomainConfirmation } from "../models/key-connector-domain-confirmation";
import { KeyConnectorUserKeyRequest } from "../models/key-connector-user-key.request";
@@ -246,22 +245,6 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
result.account_cryptographic_state,
userId,
);
// Legacy states
await this.keyService.setPrivateKey(result.account_cryptographic_state.V2.private_key, userId);
await this.keyService.setUserSigningKey(
result.account_cryptographic_state.V2.signing_key as WrappedSigningKey,
userId,
);
await this.securityStateService.setAccountSecurityState(
result.account_cryptographic_state.V2.security_state as SignedSecurityState,
userId,
);
if (result.account_cryptographic_state.V2.signed_public_key != null) {
await this.keyService.setSignedPublicKey(
result.account_cryptographic_state.V2.signed_public_key as SignedPublicKey,
userId,
);
}
}
async convertNewSsoUserToKeyConnectorV1(

View File

@@ -11,11 +11,4 @@ export abstract class SecurityStateService {
* must be used. This security state is validated on initialization of the SDK.
*/
abstract accountSecurityState$(userId: UserId): Observable<SignedSecurityState | null>;
/**
* Sets the security state for the provided user.
*/
abstract setAccountSecurityState(
accountSecurityState: SignedSecurityState,
userId: UserId,
): Promise<void>;
}

View File

@@ -1,26 +1,28 @@
import { Observable } from "rxjs";
import { map, Observable } from "rxjs";
import { StateProvider } from "@bitwarden/common/platform/state";
import { UserId } from "@bitwarden/common/types/guid";
import { AccountCryptographicStateService } from "../../account-cryptography/account-cryptographic-state.service";
import { SignedSecurityState } from "../../types";
import { SecurityStateService } from "../abstractions/security-state.service";
import { ACCOUNT_SECURITY_STATE } from "../state/security-state.state";
export class DefaultSecurityStateService implements SecurityStateService {
constructor(protected stateProvider: StateProvider) {}
constructor(private accountCryptographicStateService: AccountCryptographicStateService) {}
// Emits the provided user's security state, or null if there is no security state present for the user.
accountSecurityState$(userId: UserId): Observable<SignedSecurityState | null> {
return this.stateProvider.getUserState$(ACCOUNT_SECURITY_STATE, userId);
}
return this.accountCryptographicStateService.accountCryptographicState$(userId).pipe(
map((cryptographicState) => {
if (cryptographicState == null) {
return null;
}
// Sets the security state for the provided user.
// This is not yet validated, and is only validated upon SDK initialization.
async setAccountSecurityState(
accountSecurityState: SignedSecurityState,
userId: UserId,
): Promise<void> {
await this.stateProvider.setUserState(ACCOUNT_SECURITY_STATE, accountSecurityState, userId);
if ("V2" in cryptographicState) {
return cryptographicState.V2.security_state as SignedSecurityState;
} else {
return null;
}
}),
);
}
}

View File

@@ -1,12 +0,0 @@
import { CRYPTO_DISK, UserKeyDefinition } from "@bitwarden/common/platform/state";
import { SignedSecurityState } from "../../types";
export const ACCOUNT_SECURITY_STATE = new UserKeyDefinition<SignedSecurityState>(
CRYPTO_DISK,
"accountSecurityState",
{
deserializer: (obj) => obj,
clearOn: ["logout"],
},
);

View File

@@ -1,13 +1,4 @@
import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string";
import { EncryptionType } from "../../enums";
import { Utils } from "../../misc/utils";
import { USER_ENCRYPTED_PRIVATE_KEY, USER_EVER_HAD_USER_KEY } from "./user-key.state";
function makeEncString(data?: string) {
data ??= Utils.newGuid();
return new EncString(EncryptionType.AesCbc256_HmacSha256_B64, data, "test", "test");
}
import { USER_EVER_HAD_USER_KEY } from "./user-key.state";
describe("Ever had user key", () => {
const sut = USER_EVER_HAD_USER_KEY;
@@ -20,17 +11,3 @@ describe("Ever had user key", () => {
expect(result).toEqual(everHadUserKey);
});
});
describe("Encrypted private key", () => {
const sut = USER_ENCRYPTED_PRIVATE_KEY;
it("should deserialize encrypted private key", () => {
const encryptedPrivateKey = makeEncString().encryptedString;
const result = sut.deserializer(
JSON.parse(JSON.stringify(encryptedPrivateKey as unknown)) as unknown as EncryptedString,
);
expect(result).toEqual(encryptedPrivateKey);
});
});

View File

@@ -1,5 +1,3 @@
import { EncryptedString } from "../../../key-management/crypto/models/enc-string";
import { SignedPublicKey, 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";
@@ -13,34 +11,7 @@ export const USER_EVER_HAD_USER_KEY = new UserKeyDefinition<boolean>(
},
);
export const USER_ENCRYPTED_PRIVATE_KEY = new UserKeyDefinition<EncryptedString>(
CRYPTO_DISK,
"privateKey",
{
deserializer: (obj) => obj,
clearOn: ["logout"],
},
);
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"],
},
);
export const USER_SIGNED_PUBLIC_KEY = new UserKeyDefinition<SignedPublicKey>(
CRYPTO_DISK,
"userSignedPublicKey",
{
deserializer: (obj) => obj,
clearOn: ["logout"],
},
);

View File

@@ -199,7 +199,10 @@ describe("DefaultSyncService", () => {
new EncString("encryptedUserKey"),
user1,
);
expect(keyService.setPrivateKey).toHaveBeenCalledWith("privateKey", user1);
expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith(
{ V1: { private_key: "privateKey" } },
user1,
);
expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1);
expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1);
});
@@ -242,7 +245,10 @@ describe("DefaultSyncService", () => {
new EncString("encryptedUserKey"),
user1,
);
expect(keyService.setPrivateKey).toHaveBeenCalledWith("wrappedPrivateKey", user1);
expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith(
{ V1: { private_key: "wrappedPrivateKey" } },
user1,
);
expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1);
expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1);
});
@@ -293,12 +299,7 @@ describe("DefaultSyncService", () => {
new EncString("encryptedUserKey"),
user1,
);
expect(keyService.setPrivateKey).toHaveBeenCalledWith("wrappedPrivateKey", user1);
expect(keyService.setUserSigningKey).toHaveBeenCalledWith("wrappedSigningKey", user1);
expect(securityStateService.setAccountSecurityState).toHaveBeenCalledWith(
"securityState",
user1,
);
expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalled();
expect(keyService.setProviderKeys).toHaveBeenCalledWith([], user1);
expect(keyService.setOrgKeys).toHaveBeenCalledWith([], [], user1);
});

View File

@@ -14,6 +14,7 @@ import { SecurityStateService } from "@bitwarden/common/key-management/security-
// 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 } from "@bitwarden/key-management";
import { EncString as SdkEncString } from "@bitwarden/sdk-internal";
// 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
@@ -251,29 +252,15 @@ export class DefaultSyncService extends CoreSyncService {
response.accountKeys.toWrappedAccountCryptographicState(),
response.id,
);
// V1 and V2 users
await this.keyService.setPrivateKey(
response.accountKeys.publicKeyEncryptionKeyPair.wrappedPrivateKey,
} else {
await this.accountCryptographicStateService.setAccountCryptographicState(
{
V1: {
private_key: response.privateKey as SdkEncString,
},
},
response.id,
);
// V2 users only
if (response.accountKeys.isV2Encryption()) {
await this.keyService.setUserSigningKey(
response.accountKeys.signatureKeyPair.wrappedSigningKey,
response.id,
);
await this.securityStateService.setAccountSecurityState(
response.accountKeys.securityState.securityState,
response.id,
);
await this.keyService.setSignedPublicKey(
response.accountKeys.publicKeyEncryptionKeyPair.signedPublicKey,
response.id,
);
}
} else {
await this.keyService.setPrivateKey(response.privateKey, response.id);
}
await this.keyService.setProviderKeys(response.providers, response.id);
await this.keyService.setOrgKeys(