diff --git a/jest.config.js b/jest.config.js index 4075bb3a..1ba7c11d 100644 --- a/jest.config.js +++ b/jest.config.js @@ -41,10 +41,10 @@ module.exports = { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 71.42, - functions: 92.85, - lines: 90.68, - statements: 90.9, + branches: 79.41, + functions: 93.22, + lines: 91.5, + statements: 91.69, }, }, preset: 'ts-jest', diff --git a/package.json b/package.json index 0c005476..32203b47 100644 --- a/package.json +++ b/package.json @@ -44,12 +44,12 @@ }, "dependencies": { "@ethereumjs/tx": "^4.2.0", - "@metamask/browser-passworder": "^4.1.0", + "@metamask/browser-passworder": "^4.2.0", "@metamask/eth-hd-keyring": "^7.0.1", "@metamask/eth-sig-util": "^7.0.0", "@metamask/eth-simple-keyring": "^6.0.1", "@metamask/obs-store": "^8.1.0", - "@metamask/utils": "^8.1.0" + "@metamask/utils": "^8.2.0" }, "devDependencies": { "@ethereumjs/wallet": "^2.0.0", diff --git a/src/KeyringController.test.ts b/src/KeyringController.test.ts index b792fe02..5ef07f5a 100644 --- a/src/KeyringController.test.ts +++ b/src/KeyringController.test.ts @@ -9,17 +9,16 @@ import * as sinon from 'sinon'; import { KeyringController, keyringBuilderFactory } from '.'; import { KeyringType, KeyringControllerError } from './constants'; import { - mockEncryptor, + MockEncryptor, KeyringMockWithInit, PASSWORD, MOCK_HARDCODED_KEY, - MOCK_HEX, + MOCK_ENCRYPTION_SALT, } from './test'; import type { KeyringControllerArgs } from './types'; const MOCK_ENCRYPTION_KEY = '{"alg":"A256GCM","ext":true,"k":"wYmxkxOOFBDP6F6VuuYFcRt_Po-tSLFHCWVolsHs4VI","key_ops":["encrypt","decrypt"],"kty":"oct"}'; -const MOCK_ENCRYPTION_SALT = 'HQ5sfhsb8XAQRJtD+UqcImT7Ve4n3YMagrh05YTOsjk='; const MOCK_ENCRYPTION_DATA = `{"data":"2fOOPRKClNrisB+tmqIcETyZvDuL2iIR1Hr1nO7XZHyMqVY1cDBetw2gY5C+cIo1qkpyv3bPp+4buUjp38VBsjbijM0F/FLOqWbcuKM9h9X0uwxsgsZ96uwcIf5I46NiMgoFlhppTTMZT0Nkocz+SnvHM0IgLsFan7JqBU++vSJvx2M1PDljZSunOsqyyL+DKmbYmM4umbouKV42dipUwrCvrQJmpiUZrSkpMJrPJk9ufDQO4CyIVo0qry3aNRdYFJ6rgSyq/k6rXMwGExCMHn8UlhNnAMuMKWPWR/ymK1bzNcNs4VU14iVjEXOZGPvD9cvqVe/VtcnIba6axNEEB4HWDOCdrDh5YNWwMlQVL7vSB2yOhPZByGhnEOloYsj2E5KEb9jFGskt7EKDEYNofr6t83G0c+B72VGYZeCvgtzXzgPwzIbhTtKkP+gdBmt2JNSYrTjLypT0q+v4C9BN1xWTxPmX6TTt0NzkI9pJxgN1VQAfSU9CyWTVpd4CBkgom2cSBsxZ2MNbdKF+qSWz3fQcmJ55hxM0EGJSt9+8eQOTuoJlBapRk4wdZKHR2jdKzPjSF2MAmyVD2kU51IKa/cVsckRFEes+m7dKyHRvlNwgT78W9tBDdZb5PSlfbZXnv8z5q1KtAj2lM2ogJ7brHBdevl4FISdTkObpwcUMcvACOOO0dj6CSYjSKr0ZJ2RLChVruZyPDxEhKGb/8Kv8trLOR3mck/et6d050/NugezycNk4nnzu5iP90gPbSzaqdZI=","iv":"qTGO1afGv3waHN9KoW34Eg==","salt":"${MOCK_ENCRYPTION_SALT}"}`; const walletOneSeedWords = @@ -59,7 +58,7 @@ async function initializeKeyringController({ seedPhrase?: string; } = {}) { const keyringController = new KeyringController({ - encryptor: mockEncryptor, + encryptor: new MockEncryptor(), cacheEncryptionKey: false, keyringBuilders: [keyringBuilderFactory(KeyringMockWithInit)], ...constructorOptions, @@ -84,6 +83,25 @@ describe('KeyringController', () => { sinon.restore(); }); + describe('constructor', () => { + describe('with cacheEncryptionKey = true', () => { + it('should throw error if provided encryptor does not support key export', async () => { + expect( + () => + // @ts-expect-error we want to bypass typechecks here. + new KeyringController({ + cacheEncryptionKey: true, + encryptor: { + decrypt: async (_pass: string, _text: string) => + Promise.resolve('encrypted'), + encrypt: async (_pass: string, _obj: any) => 'decrypted', + }, + }), + ).toThrow(KeyringControllerError.UnsupportedEncryptionKeyExport); + }); + }); + }); + describe('setLocked', () => { it('setLocked correctly sets lock state', async () => { const keyringController = await initializeKeyringController({ @@ -161,39 +179,39 @@ describe('KeyringController', () => { describe('persistAllKeyrings', () => { it('should persist keyrings in _unsupportedKeyrings array', async () => { + const mockEncryptor = new MockEncryptor(); const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + encryptor: mockEncryptor, + }, }); + const encryptSpy = sinon.spy(mockEncryptor, 'encrypt'); const unsupportedKeyring = { type: 'DUMMY_KEYRING', data: {} }; keyringController.unsupportedKeyrings = [unsupportedKeyring]; + await keyringController.persistAllKeyrings(); - const { vault } = keyringController.store.getState(); - assert(vault, 'Vault is not set'); - const keyrings = await mockEncryptor.decrypt(PASSWORD, vault); - expect(keyrings).toContain(unsupportedKeyring); - expect(keyrings).toHaveLength(2); + assert(keyringController.store.getState().vault, 'Vault is not set'); + expect(encryptSpy.calledOnce).toBe(true); + expect(encryptSpy.getCalls()[0]?.args[1]).toHaveLength(2); + expect(encryptSpy.getCalls()[0]?.args[1]).toContain(unsupportedKeyring); }); describe('when `cacheEncryptionKey` is enabled', () => { it('should save an up to date encryption salt to the `memStore` when `password` is unset and `encryptionKey` is set', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); delete keyringController.password; - keyringController.cacheEncryptionKey = true; const vaultEncryptionKey = '🔑'; const vaultEncryptionSalt = '🧂'; const vault = JSON.stringify({ salt: vaultEncryptionSalt }); keyringController.store.updateState({ vault }); - expect( - keyringController.memStore.getState().encryptionKey, - ).toBeUndefined(); - expect( - keyringController.memStore.getState().encryptionSalt, - ).toBeUndefined(); - await keyringController.unlockKeyrings( undefined, vaultEncryptionKey, @@ -221,8 +239,10 @@ describe('KeyringController', () => { it('should save an up to date encryption salt to the `memStore` when `password` is set through `createNewVaultAndKeychain`', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; await keyringController.createNewVaultAndKeychain(PASSWORD); @@ -233,15 +253,17 @@ describe('KeyringController', () => { MOCK_HARDCODED_KEY, ); expect(keyringController.memStore.getState().encryptionSalt).toBe( - MOCK_HEX, + MOCK_ENCRYPTION_SALT, ); }); it('should save an up to date encryption salt to the `memStore` when `password` is set through `submitPassword`', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; await keyringController.submitPassword(PASSWORD); @@ -252,7 +274,7 @@ describe('KeyringController', () => { MOCK_HARDCODED_KEY, ); expect(keyringController.memStore.getState().encryptionSalt).toBe( - MOCK_HEX, + MOCK_ENCRYPTION_SALT, ); }); }); @@ -287,8 +309,13 @@ describe('KeyringController', () => { }); it('should encrypt keyrings with the correct password each time they are persisted', async () => { + const mockEncryptor = new MockEncryptor(); + const encryptSpy = sinon.spy(mockEncryptor, 'encrypt'); const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + encryptor: mockEncryptor, + }, }); keyringController.store.putState({}); assert(!keyringController.store.getState().vault, 'no previous vault'); @@ -297,11 +324,9 @@ describe('KeyringController', () => { const { vault } = keyringController.store.getState(); // eslint-disable-next-line jest/no-restricted-matchers expect(vault).toBeTruthy(); - keyringController.encryptor.encrypt.args.forEach( - ([actualPassword]: string[]) => { - expect(actualPassword).toBe(PASSWORD); - }, - ); + encryptSpy.args.forEach(([actualPassword]) => { + expect(actualPassword).toBe(PASSWORD); + }); }); it('should throw error if accounts are not generated correctly', async () => { @@ -321,18 +346,16 @@ describe('KeyringController', () => { it('should add an `encryptionSalt` to the `memStore` when a new vault is created', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; - const initialMemStore = keyringController.memStore.getState(); await keyringController.createNewVaultAndKeychain(PASSWORD); - const finalMemStore = keyringController.memStore.getState(); - - expect(initialMemStore.encryptionKey).toBeUndefined(); - expect(initialMemStore.encryptionSalt).toBeUndefined(); + const finalMemStore = keyringController.memStore.getState(); expect(finalMemStore.encryptionKey).toBe(MOCK_HARDCODED_KEY); - expect(finalMemStore.encryptionSalt).toBe(MOCK_HEX); + expect(finalMemStore.encryptionSalt).toBe(MOCK_ENCRYPTION_SALT); }); }); }); @@ -429,21 +452,19 @@ describe('KeyringController', () => { it('should add an `encryptionSalt` to the `memStore` when a vault is restored', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; - const initialMemStore = keyringController.memStore.getState(); await keyringController.createNewVaultAndRestore( PASSWORD, walletOneSeedWords, ); - const finalMemStore = keyringController.memStore.getState(); - - expect(initialMemStore.encryptionKey).toBeUndefined(); - expect(initialMemStore.encryptionSalt).toBeUndefined(); + const finalMemStore = keyringController.memStore.getState(); expect(finalMemStore.encryptionKey).toBe(MOCK_HARDCODED_KEY); - expect(finalMemStore.encryptionSalt).toBe(MOCK_HEX); + expect(finalMemStore.encryptionSalt).toBe(MOCK_ENCRYPTION_SALT); }); }); }); @@ -751,18 +772,96 @@ describe('KeyringController', () => { }); it('add serialized keyring to unsupportedKeyrings array if keyring type is not known', async () => { + const mockEncryptor = new MockEncryptor(); const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + encryptor: mockEncryptor, + }, }); const unsupportedKeyrings = [{ type: 'Ledger Keyring', data: 'DUMMY' }]; - mockEncryptor.encrypt(PASSWORD, unsupportedKeyrings); + await mockEncryptor.encrypt(PASSWORD, unsupportedKeyrings); await keyringController.setLocked(); + const keyrings = await keyringController.unlockKeyrings(PASSWORD); + expect(keyrings).toHaveLength(0); expect(keyringController.unsupportedKeyrings).toStrictEqual( unsupportedKeyrings, ); }); + + it('should throw error if there is no vault', async () => { + const keyringController = new KeyringController({ + cacheEncryptionKey: false, + }); + + await expect(async () => + keyringController.unlockKeyrings(PASSWORD), + ).rejects.toThrow(KeyringControllerError.VaultError); + }); + + it('should throw error if decrypted vault is not an array of serialized keyrings', async () => { + const mockEncryptor = new MockEncryptor(); + sinon + .stub(mockEncryptor, 'decrypt') + .resolves('[{"foo": "not a valid keyring}]'); + const keyringController = await initializeKeyringController({ + password: PASSWORD, + constructorOptions: { + encryptor: mockEncryptor, + }, + }); + + await expect(async () => + keyringController.unlockKeyrings(PASSWORD), + ).rejects.toThrow(KeyringControllerError.VaultDataError); + }); + + it('should throw error if decrypted vault includes an invalid keyring', async () => { + const mockEncryptor = new MockEncryptor(); + sinon + .stub(mockEncryptor, 'decrypt') + .resolves([{ foo: 'not a valid keyring' }]); + const keyringController = await initializeKeyringController({ + password: PASSWORD, + constructorOptions: { + encryptor: mockEncryptor, + }, + }); + + await expect(async () => + keyringController.unlockKeyrings(PASSWORD), + ).rejects.toThrow(KeyringControllerError.VaultDataError); + }); + + describe('with old vault format', () => { + [true, false].forEach((cacheEncryptionKey) => { + describe(`with cacheEncryptionKey = ${cacheEncryptionKey}`, () => { + it('should update the vault', async () => { + const mockEncryptor = new MockEncryptor(); + const keyringController = await initializeKeyringController({ + password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + encryptor: mockEncryptor, + }, + }); + const initialVault = keyringController.store.getState().vault; + const updatedVaultMock = + '{"vault": "updated_vault_detail", "salt": "salt"}'; + sinon.stub(mockEncryptor, 'updateVault').resolves(updatedVaultMock); + sinon.stub(mockEncryptor, 'encrypt').resolves(updatedVaultMock); + + await keyringController.unlockKeyrings(PASSWORD); + const updatedVault = keyringController.store.getState().vault; + + expect(initialVault).not.toBe(updatedVault); + expect(updatedVault).toBe(updatedVaultMock); + }); + }); + }); + }); }); describe('verifyPassword', () => { @@ -941,28 +1040,34 @@ describe('KeyringController', () => { it('sets encryption key data upon submitPassword', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; await keyringController.submitPassword(PASSWORD); expect(keyringController.password).toBe(PASSWORD); - expect(keyringController.memStore.getState().encryptionSalt).toBe('SALT'); + expect(keyringController.memStore.getState().encryptionSalt).toBe( + MOCK_ENCRYPTION_SALT, + ); expect(keyringController.memStore.getState().encryptionKey).toStrictEqual( expect.stringMatching('.+'), ); }); it('unlocks the keyrings with valid information', async () => { + const mockEncryptor = new MockEncryptor(); const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + encryptor: mockEncryptor, + }, }); - keyringController.cacheEncryptionKey = true; - const returnValue = await keyringController.encryptor.decryptWithKey(); - const decryptWithKeyStub = sinon.stub( - keyringController.encryptor, - 'decryptWithKey', - ); - decryptWithKeyStub.resolves(Promise.resolve(returnValue)); + const returnValue = await mockEncryptor.decryptWithKey('', ''); + const decryptWithKeyStub = sinon + .stub(mockEncryptor, 'decryptWithKey') + .resolves(returnValue); keyringController.store.updateState({ vault: MOCK_ENCRYPTION_DATA }); @@ -973,15 +1078,17 @@ describe('KeyringController', () => { MOCK_ENCRYPTION_SALT, ); - expect(keyringController.encryptor.decryptWithKey.calledOnce).toBe(true); + expect(decryptWithKeyStub.calledOnce).toBe(true); expect(keyringController.keyrings).toHaveLength(1); }); it('should not load keyrings when invalid encryptionKey format', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; await keyringController.setLocked(); keyringController.store.updateState({ vault: MOCK_ENCRYPTION_DATA }); @@ -997,8 +1104,10 @@ describe('KeyringController', () => { it('should not load keyrings when encryptionKey is expired', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; await keyringController.setLocked(); keyringController.store.updateState({ vault: MOCK_ENCRYPTION_DATA }); @@ -1015,8 +1124,10 @@ describe('KeyringController', () => { it('persists keyrings when actions are performed', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; await keyringController.setLocked(); keyringController.store.updateState({ vault: MOCK_ENCRYPTION_DATA }); await keyringController.submitEncryptionKey( @@ -1064,8 +1175,10 @@ describe('KeyringController', () => { it('cleans up login artifacts upon lock', async () => { const keyringController = await initializeKeyringController({ password: PASSWORD, + constructorOptions: { + cacheEncryptionKey: true, + }, }); - keyringController.cacheEncryptionKey = true; await keyringController.submitPassword(PASSWORD); expect(keyringController.password).toBe(PASSWORD); expect( diff --git a/src/KeyringController.ts b/src/KeyringController.ts index f3476c39..ed48e288 100644 --- a/src/KeyringController.ts +++ b/src/KeyringController.ts @@ -4,7 +4,12 @@ import HDKeyring from '@metamask/eth-hd-keyring'; import { normalize as normalizeToHex } from '@metamask/eth-sig-util'; import SimpleKeyring from '@metamask/eth-simple-keyring'; import { ObservableStore } from '@metamask/obs-store'; -import { remove0x, isValidHexAddress, isObject } from '@metamask/utils'; +import { + remove0x, + isValidHexAddress, + isObject, + isValidJson, +} from '@metamask/utils'; import type { Hex, Json, @@ -22,6 +27,8 @@ import type { KeyringControllerArgs, KeyringControllerState, KeyringControllerPersistentState, + GenericEncryptor, + ExportableKeyEncryptor, } from './types'; const defaultKeyringBuilders = [ @@ -36,16 +43,16 @@ class KeyringController extends EventEmitter { public memStore: ObservableStore; - public encryptor: any; - public keyrings: Keyring[]; - public cacheEncryptionKey: boolean; - public unsupportedKeyrings: SerializedKeyring[]; public password?: string; + #encryptor: GenericEncryptor | ExportableKeyEncryptor; + + #cacheEncryptionKey: boolean; + constructor({ keyringBuilders, cacheEncryptionKey, @@ -65,13 +72,16 @@ class KeyringController extends EventEmitter { keyrings: [], }); - this.encryptor = encryptor; + this.#encryptor = encryptor; this.keyrings = []; this.unsupportedKeyrings = []; // This option allows the controller to cache an exported key // for use in decrypting and encrypting data without password - this.cacheEncryptionKey = Boolean(cacheEncryptionKey); + this.#cacheEncryptionKey = Boolean(cacheEncryptionKey); + if (this.#cacheEncryptionKey) { + assertIsExportableKeyEncryptor(encryptor); + } } /** @@ -235,7 +245,7 @@ class KeyringController extends EventEmitter { if (!encryptedVault) { throw new Error(KeyringControllerError.VaultError); } - await this.encryptor.decrypt(password, encryptedVault); + await this.#encryptor.decrypt(password, encryptedVault); } /** @@ -789,10 +799,12 @@ class KeyringController extends EventEmitter { let vault; let newEncryptionKey; - if (this.cacheEncryptionKey) { + if (this.#cacheEncryptionKey) { + assertIsExportableKeyEncryptor(this.#encryptor); + if (this.password) { const { vault: newVault, exportedKeyString } = - await this.encryptor.encryptWithDetail( + await this.#encryptor.encryptWithDetail( this.password, serializedKeyrings, ); @@ -800,8 +812,8 @@ class KeyringController extends EventEmitter { vault = newVault; newEncryptionKey = exportedKeyString; } else if (encryptionKey) { - const key = await this.encryptor.importKey(encryptionKey); - const vaultJSON = await this.encryptor.encryptWithKey( + const key = await this.#encryptor.importKey(encryptionKey); + const vaultJSON = await this.#encryptor.encryptWithKey( key, serializedKeyrings, ); @@ -812,7 +824,7 @@ class KeyringController extends EventEmitter { if (typeof this.password !== 'string') { throw new TypeError(KeyringControllerError.WrongPasswordType); } - vault = await this.encryptor.encrypt(this.password, serializedKeyrings); + vault = await this.#encryptor.encrypt(this.password, serializedKeyrings); } if (!vault) { @@ -861,9 +873,11 @@ class KeyringController extends EventEmitter { let vault; - if (this.cacheEncryptionKey) { + if (this.#cacheEncryptionKey) { + assertIsExportableKeyEncryptor(this.#encryptor); + if (password) { - const result = await this.encryptor.decryptWithDetail( + const result = await this.#encryptor.decryptWithDetail( password, encryptedVault, ); @@ -885,8 +899,8 @@ class KeyringController extends EventEmitter { throw new TypeError(KeyringControllerError.WrongPasswordType); } - const key = await this.encryptor.importKey(encryptionKey); - vault = await this.encryptor.decryptWithKey(key, parsedEncryptedVault); + const key = await this.#encryptor.importKey(encryptionKey); + vault = await this.#encryptor.decryptWithKey(key, parsedEncryptedVault); // This call is required on the first call because encryptionKey // is not yet inside the memStore @@ -903,12 +917,27 @@ class KeyringController extends EventEmitter { throw new TypeError(KeyringControllerError.WrongPasswordType); } - vault = await this.encryptor.decrypt(password, encryptedVault); + vault = await this.#encryptor.decrypt(password, encryptedVault); this.password = password; } + if (!isSerializedKeyringsArray(vault)) { + throw new Error(KeyringControllerError.VaultDataError); + } + await Promise.all(vault.map(this.#restoreKeyring.bind(this))); await this.updateMemStoreKeyrings(); + + if ( + this.password && + this.#encryptor.updateVault && + (await this.#encryptor.updateVault(encryptedVault, this.password)) !== + encryptedVault + ) { + // Re-encrypt the vault with safer method if one is available + await this.persistAllKeyrings(); + } + return this.keyrings; } @@ -1092,4 +1121,44 @@ async function displayForKeyring( }; } +/** + * Assert that the provided encryptor supports + * encryption and encryption key export. + * + * @param encryptor - The encryptor to check. + * @throws If the encryptor does not support key encryption. + */ +function assertIsExportableKeyEncryptor( + encryptor: GenericEncryptor | ExportableKeyEncryptor, +): asserts encryptor is ExportableKeyEncryptor { + if ( + !( + 'importKey' in encryptor && + typeof encryptor.importKey === 'function' && + 'decryptWithKey' in encryptor && + typeof encryptor.decryptWithKey === 'function' && + 'encryptWithKey' in encryptor && + typeof encryptor.encryptWithKey === 'function' + ) + ) { + throw new Error(KeyringControllerError.UnsupportedEncryptionKeyExport); + } +} + +/** + * Checks if the provided value is a serialized keyrings array. + * + * @param array - The value to check. + * @returns True if the value is a serialized keyrings array. + */ +function isSerializedKeyringsArray( + array: unknown, +): array is SerializedKeyring[] { + return ( + typeof array === 'object' && + Array.isArray(array) && + array.every((value) => value.type && isValidJson(value.data)) + ); +} + export { KeyringController, keyringBuilderFactory }; diff --git a/src/constants.ts b/src/constants.ts index 1a1a3d27..92692a00 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -9,6 +9,8 @@ export enum KeyringControllerError { NoFirstAccount = 'KeyringController - First Account not found.', DuplicatedAccount = 'KeyringController - The account you are trying to import is a duplicate', VaultError = 'KeyringController - Cannot unlock without a previous vault.', + VaultDataError = 'KeyringController - The decrypted vault has an unexpected shape.', + UnsupportedEncryptionKeyExport = 'KeyringController - The encryptor does not support encryption key export.', UnsupportedGenerateRandomMnemonic = 'KeyringController - The current keyring does not support the method generateRandomMnemonic.', UnsupportedExportAccount = '`KeyringController - The keyring for the current address does not support the method exportAccount', UnsupportedRemoveAccount = '`KeyringController - The keyring for the current address does not support the method removeAccount', diff --git a/src/test/encryptor.mock.ts b/src/test/encryptor.mock.ts index b9e19102..0d3a4155 100644 --- a/src/test/encryptor.mock.ts +++ b/src/test/encryptor.mock.ts @@ -1,8 +1,9 @@ import type { Json } from '@metamask/utils'; -import { stub } from 'sinon'; -const PASSWORD = 'password123'; -const MOCK_ENCRYPTION_KEY = JSON.stringify({ +import type { ExportableKeyEncryptor } from '../types'; + +export const PASSWORD = 'password123'; +export const MOCK_ENCRYPTION_KEY = JSON.stringify({ alg: 'A256GCM', ext: true, k: 'wYmxkxOOFBDP6F6VuuYFcRt_Po-tSLFHCWVolsHs4VI', @@ -10,96 +11,77 @@ const MOCK_ENCRYPTION_KEY = JSON.stringify({ key_ops: ['encrypt', 'decrypt'], kty: 'oct', }); -const MOCK_ENCRYPTION_SALT = 'HQ5sfhsb8XAQRJtD+UqcImT7Ve4n3YMagrh05YTOsjk='; -const MOCK_ENCRYPTION_DATA = `{"data":"2fOOPRKClNrisB+tmqIcETyZvDuL2iIR1Hr1nO7XZHyMqVY1cDBetw2gY5C+cIo1qkpyv3bPp+4buUjp38VBsjbijM0F/FLOqWbcuKM9h9X0uwxsgsZ96uwcIf5I46NiMgoFlhppTTMZT0Nkocz+SnvHM0IgLsFan7JqBU++vSJvx2M1PDljZSunOsqyyL+DKmbYmM4umbouKV42dipUwrCvrQJmpiUZrSkpMJrPJk9ufDQO4CyIVo0qry3aNRdYFJ6rgSyq/k6rXMwGExCMHn8UlhNnAMuMKWPWR/ymK1bzNcNs4VU14iVjEXOZGPvD9cvqVe/VtcnIba6axNEEB4HWDOCdrDh5YNWwMlQVL7vSB2yOhPZByGhnEOloYsj2E5KEb9jFGskt7EKDEYNofr6t83G0c+B72VGYZeCvgtzXzgPwzIbhTtKkP+gdBmt2JNSYrTjLypT0q+v4C9BN1xWTxPmX6TTt0NzkI9pJxgN1VQAfSU9CyWTVpd4CBkgom2cSBsxZ2MNbdKF+qSWz3fQcmJ55hxM0EGJSt9+8eQOTuoJlBapRk4wdZKHR2jdKzPjSF2MAmyVD2kU51IKa/cVsckRFEes+m7dKyHRvlNwgT78W9tBDdZb5PSlfbZXnv8z5q1KtAj2lM2ogJ7brHBdevl4FISdTkObpwcUMcvACOOO0dj6CSYjSKr0ZJ2RLChVruZyPDxEhKGb/8Kv8trLOR3mck/et6d050/NugezycNk4nnzu5iP90gPbSzaqdZI=","iv":"qTGO1afGv3waHN9KoW34Eg==","salt":"${MOCK_ENCRYPTION_SALT}"}`; - -const INVALID_PASSWORD_ERROR = 'Incorrect password.'; - -const MOCK_HARDCODED_KEY = 'key'; -const MOCK_HEX = '0xabcdef0123456789'; -const MOCK_SALT = 'SALT'; +export const MOCK_ENCRYPTION_SALT = + 'HQ5sfhsb8XAQRJtD+UqcImT7Ve4n3YMagrh05YTOsjk='; +export const MOCK_HARDCODED_KEY = 'key'; +export const MOCK_HEX = '0xabcdef0123456789'; // eslint-disable-next-line no-restricted-globals const MOCK_KEY = Buffer.alloc(32); -let cacheVal: Json; - -const mockEncryptor = { - encrypt: stub().callsFake(async (_password, dataObj) => { - cacheVal = dataObj; - - return Promise.resolve(MOCK_HEX); - }), +const INVALID_PASSWORD_ERROR = 'Incorrect password.'; - encryptWithDetail: stub().callsFake(async (_password, dataObj) => { - cacheVal = dataObj; +let cacheVal: Json; - return Promise.resolve({ - vault: JSON.stringify({ salt: MOCK_HEX }), - exportedKeyString: MOCK_HARDCODED_KEY, +export class MockEncryptor implements ExportableKeyEncryptor { + async encrypt(password: string, dataObj: any) { + return JSON.stringify({ + ...(await this.encryptWithKey(password, dataObj)), + salt: this.generateSalt(), }); - }), + } async decrypt(_password: string, _text: string) { if (_password && _password !== PASSWORD) { throw new Error(INVALID_PASSWORD_ERROR); } - return Promise.resolve(cacheVal ?? {}); - }, + return cacheVal ?? {}; + } - async decryptWithEncryptedKeyString(_keyStr: string) { - const { vault } = await this.decryptWithDetail(_keyStr, 'mock vault'); - return vault; - }, + async encryptWithKey(_key: unknown, dataObj: any) { + cacheVal = dataObj; + return { + data: MOCK_HEX, + iv: 'anIv', + }; + } + + async encryptWithDetail(key: string, dataObj: any) { + return { + vault: await this.encrypt(key, dataObj), + exportedKeyString: MOCK_HARDCODED_KEY, + }; + } - async decryptWithDetail(_password: string, _text: string) { - if (_password && _password !== PASSWORD) { - throw new Error(INVALID_PASSWORD_ERROR); - } + async decryptWithDetail(key: string, text: string) { + return { + vault: await this.decrypt(key, text), + salt: MOCK_ENCRYPTION_SALT, + exportedKeyString: MOCK_ENCRYPTION_KEY, + }; + } - const result = cacheVal - ? { - vault: cacheVal, - exportedKeyString: MOCK_ENCRYPTION_KEY, - salt: MOCK_SALT, - } - : {}; - return Promise.resolve(result); - }, + async decryptWithKey(key: unknown, text: string) { + return this.decrypt(key as string, text); + } - importKey(keyString: string) { - if (keyString === '{}') { + async keyFromPassword(_password: string) { + return MOCK_KEY; + } + + async importKey(key: string) { + if (key === '{}') { throw new TypeError( `Failed to execute 'importKey' on 'SubtleCrypto': The provided value is not of type '(ArrayBuffer or ArrayBufferView or JsonWebKey)'.`, ); } return null; - }, - - encryptWithKey() { - const data = JSON.parse(MOCK_ENCRYPTION_DATA); - // Salt is not provided from this method - delete data.salt; - return data; - }, + } - async decryptWithKey(key: string, text: string) { - return this.decrypt(key, text); - }, - - async keyFromPassword(_password: string) { - return Promise.resolve(MOCK_KEY); - }, + async updateVault(_vault: string, _password: string) { + return _vault; + } generateSalt() { - return 'WHADDASALT!'; - }, -}; - -export { - mockEncryptor, - PASSWORD, - MOCK_HARDCODED_KEY, - MOCK_HEX, - MOCK_ENCRYPTION_KEY, - MOCK_SALT, -}; + return MOCK_ENCRYPTION_SALT; + } +} diff --git a/src/test/index.ts b/src/test/index.ts index 4a658c06..4e5dcbec 100644 --- a/src/test/index.ts +++ b/src/test/index.ts @@ -1,19 +1,19 @@ import { - mockEncryptor, + MockEncryptor, PASSWORD, MOCK_HARDCODED_KEY, MOCK_HEX, MOCK_ENCRYPTION_KEY, - MOCK_SALT, + MOCK_ENCRYPTION_SALT, } from './encryptor.mock'; import KeyringMockWithInit from './keyring.mock'; export { - mockEncryptor, + MockEncryptor, KeyringMockWithInit, PASSWORD, MOCK_HARDCODED_KEY, MOCK_HEX, MOCK_ENCRYPTION_KEY, - MOCK_SALT, + MOCK_ENCRYPTION_SALT, }; diff --git a/src/types.ts b/src/types.ts index 4409b4bc..67b2385f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,11 +1,20 @@ +import type { + DetailedDecryptResult, + DetailedEncryptionResult, + EncryptionResult, +} from '@metamask/browser-passworder'; import type { Json, Keyring } from '@metamask/utils'; export type KeyringControllerArgs = { keyringBuilders?: { (): Keyring; type: string }[]; - cacheEncryptionKey: boolean; initState?: KeyringControllerPersistentState; - encryptor?: any; -}; +} & ( + | { encryptor?: ExportableKeyEncryptor; cacheEncryptionKey: true } + | { + encryptor?: GenericEncryptor | ExportableKeyEncryptor; + cacheEncryptionKey: false; + } +); export type KeyringObject = { type: string; @@ -19,11 +28,105 @@ export type KeyringControllerPersistentState = { export type KeyringControllerState = { keyrings: KeyringObject[]; isUnlocked: boolean; - encryptionKey?: string; - encryptionSalt?: string; -}; +} & ( + | { encryptionKey: string; encryptionSalt: string } + | { + encryptionKey?: never; + encryptionSalt?: never; + } +); export type SerializedKeyring = { type: string; data: Json; }; + +/** + * A generic encryptor interface that supports encrypting and decrypting + * serializable data with a password. + */ +export type GenericEncryptor = { + /** + * Encrypts the given object with the given password. + * + * @param password - The password to encrypt with. + * @param object - The object to encrypt. + * @returns The encrypted string. + */ + encrypt: (password: string, object: Json) => Promise; + /** + * Decrypts the given encrypted string with the given password. + * + * @param password - The password to decrypt with. + * @param encryptedString - The encrypted string to decrypt. + * @returns The decrypted object. + */ + decrypt: (password: string, encryptedString: string) => Promise; + /** + * Optional vault migration helper. Updates the provided vault, re-encrypting + * data with a safer algorithm if one is available. + * + * @param vault - The encrypted string to update. + * @param password - The password to decrypt the vault with. + * @returns The updated encrypted string. + */ + updateVault?: (vault: string, password: string) => Promise; +}; + +/** + * An encryptor interface that supports encrypting and decrypting + * serializable data with a password, and exporting and importing keys. + */ +export type ExportableKeyEncryptor = GenericEncryptor & { + /** + * Encrypts the given object with the given encryption key. + * + * @param key - The encryption key to encrypt with. + * @param object - The object to encrypt. + * @returns The encryption result. + */ + encryptWithKey: (key: unknown, object: Json) => Promise; + /** + * Encrypts the given object with the given password, and returns the + * encryption result and the exported key string. + * + * @param password - The password to encrypt with. + * @param object - The object to encrypt. + * @param salt - The optional salt to use for encryption. + * @returns The encrypted string and the exported key string. + */ + encryptWithDetail: ( + password: string, + object: Json, + salt?: string, + ) => Promise; + /** + * Decrypts the given encrypted string with the given encryption key. + * + * @param key - The encryption key to decrypt with. + * @param encryptedString - The encrypted string to decrypt. + * @returns The decrypted object. + */ + decryptWithKey: (key: unknown, encryptedString: string) => Promise; + /** + * Decrypts the given encrypted string with the given password, and returns + * the decrypted object and the salt and exported key string used for + * encryption. + * + * @param password - The password to decrypt with. + * @param encryptedString - The encrypted string to decrypt. + * @returns The decrypted object and the salt and exported key string used for + * encryption. + */ + decryptWithDetail: ( + password: string, + encryptedString: string, + ) => Promise; + /** + * Generates an encryption key from exported key string. + * + * @param key - The exported key string. + * @returns The encryption key. + */ + importKey: (key: string) => Promise; +}; diff --git a/yarn.lock b/yarn.lock index 1c6105ff..078307dc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1154,10 +1154,12 @@ __metadata: languageName: node linkType: hard -"@metamask/browser-passworder@npm:^4.1.0": - version: 4.1.0 - resolution: "@metamask/browser-passworder@npm:4.1.0" - checksum: f1edb3b75594b8e8d075b3134c9ce6c73573160eb48184ef722b9d96a5763db1e2e9acb166fc5c66c7c82936e134a02a3fb4c0022ca9a948857a30181cb84d7e +"@metamask/browser-passworder@npm:^4.2.0": + version: 4.2.0 + resolution: "@metamask/browser-passworder@npm:4.2.0" + dependencies: + "@metamask/utils": ^8.2.0 + checksum: 03b76357942d25a6316d6a03a8bc839cb18e53d9f95fc2787e0fbbcf13eeb2485ece47a2758e928d04635f8dbaa598794f2ecd0313e7c91f989bf11f2a0adec5 languageName: node linkType: hard @@ -1233,7 +1235,7 @@ __metadata: "@lavamoat/allow-scripts": ^2.3.1 "@lavamoat/preinstall-always-fail": ^1.0.0 "@metamask/auto-changelog": ^3.0.0 - "@metamask/browser-passworder": ^4.1.0 + "@metamask/browser-passworder": ^4.2.0 "@metamask/eslint-config": ^12.2.0 "@metamask/eslint-config-jest": ^12.1.0 "@metamask/eslint-config-nodejs": ^12.1.0 @@ -1242,7 +1244,7 @@ __metadata: "@metamask/eth-sig-util": ^7.0.0 "@metamask/eth-simple-keyring": ^6.0.1 "@metamask/obs-store": ^8.1.0 - "@metamask/utils": ^8.1.0 + "@metamask/utils": ^8.2.0 "@types/jest": ^29.4.0 "@types/node": ^16.18.46 "@types/sinon": ^10.0.13 @@ -1329,7 +1331,7 @@ __metadata: languageName: node linkType: hard -"@metamask/utils@npm:^8.0.0, @metamask/utils@npm:^8.1.0": +"@metamask/utils@npm:^8.0.0, @metamask/utils@npm:^8.1.0, @metamask/utils@npm:^8.2.0": version: 8.2.1 resolution: "@metamask/utils@npm:8.2.1" dependencies: