From e4ed4a38583be41ccecd8a24abad79ccb6926056 Mon Sep 17 00:00:00 2001
From: Todd Martin <106564991+trmartin4@users.noreply.github.com>
Date: Tue, 6 Aug 2024 18:23:38 -0400
Subject: [PATCH] Add consistent and contextual logging around decryption
 failure (#10404)

* Added more context to logging messages around decryption failure

* Added missing period.
---
 apps/browser/src/background/main.background.ts        |  3 +++
 .../cryptography/encrypt.service.implementation.ts    | 11 +++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts
index f5fd5185bf..d514c417ef 100644
--- a/apps/browser/src/background/main.background.ts
+++ b/apps/browser/src/background/main.background.ts
@@ -452,6 +452,9 @@ export default class MainBackground {
         return new ForegroundMemoryStorageService();
       }
 
+      // For local backed session storage, we expect that the encrypted data on disk will persist longer than the encryption key in memory
+      // and failures to decrypt because of that are completely expected. For this reason, we pass in `false` to the `EncryptServiceImplementation`
+      // so that MAC failures are not logged.
       return new LocalBackedSessionStorageService(
         sessionKey,
         this.storageService,
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 228f0c5417..681972e7e4 100644
--- a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts
+++ b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts
@@ -71,12 +71,12 @@ export class EncryptServiceImplementation implements EncryptService {
     key = this.resolveLegacyKey(key, encString);
 
     if (key.macKey != null && encString?.mac == null) {
-      this.logService.error("mac required.");
+      this.logService.error("MAC required but not provided.");
       return null;
     }
 
     if (key.encType !== encString.encryptionType) {
-      this.logService.error("encType unavailable.");
+      this.logService.error("Key encryption type does not match payload encryption type.");
       return null;
     }
 
@@ -94,7 +94,7 @@ export class EncryptServiceImplementation implements EncryptService {
       );
       const macsEqual = await this.cryptoFunctionService.compareFast(fastParams.mac, computedMac);
       if (!macsEqual) {
-        this.logMacFailed("mac failed.");
+        this.logMacFailed("MAC comparison failed. Key or payload has changed.");
         return null;
       }
     }
@@ -114,10 +114,12 @@ export class EncryptServiceImplementation implements EncryptService {
     key = this.resolveLegacyKey(key, encThing);
 
     if (key.macKey != null && encThing.macBytes == null) {
+      this.logService.error("MAC required but not provided.");
       return null;
     }
 
     if (key.encType !== encThing.encryptionType) {
+      this.logService.error("Key encryption type does not match payload encryption type.");
       return null;
     }
 
@@ -127,12 +129,13 @@ export class EncryptServiceImplementation implements EncryptService {
       macData.set(new Uint8Array(encThing.dataBytes), encThing.ivBytes.byteLength);
       const computedMac = await this.cryptoFunctionService.hmac(macData, key.macKey, "sha256");
       if (computedMac === null) {
+        this.logMacFailed("Failed to compute MAC.");
         return null;
       }
 
       const macsMatch = await this.cryptoFunctionService.compare(encThing.macBytes, computedMac);
       if (!macsMatch) {
-        this.logMacFailed("mac failed.");
+        this.logMacFailed("MAC comparison failed. Key or payload has changed.");
         return null;
       }
     }