1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 13:53:34 +00:00

[PM-18243] Improve type safety in decryption (#12885)

* Improve decrypt failure logging

* Rename decryptcontext to decrypttrace

* Improve docs

* PM-16984: Improving type safety of decryption

* Improving type safety of decryption

---------

Co-authored-by: Bernd Schoolmann <mail@quexten.com>
This commit is contained in:
Maciej Zieniuk
2025-03-11 14:06:44 +01:00
committed by GitHub
parent 7e6f2fa798
commit 5cd47ac907
18 changed files with 111 additions and 154 deletions

View File

@@ -39,11 +39,10 @@ export class Collection extends Domain {
} }
decrypt(orgKey: OrgKey): Promise<CollectionView> { decrypt(orgKey: OrgKey): Promise<CollectionView> {
return this.decryptObj( return this.decryptObj<Collection, CollectionView>(
this,
new CollectionView(this), new CollectionView(this),
{ ["name"],
name: null,
},
this.organizationId, this.organizationId,
orgKey, orgKey,
); );

View File

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { ConditionalExcept, ConditionalKeys, Constructor } from "type-fest"; import { ConditionalExcept, ConditionalKeys, Constructor } from "type-fest";
import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service";
@@ -15,6 +13,19 @@ export type DecryptedObject<
TDecryptedKeys extends EncStringKeys<TEncryptedObject>, TDecryptedKeys extends EncStringKeys<TEncryptedObject>,
> = Record<TDecryptedKeys, string> & Omit<TEncryptedObject, TDecryptedKeys>; > = Record<TDecryptedKeys, string> & Omit<TEncryptedObject, TDecryptedKeys>;
// extracts shared keys from the domain and view types
type EncryptableKeys<D extends Domain, V extends View> = (keyof D &
ConditionalKeys<D, EncString | null>) &
(keyof V & ConditionalKeys<V, string | null>);
type DomainEncryptableKeys<D extends Domain> = {
[key in ConditionalKeys<D, EncString | null>]: EncString | null;
};
type ViewEncryptableKeys<V extends View> = {
[key in ConditionalKeys<V, string | null>]: string | null;
};
// https://contributing.bitwarden.com/architecture/clients/data-model#domain // https://contributing.bitwarden.com/architecture/clients/data-model#domain
export default class Domain { export default class Domain {
protected buildDomainModel<D extends Domain>( protected buildDomainModel<D extends Domain>(
@@ -37,6 +48,7 @@ export default class Domain {
} }
} }
} }
protected buildDataModel<D extends Domain>( protected buildDataModel<D extends Domain>(
domain: D, domain: D,
dataObj: any, dataObj: any,
@@ -58,31 +70,24 @@ export default class Domain {
} }
} }
protected async decryptObj<T extends View>( protected async decryptObj<D extends Domain, V extends View>(
viewModel: T, domain: DomainEncryptableKeys<D>,
map: any, viewModel: ViewEncryptableKeys<V>,
orgId: string, props: EncryptableKeys<D, V>[],
key: SymmetricCryptoKey = null, orgId: string | null,
key: SymmetricCryptoKey | null = null,
objectContext: string = "No Domain Context", objectContext: string = "No Domain Context",
): Promise<T> { ): Promise<V> {
const self: any = this; for (const prop of props) {
viewModel[prop] =
for (const prop in map) { (await domain[prop]?.decrypt(
// eslint-disable-next-line
if (!map.hasOwnProperty(prop)) {
continue;
}
const mapProp = map[prop] || prop;
if (self[mapProp]) {
(viewModel as any)[prop] = await self[mapProp].decrypt(
orgId, orgId,
key, key,
`Property: ${prop}; ObjectContext: ${objectContext}`, `Property: ${prop as string}; ObjectContext: ${objectContext}`,
); )) ?? null;
}
} }
return viewModel;
return viewModel as V;
} }
/** /**
@@ -111,7 +116,7 @@ export default class Domain {
const decryptedObjects = []; const decryptedObjects = [];
for (const prop of encryptedProperties) { for (const prop of encryptedProperties) {
const value = (this as any)[prop] as EncString; const value = this[prop] as EncString;
const decrypted = await this.decryptProperty( const decrypted = await this.decryptProperty(
prop, prop,
value, value,
@@ -138,11 +143,9 @@ export default class Domain {
encryptService: EncryptService, encryptService: EncryptService,
decryptTrace: string, decryptTrace: string,
) { ) {
let decrypted: string = null; let decrypted: string | null = null;
if (value) { if (value) {
decrypted = await value.decryptWithKey(key, encryptService, decryptTrace); decrypted = await value.decryptWithKey(key, encryptService, decryptTrace);
} else {
decrypted = null;
} }
return { return {
[propertyKey]: decrypted, [propertyKey]: decrypted,

View File

@@ -160,7 +160,7 @@ export class EncString implements Encrypted {
async decrypt( async decrypt(
orgId: string | null, orgId: string | null,
key: SymmetricCryptoKey = null, key: SymmetricCryptoKey | null = null,
context?: string, context?: string,
): Promise<string> { ): Promise<string> {
if (this.decryptedValue != null) { if (this.decryptedValue != null) {

View File

@@ -54,14 +54,7 @@ export class SendAccess extends Domain {
async decrypt(key: SymmetricCryptoKey): Promise<SendAccessView> { async decrypt(key: SymmetricCryptoKey): Promise<SendAccessView> {
const model = new SendAccessView(this); const model = new SendAccessView(this);
await this.decryptObj( await this.decryptObj<SendAccess, SendAccessView>(this, model, ["name"], null, key);
model,
{
name: null,
},
null,
key,
);
switch (this.type) { switch (this.type) {
case SendType.File: case SendType.File:

View File

@@ -34,15 +34,13 @@ export class SendFile extends Domain {
} }
async decrypt(key: SymmetricCryptoKey): Promise<SendFileView> { async decrypt(key: SymmetricCryptoKey): Promise<SendFileView> {
const view = await this.decryptObj( return await this.decryptObj<SendFile, SendFileView>(
this,
new SendFileView(this), new SendFileView(this),
{ ["fileName"],
fileName: null,
},
null, null,
key, key,
); );
return view;
} }
static fromJSON(obj: Jsonify<SendFile>) { static fromJSON(obj: Jsonify<SendFile>) {

View File

@@ -30,11 +30,10 @@ export class SendText extends Domain {
} }
decrypt(key: SymmetricCryptoKey): Promise<SendTextView> { decrypt(key: SymmetricCryptoKey): Promise<SendTextView> {
return this.decryptObj( return this.decryptObj<SendText, SendTextView>(
this,
new SendTextView(this), new SendTextView(this),
{ ["text"],
text: null,
},
null, null,
key, key,
); );

View File

@@ -87,15 +87,7 @@ export class Send extends Domain {
// TODO: error? // TODO: error?
} }
await this.decryptObj( await this.decryptObj<Send, SendView>(this, model, ["name", "notes"], null, model.cryptoKey);
model,
{
name: null,
notes: null,
},
null,
model.cryptoKey,
);
switch (this.type) { switch (this.type) {
case SendType.File: case SendType.File:

View File

@@ -43,11 +43,10 @@ export class Attachment extends Domain {
context = "No Cipher Context", context = "No Cipher Context",
encKey?: SymmetricCryptoKey, encKey?: SymmetricCryptoKey,
): Promise<AttachmentView> { ): Promise<AttachmentView> {
const view = await this.decryptObj( const view = await this.decryptObj<Attachment, AttachmentView>(
this,
new AttachmentView(this), new AttachmentView(this),
{ ["fileName"],
fileName: null,
},
orgId, orgId,
encKey, encKey,
"DomainType: Attachment; " + context, "DomainType: Attachment; " + context,

View File

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

View File

@@ -154,12 +154,10 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
bypassValidation = false; bypassValidation = false;
} }
await this.decryptObj( await this.decryptObj<Cipher, CipherView>(
this,
model, model,
{ ["name", "notes"],
name: null,
notes: null,
},
this.organizationId, this.organizationId,
encKey, encKey,
); );

View File

@@ -52,41 +52,38 @@ export class Fido2Credential extends Domain {
} }
async decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise<Fido2CredentialView> { async decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise<Fido2CredentialView> {
const view = await this.decryptObj( const view = await this.decryptObj<Fido2Credential, Fido2CredentialView>(
this,
new Fido2CredentialView(), new Fido2CredentialView(),
{ [
credentialId: null, "credentialId",
keyType: null, "keyType",
keyAlgorithm: null, "keyAlgorithm",
keyCurve: null, "keyCurve",
keyValue: null, "keyValue",
rpId: null, "rpId",
userHandle: null, "userHandle",
userName: null, "userName",
rpName: null, "rpName",
userDisplayName: null, "userDisplayName",
discoverable: null, ],
},
orgId, orgId,
encKey, encKey,
); );
const { counter } = await this.decryptObj( const { counter } = await this.decryptObj<
{ counter: "" }, Fido2Credential,
{ {
counter: null, counter: string;
}, }
orgId, >(this, { counter: "" }, ["counter"], orgId, encKey);
encKey,
);
// Counter will end up as NaN if this fails // Counter will end up as NaN if this fails
view.counter = parseInt(counter); view.counter = parseInt(counter);
const { discoverable } = await this.decryptObj( const { discoverable } = await this.decryptObj<Fido2Credential, { discoverable: string }>(
this,
{ discoverable: "" }, { discoverable: "" },
{ ["discoverable"],
discoverable: null,
},
orgId, orgId,
encKey, encKey,
); );

View File

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

View File

@@ -40,13 +40,7 @@ export class Folder extends Domain {
} }
decrypt(): Promise<FolderView> { decrypt(): Promise<FolderView> {
return this.decryptObj( return this.decryptObj<Folder, FolderView>(this, new FolderView(this), ["name"], null);
new FolderView(this),
{
name: null,
},
null,
);
} }
async decryptWithKey( async decryptWithKey(

View File

@@ -66,28 +66,29 @@ export class Identity extends Domain {
context: string = "No Cipher Context", context: string = "No Cipher Context",
encKey?: SymmetricCryptoKey, encKey?: SymmetricCryptoKey,
): Promise<IdentityView> { ): Promise<IdentityView> {
return this.decryptObj( return this.decryptObj<Identity, IdentityView>(
this,
new IdentityView(), new IdentityView(),
{ [
title: null, "title",
firstName: null, "firstName",
middleName: null, "middleName",
lastName: null, "lastName",
address1: null, "address1",
address2: null, "address2",
address3: null, "address3",
city: null, "city",
state: null, "state",
postalCode: null, "postalCode",
country: null, "country",
company: null, "company",
email: null, "email",
phone: null, "phone",
ssn: null, "ssn",
username: null, "username",
passportNumber: null, "passportNumber",
licenseNumber: null, "licenseNumber",
}, ],
orgId, orgId,
encKey, encKey,
"DomainType: Identity; " + context, "DomainType: Identity; " + context,

View File

@@ -38,11 +38,10 @@ export class LoginUri extends Domain {
context: string = "No Cipher Context", context: string = "No Cipher Context",
encKey?: SymmetricCryptoKey, encKey?: SymmetricCryptoKey,
): Promise<LoginUriView> { ): Promise<LoginUriView> {
return this.decryptObj( return this.decryptObj<LoginUri, LoginUriView>(
this,
new LoginUriView(this), new LoginUriView(this),
{ ["uri"],
uri: null,
},
orgId, orgId,
encKey, encKey,
context, context,

View File

@@ -58,13 +58,10 @@ export class Login extends Domain {
context: string = "No Cipher Context", context: string = "No Cipher Context",
encKey?: SymmetricCryptoKey, encKey?: SymmetricCryptoKey,
): Promise<LoginView> { ): Promise<LoginView> {
const view = await this.decryptObj( const view = await this.decryptObj<Login, LoginView>(
this,
new LoginView(this), new LoginView(this),
{ ["username", "password", "totp"],
username: null,
password: null,
totp: null,
},
orgId, orgId,
encKey, encKey,
`DomainType: Login; ${context}`, `DomainType: Login; ${context}`,

View File

@@ -25,11 +25,10 @@ export class Password extends Domain {
} }
decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise<PasswordHistoryView> { decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise<PasswordHistoryView> {
return this.decryptObj( return this.decryptObj<Password, PasswordHistoryView>(
this,
new PasswordHistoryView(this), new PasswordHistoryView(this),
{ ["password"],
password: null,
},
orgId, orgId,
encKey, encKey,
"DomainType: PasswordHistory", "DomainType: PasswordHistory",

View File

@@ -36,13 +36,10 @@ export class SshKey extends Domain {
context = "No Cipher Context", context = "No Cipher Context",
encKey?: SymmetricCryptoKey, encKey?: SymmetricCryptoKey,
): Promise<SshKeyView> { ): Promise<SshKeyView> {
return this.decryptObj( return this.decryptObj<SshKey, SshKeyView>(
this,
new SshKeyView(), new SshKeyView(),
{ ["privateKey", "publicKey", "keyFingerprint"],
privateKey: null,
publicKey: null,
keyFingerprint: null,
},
orgId, orgId,
encKey, encKey,
"DomainType: SshKey; " + context, "DomainType: SshKey; " + context,