From 29be72b3f51c6726579f3c88ae87af3e5f745272 Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Wed, 14 Dec 2022 14:19:17 +0100 Subject: [PATCH] Move encryption logic to rpc-methods --- packages/rpc-methods/jest.config.js | 6 +- packages/rpc-methods/package.json | 1 + .../src/restricted/getBip32Entropy.test.ts | 4 +- .../src/restricted/manageState.test.ts | 173 ++++++++++++++++-- .../rpc-methods/src/restricted/manageState.ts | 154 ++++++++++++++-- packages/snaps-controllers/jest.config.js | 8 +- packages/snaps-controllers/package.json | 1 - .../src/snaps/SnapController.test.ts | 127 +------------ .../src/snaps/SnapController.ts | 51 +----- packages/snaps-utils/src/test-utils/common.ts | 3 + packages/snaps-utils/src/test-utils/index.ts | 1 + yarn.lock | 2 +- 12 files changed, 326 insertions(+), 205 deletions(-) diff --git a/packages/rpc-methods/jest.config.js b/packages/rpc-methods/jest.config.js index 0bee990553..018cbc7569 100644 --- a/packages/rpc-methods/jest.config.js +++ b/packages/rpc-methods/jest.config.js @@ -11,9 +11,9 @@ module.exports = deepmerge(baseConfig, { coverageThreshold: { global: { branches: 75.18, - functions: 86.56, - lines: 87.27, - statements: 86.78, + functions: 87.14, + lines: 87.96, + statements: 87.57, }, }, testTimeout: 2500, diff --git a/packages/rpc-methods/package.json b/packages/rpc-methods/package.json index 875d906462..abb59558a0 100644 --- a/packages/rpc-methods/package.json +++ b/packages/rpc-methods/package.json @@ -26,6 +26,7 @@ "publish:package": "../../scripts/publish-package.sh" }, "dependencies": { + "@metamask/browser-passworder": "^4.0.2", "@metamask/key-tree": "^6.0.0", "@metamask/permission-controller": "^1.0.1", "@metamask/snaps-utils": "^0.26.2", diff --git a/packages/rpc-methods/src/restricted/getBip32Entropy.test.ts b/packages/rpc-methods/src/restricted/getBip32Entropy.test.ts index e93f5c432c..029c3ff8c0 100644 --- a/packages/rpc-methods/src/restricted/getBip32Entropy.test.ts +++ b/packages/rpc-methods/src/restricted/getBip32Entropy.test.ts @@ -1,4 +1,5 @@ import { SIP_6_MAGIC_VALUE, SnapCaveatType } from '@metamask/snaps-utils'; +import { TEST_SECRET_RECOVERY_PHRASE } from '@metamask/snaps-utils/test-utils'; import { getBip32EntropyBuilder, @@ -8,9 +9,6 @@ import { validateCaveatPaths, } from './getBip32Entropy'; -const TEST_SECRET_RECOVERY_PHRASE = - 'test test test test test test test test test test test ball'; - describe('validateCaveatPaths', () => { it.each([[], null, undefined, 'foo'])( 'throws if the value is not an array or empty', diff --git a/packages/rpc-methods/src/restricted/manageState.test.ts b/packages/rpc-methods/src/restricted/manageState.test.ts index 5100b4af9b..7478d4865f 100644 --- a/packages/rpc-methods/src/restricted/manageState.test.ts +++ b/packages/rpc-methods/src/restricted/manageState.test.ts @@ -1,18 +1,44 @@ +import { encrypt } from '@metamask/browser-passworder'; +import { + deriveEntropy, + STATE_ENCRYPTION_MAGIC_VALUE, +} from '@metamask/snaps-utils'; +import { + MOCK_LOCAL_SNAP_ID, + MOCK_SNAP_ID, + TEST_SECRET_RECOVERY_PHRASE, +} from '@metamask/snaps-utils/test-utils'; +import { ethErrors } from 'eth-rpc-errors'; + import { getManageStateImplementation, getValidatedParams, ManageStateOperation, specificationBuilder, + STATE_ENCRYPTION_SALT, } from './manageState'; +Object.defineProperty(global, 'crypto', { + value: { + /* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ + ...require('crypto').webcrypto, + subtle: require('crypto').webcrypto.subtle, + /* eslint-enable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ + getRandomValues: (input: Uint8Array) => input.fill(0), + }, +}); + describe('snap_manageState', () => { const MOCK_SMALLER_STORAGE_SIZE_LIMIT = 10; // In bytes + describe('specification', () => { it('builds specification', () => { const methodHooks = { clearSnapState: jest.fn(), getSnapState: jest.fn(), updateSnapState: jest.fn(), + getMnemonic: jest.fn(), + getUnlockPromise: jest.fn(), }; expect( @@ -31,28 +57,40 @@ describe('snap_manageState', () => { describe('getManageStateImplementation', () => { it('gets snap state', async () => { + const encryptionKey = await deriveEntropy({ + input: MOCK_SNAP_ID, + salt: STATE_ENCRYPTION_SALT, + mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, + magic: STATE_ENCRYPTION_MAGIC_VALUE, + }); + const mockSnapState = { some: { data: 'for a snap state', }, }; + + const mockEncryptedState = encrypt(encryptionKey, mockSnapState); + const clearSnapState = jest.fn().mockResolvedValueOnce(true); - const getSnapState = jest.fn().mockResolvedValueOnce(mockSnapState); + const getSnapState = jest.fn().mockResolvedValueOnce(mockEncryptedState); const updateSnapState = jest.fn().mockResolvedValueOnce(true); const manageStateImplementation = getManageStateImplementation({ clearSnapState, getSnapState, updateSnapState, + getMnemonic: jest.fn().mockResolvedValue(TEST_SECRET_RECOVERY_PHRASE), + getUnlockPromise: jest.fn(), }); const result = await manageStateImplementation({ - context: { origin: 'snap-origin' }, + context: { origin: MOCK_SNAP_ID }, method: 'snap_manageState', params: { operation: ManageStateOperation.GetState }, }); - expect(getSnapState).toHaveBeenCalledWith('snap-origin'); + expect(getSnapState).toHaveBeenCalledWith(MOCK_SNAP_ID); expect(result).toStrictEqual(mockSnapState); }); @@ -65,18 +103,78 @@ describe('snap_manageState', () => { clearSnapState, getSnapState, updateSnapState, + getMnemonic: jest.fn().mockResolvedValue(TEST_SECRET_RECOVERY_PHRASE), + getUnlockPromise: jest.fn(), }); await manageStateImplementation({ - context: { origin: 'snap-origin' }, + context: { origin: MOCK_SNAP_ID }, method: 'snap_manageState', params: { operation: ManageStateOperation.ClearState }, }); - expect(clearSnapState).toHaveBeenCalledWith('snap-origin'); + expect(clearSnapState).toHaveBeenCalledWith(MOCK_SNAP_ID); }); it('updates snap state', async () => { + const encryptionKey = await deriveEntropy({ + input: MOCK_SNAP_ID, + salt: STATE_ENCRYPTION_SALT, + mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, + magic: STATE_ENCRYPTION_MAGIC_VALUE, + }); + + const mockSnapState = { + some: { + data: 'for a snap state', + }, + }; + + const mockEncryptedState = await encrypt(encryptionKey, mockSnapState); + + const clearSnapState = jest.fn().mockResolvedValueOnce(true); + const getSnapState = jest.fn().mockResolvedValueOnce(true); + const updateSnapState = jest.fn().mockResolvedValueOnce(true); + + const manageStateImplementation = getManageStateImplementation({ + clearSnapState, + getSnapState, + updateSnapState, + getMnemonic: jest.fn().mockResolvedValue(TEST_SECRET_RECOVERY_PHRASE), + getUnlockPromise: jest.fn(), + }); + + await manageStateImplementation({ + context: { origin: MOCK_SNAP_ID }, + method: 'snap_manageState', + params: { + operation: ManageStateOperation.UpdateState, + newState: mockSnapState, + }, + }); + + expect(updateSnapState).toHaveBeenCalledWith( + MOCK_SNAP_ID, + mockEncryptedState, + ); + }); + + it('uses different encryption for different snap IDs', async () => { + const encryptionKey = await deriveEntropy({ + input: MOCK_SNAP_ID, + salt: STATE_ENCRYPTION_SALT, + mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, + magic: STATE_ENCRYPTION_MAGIC_VALUE, + }); + + const mockSnapState = { + some: { + data: 'for a snap state', + }, + }; + + const mockEncryptedState = await encrypt(encryptionKey, mockSnapState); + const clearSnapState = jest.fn().mockResolvedValueOnce(true); const getSnapState = jest.fn().mockResolvedValueOnce(true); const updateSnapState = jest.fn().mockResolvedValueOnce(true); @@ -85,19 +183,66 @@ describe('snap_manageState', () => { clearSnapState, getSnapState, updateSnapState, + getMnemonic: jest.fn().mockResolvedValue(TEST_SECRET_RECOVERY_PHRASE), + getUnlockPromise: jest.fn(), + }); + + await manageStateImplementation({ + context: { origin: MOCK_SNAP_ID }, + method: 'snap_manageState', + params: { + operation: ManageStateOperation.UpdateState, + newState: mockSnapState, + }, }); - const newState = { data: 'updated data' }; await manageStateImplementation({ - context: { origin: 'snap-origin' }, + context: { origin: MOCK_LOCAL_SNAP_ID }, method: 'snap_manageState', params: { operation: ManageStateOperation.UpdateState, - newState, + newState: mockSnapState, }, }); - expect(updateSnapState).toHaveBeenCalledWith('snap-origin', newState); + expect(updateSnapState).toHaveBeenCalledTimes(2); + expect(updateSnapState).toHaveBeenNthCalledWith( + 1, + MOCK_SNAP_ID, + mockEncryptedState, + ); + + expect(updateSnapState).not.toHaveBeenNthCalledWith( + 2, + MOCK_LOCAL_SNAP_ID, + mockEncryptedState, + ); + }); + + it('throws an error if the state is corrupt', async () => { + const clearSnapState = jest.fn().mockResolvedValueOnce(true); + const getSnapState = jest.fn().mockResolvedValueOnce('foo'); + const updateSnapState = jest.fn().mockResolvedValueOnce(true); + + const manageStateImplementation = getManageStateImplementation({ + clearSnapState, + getSnapState, + updateSnapState, + getMnemonic: jest.fn().mockResolvedValue(TEST_SECRET_RECOVERY_PHRASE), + getUnlockPromise: jest.fn(), + }); + + await expect( + manageStateImplementation({ + context: { origin: MOCK_SNAP_ID }, + method: 'snap_manageState', + params: { operation: ManageStateOperation.GetState }, + }), + ).rejects.toThrow( + ethErrors.rpc.internal({ + message: 'Failed to decrypt snap state, the state must be corrupted.', + }), + ); }); it('throws an error on update if the new state is not plain object', async () => { @@ -109,14 +254,17 @@ describe('snap_manageState', () => { clearSnapState, getSnapState, updateSnapState, + getMnemonic: jest.fn().mockResolvedValue(TEST_SECRET_RECOVERY_PHRASE), + getUnlockPromise: jest.fn(), }); + const newState = (a: unknown) => { return a; }; await expect( manageStateImplementation({ - context: { origin: 'snap-origin' }, + context: { origin: MOCK_SNAP_ID }, method: 'snap_manageState', params: { operation: ManageStateOperation.UpdateState, @@ -129,7 +277,7 @@ describe('snap_manageState', () => { 'Invalid snap_manageState "updateState" parameter: The new state must be a plain object.', ); - expect(updateSnapState).not.toHaveBeenCalledWith('snap-origin', newState); + expect(updateSnapState).not.toHaveBeenCalledWith(MOCK_SNAP_ID, newState); }); it('throws an error on update if the new state is not valid json serializable object', async () => { @@ -141,7 +289,10 @@ describe('snap_manageState', () => { clearSnapState, getSnapState, updateSnapState, + getMnemonic: jest.fn().mockResolvedValue(TEST_SECRET_RECOVERY_PHRASE), + getUnlockPromise: jest.fn(), }); + const newState = { something: { something: { diff --git a/packages/rpc-methods/src/restricted/manageState.ts b/packages/rpc-methods/src/restricted/manageState.ts index 0d8140285f..75d6a761ab 100644 --- a/packages/rpc-methods/src/restricted/manageState.ts +++ b/packages/rpc-methods/src/restricted/manageState.ts @@ -1,41 +1,60 @@ +import { decrypt, encrypt } from '@metamask/browser-passworder'; import { PermissionSpecificationBuilder, PermissionType, RestrictedMethodOptions, ValidPermissionSpecification, } from '@metamask/permission-controller'; +import { + deriveEntropy, + STATE_ENCRYPTION_MAGIC_VALUE, +} from '@metamask/snaps-utils'; import { Json, NonEmptyArray, isObject, validateJsonAndGetSize, + assert, + isValidJson, } from '@metamask/utils'; import { ethErrors } from 'eth-rpc-errors'; +// The salt used for SIP-6-based entropy derivation. +export const STATE_ENCRYPTION_SALT = 'snap_manageState encryption'; + const methodName = 'snap_manageState'; export type ManageStateMethodHooks = { + /** + * @returns The mnemonic of the user's primary keyring. + */ + getMnemonic: () => Promise; + + /** + * Waits for the extension to be unlocked. + * + * @returns A promise that resolves once the extension is unlocked. + */ + getUnlockPromise: (shouldShowUnlockRequest: boolean) => Promise; + /** * A function that clears the state of the requesting Snap. */ clearSnapState: (snapId: string) => Promise; /** - * A function that gets the state of the requesting Snap. + * A function that gets the encrypted state of the requesting Snap. * * @returns The current state of the Snap. */ - getSnapState: (snapId: string) => Promise>; + getSnapState: (snapId: string) => Promise; /** * A function that updates the state of the requesting Snap. * * @param newState - The new state of the Snap. */ - updateSnapState: ( - snapId: string, - newState: Record, - ) => Promise; + updateSnapState: (snapId: string, newState: string) => Promise; }; type ManageStateSpecificationBuilderOptions = { @@ -99,17 +118,108 @@ export type ManageStateArgs = { export const STORAGE_SIZE_LIMIT = 104857600; // In bytes (100MB) +type GetEncryptionKeyArgs = { + snapId: string; + mnemonicPhrase: string; +}; + +/** + * Get a deterministic encryption key to use for encrypting and decrypting the + * state. + * + * This key should only be used for state encryption using `snap_manageState`. + * To get other encryption keys, a different salt can be used. + * + * @param args - The encryption key args. + * @param args.snapId - The ID of the snap to get the encryption key for. + * @param args.mnemonicPhrase - The mnemonic phrase to derive the encryption key + * from. + * @returns The state encryption key. + */ +async function getEncryptionKey({ + mnemonicPhrase, + snapId, +}: GetEncryptionKeyArgs) { + return await deriveEntropy({ + mnemonicPhrase, + input: snapId, + salt: STATE_ENCRYPTION_SALT, + magic: STATE_ENCRYPTION_MAGIC_VALUE, + }); +} + +type EncryptStateArgs = GetEncryptionKeyArgs & { + state: Json; +}; + +/** + * Encrypt the state using a deterministic encryption algorithm, based on the + * snap ID and mnemonic phrase. + * + * @param args - The encryption args. + * @param args.state - The state to encrypt. + * @param args.snapId - The ID of the snap to get the encryption key for. + * @param args.mnemonicPhrase - The mnemonic phrase to derive the encryption key + * from. + * @returns The encrypted state. + */ +async function encryptState({ state, ...keyArgs }: EncryptStateArgs) { + const encryptionKey = await getEncryptionKey(keyArgs); + return await encrypt(encryptionKey, state); +} + +type DecryptStateArgs = GetEncryptionKeyArgs & { + state: string; +}; + +/** + * Decrypt the state using a deterministic decryption algorithm, based on the + * snap ID and mnemonic phrase. + * + * @param args - The encryption args. + * @param args.state - The state to decrypt. + * @param args.snapId - The ID of the snap to get the encryption key for. + * @param args.mnemonicPhrase - The mnemonic phrase to derive the encryption key + * from. + * @returns The encrypted state. + */ +async function decryptState({ state, ...keyArgs }: DecryptStateArgs) { + try { + const encryptionKey = await getEncryptionKey(keyArgs); + const decryptedState = await decrypt(encryptionKey, state); + + assert(isValidJson(decryptedState)); + + return decryptedState as Record; + } catch { + throw ethErrors.rpc.internal({ + message: 'Failed to decrypt snap state, the state must be corrupted.', + }); + } +} + /** * Builds the method implementation for `snap_manageState`. * * @param hooks - The RPC method hooks. - * @param hooks.clearSnapState - A function that clears the state stored for a snap. - * @param hooks.getSnapState - A function that fetches the persisted decrypted state for a snap. - * @param hooks.updateSnapState - A function that updates the state stored for a snap. - * @returns The method implementation which either returns `null` for a successful state update/deletion or returns the decrypted state. + * @param hooks.clearSnapState - A function that clears the state stored for a + * snap. + * @param hooks.getSnapState - A function that fetches the persisted decrypted + * state for a snap. + * @param hooks.updateSnapState - A function that updates the state stored for a + * snap. + * @param hooks.getMnemonic - A function to retrieve the Secret Recovery Phrase + * of the user. + * @param hooks.getUnlockPromise - A function that resolves once the MetaMask + * extension is unlocked and prompts the user to unlock their MetaMask if it is + * locked. + * @returns The method implementation which either returns `null` for a + * successful state update/deletion or returns the decrypted state. * @throws If the params are invalid. */ export function getManageStateImplementation({ + getMnemonic, + getUnlockPromise, clearSnapState, getSnapState, updateSnapState, @@ -124,18 +234,36 @@ export function getManageStateImplementation({ } = options; const { operation, newState } = getValidatedParams(params, method); + await getUnlockPromise(true); + const mnemonicPhrase = await getMnemonic(); + switch (operation) { case ManageStateOperation.ClearState: await clearSnapState(origin); return null; - case ManageStateOperation.GetState: - return await getSnapState(origin); + case ManageStateOperation.GetState: { + const state = await getSnapState(origin); + return await decryptState({ + state, + mnemonicPhrase, + snapId: origin, + }); + } case ManageStateOperation.UpdateState: { - await updateSnapState(origin, newState as Record); + assert(newState); + + const encryptedState = await encryptState({ + state: newState, + mnemonicPhrase, + snapId: origin, + }); + + await updateSnapState(origin, encryptedState); return null; } + default: throw ethErrors.rpc.invalidParams( `Invalid ${method} operation: "${operation as string}"`, diff --git a/packages/snaps-controllers/jest.config.js b/packages/snaps-controllers/jest.config.js index 76c2b8185f..44019d3737 100644 --- a/packages/snaps-controllers/jest.config.js +++ b/packages/snaps-controllers/jest.config.js @@ -5,10 +5,10 @@ const baseConfig = require('../../jest.config.base'); module.exports = deepmerge(baseConfig, { coverageThreshold: { global: { - branches: 87.88, - functions: 94.42, - lines: 95.59, - statements: 95.52, + branches: 87.92, + functions: 94.37, + lines: 95.55, + statements: 95.47, }, }, projects: [ diff --git a/packages/snaps-controllers/package.json b/packages/snaps-controllers/package.json index a08e2f2fb8..7665b56531 100644 --- a/packages/snaps-controllers/package.json +++ b/packages/snaps-controllers/package.json @@ -33,7 +33,6 @@ "dependencies": { "@metamask/approval-controller": "^1.0.1", "@metamask/base-controller": "^1.1.1", - "@metamask/browser-passworder": "^4.0.2", "@metamask/object-multiplex": "^1.1.0", "@metamask/permission-controller": "^1.0.1", "@metamask/post-message-stream": "^6.0.0", diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.ts b/packages/snaps-controllers/src/snaps/SnapController.test.ts index 86c78ad32d..e48a8c835b 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.test.ts @@ -1,5 +1,4 @@ import { getPersistentState } from '@metamask/base-controller'; -import { encrypt } from '@metamask/browser-passworder'; import { Caveat, SubjectPermissions, @@ -17,8 +16,6 @@ import { VirtualFile, SnapManifest, NpmSnapFileNames, - deriveEntropy, - STATE_ENCRYPTION_MAGIC_VALUE, } from '@metamask/snaps-utils'; import { DEFAULT_SNAP_BUNDLE, @@ -62,7 +59,6 @@ import { sleep, loopbackDetect, LoopbackLocation, - TEST_SECRET_RECOVERY_PHRASE, } from '../test-utils'; import { delay } from '../utils'; import { handlerEndowments, SnapEndowments } from './endowments'; @@ -105,24 +101,17 @@ describe('SnapController', () => { ); const snap = snapController.getExpect(MOCK_SNAP_ID); - const state = { hello: 'world' }; + const state = 'foo'; await snapController.startSnap(snap.id); await snapController.updateSnapState(snap.id, state); const snapState = await snapController.getSnapState(snap.id); expect(snapState).toStrictEqual(state); - const encryptionKey = await deriveEntropy({ - input: MOCK_SNAP_ID, - salt: 'state encryption', - mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, - magic: STATE_ENCRYPTION_MAGIC_VALUE, - }); - expect( // @ts-expect-error Accessing private property snapController.snapsRuntimeData.get(MOCK_SNAP_ID).state, - ).toStrictEqual(await encrypt(encryptionKey, state)); + ).toStrictEqual(state); snapController.destroy(); await service.terminateAllSnaps(); }); @@ -3712,18 +3701,8 @@ describe('SnapController', () => { it(`gets the snap's state`, async () => { const messenger = getSnapControllerMessenger(); - const state = { - fizz: 'buzz', - }; - - const encryptionKey = await deriveEntropy({ - input: MOCK_SNAP_ID, - salt: 'state encryption', - mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, - magic: STATE_ENCRYPTION_MAGIC_VALUE, - }); + const state = 'foo'; - const encrypted = await encrypt(encryptionKey, state); const snapController = getSnapController( getSnapControllerOptions({ messenger, @@ -3732,7 +3711,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: getPersistedSnapObject(), }, snapStates: { - [MOCK_SNAP_ID]: encrypted, + [MOCK_SNAP_ID]: state, }, }, }), @@ -3747,30 +3726,6 @@ describe('SnapController', () => { expect(getSnapStateSpy).toHaveBeenCalledTimes(1); expect(result).toStrictEqual(state); }); - - it('throws custom error message in case decryption fails', async () => { - const messenger = getSnapControllerMessenger(); - - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snapStates: { [MOCK_SNAP_ID]: 'foo' }, - snaps: getPersistedSnapsState( - getPersistedSnapObject({ status: SnapStatus.Installing }), - ), - }, - }), - ); - - const getSnapStateSpy = jest.spyOn(snapController, 'getSnapState'); - await expect( - messenger.call('SnapController:getSnapState', MOCK_SNAP_ID), - ).rejects.toThrow( - 'Failed to decrypt snap state, the state must be corrupted.', - ); - expect(getSnapStateSpy).toHaveBeenCalledTimes(1); - }); }); describe('SnapController:has', () => { @@ -3818,88 +3773,18 @@ describe('SnapController', () => { ); const updateSnapStateSpy = jest.spyOn(snapController, 'updateSnapState'); - const state = { - bar: 'baz', - }; + const state = 'bar'; await messenger.call( 'SnapController:updateSnapState', MOCK_SNAP_ID, state, ); - const encryptionKey = await deriveEntropy({ - input: MOCK_SNAP_ID, - salt: 'state encryption', - mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, - magic: STATE_ENCRYPTION_MAGIC_VALUE, - }); - expect(updateSnapStateSpy).toHaveBeenCalledTimes(1); expect( // @ts-expect-error Accessing private property snapController.snapsRuntimeData.get(MOCK_SNAP_ID).state, - ).toStrictEqual(await encrypt(encryptionKey, state)); - }); - - it('has different encryption for the same data stored by two different snaps', async () => { - const messenger = getSnapControllerMessenger(); - - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snaps: getPersistedSnapsState( - getPersistedSnapObject(), - getPersistedSnapObject({ - id: MOCK_LOCAL_SNAP_ID, - }), - ), - }, - }), - ); - - const updateSnapStateSpy = jest.spyOn(snapController, 'updateSnapState'); - const state = { - bar: 'baz', - }; - await messenger.call( - 'SnapController:updateSnapState', - MOCK_SNAP_ID, - state, - ); - - await messenger.call( - 'SnapController:updateSnapState', - MOCK_LOCAL_SNAP_ID, - state, - ); - - expect(updateSnapStateSpy).toHaveBeenCalledTimes(2); - const snapState1 = - // @ts-expect-error Accessing private property - snapController.snapsRuntimeData.get(MOCK_SNAP_ID).state; - - const snapState2 = - // @ts-expect-error Accessing private property - snapController.snapsRuntimeData.get(MOCK_LOCAL_SNAP_ID).state; - - const encryptionKey1 = await deriveEntropy({ - input: MOCK_SNAP_ID, - salt: 'state encryption', - mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, - magic: STATE_ENCRYPTION_MAGIC_VALUE, - }); - - const encryptionKey2 = await deriveEntropy({ - input: MOCK_LOCAL_SNAP_ID, - salt: 'state encryption', - mnemonicPhrase: TEST_SECRET_RECOVERY_PHRASE, - magic: STATE_ENCRYPTION_MAGIC_VALUE, - }); - - expect(snapState1).toStrictEqual(await encrypt(encryptionKey1, state)); - expect(snapState2).toStrictEqual(await encrypt(encryptionKey2, state)); - expect(snapState1).not.toStrictEqual(snapState2); + ).toStrictEqual(state); }); }); diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index e097f8f56e..290aa4a20a 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -3,7 +3,6 @@ import { BaseControllerV2 as BaseController, RestrictedControllerMessenger, } from '@metamask/base-controller'; -import { encrypt, decrypt } from '@metamask/browser-passworder'; import { Caveat, GetEndowments, @@ -57,8 +56,6 @@ import { validateSnapId, validateSnapShasum, VirtualFile, - deriveEntropy, - STATE_ENCRYPTION_MAGIC_VALUE, } from '@metamask/snaps-utils'; import { GetSubjectMetadata, @@ -70,7 +67,6 @@ import { hasProperty, inMilliseconds, isNonEmptyArray, - isValidJson, Json, NonEmptyArray, timeSince, @@ -659,8 +655,6 @@ export class SnapController extends BaseController< #rollbackSnapshots: Map; - #getMnemonicPhrase: GetMnemonicPhrase; - #timeoutForLastRequestStatus?: number; #statusMachine!: StateMachine.Machine< @@ -673,7 +667,6 @@ export class SnapController extends BaseController< closeAllConnections, messenger, state, - getMnemonicPhrase, environmentEndowmentPermissions = [], idleTimeCheckInterval = inMilliseconds(5, Duration.Second), checkBlockList, @@ -742,7 +735,6 @@ export class SnapController extends BaseController< this.#environmentEndowmentPermissions = environmentEndowmentPermissions; this.#featureFlags = featureFlags; this.#fetchFunction = fetchFunction; - this.#getMnemonicPhrase = getMnemonicPhrase; this.#idleTimeCheckInterval = idleTimeCheckInterval; this.#checkSnapBlockList = checkBlockList; this.#maxIdleTime = maxIdleTime; @@ -1322,10 +1314,9 @@ export class SnapController extends BaseController< * @param snapId - The id of the Snap whose state should be updated. * @param newSnapState - The new state of the snap. */ - async updateSnapState(snapId: SnapId, newSnapState: Json): Promise { - const encrypted = await this.#encryptSnapState(snapId, newSnapState); + async updateSnapState(snapId: SnapId, newSnapState: string): Promise { const runtime = this.#getRuntimeExpect(snapId); - runtime.state = encrypted; + runtime.state = newSnapState; } /** @@ -1384,43 +1375,7 @@ export class SnapController extends BaseController< */ async getSnapState(snapId: SnapId): Promise { const { state } = this.#getRuntimeExpect(snapId); - return state ? this.#decryptSnapState(snapId, state) : null; - } - - /** - * Get a deterministic encryption key based on the given snap ID. - * - * @param snapId - The ID of the snap to get the encryption key for. - * @returns The encryption key for the given snap ID. - */ - async #getEncryptionKey(snapId: SnapId): Promise { - const mnemonicPhrase = await this.#getMnemonicPhrase(); - - return deriveEntropy({ - mnemonicPhrase, - input: snapId, - salt: 'state encryption', - magic: STATE_ENCRYPTION_MAGIC_VALUE, - }); - } - - async #encryptSnapState(snapId: SnapId, state: Json): Promise { - const appKey = await this.#getEncryptionKey(snapId); - return encrypt(appKey, state); - } - - async #decryptSnapState(snapId: SnapId, encrypted: string): Promise { - const appKey = await this.#getEncryptionKey(snapId); - try { - const value = await decrypt(appKey, encrypted); - - assert(isValidJson(value)); - return value; - } catch (error) { - throw new Error( - 'Failed to decrypt snap state, the state must be corrupted.', - ); - } + return state ?? null; } /** diff --git a/packages/snaps-utils/src/test-utils/common.ts b/packages/snaps-utils/src/test-utils/common.ts index a15d6c88f5..ecb9838f7c 100644 --- a/packages/snaps-utils/src/test-utils/common.ts +++ b/packages/snaps-utils/src/test-utils/common.ts @@ -1,5 +1,8 @@ import { SemVerVersion } from '../versions'; +export const TEST_SECRET_RECOVERY_PHRASE = + 'test test test test test test test test test test test ball'; + /** * Tens/hundreds legacy tests use creation utils. * diff --git a/packages/snaps-utils/src/test-utils/index.ts b/packages/snaps-utils/src/test-utils/index.ts index e4d005f7cf..a32bd5fc3b 100644 --- a/packages/snaps-utils/src/test-utils/index.ts +++ b/packages/snaps-utils/src/test-utils/index.ts @@ -4,5 +4,6 @@ * only be used in tests, and should not be used in production code. */ +export * from './common'; export * from './manifest'; export * from './snap'; diff --git a/yarn.lock b/yarn.lock index 882b35e9e9..3738a00601 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2678,6 +2678,7 @@ __metadata: dependencies: "@lavamoat/allow-scripts": ^2.0.3 "@metamask/auto-changelog": ^3.1.0 + "@metamask/browser-passworder": ^4.0.2 "@metamask/eslint-config": ^11.0.0 "@metamask/eslint-config-jest": ^11.0.0 "@metamask/eslint-config-nodejs": ^11.0.1 @@ -2830,7 +2831,6 @@ __metadata: "@metamask/approval-controller": ^1.0.1 "@metamask/auto-changelog": ^3.1.0 "@metamask/base-controller": ^1.1.1 - "@metamask/browser-passworder": ^4.0.2 "@metamask/eslint-config": ^11.0.0 "@metamask/eslint-config-jest": ^11.0.0 "@metamask/eslint-config-nodejs": ^11.0.1