diff --git a/packages/keyring-controller/CHANGELOG.md b/packages/keyring-controller/CHANGELOG.md index 9bb57e1f03..f6d46c3dd6 100644 --- a/packages/keyring-controller/CHANGELOG.md +++ b/packages/keyring-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** `exportSeedPhrase` and `exportAccount` now take `VerificationCredentials` (`{ password }` | `{ encryptionKey }`) instead of a bare password string ([#8996](https://github.com/MetaMask/core/pull/8996)) + ### Fixed - Automatically remove and destroy non-primary keyrings whose last account is removed during a `withKeyring` or `withKeyringV2` callback ([#8951](https://github.com/MetaMask/core/pull/8951)) diff --git a/packages/keyring-controller/src/KeyringController-method-action-types.ts b/packages/keyring-controller/src/KeyringController-method-action-types.ts index 1abd9cf97e..b469bc643d 100644 --- a/packages/keyring-controller/src/KeyringController-method-action-types.ts +++ b/packages/keyring-controller/src/KeyringController-method-action-types.ts @@ -82,7 +82,8 @@ export type KeyringControllerIsUnlockedAction = { /** * Gets the seed phrase of the HD keyring. * - * @param password - Password of the keyring. + * @param credentials - Object holding either the `password` or the vault + * `encryptionKey`. * @param keyringId - The id of the keyring. * @returns Promise resolving to the seed phrase. */ @@ -94,7 +95,8 @@ export type KeyringControllerExportSeedPhraseAction = { /** * Gets the private key from the keyring controlling an address. * - * @param password - Password of the keyring. + * @param credentials - Object holding either the `password` or the vault + * `encryptionKey`. * @param address - Address to export. * @returns Promise resolving to the private key for an address. */ diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 0924cb581d..4fad5575b9 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -605,7 +605,7 @@ describe('KeyringController', () => { { type: 'HD Key Tree' }, async ({ keyring }) => keyring.serialize(), ); - const currentSeedWord = await controller.exportSeedPhrase(password); + const currentSeedWord = await controller.exportSeedPhrase({ password }); await controller.createNewVaultAndRestore(password, currentSeedWord); @@ -679,8 +679,9 @@ describe('KeyringController', () => { async ({ controller }) => { await controller.createNewVaultAndKeychain(password); - const currentSeedPhrase = - await controller.exportSeedPhrase(password); + const currentSeedPhrase = await controller.exportSeedPhrase({ + password, + }); expect(currentSeedPhrase.length).toBeGreaterThan(0); expect( @@ -794,13 +795,17 @@ describe('KeyringController', () => { describe('when there is an existing vault', () => { it('should not create a new vault or keychain', async () => { await withController(async ({ controller, initialState }) => { - const initialSeedWord = await controller.exportSeedPhrase(password); + const initialSeedWord = await controller.exportSeedPhrase({ + password, + }); expect(initialSeedWord).toBeDefined(); const initialVault = controller.state.vault; await controller.createNewVaultAndKeychain(password); - const currentSeedWord = await controller.exportSeedPhrase(password); + const currentSeedWord = await controller.exportSeedPhrase({ + password, + }); expect(initialState).toStrictEqual(controller.state); expect(initialSeedWord).toBe(currentSeedWord); expect(initialVault).toStrictEqual(controller.state.vault); @@ -867,9 +872,9 @@ describe('KeyringController', () => { primaryKeyring.mnemonic = ''; - await expect(controller.exportSeedPhrase(password)).rejects.toThrow( - "Can't get mnemonic bytes from keyring", - ); + await expect( + controller.exportSeedPhrase({ password }), + ).rejects.toThrow("Can't get mnemonic bytes from keyring"); }); }); }); @@ -878,7 +883,7 @@ describe('KeyringController', () => { describe('when correct password is provided', () => { it('should export seed phrase without keyringId', async () => { await withController(async ({ controller }) => { - const seed = await controller.exportSeedPhrase(password); + const seed = await controller.exportSeedPhrase({ password }); expect(seed).not.toBe(''); }); }); @@ -886,7 +891,10 @@ describe('KeyringController', () => { it('should export seed phrase with valid keyringId', async () => { await withController(async ({ controller, initialState }) => { const keyringId = initialState.keyrings[0].metadata.id; - const seed = await controller.exportSeedPhrase(password, keyringId); + const seed = await controller.exportSeedPhrase( + { password }, + keyringId, + ); expect(seed).not.toBe(''); }); }); @@ -894,7 +902,7 @@ describe('KeyringController', () => { it('should throw error if keyringId is invalid', async () => { await withController(async ({ controller }) => { await expect( - controller.exportSeedPhrase(password, 'invalid-id'), + controller.exportSeedPhrase({ password }, 'invalid-id'), ).rejects.toThrow('Keyring not found'); }); }); @@ -906,9 +914,9 @@ describe('KeyringController', () => { jest .spyOn(encryptor, 'decrypt') .mockRejectedValueOnce(new Error('Invalid password')); - await expect(controller.exportSeedPhrase('')).rejects.toThrow( - 'Invalid password', - ); + await expect( + controller.exportSeedPhrase({ password: '' }), + ).rejects.toThrow('Invalid password'); }); }); @@ -920,19 +928,75 @@ describe('KeyringController', () => { .spyOn(encryptor, 'decrypt') .mockRejectedValueOnce(new Error('Invalid password')); await expect( - controller.exportSeedPhrase('', keyringId), + controller.exportSeedPhrase({ password: '' }, keyringId), ).rejects.toThrow('Invalid password'); }, ); }); }); + + describe('when correct encryption key is provided', () => { + it('should export seed phrase with an encryption key credential', async () => { + await withController(async ({ controller }) => { + const encryptionKey = await controller.exportEncryptionKey(); + const seed = await controller.exportSeedPhrase({ encryptionKey }); + expect(seed).not.toBe(''); + }); + }); + + it('should export seed phrase with an encryption key and a valid keyringId', async () => { + await withController(async ({ controller, initialState }) => { + const keyringId = initialState.keyrings[0].metadata.id; + const encryptionKey = await controller.exportEncryptionKey(); + const seed = await controller.exportSeedPhrase( + { encryptionKey }, + keyringId, + ); + expect(seed).not.toBe(''); + }); + }); + }); + + describe('when wrong encryption key is provided', () => { + it('should throw the decryption error', async () => { + await withController(async ({ controller, encryptor }) => { + const encryptionKey = await controller.exportEncryptionKey(); + jest + .spyOn(encryptor, 'decryptWithKey') + .mockRejectedValueOnce(new Error('Invalid key')); + await expect( + controller.exportSeedPhrase({ encryptionKey }), + ).rejects.toThrow('Invalid key'); + }); + }); + }); + + describe('when vault is missing', () => { + it('should throw error', async () => { + await withController( + { + skipVaultCreation: true, + state: { + isUnlocked: true, + } as KeyringControllerState, + }, + async ({ controller }) => { + await expect( + controller.exportSeedPhrase({ + encryptionKey: 'encryption-key', + }), + ).rejects.toThrow(KeyringControllerErrorMessage.VaultError); + }, + ); + }); + }); }); it('should throw error when the controller is locked', async () => { await withController(async ({ controller }) => { await controller.setLocked(); - await expect(controller.exportSeedPhrase(password)).rejects.toThrow( + await expect(controller.exportSeedPhrase({ password })).rejects.toThrow( KeyringControllerErrorMessage.ControllerLocked, ); }); @@ -947,7 +1011,7 @@ describe('KeyringController', () => { await withController(async ({ controller, initialState }) => { const account = initialState.keyrings[0].accounts[0]; const newPrivateKey = await controller.exportAccount( - password, + { password }, account, ); expect(newPrivateKey).not.toBe(''); @@ -959,7 +1023,7 @@ describe('KeyringController', () => { it('should throw error', async () => { await withController(async ({ controller }) => { await expect( - controller.exportAccount(password, ''), + controller.exportAccount({ password }, ''), ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound); }); }); @@ -968,17 +1032,63 @@ describe('KeyringController', () => { describe('when wrong password is provided', () => { it('should throw error', async () => { - await withController(async ({ controller, encryptor }) => { - jest - .spyOn(encryptor, 'decrypt') - .mockRejectedValueOnce(new Error('Invalid password')); - await expect(controller.exportSeedPhrase('')).rejects.toThrow( - 'Invalid password', + await withController( + async ({ controller, initialState, encryptor }) => { + const account = initialState.keyrings[0].accounts[0]; + jest + .spyOn(encryptor, 'decrypt') + .mockRejectedValueOnce(new Error('Invalid password')); + await expect( + controller.exportAccount({ password: '' }, account), + ).rejects.toThrow('Invalid password'); + }, + ); + }); + }); + + describe('when correct encryption key is provided', () => { + it('should export account with an encryption key credential', async () => { + await withController(async ({ controller, initialState }) => { + const account = initialState.keyrings[0].accounts[0]; + const encryptionKey = await controller.exportEncryptionKey(); + const newPrivateKey = await controller.exportAccount( + { encryptionKey }, + account, ); + expect(newPrivateKey).not.toBe(''); }); }); }); + + describe('when wrong encryption key is provided', () => { + it('should throw the decryption error', async () => { + await withController( + async ({ controller, initialState, encryptor }) => { + const account = initialState.keyrings[0].accounts[0]; + const encryptionKey = await controller.exportEncryptionKey(); + jest + .spyOn(encryptor, 'decryptWithKey') + .mockRejectedValueOnce(new Error('Invalid key')); + await expect( + controller.exportAccount({ encryptionKey }, account), + ).rejects.toThrow('Invalid key'); + }, + ); + }); + }); }); + + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller, initialState }) => { + const account = initialState.keyrings[0].accounts[0]; + await controller.setLocked(); + + await expect( + controller.exportAccount({ password }, account), + ).rejects.toThrow(KeyringControllerErrorMessage.ControllerLocked); + }); + }); + describe('when the keyring for the given address does not support exportAccount', () => { it('should throw error', async () => { const address = '0x5AC6D462f054690a373FABF8CC28e161003aEB19'; @@ -989,7 +1099,7 @@ describe('KeyringController', () => { await controller.addNewKeyring(MockKeyring.type); await expect( - controller.exportAccount(password, address), + controller.exportAccount({ password }, address), ).rejects.toThrow( KeyringControllerErrorMessage.UnsupportedExportAccount, ); @@ -5686,7 +5796,7 @@ describe('KeyringController', () => { expect(controller.state).toStrictEqual(initialState); await expect( - controller.exportAccount(password, mockAddress), + controller.exportAccount({ password }, mockAddress), ).rejects.toThrow(KeyringControllerErrorMessage.KeyringNotFound); }, ); diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 35603b8395..c6f5528deb 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -48,6 +48,7 @@ import { KeyringControllerError } from './errors'; import type { KeyringControllerMethodActions } from './KeyringController-method-action-types'; import type { Eip7702AuthorizationParams, + Credentials, PersonalMessageParams, TypedMessageParams, } from './types'; @@ -1045,6 +1046,39 @@ export class KeyringController< await this.#encryptor.decrypt(password, this.state.vault); } + /** + * Method to verify a given encryption key validity. Throws an error if the + * encryption key is invalid, i.e. it cannot decrypt the vault. + * + * @param encryptionKey - Serialized vault encryption key. + */ + async #verifyEncryptionKey(encryptionKey: string): Promise { + if (!this.state.vault) { + throw new KeyringControllerError( + KeyringControllerErrorMessage.VaultError, + ); + } + + const key = await this.#encryptor.importKey(encryptionKey); + await this.#encryptor.decryptWithKey(key, JSON.parse(this.state.vault)); + } + + /** + * Verifies export credentials by checking either the wallet password or the + * vault encryption key. + * + * @param credentials - Object holding either the `password` or the vault + * `encryptionKey`. + */ + async #verifyCredentials(credentials: Credentials): Promise { + // eslint-disable-next-line no-restricted-syntax + if ('password' in credentials) { + await this.verifyPassword(credentials.password); + } else { + await this.#verifyEncryptionKey(credentials.encryptionKey); + } + } + /** * Returns the status of the vault. * @@ -1057,16 +1091,19 @@ export class KeyringController< /** * Gets the seed phrase of the HD keyring. * - * @param password - Password of the keyring. + * @param credentials - Object holding either the `password` or the vault + * `encryptionKey`. * @param keyringId - The id of the keyring. * @returns Promise resolving to the seed phrase. */ async exportSeedPhrase( - password: string, + credentials: Credentials, keyringId?: string, ): Promise { this.#assertIsUnlocked(); - await this.verifyPassword(password); + + await this.#verifyCredentials(credentials); + const selectedKeyring = this.#getKeyringByIdOrDefault(keyringId); if (!selectedKeyring) { throw new KeyringControllerError('Keyring not found'); @@ -1079,12 +1116,18 @@ export class KeyringController< /** * Gets the private key from the keyring controlling an address. * - * @param password - Password of the keyring. + * @param credentials - Object holding either the `password` or the vault + * `encryptionKey`. * @param address - Address to export. * @returns Promise resolving to the private key for an address. */ - async exportAccount(password: string, address: string): Promise { - await this.verifyPassword(password); + async exportAccount( + credentials: Credentials, + address: string, + ): Promise { + this.#assertIsUnlocked(); + + await this.#verifyCredentials(credentials); const keyring = (await this.getKeyringForAccount(address)) as EthKeyring; if (!keyring.exportAccount) { diff --git a/packages/keyring-controller/src/types.ts b/packages/keyring-controller/src/types.ts index f4595db90f..11487b8621 100644 --- a/packages/keyring-controller/src/types.ts +++ b/packages/keyring-controller/src/types.ts @@ -72,3 +72,9 @@ export type SignTypedDataMessageV3V4 = { export type TypedMessageParams = { data: Record[] | string | SignTypedDataMessageV3V4; } & AbstractMessageParams; + +/** + * Credentials for re-authenticating the keyring during sensitive operations + * such as `exportSeedPhrase` and `exportAccount`. + */ +export type Credentials = { password: string } | { encryptionKey: string };