Uh oh!
There was an error while loading. Please reload this page.
This repository was archived by the owner on Oct 7, 2024. It is now read-only.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 126
Provide method for unlocking keyring by decrypted key#147
Closed
Uh oh!
There was an error while loading. Please reload this page.
Closed
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
34052f3
Provide method for unlocking keyring by decrypted key
darkwing 63240f1
Add code for salt
darkwing 6364e39
Add test outlines
darkwing de92b6f
Simplify logic
darkwing 6e2a2cb
Progress
darkwing 3148567
Don't assume password if there's a salted vault
darkwing d86ea18
Use encryptor salt
darkwing 1cd75b6
Abstract the salt and vault separation
darkwing 4af6bfa
Test progress
darkwing 86ef9b2
Improve test, implement encryption key generation
darkwing b493d5d
Fix tests, add TextEncoder shim
darkwing 39650d3
Add bad encryption key test
darkwing bfc85a2
test progress
darkwing f9e6dd2
Decode the ArrayBuffer
darkwing e7267d9
Only try to decrypt the salt-less vault
darkwing e333992
Format TextEncoder shim
darkwing c0585bb
Test improvements
darkwing f23e721
Address majority of feedback
darkwing 11c6cf9
Use TextEncoder provided by Node utils package
darkwing 2787442
Implement Dan Miller's suggestions
darkwing ba2c840
Fix lint
darkwing b950ee6
Remove unwanted code
darkwing ae049f1
Code cleanups and documentation
darkwing 36e5287
Remove unnecessary variable
darkwing 5e40e38
Remove unnecessary variables change
darkwing 5d0ec45
Update comment
darkwing 3009228
Move comment
darkwing 671d072
Add debug for differing passwords
darkwing 096d435
WIP
darkwing File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,16 +4,21 @@ const bip39 = require('@metamask/bip39'); | ||
| const ObservableStore = require('obs-store'); | ||
| const encryptor = require('browser-passworder'); | ||
| const { normalize: normalizeAddress } = require('eth-sig-util'); | ||
| //const { sha256 } = require('ethereum-cryptography/sha256'); | ||
| //const { utf8ToBytes, toHex } = require('ethereum-cryptography/utils'); | ||
| const SimpleKeyring = require('eth-simple-keyring'); | ||
| const HdKeyring = require('@metamask/eth-hd-keyring'); | ||
| const keyringTypes = [SimpleKeyring, HdKeyring]; | ||
| const VAULT_SEPARATOR = ':::'; | ||
| const KEYRINGS_TYPE_MAP = { | ||
| HD_KEYRING: 'HD Key Tree', | ||
| SIMPLE_KEYRING: 'Simple Key Pair', | ||
| }; | ||
| /** | ||
| * Strip the hex prefix from an address, if present | ||
| * @param {string} address - The address that might be hex prefixed. | ||
| @@ -125,6 +130,7 @@ class KeyringController extends EventEmitter { | ||
| mnemonic: seedPhraseAsBuffer, | ||
| numberOfAccounts: 1, | ||
| }, | ||
| password, | ||
| ); | ||
| const [firstAccount] = await firstKeyring.getAccounts(); | ||
| if (!firstAccount) { | ||
| @@ -145,8 +151,8 @@ class KeyringController extends EventEmitter { | ||
| */ | ||
| async setLocked() { | ||
| // set locked | ||
| this.password = null; | ||
| this.memStore.updateState({ isUnlocked: false }); | ||
| delete this.encryptionKey; | ||
| // remove keyrings | ||
| this.keyrings = []; | ||
| await this._updateMemStoreKeyrings(); | ||
| @@ -168,7 +174,29 @@ class KeyringController extends EventEmitter { | ||
| * @returns {Promise<Object>} A Promise that resolves to the state. | ||
| */ | ||
| async submitPassword(password) { | ||
| await this.verifyPassword(password); | ||
| this.keyrings = await this.unlockKeyrings(password); | ||
| // We should persist keyrings so that we can either | ||
| // (1) migrate or (2) create a new salt | ||
| await this.persistAllKeyrings(password); | ||
| this.setUnlocked(); | ||
| this.fullUpdate(); | ||
| return this.encryptionKey; | ||
| } | ||
| /** | ||
| * Submit Encrypted Key | ||
| * | ||
| * Attempts to decrypt the current vault with a given encryption key | ||
| * and loads its keyrings into memory | ||
| * | ||
| * @param {string} password | ||
| */ | ||
| async submitEncryptionKey(encryptionKey) { | ||
| this.keyrings = await this.unlockKeyrings(undefined, encryptionKey); | ||
| this.setUnlocked(); | ||
| this.fullUpdate(); | ||
| } | ||
| @@ -186,7 +214,12 @@ class KeyringController extends EventEmitter { | ||
| if (!encryptedVault) { | ||
| throw new Error('Cannot unlock without a previous vault.'); | ||
| } | ||
| await this.encryptor.decrypt(password, encryptedVault); | ||
| const result = await this.attemptGetDecryptedVault( | ||
| encryptedVault, | ||
| password, | ||
| ); | ||
| return result; | ||
darkwing marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| /** | ||
| @@ -202,7 +235,7 @@ class KeyringController extends EventEmitter { | ||
| * @param {Object} opts - The constructor options for the keyring. | ||
| * @returns {Promise<Keyring>} The new keyring. | ||
| */ | ||
| async addNewKeyring(type, opts) { | ||
| async addNewKeyring(type, opts, password) { | ||
| const Keyring = this.getKeyringClassForType(type); | ||
| const keyring = new Keyring(opts); | ||
| if ((!opts || !opts.mnemonic) && type === KEYRINGS_TYPE_MAP.HD_KEYRING) { | ||
| @@ -214,7 +247,7 @@ class KeyringController extends EventEmitter { | ||
| await this.checkForDuplicate(type, accounts); | ||
| this.keyrings.push(keyring); | ||
| await this.persistAllKeyrings(); | ||
| await this.persistAllKeyrings(password); | ||
| await this._updateMemStoreKeyrings(); | ||
| this.fullUpdate(); | ||
| @@ -491,10 +524,13 @@ class KeyringController extends EventEmitter { | ||
| * @returns {Promise<void>} - A promise that resolves if the operation was successful. | ||
| */ | ||
| async createFirstKeyTree(password) { | ||
| this.password = password; | ||
| this.clearKeyrings(); | ||
| const keyring = await this.addNewKeyring(KEYRINGS_TYPE_MAP.HD_KEYRING); | ||
| const keyring = await this.addNewKeyring( | ||
| KEYRINGS_TYPE_MAP.HD_KEYRING, | ||
| {}, | ||
| password, | ||
| ); | ||
| const [firstAccount] = await keyring.getAccounts(); | ||
| if (!firstAccount) { | ||
| throw new Error('KeyringController - No account found on keychain.'); | ||
| @@ -516,12 +552,37 @@ class KeyringController extends EventEmitter { | ||
| * @param {string} password - The keyring controller password. | ||
| * @returns {Promise<boolean>} Resolves to true once keyrings are persisted. | ||
| */ | ||
| async persistAllKeyrings(password = this.password) { | ||
| if (typeof password !== 'string') { | ||
| async persistAllKeyrings(password) { | ||
| if (password && typeof password !== 'string') { | ||
| throw new Error('KeyringController - password is not a string'); | ||
| } | ||
| this.password = password; | ||
| // Since we also allow persisting without a password, | ||
| // we should require this.encryptionKey | ||
| if (password === undefined && this.encryptionKey === undefined) { | ||
| throw new Error( | ||
| 'KeyringController - a password or encryptionKey must exist to persist keyrings', | ||
| ); | ||
| } | ||
darkwing marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| let salt = null; | ||
| if (password) { | ||
| // If this is a migration or new password-driven login, we should | ||
| // create or rotate the salt | ||
| salt = this.encryptor.generateSalt(); | ||
| // Since there's a new salt, we need to generate a new encrypted key | ||
| // for use in the | ||
| this.encryptionKey = await this._generateEncryptionKey(password, salt); | ||
| } else { | ||
| const encryptedVault = this.store.getState().vault; | ||
| if (!encryptedVault) { | ||
| throw new Error('Cannot unlock without a previous vault.'); | ||
| } | ||
| // We can use an existing salt if one exists in the encrypted key | ||
| salt = this.parseVault(encryptedVault).salt; | ||
| } | ||
| const serializedKeyrings = await Promise.all( | ||
| this.keyrings.map(async (keyring) => { | ||
| const [type, data] = await Promise.all([ | ||
| @@ -531,14 +592,50 @@ class KeyringController extends EventEmitter { | ||
| return { type, data }; | ||
| }), | ||
| ); | ||
| const encryptedString = await this.encryptor.encrypt( | ||
| this.password, | ||
| this.encryptionKey, | ||
| serializedKeyrings, | ||
| ); | ||
| this.store.updateState({ vault: encryptedString }); | ||
| if (!encryptedString || !salt) { | ||
| throw new Error( | ||
| 'Cannot persist vault without salt or encrypted vault string', | ||
| ); | ||
| } | ||
| const newVault = [encryptedString, VAULT_SEPARATOR, salt].join(''); | ||
| // The encrypted string gets concatenated with a separator and salt | ||
| this.store.updateState({ vault: newVault }); | ||
| return true; | ||
| } | ||
| async attemptGetDecryptedVault(encryptedVault, password, encryptionKey) { | ||
| if (password === undefined && encryptionKey === undefined) { | ||
| throw new Error( | ||
| 'No way to decrypt a salted vault without a password or encryption key', | ||
| ); | ||
| } | ||
| // If the separator string is in the vault string, the user has already migrated | ||
| // from the previous password-only model | ||
| if (encryptedVault.includes(VAULT_SEPARATOR)) { | ||
| const { salt, vault } = this.parseVault(encryptedVault); | ||
| if (encryptionKey) { | ||
| this.encryptionKey = encryptionKey; | ||
| } else { | ||
| this.encryptionKey = await this._generateEncryptionKey(password, salt); | ||
| } | ||
| //return await this.encryptor.decrypt(this.encryptionKey, vault); | ||
| return await this.encryptor.decryptWithKey(this.encryptionKey, encryptedVault); | ||
| } | ||
| return await this.encryptor.decrypt(password, encryptedVault); | ||
| } | ||
| /** | ||
| * Unlock Keyrings | ||
| * | ||
| @@ -548,20 +645,71 @@ class KeyringController extends EventEmitter { | ||
| * @param {string} password - The keyring controller password. | ||
| * @returns {Promise<Array<Keyring>>} The keyrings. | ||
| */ | ||
| async unlockKeyrings(password) { | ||
| async unlockKeyrings(password, encryptionKey) { | ||
| const encryptedVault = this.store.getState().vault; | ||
| if (!encryptedVault) { | ||
| throw new Error('Cannot unlock without a previous vault.'); | ||
| } | ||
| if (password === undefined && encryptionKey === undefined) { | ||
| throw new Error( | ||
| 'No way to decrypt a salted vault without a password or encrypted key', | ||
| ); | ||
| } | ||
| await this.clearKeyrings(); | ||
| const vault = await this.encryptor.decrypt(password, encryptedVault); | ||
| this.password = password; | ||
| const vault = await this.attemptGetDecryptedVault( | ||
| encryptedVault, | ||
| password, | ||
| encryptionKey, | ||
| ); | ||
| await Promise.all(vault.map(this._restoreKeyring.bind(this))); | ||
| await this._updateMemStoreKeyrings(); | ||
| return this.keyrings; | ||
| } | ||
| /** | ||
| * Parse Vault | ||
| * | ||
| * Parses the vault string for vault and salt | ||
| * | ||
| * @param {string} encryptedVault - The stored, encrypted vault | ||
| * @returns {Boject} Contains salt and vault properties | ||
| */ | ||
| parseVault(encryptedVault) { | ||
| const [vault, salt] = encryptedVault.split(VAULT_SEPARATOR); | ||
| return { vault, salt }; | ||
| } | ||
| /** | ||
| * Generate Encryption Key | ||
| * | ||
| * Generates an encryption key which will be used to decrypt | ||
| * the vault. | ||
| * | ||
| * @param {Object} password - The user's submitted password | ||
| * @param {Object} salt - Salt to generate the encryption key | ||
| * @returns {string} The encryption key | ||
| */ | ||
| async _generateEncryptionKey(password, salt) { | ||
| //return toHex(sha256(utf8ToBytes(password + salt))); | ||
| const key = await encryptor.keyFromPassword(password, salt); | ||
| const exportedKey = await window.crypto.subtle.exportKey('jwk', key); | ||
| const keyString = JSON.stringify(exportedKey); | ||
| console.log("key is: ", key, exportedKey); | ||
| console.log("keystring is: ", keyString); | ||
| // recreate the key? | ||
| const restoredKey = await window.crypto.subtle.importKey('jwk', JSON.parse(keyString), 'AES-GCM', true, ['encrypt', 'decrypt']); | ||
| console.log('restored key is: ', restoredKey); | ||
| return keyString; | ||
| } | ||
| /** | ||
| * Restore Keyring | ||
| * | ||
| @@ -762,4 +910,4 @@ class KeyringController extends EventEmitter { | ||
| } | ||
| } | ||
| module.exports = KeyringController; | ||
| module.exports = KeyringController; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what this part is doing, assuming some signaling that's not relevant to the security or lock/unlock process.