From d7713a70cece4eb5aa81a91a8d6ee70f906cd91f Mon Sep 17 00:00:00 2001 From: ryanml Date: Fri, 11 Mar 2022 13:26:37 -0700 Subject: [PATCH] Revert "Update SRP representation" This reverts commit c3feabf4de3c978ecb25b5a960cbd2c919d9f20c. --- app/scripts/lib/seed-phrase-verifier.js | 8 +- app/scripts/metamask-controller.js | 18 ++-- lavamoat/browserify/beta/policy.json | 5 -- lavamoat/browserify/flask/policy.json | 5 -- lavamoat/browserify/main/policy.json | 5 -- patches/bip39+2.5.0.patch | 99 ---------------------- patches/eth-hd-keyring+3.6.0.patch | 43 ---------- patches/eth-keyring-controller+6.2.1.patch | 37 -------- ui/store/actions.js | 65 ++++++-------- ui/store/actions.test.js | 12 +-- 10 files changed, 42 insertions(+), 255 deletions(-) delete mode 100644 patches/bip39+2.5.0.patch delete mode 100644 patches/eth-hd-keyring+3.6.0.patch delete mode 100644 patches/eth-keyring-controller+6.2.1.patch diff --git a/app/scripts/lib/seed-phrase-verifier.js b/app/scripts/lib/seed-phrase-verifier.js index e1c18c1eb..225a57896 100644 --- a/app/scripts/lib/seed-phrase-verifier.js +++ b/app/scripts/lib/seed-phrase-verifier.js @@ -11,10 +11,10 @@ const seedPhraseVerifier = { * - The keyring always creates the accounts in the same sequence. * * @param {Array} createdAccounts - The accounts to restore - * @param {Buffer} seedPhrase - The seed words to verify, encoded as a Buffer - * @returns {Promise} + * @param {string} seedWords - The seed words to verify + * @returns {Promise} Promises undefined */ - async verifyAccounts(createdAccounts, seedPhrase) { + async verifyAccounts(createdAccounts, seedWords) { if (!createdAccounts || createdAccounts.length < 1) { throw new Error('No created accounts defined.'); } @@ -22,7 +22,7 @@ const seedPhraseVerifier = { const keyringController = new KeyringController({}); const Keyring = keyringController.getKeyringClassForType('HD Key Tree'); const opts = { - mnemonic: seedPhrase, + mnemonic: seedWords, numberOfAccounts: createdAccounts.length, }; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index ef6b9b86e..eb5381a6b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1838,16 +1838,13 @@ export default class MetamaskController extends EventEmitter { * Create a new Vault and restore an existent keyring. * * @param {string} password - * @param {number[]} encodedSeedPhrase - The seed phrase, encoded as an array - * of UTF-8 bytes. + * @param {string} seed */ - async createNewVaultAndRestore(password, encodedSeedPhrase) { + async createNewVaultAndRestore(password, seed) { const releaseLock = await this.createVaultMutex.acquire(); try { let accounts, lastBalance; - const seedPhraseAsBuffer = Buffer.from(encodedSeedPhrase); - const { keyringController } = this; // clear known identities @@ -1868,7 +1865,7 @@ export default class MetamaskController extends EventEmitter { // create new vault const vault = await keyringController.createNewVaultAndRestore( password, - seedPhraseAsBuffer, + seed, ); const ethQuery = new EthQuery(this.provider); @@ -2376,8 +2373,7 @@ export default class MetamaskController extends EventEmitter { * * Called when the first account is created and on unlocking the vault. * - * @returns {Promise} The seed phrase to be confirmed by the user, - * encoded as an array of UTF-8 bytes. + * @returns {Promise} Seed phrase to be confirmed by the user. */ async verifySeedPhrase() { const primaryKeyring = this.keyringController.getKeyringsByType( @@ -2388,7 +2384,7 @@ export default class MetamaskController extends EventEmitter { } const serialized = await primaryKeyring.serialize(); - const seedPhraseAsBuffer = Buffer.from(serialized.mnemonic); + const seedWords = serialized.mnemonic; const accounts = await primaryKeyring.getAccounts(); if (accounts.length < 1) { @@ -2396,8 +2392,8 @@ export default class MetamaskController extends EventEmitter { } try { - await seedPhraseVerifier.verifyAccounts(accounts, seedPhraseAsBuffer); - return Array.from(seedPhraseAsBuffer.values()); + await seedPhraseVerifier.verifyAccounts(accounts, seedWords); + return seedWords; } catch (err) { log.error(err.message); throw err; diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 10a1dfc14..86a77a3df 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -1178,9 +1178,6 @@ } }, "bip39": { - "globals": { - "console.log": true - }, "packages": { "buffer": true, "create-hash": true, @@ -1895,7 +1892,6 @@ "eth-hd-keyring": { "packages": { "bip39": true, - "buffer": true, "eth-sig-util": true, "eth-simple-keyring": true, "ethereumjs-wallet": true @@ -1954,7 +1950,6 @@ "packages": { "bip39": true, "browser-passworder": true, - "buffer": true, "eth-hd-keyring": true, "eth-sig-util": true, "eth-simple-keyring": true, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index df637e627..9882d00ed 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -1197,9 +1197,6 @@ } }, "bip39": { - "globals": { - "console.log": true - }, "packages": { "buffer": true, "create-hash": true, @@ -1914,7 +1911,6 @@ "eth-hd-keyring": { "packages": { "bip39": true, - "buffer": true, "eth-sig-util": true, "eth-simple-keyring": true, "ethereumjs-wallet": true @@ -1973,7 +1969,6 @@ "packages": { "bip39": true, "browser-passworder": true, - "buffer": true, "eth-hd-keyring": true, "eth-sig-util": true, "eth-simple-keyring": true, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 10a1dfc14..86a77a3df 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -1178,9 +1178,6 @@ } }, "bip39": { - "globals": { - "console.log": true - }, "packages": { "buffer": true, "create-hash": true, @@ -1895,7 +1892,6 @@ "eth-hd-keyring": { "packages": { "bip39": true, - "buffer": true, "eth-sig-util": true, "eth-simple-keyring": true, "ethereumjs-wallet": true @@ -1954,7 +1950,6 @@ "packages": { "bip39": true, "browser-passworder": true, - "buffer": true, "eth-hd-keyring": true, "eth-sig-util": true, "eth-simple-keyring": true, diff --git a/patches/bip39+2.5.0.patch b/patches/bip39+2.5.0.patch deleted file mode 100644 index 2976f3bb2..000000000 --- a/patches/bip39+2.5.0.patch +++ /dev/null @@ -1,99 +0,0 @@ -diff --git a/node_modules/bip39/index.js b/node_modules/bip39/index.js -index aa0f29f..bee8008 100644 ---- a/node_modules/bip39/index.js -+++ b/node_modules/bip39/index.js -@@ -48,7 +48,9 @@ function salt (password) { - } - - function mnemonicToSeed (mnemonic, password) { -- var mnemonicBuffer = Buffer.from(unorm.nfkd(mnemonic), 'utf8') -+ var mnemonicBuffer = typeof mnemonic === 'string' -+ ? Buffer.from(unorm.nfkd(mnemonic), 'utf8') -+ : mnemonic - var saltBuffer = Buffer.from(salt(unorm.nfkd(password)), 'utf8') - - return pbkdf2(mnemonicBuffer, saltBuffer, 2048, 64, 'sha512') -@@ -61,12 +63,28 @@ function mnemonicToSeedHex (mnemonic, password) { - function mnemonicToEntropy (mnemonic, wordlist) { - wordlist = wordlist || DEFAULT_WORDLIST - -- var words = unorm.nfkd(mnemonic).split(' ') -+ var mnemonicAsBuffer = typeof mnemonic === 'string' -+ ? Buffer.from(unorm.nfkd(mnemonic), 'utf8') -+ : mnemonic -+ -+ var words = []; -+ var currentWord = []; -+ for (const byte of mnemonicAsBuffer.values()) { -+ // split at space or \u3000 (ideographic space, for Japanese wordlists) -+ if (byte === 0x20 || byte === 0x3000) { -+ words.push(Buffer.from(currentWord)); -+ currentWord = []; -+ } else { -+ currentWord.push(byte); -+ } -+ } -+ words.push(Buffer.from(currentWord)); -+ - if (words.length % 3 !== 0) throw new Error(INVALID_MNEMONIC) - - // convert word indices to 11 bit binary strings - var bits = words.map(function (word) { -- var index = wordlist.indexOf(word) -+ var index = wordlist.indexOf(word.toString('utf8')) - if (index === -1) throw new Error(INVALID_MNEMONIC) - - return lpad(index.toString(2), '0', 11) -@@ -104,12 +122,41 @@ function entropyToMnemonic (entropy, wordlist) { - - var bits = entropyBits + checksumBits - var chunks = bits.match(/(.{1,11})/g) -- var words = chunks.map(function (binary) { -+ var wordsAsBuffers = chunks.map(function (binary) { - var index = binaryToByte(binary) -- return wordlist[index] -+ return Buffer.from(wordlist[index], 'utf8') - }) - -- return wordlist === JAPANESE_WORDLIST ? words.join('\u3000') : words.join(' ') -+ var bufferSize = wordsAsBuffers.reduce(function (bufferSize, wordAsBuffer, i) { -+ var shouldAddSeparator = i < wordsAsBuffers.length - 1 -+ return ( -+ bufferSize + -+ wordAsBuffer.length + -+ (shouldAddSeparator ? 1 : 0) -+ ) -+ }, 0) -+ var separator = wordlist === JAPANESE_WORDLIST ? '\u3000' : ' ' -+ var result = wordsAsBuffers.reduce(function (result, wordAsBuffer, i) { -+ var shouldAddSeparator = i < wordsAsBuffers.length - 1 -+ result.workingBuffer.set(wordAsBuffer, result.offset) -+ if (shouldAddSeparator) { -+ result.workingBuffer.write( -+ separator, -+ result.offset + wordAsBuffer.length, -+ separator.length, -+ 'utf8' -+ ) -+ } -+ return { -+ workingBuffer: result.workingBuffer, -+ offset: ( -+ result.offset + -+ wordAsBuffer.length + -+ (shouldAddSeparator ? 1 : 0) -+ ) -+ } -+ }, { workingBuffer: Buffer.alloc(bufferSize), offset: 0 }) -+ return result.workingBuffer; - } - - function generateMnemonic (strength, rng, wordlist) { -@@ -124,6 +171,7 @@ function validateMnemonic (mnemonic, wordlist) { - try { - mnemonicToEntropy(mnemonic, wordlist) - } catch (e) { -+ console.log('could not validate mnemonic', e) - return false - } - diff --git a/patches/eth-hd-keyring+3.6.0.patch b/patches/eth-hd-keyring+3.6.0.patch deleted file mode 100644 index 211cb89dd..000000000 --- a/patches/eth-hd-keyring+3.6.0.patch +++ /dev/null @@ -1,43 +0,0 @@ -diff --git a/node_modules/eth-hd-keyring/index.js b/node_modules/eth-hd-keyring/index.js -index 19d1d7f..350d6b8 100644 ---- a/node_modules/eth-hd-keyring/index.js -+++ b/node_modules/eth-hd-keyring/index.js -@@ -17,8 +17,11 @@ class HdKeyring extends SimpleKeyring { - } - - serialize () { -+ const mnemonicAsBuffer = typeof this.mnemonic === 'string' -+ ? Buffer.from(this.mnemonic, 'utf8') -+ : this.mnemonic - return Promise.resolve({ -- mnemonic: this.mnemonic, -+ mnemonic: Array.from(mnemonicAsBuffer.values()), - numberOfAccounts: this.wallets.length, - hdPath: this.hdPath, - }) -@@ -69,9 +72,22 @@ class HdKeyring extends SimpleKeyring { - - /* PRIVATE METHODS */ - -- _initFromMnemonic (mnemonic) { -- this.mnemonic = mnemonic -- const seed = bip39.mnemonicToSeed(mnemonic) -+ /** -+ * Sets appropriate properties for the keyring based on the given -+ * BIP39-compliant mnemonic. -+ * -+ * @param {string|Array|Buffer} mnemonic - A seed phrase represented -+ * as a string, an array of UTF-8 bytes, or a Buffer. -+ */ -+ _initFromMnemonic(mnemonic) { -+ if (typeof mnemonic === 'string') { -+ this.mnemonic = Buffer.from(mnemonic, 'utf8') -+ } else if (Array.isArray(mnemonic)) { -+ this.mnemonic = Buffer.from(mnemonic) -+ } else { -+ this.mnemonic = mnemonic -+ } -+ const seed = bip39.mnemonicToSeed(this.mnemonic) - this.hdWallet = hdkey.fromMasterSeed(seed) - this.root = this.hdWallet.derivePath(this.hdPath) - } diff --git a/patches/eth-keyring-controller+6.2.1.patch b/patches/eth-keyring-controller+6.2.1.patch deleted file mode 100644 index aec0c7168..000000000 --- a/patches/eth-keyring-controller+6.2.1.patch +++ /dev/null @@ -1,37 +0,0 @@ -diff --git a/node_modules/eth-keyring-controller/index.js b/node_modules/eth-keyring-controller/index.js -index 250ab98..38615aa 100644 ---- a/node_modules/eth-keyring-controller/index.js -+++ b/node_modules/eth-keyring-controller/index.js -@@ -84,15 +84,20 @@ class KeyringController extends EventEmitter { - * - * @emits KeyringController#unlock - * @param {string} password - The password to encrypt the vault with -- * @param {string} seed - The BIP44-compliant seed phrase. -+ * @param {string|Array} seedPhrase - The BIP39-compliant seed phrase, -+ * either as a string or an array of UTF-8 bytes that represent the string. - * @returns {Promise} A Promise that resolves to the state. - */ -- createNewVaultAndRestore (password, seed) { -+ createNewVaultAndRestore(password, seedPhrase) { -+ const seedPhraseAsBuffer = typeof seedPhrase === 'string' -+ ? Buffer.from(seedPhrase, 'utf8') -+ : Buffer.from(seedPhrase) -+ - if (typeof password !== 'string') { - return Promise.reject(new Error('Password must be text.')) - } - -- if (!bip39.validateMnemonic(seed)) { -+ if (!bip39.validateMnemonic(seedPhraseAsBuffer)) { - return Promise.reject(new Error('Seed phrase is invalid.')) - } - -@@ -101,7 +106,7 @@ class KeyringController extends EventEmitter { - return this.persistAllKeyrings(password) - .then(() => { - return this.addNewKeyring('HD Key Tree', { -- mnemonic: seed, -+ mnemonic: seedPhraseAsBuffer, - numberOfAccounts: 1, - }) - }) diff --git a/ui/store/actions.js b/ui/store/actions.js index 5925120b0..6461f29da 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -80,39 +80,20 @@ export function tryUnlockMetamask(password) { }; } -/** - * Adds a new account where all data is encrypted using the given password and - * where all addresses are generated from a given seed phrase. - * - * @param {string} password - The password. - * @param {string} seedPhrase - The seed phrase. - * @returns {Object} The updated state of the keyring controller. - */ -export function createNewVaultAndRestore(password, seedPhrase) { +export function createNewVaultAndRestore(password, seed) { return (dispatch) => { dispatch(showLoadingIndication()); log.debug(`background.createNewVaultAndRestore`); - - // Encode the secret recovery phrase as an array of integers so that it is - // serialized as JSON properly. - const encodedSeedPhrase = Array.from( - Buffer.from(seedPhrase, 'utf8').values(), - ); - let vault; return new Promise((resolve, reject) => { - background.createNewVaultAndRestore( - password, - encodedSeedPhrase, - (err, _vault) => { - if (err) { - reject(err); - return; - } - vault = _vault; - resolve(); - }, - ); + background.createNewVaultAndRestore(password, seed, (err, _vault) => { + if (err) { + reject(err); + return; + } + vault = _vault; + resolve(); + }); }) .then(() => dispatch(unMarkPasswordForgotten())) .then(() => { @@ -134,8 +115,8 @@ export function createNewVaultAndGetSeedPhrase(password) { try { await createNewVault(password); - const seedPhrase = await verifySeedPhrase(); - return seedPhrase; + const seedWords = await verifySeedPhrase(); + return seedWords; } catch (error) { dispatch(displayWarning(error.message)); throw new Error(error.message); @@ -151,9 +132,9 @@ export function unlockAndGetSeedPhrase(password) { try { await submitPassword(password); - const seedPhrase = await verifySeedPhrase(); + const seedWords = await verifySeedPhrase(); await forceUpdateMetamaskState(dispatch); - return seedPhrase; + return seedWords; } catch (error) { dispatch(displayWarning(error.message)); throw new Error(error.message); @@ -202,9 +183,17 @@ export function verifyPassword(password) { }); } -export async function verifySeedPhrase() { - const encodedSeedPhrase = await promisifiedBackground.verifySeedPhrase(); - return Buffer.from(encodedSeedPhrase).toString('utf8'); +export function verifySeedPhrase() { + return new Promise((resolve, reject) => { + background.verifySeedPhrase((error, seedWords) => { + if (error) { + reject(error); + return; + } + + resolve(seedWords); + }); + }); } export function requestRevealSeedWords(password) { @@ -214,11 +203,11 @@ export function requestRevealSeedWords(password) { try { await verifyPassword(password); - const seedPhrase = await verifySeedPhrase(); - return seedPhrase; + const seedWords = await verifySeedPhrase(); + return seedWords; } catch (error) { dispatch(displayWarning(error.message)); - throw error; + throw new Error(error.message); } finally { dispatch(hideLoadingIndication()); } diff --git a/ui/store/actions.test.js b/ui/store/actions.test.js index 727835ecf..0866713b4 100644 --- a/ui/store/actions.test.js +++ b/ui/store/actions.test.js @@ -111,9 +111,7 @@ describe('Actions', () => { actions._setBackgroundConnection(background); - await store.dispatch( - actions.createNewVaultAndRestore('password', 'test'), - ); + await store.dispatch(actions.createNewVaultAndRestore()); expect(createNewVaultAndRestore.callCount).toStrictEqual(1); }); @@ -136,9 +134,7 @@ describe('Actions', () => { { type: 'HIDE_LOADING_INDICATION' }, ]; - await store.dispatch( - actions.createNewVaultAndRestore('password', 'test'), - ); + await store.dispatch(actions.createNewVaultAndRestore()); expect(store.getActions()).toStrictEqual(expectedActions); }); @@ -159,7 +155,7 @@ describe('Actions', () => { ]; await expect( - store.dispatch(actions.createNewVaultAndRestore('password', 'test')), + store.dispatch(actions.createNewVaultAndRestore()), ).rejects.toThrow('error'); expect(store.getActions()).toStrictEqual(expectedActions); @@ -178,7 +174,7 @@ describe('Actions', () => { cb(), ); const verifySeedPhrase = background.verifySeedPhrase.callsFake((cb) => - cb(null, Array.from(Buffer.from('test').values())), + cb(), ); actions._setBackgroundConnection(background);