1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-11 05:53:42 +00:00

Merge branch 'km/encstring-remove-decrypt-vault' into km/decrypt-obj

This commit is contained in:
Bernd Schoolmann
2025-10-29 12:51:46 +01:00
13 changed files with 56 additions and 116 deletions

View File

@@ -9,7 +9,7 @@ import { EncryptService } from "../../../key-management/crypto/abstractions/encr
import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string";
import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key";
import { ContainerService } from "../../../platform/services/container.service";
import { OrgKey, UserKey } from "../../../types/key";
import { UserKey } from "../../../types/key";
import { AttachmentData } from "../../models/data/attachment.data";
import { Attachment } from "../../models/domain/attachment";
@@ -110,31 +110,11 @@ describe("Attachment", () => {
it("uses the provided key without depending on KeyService", async () => {
const providedKey = mock<SymmetricCryptoKey>();
await attachment.decrypt(null, "", providedKey);
await attachment.decrypt(providedKey, "");
expect(keyService.getUserKey).not.toHaveBeenCalled();
expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(attachment.key, providedKey);
});
it("gets an organization key if required", async () => {
const orgKey = mock<OrgKey>();
keyService.getOrgKey.calledWith("orgId").mockResolvedValue(orgKey);
await attachment.decrypt("orgId", "", null);
expect(keyService.getOrgKey).toHaveBeenCalledWith("orgId");
expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(attachment.key, orgKey);
});
it("gets the user's decryption key if required", async () => {
const userKey = mock<UserKey>();
keyService.getUserKey.mockResolvedValue(userKey);
await attachment.decrypt(null, "", null);
expect(keyService.getUserKey).toHaveBeenCalled();
expect(encryptService.unwrapSymmetricKey).toHaveBeenCalledWith(attachment.key, userKey);
});
});
});

View File

@@ -1,6 +1,5 @@
import { Jsonify } from "type-fest";
import { OrgKey, UserKey } from "@bitwarden/common/types/key";
import { Attachment as SdkAttachment } from "@bitwarden/sdk-internal";
import { EncString } from "../../../key-management/crypto/models/enc-string";
@@ -34,21 +33,20 @@ export class Attachment extends Domain {
}
async decrypt(
orgId: string | undefined,
decryptionKey: SymmetricCryptoKey,
context = "No Cipher Context",
encKey?: SymmetricCryptoKey,
): Promise<AttachmentView> {
const view = await this.decryptObj<Attachment, AttachmentView>(
this,
new AttachmentView(this),
["fileName"],
orgId ?? null,
encKey,
null,
decryptionKey,
"DomainType: Attachment; " + context,
);
if (this.key != null) {
view.key = await this.decryptAttachmentKey(orgId, encKey);
view.key = await this.decryptAttachmentKey(decryptionKey);
view.encryptedKey = this.key; // Keep the encrypted key for the view
}
@@ -56,27 +54,15 @@ export class Attachment extends Domain {
}
private async decryptAttachmentKey(
orgId: string | undefined,
encKey?: SymmetricCryptoKey,
decryptionKey: SymmetricCryptoKey,
): Promise<SymmetricCryptoKey | undefined> {
try {
if (this.key == null) {
return undefined;
}
if (encKey == null) {
const key = await this.getKeyForDecryption(orgId);
// If we don't have a key, we can't decrypt
if (key == null) {
return undefined;
}
encKey = key;
}
const encryptService = Utils.getContainerService().getEncryptService();
const decValue = await encryptService.unwrapSymmetricKey(this.key, encKey);
const decValue = await encryptService.unwrapSymmetricKey(this.key, decryptionKey);
return decValue;
} catch (e) {
// eslint-disable-next-line no-console
@@ -85,11 +71,6 @@ export class Attachment extends Domain {
}
}
private async getKeyForDecryption(orgId: string | undefined): Promise<OrgKey | UserKey | null> {
const keyService = Utils.getContainerService().getKeyService();
return orgId != null ? await keyService.getOrgKey(orgId) : await keyService.getUserKey();
}
toAttachmentData(): AttachmentData {
const a = new AttachmentData();
if (this.size != null) {

View File

@@ -31,16 +31,12 @@ export class Card extends Domain {
this.code = conditionalEncString(obj.code);
}
async decrypt(
orgId: string | undefined,
context = "No Cipher Context",
encKey?: SymmetricCryptoKey,
): Promise<CardView> {
async decrypt(encKey: SymmetricCryptoKey, context = "No Cipher Context"): Promise<CardView> {
return this.decryptObj<Card, CardView>(
this,
new CardView(),
["cardholderName", "brand", "number", "expMonth", "expYear", "code"],
orgId ?? null,
null,
encKey,
"DomainType: Card; " + context,
);

View File

@@ -1,5 +1,6 @@
import { Jsonify } from "type-fest";
import { assertNonNullish } from "@bitwarden/common/auth/utils";
import { Cipher as SdkCipher } from "@bitwarden/sdk-internal";
import { EncString } from "../../../key-management/crypto/models/enc-string";
@@ -123,16 +124,22 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
// We are passing the organizationId into the EncString.decrypt() method here, but because the encKey will always be
// present and so the organizationId will not be used.
// We will refactor the EncString.decrypt() in https://bitwarden.atlassian.net/browse/PM-3762 to remove the dependency on the organizationId.
async decrypt(encKey: SymmetricCryptoKey): Promise<CipherView> {
async decrypt(userKeyOrOrgKey: SymmetricCryptoKey): Promise<CipherView> {
assertNonNullish(userKeyOrOrgKey, "userKeyOrOrgKey", "Cipher decryption");
const model = new CipherView(this);
let bypassValidation = true;
// By default, the user/organization key is used for decryption
let cipherDecryptionKey = userKeyOrOrgKey;
// If there is a cipher key present, unwrap it and use it for decryption
if (this.key != null) {
const encryptService = Utils.getContainerService().getEncryptService();
try {
const cipherKey = await encryptService.unwrapSymmetricKey(this.key, encKey);
encKey = cipherKey;
const cipherKey = await encryptService.unwrapSymmetricKey(this.key, userKeyOrOrgKey);
cipherDecryptionKey = cipherKey;
bypassValidation = false;
} catch {
model.name = "[error: cannot decrypt]";
@@ -145,18 +152,17 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
this,
model,
["name", "notes"],
this.organizationId ?? null,
encKey,
null,
cipherDecryptionKey,
);
switch (this.type) {
case CipherType.Login:
if (this.login != null) {
model.login = await this.login.decrypt(
this.organizationId,
bypassValidation,
userKeyOrOrgKey,
`Cipher Id: ${this.id}`,
encKey,
);
}
break;
@@ -167,29 +173,17 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
break;
case CipherType.Card:
if (this.card != null) {
model.card = await this.card.decrypt(
this.organizationId,
`Cipher Id: ${this.id}`,
encKey,
);
model.card = await this.card.decrypt(userKeyOrOrgKey, `Cipher Id: ${this.id}`);
}
break;
case CipherType.Identity:
if (this.identity != null) {
model.identity = await this.identity.decrypt(
this.organizationId,
`Cipher Id: ${this.id}`,
encKey,
);
model.identity = await this.identity.decrypt(userKeyOrOrgKey, `Cipher Id: ${this.id}`);
}
break;
case CipherType.SshKey:
if (this.sshKey != null) {
model.sshKey = await this.sshKey.decrypt(
this.organizationId,
`Cipher Id: ${this.id}`,
encKey,
);
model.sshKey = await this.sshKey.decrypt(userKeyOrOrgKey, `Cipher Id: ${this.id}`);
}
break;
default:
@@ -200,9 +194,8 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
const attachments: AttachmentView[] = [];
for (const attachment of this.attachments) {
const decryptedAttachment = await attachment.decrypt(
this.organizationId,
userKeyOrOrgKey,
`Cipher Id: ${this.id}`,
encKey,
);
attachments.push(decryptedAttachment);
}
@@ -212,7 +205,7 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
if (this.fields != null && this.fields.length > 0) {
const fields: FieldView[] = [];
for (const field of this.fields) {
const decryptedField = await field.decrypt(this.organizationId, encKey);
const decryptedField = await field.decrypt(userKeyOrOrgKey);
fields.push(decryptedField);
}
model.fields = fields;
@@ -221,7 +214,7 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
if (this.passwordHistory != null && this.passwordHistory.length > 0) {
const passwordHistory: PasswordHistoryView[] = [];
for (const ph of this.passwordHistory) {
const decryptedPh = await ph.decrypt(this.organizationId, encKey);
const decryptedPh = await ph.decrypt(userKeyOrOrgKey);
passwordHistory.push(decryptedPh);
}
model.passwordHistory = passwordHistory;

View File

@@ -46,10 +46,7 @@ export class Fido2Credential extends Domain {
this.creationDate = new Date(obj.creationDate);
}
async decrypt(
orgId: string | undefined,
encKey?: SymmetricCryptoKey,
): Promise<Fido2CredentialView> {
async decrypt(decryptionKey: SymmetricCryptoKey): Promise<Fido2CredentialView> {
const view = await this.decryptObj<Fido2Credential, Fido2CredentialView>(
this,
new Fido2CredentialView(),
@@ -65,8 +62,8 @@ export class Fido2Credential extends Domain {
"rpName",
"userDisplayName",
],
orgId ?? null,
encKey,
null,
decryptionKey,
);
const { counter } = await this.decryptObj<
@@ -74,7 +71,7 @@ export class Fido2Credential extends Domain {
{
counter: string;
}
>(this, { counter: "" }, ["counter"], orgId ?? null, encKey);
>(this, { counter: "" }, ["counter"], null, decryptionKey);
// Counter will end up as NaN if this fails
view.counter = parseInt(counter);
@@ -82,8 +79,8 @@ export class Fido2Credential extends Domain {
this,
{ discoverable: "" },
["discoverable"],
orgId ?? null,
encKey,
null,
decryptionKey,
);
view.discoverable = discoverable === "true";
view.creationDate = this.creationDate;

View File

@@ -28,12 +28,12 @@ export class Field extends Domain {
this.value = conditionalEncString(obj.value);
}
decrypt(orgId: string | undefined, encKey?: SymmetricCryptoKey): Promise<FieldView> {
decrypt(encKey: SymmetricCryptoKey): Promise<FieldView> {
return this.decryptObj<Field, FieldView>(
this,
new FieldView(this),
["name", "value"],
orgId ?? null,
null,
encKey,
);
}

View File

@@ -56,9 +56,8 @@ export class Identity extends Domain {
}
decrypt(
orgId: string | undefined,
encKey: SymmetricCryptoKey,
context: string = "No Cipher Context",
encKey?: SymmetricCryptoKey,
): Promise<IdentityView> {
return this.decryptObj<Identity, IdentityView>(
this,
@@ -83,7 +82,7 @@ export class Identity extends Domain {
"passportNumber",
"licenseNumber",
],
orgId ?? null,
null,
encKey,
"DomainType: Identity; " + context,
);

View File

@@ -77,7 +77,7 @@ describe("LoginUri", () => {
loginUri.uriChecksum = mockEnc("checksum");
encryptService.hash.mockResolvedValue("checksum");
const actual = await loginUri.validateChecksum("uri", undefined, undefined);
const actual = await loginUri.validateChecksum("uri", undefined);
expect(actual).toBe(true);
expect(encryptService.hash).toHaveBeenCalledWith("uri", "sha256");
@@ -88,7 +88,7 @@ describe("LoginUri", () => {
loginUri.uriChecksum = mockEnc("checksum");
encryptService.hash.mockResolvedValue("incorrect checksum");
const actual = await loginUri.validateChecksum("uri", undefined, undefined);
const actual = await loginUri.validateChecksum("uri", undefined);
expect(actual).toBe(false);
});

View File

@@ -28,21 +28,20 @@ export class LoginUri extends Domain {
}
decrypt(
orgId: string | undefined,
encKey: SymmetricCryptoKey,
context: string = "No Cipher Context",
encKey?: SymmetricCryptoKey,
): Promise<LoginUriView> {
return this.decryptObj<LoginUri, LoginUriView>(
this,
new LoginUriView(this),
["uri"],
orgId ?? null,
null,
encKey,
context,
);
}
async validateChecksum(clearTextUri: string, orgId?: string, encKey?: SymmetricCryptoKey) {
async validateChecksum(clearTextUri: string, encKey: SymmetricCryptoKey) {
if (this.uriChecksum == null) {
return false;
}
@@ -50,7 +49,7 @@ export class LoginUri extends Domain {
const keyService = Utils.getContainerService().getEncryptService();
const localChecksum = await keyService.hash(clearTextUri, "sha256");
const remoteChecksum = await this.uriChecksum.decrypt(orgId ?? null, encKey);
const remoteChecksum = await this.uriChecksum.decrypt(null, encKey);
return remoteChecksum === localChecksum;
}

View File

@@ -99,7 +99,7 @@ describe("Login DTO", () => {
loginUri.validateChecksum.mockResolvedValue(true);
login.uris = [loginUri];
const loginView = await login.decrypt(null, true);
const loginView = await login.decrypt(true, null);
expect(loginView).toEqual(expectedView);
});
@@ -111,7 +111,7 @@ describe("Login DTO", () => {
.mockResolvedValueOnce(true);
login.uris = [loginUri, loginUri, loginUri];
const loginView = await login.decrypt(null, false);
const loginView = await login.decrypt(false, null);
expect(loginView).toEqual(expectedView);
});
});

View File

@@ -44,16 +44,15 @@ export class Login extends Domain {
}
async decrypt(
orgId: string | undefined,
bypassValidation: boolean,
encKey: SymmetricCryptoKey,
context: string = "No Cipher Context",
encKey?: SymmetricCryptoKey,
): Promise<LoginView> {
const view = await this.decryptObj<Login, LoginView>(
this,
new LoginView(this),
["username", "password", "totp"],
orgId ?? null,
null,
encKey,
`DomainType: Login; ${context}`,
);
@@ -66,7 +65,7 @@ export class Login extends Domain {
continue;
}
const uri = await this.uris[i].decrypt(orgId, context, encKey);
const uri = await this.uris[i].decrypt(encKey, context);
const uriString = uri.uri;
if (uriString == null) {
@@ -79,7 +78,7 @@ export class Login extends Domain {
// So we bypass the validation if there's no cipher.key or proceed with the validation and
// Skip the value if it's been tampered with.
const isValidUri =
bypassValidation || (await this.uris[i].validateChecksum(uriString, orgId, encKey));
bypassValidation || (await this.uris[i].validateChecksum(uriString, encKey));
if (isValidUri) {
view.uris.push(uri);
@@ -89,7 +88,7 @@ export class Login extends Domain {
if (this.fido2Credentials != null) {
view.fido2Credentials = await Promise.all(
this.fido2Credentials.map((key) => key.decrypt(orgId, encKey)),
this.fido2Credentials.map((key) => key.decrypt(encKey)),
);
}

View File

@@ -22,12 +22,12 @@ export class Password extends Domain {
this.lastUsedDate = new Date(obj.lastUsedDate);
}
decrypt(orgId: string | undefined, encKey?: SymmetricCryptoKey): Promise<PasswordHistoryView> {
decrypt(encKey: SymmetricCryptoKey): Promise<PasswordHistoryView> {
return this.decryptObj<Password, PasswordHistoryView>(
this,
new PasswordHistoryView(this),
["password"],
orgId ?? null,
null,
encKey,
"DomainType: PasswordHistory",
);

View File

@@ -24,16 +24,12 @@ export class SshKey extends Domain {
this.keyFingerprint = new EncString(obj.keyFingerprint);
}
decrypt(
orgId: string | undefined,
context = "No Cipher Context",
encKey?: SymmetricCryptoKey,
): Promise<SshKeyView> {
decrypt(encKey: SymmetricCryptoKey, context = "No Cipher Context"): Promise<SshKeyView> {
return this.decryptObj<SshKey, SshKeyView>(
this,
new SshKeyView(),
["privateKey", "publicKey", "keyFingerprint"],
orgId ?? null,
null,
encKey,
"DomainType: SshKey; " + context,
);