1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-22 11:45:59 +01:00

Add context to logs for decryption failures (#11684)

* Add logging to decryption routines

* Fix case of uknown encryption type

* Add decryption context to log where failures occur

* Update log message

* Fix linting

* Add more context logs

* Add more fine grained logging

* Update log message

* Fix tests
This commit is contained in:
Bernd Schoolmann 2024-10-25 15:22:30 +02:00 committed by GitHub
parent adabc59c03
commit 122c3c7809
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 61 additions and 17 deletions

View File

@ -48,8 +48,12 @@ describe("LocalBackedSessionStorage", () => {
localStorage.internalStore["session_test"] = encrypted.encryptedString; localStorage.internalStore["session_test"] = encrypted.encryptedString;
encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted"));
const result = await sut.get("test"); const result = await sut.get("test");
expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encrypted, sessionKey); expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(
expect(result).toEqual("decrypted"); encrypted,
sessionKey,
"browser-session-key",
),
expect(result).toEqual("decrypted");
}); });
it("caches the decrypted value when one is stored in local storage", async () => { it("caches the decrypted value when one is stored in local storage", async () => {
@ -65,8 +69,12 @@ describe("LocalBackedSessionStorage", () => {
localStorage.internalStore["session_test"] = encrypted.encryptedString; localStorage.internalStore["session_test"] = encrypted.encryptedString;
encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted"));
const result = await sut.get("test"); const result = await sut.get("test");
expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encrypted, sessionKey); expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(
expect(result).toEqual("decrypted"); encrypted,
sessionKey,
"browser-session-key",
),
expect(result).toEqual("decrypted");
}); });
it("caches the decrypted value when one is stored in local storage", async () => { it("caches the decrypted value when one is stored in local storage", async () => {

View File

@ -117,7 +117,11 @@ export class LocalBackedSessionStorageService
return null; return null;
} }
const valueJson = await this.encryptService.decryptToUtf8(new EncString(local), encKey); const valueJson = await this.encryptService.decryptToUtf8(
new EncString(local),
encKey,
"browser-session-key",
);
if (valueJson == null) { if (valueJson == null) {
// error with decryption, value is lost, delete state and start over // error with decryption, value is lost, delete state and start over
await this.localStorage.remove(this.sessionStorageKey(key)); await this.localStorage.remove(this.sessionStorageKey(key));

View File

@ -455,6 +455,7 @@ export class GetCommand extends DownloadCommand {
decCollection.name = await this.encryptService.decryptToUtf8( decCollection.name = await this.encryptService.decryptToUtf8(
new EncString(response.name), new EncString(response.name),
orgKey, orgKey,
`orgkey-${options.organizationId}`,
); );
const groups = const groups =
response.groups == null response.groups == null

View File

@ -8,7 +8,11 @@ import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key";
export abstract class EncryptService { export abstract class EncryptService {
abstract encrypt(plainValue: string | Uint8Array, key: SymmetricCryptoKey): Promise<EncString>; abstract encrypt(plainValue: string | Uint8Array, key: SymmetricCryptoKey): Promise<EncString>;
abstract encryptToBytes(plainValue: Uint8Array, key: SymmetricCryptoKey): Promise<EncArrayBuffer>; abstract encryptToBytes(plainValue: Uint8Array, key: SymmetricCryptoKey): Promise<EncArrayBuffer>;
abstract decryptToUtf8(encString: EncString, key: SymmetricCryptoKey): Promise<string>; abstract decryptToUtf8(
encString: EncString,
key: SymmetricCryptoKey,
decryptContext?: string,
): Promise<string>;
abstract decryptToBytes(encThing: Encrypted, key: SymmetricCryptoKey): Promise<Uint8Array>; abstract decryptToBytes(encThing: Encrypted, key: SymmetricCryptoKey): Promise<Uint8Array>;
abstract rsaEncrypt(data: Uint8Array, publicKey: Uint8Array): Promise<EncString>; abstract rsaEncrypt(data: Uint8Array, publicKey: Uint8Array): Promise<EncString>;
abstract rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise<Uint8Array>; abstract rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise<Uint8Array>;

View File

@ -149,7 +149,7 @@ describe("EncString", () => {
const key = new SymmetricCryptoKey(makeStaticByteArray(32)); const key = new SymmetricCryptoKey(makeStaticByteArray(32));
await encString.decryptWithKey(key, encryptService); await encString.decryptWithKey(key, encryptService);
expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, key); expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, key, "domain-withkey");
}); });
it("fails to decrypt when key is null", async () => { it("fails to decrypt when key is null", async () => {
@ -351,7 +351,7 @@ describe("EncString", () => {
await encString.decrypt(null, key); await encString.decrypt(null, key);
expect(keyService.getUserKeyWithLegacySupport).not.toHaveBeenCalled(); expect(keyService.getUserKeyWithLegacySupport).not.toHaveBeenCalled();
expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, key); expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, key, "provided-key");
}); });
it("gets an organization key if required", async () => { it("gets an organization key if required", async () => {
@ -362,7 +362,11 @@ describe("EncString", () => {
await encString.decrypt("orgId", null); await encString.decrypt("orgId", null);
expect(keyService.getOrgKey).toHaveBeenCalledWith("orgId"); expect(keyService.getOrgKey).toHaveBeenCalledWith("orgId");
expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, orgKey); expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(
encString,
orgKey,
"domain-orgkey-orgId",
);
}); });
it("gets the user's decryption key if required", async () => { it("gets the user's decryption key if required", async () => {
@ -373,7 +377,11 @@ describe("EncString", () => {
await encString.decrypt(null, null); await encString.decrypt(null, null);
expect(keyService.getUserKeyWithLegacySupport).toHaveBeenCalledWith(); expect(keyService.getUserKeyWithLegacySupport).toHaveBeenCalledWith();
expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, userKey); expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(
encString,
userKey,
"domain-withlegacysupport-masterkey",
);
}); });
}); });

View File

@ -159,16 +159,27 @@ export class EncString implements Encrypted {
return this.decryptedValue; return this.decryptedValue;
} }
let keyContext = "provided-key";
try { try {
if (key == null) { if (key == null) {
key = await this.getKeyForDecryption(orgId); key = await this.getKeyForDecryption(orgId);
keyContext = orgId == null ? `domain-orgkey-${orgId}` : "domain-userkey|masterkey";
if (orgId != null) {
keyContext = `domain-orgkey-${orgId}`;
} else {
const cryptoService = Utils.getContainerService().getKeyService();
keyContext =
(await cryptoService.getUserKey()) == null
? "domain-withlegacysupport-masterkey"
: "domain-withlegacysupport-userkey";
}
} }
if (key == null) { if (key == null) {
throw new Error("No key to decrypt EncString with orgId " + orgId); throw new Error("No key to decrypt EncString with orgId " + orgId);
} }
const encryptService = Utils.getContainerService().getEncryptService(); const encryptService = Utils.getContainerService().getEncryptService();
this.decryptedValue = await encryptService.decryptToUtf8(this, key); this.decryptedValue = await encryptService.decryptToUtf8(this, key, keyContext);
} catch (e) { } catch (e) {
this.decryptedValue = DECRYPT_ERROR; this.decryptedValue = DECRYPT_ERROR;
} }
@ -181,7 +192,7 @@ export class EncString implements Encrypted {
throw new Error("No key to decrypt EncString"); throw new Error("No key to decrypt EncString");
} }
this.decryptedValue = await encryptService.decryptToUtf8(this, key); this.decryptedValue = await encryptService.decryptToUtf8(this, key, "domain-withkey");
} catch (e) { } catch (e) {
this.decryptedValue = DECRYPT_ERROR; this.decryptedValue = DECRYPT_ERROR;
} }

View File

@ -63,7 +63,11 @@ export class EncryptServiceImplementation implements EncryptService {
return new EncArrayBuffer(encBytes); return new EncArrayBuffer(encBytes);
} }
async decryptToUtf8(encString: EncString, key: SymmetricCryptoKey): Promise<string> { async decryptToUtf8(
encString: EncString,
key: SymmetricCryptoKey,
decryptContext: string = "no context",
): Promise<string> {
if (key == null) { if (key == null) {
throw new Error("No key provided for decryption."); throw new Error("No key provided for decryption.");
} }
@ -75,8 +79,9 @@ export class EncryptServiceImplementation implements EncryptService {
this.logService.error( this.logService.error(
"[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " +
encryptionTypeName(key.encType) + encryptionTypeName(key.encType) +
" Payload type " + "Payload type " +
encryptionTypeName(encString.encryptionType), encryptionTypeName(encString.encryptionType),
"Decrypt context: " + decryptContext,
); );
return null; return null;
} }
@ -85,8 +90,9 @@ export class EncryptServiceImplementation implements EncryptService {
this.logService.error( this.logService.error(
"[Encrypt service] Key encryption type does not match payload encryption type. Key type " + "[Encrypt service] Key encryption type does not match payload encryption type. Key type " +
encryptionTypeName(key.encType) + encryptionTypeName(key.encType) +
" Payload type " + "Payload type " +
encryptionTypeName(encString.encryptionType), encryptionTypeName(encString.encryptionType),
"Decrypt context: " + decryptContext,
); );
return null; return null;
} }
@ -108,8 +114,10 @@ export class EncryptServiceImplementation implements EncryptService {
this.logMacFailed( this.logMacFailed(
"[Encrypt service] MAC comparison failed. Key or payload has changed. Key type " + "[Encrypt service] MAC comparison failed. Key or payload has changed. Key type " +
encryptionTypeName(key.encType) + encryptionTypeName(key.encType) +
" Payload type " + "Payload type " +
encryptionTypeName(encString.encryptionType), encryptionTypeName(encString.encryptionType) +
" Decrypt context: " +
decryptContext,
); );
return null; return null;
} }