From 122c3c780999b63c76515c35c4dad17666d145d0 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 25 Oct 2024 15:22:30 +0200 Subject: [PATCH] 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 --- ...ocal-backed-session-storage.service.spec.ts | 16 ++++++++++++---- .../local-backed-session-storage.service.ts | 6 +++++- apps/cli/src/commands/get.command.ts | 1 + .../platform/abstractions/encrypt.service.ts | 6 +++++- .../platform/models/domain/enc-string.spec.ts | 16 ++++++++++++---- .../src/platform/models/domain/enc-string.ts | 15 +++++++++++++-- .../encrypt.service.implementation.ts | 18 +++++++++++++----- 7 files changed, 61 insertions(+), 17 deletions(-) diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts index 8d43c8f2fe..949ecebde8 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.spec.ts @@ -48,8 +48,12 @@ describe("LocalBackedSessionStorage", () => { localStorage.internalStore["session_test"] = encrypted.encryptedString; encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); const result = await sut.get("test"); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encrypted, sessionKey); - expect(result).toEqual("decrypted"); + expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( + encrypted, + sessionKey, + "browser-session-key", + ), + expect(result).toEqual("decrypted"); }); 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; encryptService.decryptToUtf8.mockResolvedValue(JSON.stringify("decrypted")); const result = await sut.get("test"); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encrypted, sessionKey); - expect(result).toEqual("decrypted"); + expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( + encrypted, + sessionKey, + "browser-session-key", + ), + expect(result).toEqual("decrypted"); }); it("caches the decrypted value when one is stored in local storage", async () => { diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index 28abb78b19..e4256d9c0c 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -117,7 +117,11 @@ export class LocalBackedSessionStorageService 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) { // error with decryption, value is lost, delete state and start over await this.localStorage.remove(this.sessionStorageKey(key)); diff --git a/apps/cli/src/commands/get.command.ts b/apps/cli/src/commands/get.command.ts index 0bf1487b2e..fc014534e0 100644 --- a/apps/cli/src/commands/get.command.ts +++ b/apps/cli/src/commands/get.command.ts @@ -455,6 +455,7 @@ export class GetCommand extends DownloadCommand { decCollection.name = await this.encryptService.decryptToUtf8( new EncString(response.name), orgKey, + `orgkey-${options.organizationId}`, ); const groups = response.groups == null diff --git a/libs/common/src/platform/abstractions/encrypt.service.ts b/libs/common/src/platform/abstractions/encrypt.service.ts index c70042e418..5b28b98803 100644 --- a/libs/common/src/platform/abstractions/encrypt.service.ts +++ b/libs/common/src/platform/abstractions/encrypt.service.ts @@ -8,7 +8,11 @@ import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; export abstract class EncryptService { abstract encrypt(plainValue: string | Uint8Array, key: SymmetricCryptoKey): Promise; abstract encryptToBytes(plainValue: Uint8Array, key: SymmetricCryptoKey): Promise; - abstract decryptToUtf8(encString: EncString, key: SymmetricCryptoKey): Promise; + abstract decryptToUtf8( + encString: EncString, + key: SymmetricCryptoKey, + decryptContext?: string, + ): Promise; abstract decryptToBytes(encThing: Encrypted, key: SymmetricCryptoKey): Promise; abstract rsaEncrypt(data: Uint8Array, publicKey: Uint8Array): Promise; abstract rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise; diff --git a/libs/common/src/platform/models/domain/enc-string.spec.ts b/libs/common/src/platform/models/domain/enc-string.spec.ts index 462a977ff8..85108a9609 100644 --- a/libs/common/src/platform/models/domain/enc-string.spec.ts +++ b/libs/common/src/platform/models/domain/enc-string.spec.ts @@ -149,7 +149,7 @@ describe("EncString", () => { const key = new SymmetricCryptoKey(makeStaticByteArray(32)); 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 () => { @@ -351,7 +351,7 @@ describe("EncString", () => { await encString.decrypt(null, key); 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 () => { @@ -362,7 +362,11 @@ describe("EncString", () => { await encString.decrypt("orgId", null); 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 () => { @@ -373,7 +377,11 @@ describe("EncString", () => { await encString.decrypt(null, null); expect(keyService.getUserKeyWithLegacySupport).toHaveBeenCalledWith(); - expect(encryptService.decryptToUtf8).toHaveBeenCalledWith(encString, userKey); + expect(encryptService.decryptToUtf8).toHaveBeenCalledWith( + encString, + userKey, + "domain-withlegacysupport-masterkey", + ); }); }); diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index 40f36306bf..6f01f46439 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -159,16 +159,27 @@ export class EncString implements Encrypted { return this.decryptedValue; } + let keyContext = "provided-key"; try { if (key == null) { 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) { throw new Error("No key to decrypt EncString with orgId " + orgId); } const encryptService = Utils.getContainerService().getEncryptService(); - this.decryptedValue = await encryptService.decryptToUtf8(this, key); + this.decryptedValue = await encryptService.decryptToUtf8(this, key, keyContext); } catch (e) { this.decryptedValue = DECRYPT_ERROR; } @@ -181,7 +192,7 @@ export class EncString implements Encrypted { 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) { this.decryptedValue = DECRYPT_ERROR; } diff --git a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts index 8c42c724b2..137d67ca0f 100644 --- a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts +++ b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts @@ -63,7 +63,11 @@ export class EncryptServiceImplementation implements EncryptService { return new EncArrayBuffer(encBytes); } - async decryptToUtf8(encString: EncString, key: SymmetricCryptoKey): Promise { + async decryptToUtf8( + encString: EncString, + key: SymmetricCryptoKey, + decryptContext: string = "no context", + ): Promise { if (key == null) { throw new Error("No key provided for decryption."); } @@ -75,8 +79,9 @@ export class EncryptServiceImplementation implements EncryptService { this.logService.error( "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + encryptionTypeName(key.encType) + - " Payload type " + + "Payload type " + encryptionTypeName(encString.encryptionType), + "Decrypt context: " + decryptContext, ); return null; } @@ -85,8 +90,9 @@ export class EncryptServiceImplementation implements EncryptService { this.logService.error( "[Encrypt service] Key encryption type does not match payload encryption type. Key type " + encryptionTypeName(key.encType) + - " Payload type " + + "Payload type " + encryptionTypeName(encString.encryptionType), + "Decrypt context: " + decryptContext, ); return null; } @@ -108,8 +114,10 @@ export class EncryptServiceImplementation implements EncryptService { this.logMacFailed( "[Encrypt service] MAC comparison failed. Key or payload has changed. Key type " + encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encString.encryptionType), + "Payload type " + + encryptionTypeName(encString.encryptionType) + + " Decrypt context: " + + decryptContext, ); return null; }