fix: properly decrypt legacy state blobs (#2472)

FrederikBolding · web-flow · commit e71713915ee8 · 2024-06-10T12:15:29.000+02:00
Fixes a problem where encrypted state blobs produced before `keyMetadata` was added would fail to decrypt. The encryptor would attempt to use the latest key derivation parameters when no `keyMetadata` was passed in, causing a failure to decrypt: https://github.com/MetaMask/metamask-extension/blob/ed5ec2d28f7e118803c6fdc74abe90db4e00f480/app/scripts/lib/encryptor-factory.ts#L78-L82 This PR simply falls back to the legacy key derivation parameters whenever `keyMetadata` isn't present strictly for decryption purposes. The default key derivation parameters are still in use for all encryption.
diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json
@@ -1,5 +1,5 @@
 {
-  "branches": 91.64,
+  "branches": 91.68,
   "functions": 96.77,
   "lines": 97.89,
   "statements": 97.57
diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx
@@ -95,6 +95,7 @@ import {
   sleep,
 } from '../test-utils';
 import { delay } from '../utils';
+import { LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS } from './constants';
 import { SnapsRegistryStatus } from './registry';
 import type { SnapControllerState } from './SnapController';
 import {
@@ -8178,6 +8179,42 @@ describe('SnapController', () => {
       snapController.destroy();
     });
 
+    it('uses legacy decryption where needed', async () => {
+      const messenger = getSnapControllerMessenger();
+
+      const state = { foo: 'bar' };
+
+      const { data, iv, salt } = JSON.parse(
+        await encrypt(
+          ENCRYPTION_KEY,
+          state,
+          undefined,
+          undefined,
+          LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS,
+        ),
+      );
+
+      const snapController = getSnapController(
+        getSnapControllerOptions({
+          messenger,
+          state: {
+            snaps: {
+              [MOCK_SNAP_ID]: getPersistedSnapObject(),
+            },
+            snapStates: {
+              [MOCK_SNAP_ID]: JSON.stringify({ data, iv, salt }),
+            },
+          },
+        }),
+      );
+
+      expect(
+        await messenger.call('SnapController:getSnapState', MOCK_SNAP_ID, true),
+      ).toStrictEqual(state);
+
+      snapController.destroy();
+    });
+
     it('throws an error if the state is corrupt', async () => {
       const messenger = getSnapControllerMessenger();
 
diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts
@@ -128,7 +128,10 @@ import {
   setDiff,
   withTimeout,
 } from '../utils';
-import { ALLOWED_PERMISSIONS } from './constants';
+import {
+  ALLOWED_PERMISSIONS,
+  LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS,
+} from './constants';
 import type { SnapLocation } from './location';
 import { detectSnapLocation } from './location';
 import type {
@@ -1676,7 +1679,9 @@ export class SnapController extends BaseController<
         snapId,
         salt,
         useCache,
-        keyMetadata,
+        // When decrypting state we expect key metadata to be present.
+        // If it isn't present, we assume that the Snap state we are decrypting is old enough to use the legacy encryption params.
+        keyMetadata: keyMetadata ?? LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS,
       });
       const decryptedState = await this.#encryptor.decryptWithKey(key, parsed);
 
diff --git a/packages/snaps-controllers/src/snaps/constants.ts b/packages/snaps-controllers/src/snaps/constants.ts
@@ -13,3 +13,10 @@ export const ALLOWED_PERMISSIONS = Object.freeze([
   SnapEndowments.TransactionInsight,
   SnapEndowments.SignatureInsight,
 ]);
+
+export const LEGACY_ENCRYPTION_KEY_DERIVATION_OPTIONS = {
+  algorithm: 'PBKDF2' as const,
+  params: {
+    iterations: 10_000,
+  },
+};

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`{`
`2`		`- "branches": 91.64,`
	`2`	`+ "branches": 91.68,`
`3`	`3`	`"functions": 96.77,`
`4`	`4`	`"lines": 97.89,`
`5`	`5`	`"statements": 97.57`