From c37c050c8a119e4c2de1edea474c3c3d5ad1cf99 Mon Sep 17 00:00:00 2001 From: Frankie Date: Mon, 12 Sep 2016 10:34:06 -0700 Subject: [PATCH] Revert "Add new eth-lightwallet salting to vault." --- CHANGELOG.md | 1 - app/scripts/lib/idStore.js | 133 +++++++++++++-------------- test/unit/idStore-test.js | 81 ++-------------- ui/app/accounts/account-list-item.js | 13 ++- ui/app/accounts/index.js | 4 +- ui/app/components/shapeshift-form.js | 4 +- ui/app/first-time/restore-vault.js | 6 +- ui/app/send.js | 6 +- 8 files changed, 93 insertions(+), 155 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff1d4c2e7..9d89c2438 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,6 @@ - MetaMask logo now renders as super lightweight SVG, improving compatibility and performance. - Now showing loading indication during vault unlocking, to clarify behavior for users who are experience slow unlocks. - Now only initially creates one wallet when restoring a vault, to reduce some users' confusion. -- Improved the security of vault encryption by ensuring passwords are always uniquely salted. ## 2.10.2 2016-09-02 diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 8b7e3ad3b..26aa02ef7 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -3,7 +3,7 @@ const inherits = require('util').inherits const async = require('async') const ethUtil = require('ethereumjs-util') const EthQuery = require('eth-query') -const KeyStore = require('eth-lightwallet').keystore +const LightwalletKeyStore = require('eth-lightwallet').keystore const clone = require('clone') const extend = require('xtend') const createId = require('web3-provider-engine/util/random-id') @@ -50,16 +50,15 @@ IdentityStore.prototype.createNewVault = function (password, entropy, cb) { if (serializedKeystore) { this.configManager.setData({}) } - this._createIdmgmt(password, null, entropy, (err) => { if (err) return cb(err) + this._loadIdentities() + this._didUpdate() this._autoFaucet() this.configManager.setShowSeedWords(true) var seedWords = this._idmgmt.getSeed() - - cb(null, seedWords) }) } @@ -76,6 +75,7 @@ IdentityStore.prototype.recoverFromSeed = function (password, seed, cb) { if (err) return cb(err) this._loadIdentities() + this._didUpdate() cb(null, this.getState()) }) } @@ -125,7 +125,7 @@ IdentityStore.prototype.getSelectedAddress = function () { return configManager.getSelectedAccount() } -IdentityStore.prototype.setSelectedAddressSync = function (address) { +IdentityStore.prototype.setSelectedAddress = function (address, cb) { const configManager = this.configManager if (!address) { var addresses = this._getAddresses() @@ -133,12 +133,7 @@ IdentityStore.prototype.setSelectedAddressSync = function (address) { } configManager.setSelectedAccount(address) - return address -} - -IdentityStore.prototype.setSelectedAddress = function (address, cb) { - const resultAddress = this.setSelectedAddressSync(address) - if (cb) return cb(null, resultAddress) + if (cb) return cb(null, address) } IdentityStore.prototype.revealAccount = function (cb) { @@ -148,7 +143,6 @@ IdentityStore.prototype.revealAccount = function (cb) { keyStore.setDefaultHdDerivationPath(this.hdPathString) keyStore.generateNewAddress(derivedKey, 1) - configManager.setWallet(keyStore.serialize()) this._loadIdentities() @@ -399,6 +393,7 @@ IdentityStore.prototype._loadIdentities = function () { var addresses = this._getAddresses() addresses.forEach((address, i) => { // // add to ethStore + this._ethStore.addAccount(address) // add to identities const defaultLabel = 'Wallet ' + (i + 1) const nickname = configManager.nicknameForWallet(address) @@ -417,6 +412,7 @@ IdentityStore.prototype.saveAccountLabel = function (account, label, cb) { configManager.setNicknameForWallet(account, label) this._loadIdentities() cb(null, label) + this._didUpdate() } // mayBeFauceting @@ -440,76 +436,77 @@ IdentityStore.prototype._mayBeFauceting = function (i) { // IdentityStore.prototype.tryPassword = function (password, cb) { - var serializedKeystore = this.configManager.getWallet() - var keyStore = KeyStore.deserialize(serializedKeystore) + this._createIdmgmt(password, null, null, cb) +} - keyStore.keyFromPassword(password, (err, pwDerivedKey) => { +IdentityStore.prototype._createIdmgmt = function (password, seed, entropy, cb) { + const configManager = this.configManager + + var keyStore = null + LightwalletKeyStore.deriveKeyFromPassword(password, (err, derivedKey) => { if (err) return cb(err) + var serializedKeystore = configManager.getWallet() - const isCorrect = keyStore.isDerivedKeyCorrect(pwDerivedKey) - if (!isCorrect) return cb(new Error('Lightwallet - password incorrect')) + if (seed) { + try { + keyStore = this._restoreFromSeed(password, seed, derivedKey) + } catch (e) { + return cb(e) + } + + // returning user, recovering from storage + } else if (serializedKeystore) { + keyStore = LightwalletKeyStore.deserialize(serializedKeystore) + var isCorrect = keyStore.isDerivedKeyCorrect(derivedKey) + if (!isCorrect) return cb(new Error('Lightwallet - password incorrect')) + + // first time here + } else { + keyStore = this._createFirstWallet(entropy, derivedKey) + } + + this._keyStore = keyStore + this._idmgmt = new IdManagement({ + keyStore: keyStore, + derivedKey: derivedKey, + hdPathSTring: this.hdPathString, + configManager: this.configManager, + }) cb() }) } -IdentityStore.prototype._createIdmgmt = function (password, seedPhrase, entropy, cb) { - const opts = { - password, - hdPathString: this.hdPathString, - } - - if (seedPhrase) { - opts.seedPhrase = seedPhrase - } - - KeyStore.createVault(opts, (err, keyStore) => { - if (err) return cb(err) - - this._keyStore = keyStore - - keyStore.keyFromPassword(password, (err, derivedKey) => { - if (err) return cb(err) - - this.purgeCache() - - keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) - - this._createFirstWallet(derivedKey) - - this._idmgmt = new IdManagement({ - keyStore: keyStore, - derivedKey: derivedKey, - configManager: this.configManager, - }) - - this.setSelectedAddressSync() - - cb() - }) - }) -} - -IdentityStore.prototype.purgeCache = function () { - this._getAddresses().forEach((address) => { - this._ethStore.del(address) - }) -} - -IdentityStore.prototype._createFirstWallet = function (derivedKey) { - const keyStore = this._keyStore +IdentityStore.prototype._restoreFromSeed = function (password, seed, derivedKey) { + const configManager = this.configManager + var keyStore = new LightwalletKeyStore(seed, derivedKey, this.hdPathString) + keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) keyStore.setDefaultHdDerivationPath(this.hdPathString) + keyStore.generateNewAddress(derivedKey, 1) - this.configManager.setWallet(keyStore.serialize()) - var addresses = keyStore.getAddresses() - this._ethStore.addAccount(ethUtil.addHexPrefix(addresses[0])) + configManager.setWallet(keyStore.serialize()) + if (global.METAMASK_DEBUG) { + console.log('restored from seed. saved to keystore') + } + return keyStore +} + +IdentityStore.prototype._createFirstWallet = function (entropy, derivedKey) { + const configManager = this.configManager + var secretSeed = LightwalletKeyStore.generateRandomSeed(entropy) + var keyStore = new LightwalletKeyStore(secretSeed, derivedKey, this.hdPathString) + keyStore.addHdDerivationPath(this.hdPathString, derivedKey, {curve: 'secp256k1', purpose: 'sign'}) + keyStore.setDefaultHdDerivationPath(this.hdPathString) + + keyStore.generateNewAddress(derivedKey, 1) + configManager.setWallet(keyStore.serialize()) + console.log('saved to keystore') + return keyStore } // get addresses and normalize address hexString IdentityStore.prototype._getAddresses = function () { - return this._keyStore.getAddresses(this.hdPathString).map((address) => { - return ethUtil.addHexPrefix(address) - }) + return this._keyStore.getAddresses(this.hdPathString).map((address) => { return '0x' + address }) } IdentityStore.prototype._autoFaucet = function () { diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 1ed1bf9a7..ee4613236 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -1,8 +1,6 @@ var assert = require('assert') var IdentityStore = require('../../app/scripts/lib/idStore') var configManagerGen = require('../lib/mock-config-manager') -const ethUtil = require('ethereumjs-util') -const async = require('async') describe('IdentityStore', function() { @@ -20,12 +18,11 @@ describe('IdentityStore', function() { idStore = new IdentityStore({ configManager: configManagerGen(), ethStore: { - addAccount(acct) { accounts.push(ethUtil.addHexPrefix(acct)) }, + addAccount(acct) { accounts.push(acct) }, }, }) idStore.createNewVault(password, entropy, (err, seeds) => { - assert.ifError(err, 'createNewVault threw error') seedWords = seeds originalKeystore = idStore._idmgmt.keyStore done() @@ -41,7 +38,7 @@ describe('IdentityStore', function() { idStore = new IdentityStore({ configManager: configManagerGen(), ethStore: { - addAccount(acct) { newAccounts.push(ethUtil.addHexPrefix(acct)) }, + addAccount(acct) { newAccounts.push(acct) }, }, }) }) @@ -60,91 +57,33 @@ describe('IdentityStore', function() { }) describe('#recoverFromSeed BIP44 compliance', function() { - const salt = 'lightwalletSalt' + let seedWords = 'picnic injury awful upper eagle junk alert toss flower renew silly vague' + let firstAccount = '0x5d8de92c205279c10e5669f797b853ccef4f739a' let password = 'secret!' let accounts = [] let idStore - var assertions = [ - { - seed: 'picnic injury awful upper eagle junk alert toss flower renew silly vague', - account: '0x5d8de92c205279c10e5669f797b853ccef4f739a', - }, - { - seed: 'radar blur cabbage chef fix engine embark joy scheme fiction master release', - account: '0xe15d894becb0354c501ae69429b05143679f39e0', - }, - { - seed: 'phone coyote caught pattern found table wedding list tumble broccoli chief swing', - account: '0xb0e868f24bc7fec2bce2efc2b1c344d7569cd9d2', - }, - { - seed: 'recycle tag bird palace blue village anxiety census cook soldier example music', - account: '0xab34a45920afe4af212b96ec51232aaa6a33f663', - }, - { - seed: 'half glimpse tape cute harvest sweet bike voyage actual floor poet lazy', - account: '0x28e9044597b625ac4beda7250011670223de43b2', - }, - { - seed: 'flavor tiger carpet motor angry hungry document inquiry large critic usage liar', - account: '0xb571be96558940c4e9292e1999461aa7499fb6cd', - }, - ] - before(function() { window.localStorage = {} // Hacking localStorage support into JSDom idStore = new IdentityStore({ configManager: configManagerGen(), ethStore: { - addAccount(acct) { accounts.push(ethUtil.addHexPrefix(acct)) }, + addAccount(acct) { accounts.push(acct) }, }, }) }) - beforeEach(function() { - accounts = [] - }) + it('should return the expected first account', function (done) { - it('should enforce seed compliance with TestRPC', function (done) { - const tests = assertions.map((assertion) => { - return function (cb) { - accounts = [] - idStore.recoverFromSeed(password, assertion.seed, (err) => { - assert.ifError(err) - - var received = accounts[0].toLowerCase() - var expected = assertion.account.toLowerCase() - assert.equal(received, expected) - cb() - }) - } - }) - - async.series(tests, function(err, results) { + idStore.recoverFromSeed(password, seedWords, (err) => { assert.ifError(err) + + let newKeystore = idStore._idmgmt.keyStore + assert.equal(accounts[0], firstAccount) done() }) }) - - it('should allow restoring and unlocking again', function (done) { - const assertion = assertions[0] - idStore.recoverFromSeed(password, assertion.seed, (err) => { - assert.ifError(err) - - var received = accounts[0].toLowerCase() - var expected = assertion.account.toLowerCase() - assert.equal(received, expected) - - - idStore.submitPassword(password, function(err, account) { - assert.ifError(err) - assert.equal(account, expected) - done() - }) - }) - }) }) }) diff --git a/ui/app/accounts/account-list-item.js b/ui/app/accounts/account-list-item.js index 4e0d69ed7..0b4acdfec 100644 --- a/ui/app/accounts/account-list-item.js +++ b/ui/app/accounts/account-list-item.js @@ -7,14 +7,14 @@ const EthBalance = require('../components/eth-balance') const CopyButton = require('../components/copyButton') const Identicon = require('../components/identicon') -module.exports = AccountListItem +module.exports = NewComponent -inherits(AccountListItem, Component) -function AccountListItem () { +inherits(NewComponent, Component) +function NewComponent () { Component.call(this) } -AccountListItem.prototype.render = function () { +NewComponent.prototype.render = function () { const identity = this.props.identity var isSelected = this.props.selectedAddress === identity.address var account = this.props.accounts[identity.address] @@ -23,6 +23,9 @@ AccountListItem.prototype.render = function () { return ( h(`.accounts-list-option.flex-row.flex-space-between.pointer.hover-white${selectedClass}`, { key: `account-panel-${identity.address}`, + style: { + flex: '1 0 auto', + }, onClick: (event) => this.props.onShowDetail(identity.address, event), }, [ @@ -70,7 +73,7 @@ AccountListItem.prototype.render = function () { ) } -AccountListItem.prototype.pendingOrNot = function () { +NewComponent.prototype.pendingOrNot = function () { const pending = this.props.pending if (pending.length === 0) return null return h('.pending-dot', pending.length) diff --git a/ui/app/accounts/index.js b/ui/app/accounts/index.js index d3c84d387..c20900c1e 100644 --- a/ui/app/accounts/index.js +++ b/ui/app/accounts/index.js @@ -84,7 +84,7 @@ AccountsScreen.prototype.render = function () { }) }), - h('hr.horizontal-line'), + h('hr.horizontal-line', {key: 'horizontal-line1'}), h('div.footer.hover-white.pointer', { key: 'reveal-account-bar', onClick: () => { @@ -92,6 +92,7 @@ AccountsScreen.prototype.render = function () { }, style: { display: 'flex', + flex: '1 0 auto', height: '40px', paddint: '10px', justifyContent: 'center', @@ -100,7 +101,6 @@ AccountsScreen.prototype.render = function () { }, [ h('i.fa.fa-plus.fa-lg', {key: ''}), ]), - h('hr.horizontal-line'), ]), unconfTxList.length ? ( diff --git a/ui/app/components/shapeshift-form.js b/ui/app/components/shapeshift-form.js index 77c0d12b1..58b7942c3 100644 --- a/ui/app/components/shapeshift-form.js +++ b/ui/app/components/shapeshift-form.js @@ -69,7 +69,7 @@ ShapeshiftForm.prototype.renderMain = function () { h('input#fromCoin.buy-inputs.ex-coins', { type: 'text', list: 'coinList', - dataSet: { + dataset: { persistentFormId: 'input-coin', }, style: { @@ -165,7 +165,7 @@ ShapeshiftForm.prototype.renderMain = function () { h('input#fromCoinAddress.buy-inputs', { type: 'text', placeholder: `Your ${coin} Refund Address`, - dataSet: { + dataset: { persistentFormId: 'refund-address', }, style: { diff --git a/ui/app/first-time/restore-vault.js b/ui/app/first-time/restore-vault.js index c34cd7e66..4c1f21008 100644 --- a/ui/app/first-time/restore-vault.js +++ b/ui/app/first-time/restore-vault.js @@ -41,7 +41,7 @@ RestoreVaultScreen.prototype.render = function () { // wallet seed entry h('h3', 'Wallet Seed'), h('textarea.twelve-word-phrase.letter-spacey', { - dataSet: { + dataset: { persistentFormId: 'wallet-seed', }, placeholder: 'Enter your secret twelve word phrase here to restore your vault.', @@ -52,7 +52,7 @@ RestoreVaultScreen.prototype.render = function () { type: 'password', id: 'password-box', placeholder: 'New Password (min 8 chars)', - dataSet: { + dataset: { persistentFormId: 'password', }, style: { @@ -67,7 +67,7 @@ RestoreVaultScreen.prototype.render = function () { id: 'password-box-confirm', placeholder: 'Confirm Password', onKeyPress: this.onMaybeCreate.bind(this), - dataSet: { + dataset: { persistentFormId: 'password-confirmation', }, style: { diff --git a/ui/app/send.js b/ui/app/send.js index e660e90a8..009866cf7 100644 --- a/ui/app/send.js +++ b/ui/app/send.js @@ -138,7 +138,7 @@ SendTransactionScreen.prototype.render = function () { h('input.large-input', { name: 'address', placeholder: 'Recipient Address', - dataSet: { + dataset: { persistentFormId: 'recipient-address', }, }), @@ -154,7 +154,7 @@ SendTransactionScreen.prototype.render = function () { style: { marginRight: 6, }, - dataSet: { + dataset: { persistentFormId: 'tx-amount', }, }), @@ -192,7 +192,7 @@ SendTransactionScreen.prototype.render = function () { width: '100%', resize: 'none', }, - dataSet: { + dataset: { persistentFormId: 'tx-data', }, }),