MetaMask / MetaMask/metamask-extension
Move Encryptor outside of KeyringController
- Dominant language
- TypeScript
- Stars
- 13.2k
- Forks
- 5.6k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 451
Description
Once #4034 is completed, the next function that could be moved outside of the KeyringController is the encryption functions.
Currently the KeyringController stores accounts and their schema, but these aren't actually the only portions of MetaMask state that might need to be encrypted.
Instead, we should move our encryption related functions into their own module at the `MetaMaskController` level, so that it can combine encryption-needing state as needed.
Two functions involving the password currently pass through the KeyringController:
- [ ] setLocked
- [ ] submitPassword
Also, a couple methods that assume keyringController manages the password encryption exist:
- [ ] createNewVaultAndKeychain
- [ ] createNewVaultAndRestore
Since we are using [obs-store](https://www.npmjs.com/package/obs-store) for most of our state objects now, it would probably make sense to take advantage of this architecture for the new encryption strategy.
## One implementation idea: SecureComposableObservableStore
We could maybe achieve this goal with a subclass of [ComposableObservableStore](https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/lib/ComposableObservableStore.js), [as the `metamaskController.store` instead](https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/metamask-controller.js#L70).
### The `SecureComposableObservableStore` class
- Has a method named `subscribeToEncrypted`, which emits an encrypted blob instead of the clear text contents.
- Has a method named `submitPassword`, and does not emit any events until it has been unlocked.
- Has a `setLocked` method that deallocates any secret material and stops emitting updates.
- Emits events for `locked` and `unlocked`.
- `MetaMaskController` uses `locked` and `unlocked` events to setup the controllers managed by this store.
- `MetaMaskController` must also call [`updateStructure(opts)`](https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/lib/ComposableObservableStore.js#L24) on its `.memStore` on both of these events, to add/remove the added/removed controllers from the memStore.
Contributor guide
Research direction
Review app/scripts/lib/ComposableObservableStore.js, app/scripts/metamask-controller.js around line 70, and the listed KeyringController methods after #4034 is complete. Determine how encrypted subscriptions, password submission, locking, and controller setup should work through the MetaMaskController store. Done means encryption no longer passes through KeyringController and locked/unlocked store events update memStore structure as described.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100