Merge pull request #878 from MetaMask/i843-MoveSaltIntoEncryptor

Simplify Encryptor API Surface

4 files changed, 42 insertions, 49 deletions

diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js
index 7020c188a..4fa2b4ee8 100644
--- a/app/scripts/keyring-controller.js
+++ b/app/scripts/keyring-controller.js
@@ -55,7 +55,7 @@ module.exports = class KeyringController extends EventEmitter {
     return {
       seedWords: this.configManager.getSeedWords(),
       isInitialized: (!!wallet || !!vault),
-      isUnlocked: !!this.key,
+      isUnlocked: Boolean(this.password),
       isDisclaimerConfirmed: this.configManager.getConfirmedDisclaimer(), // AUDIT this.configManager.getConfirmedDisclaimer(),
       unconfTxs: this.configManager.unconfirmedTxs(),
       transactions: this.configManager.getTxList(),
@@ -113,34 +113,24 @@ module.exports = class KeyringController extends EventEmitter {
     })
   }
 
-  migrateAndGetKey (password) {
-    let key
+  migrateOldVaultIfAny (password) {
     const shouldMigrate = !!this.configManager.getWallet() && !this.configManager.getVault()
-    return this.loadKey(password)
-    .then((derivedKey) => {
-      key = derivedKey
-      this.key = key
-      return this.idStoreMigrator.migratedVaultForPassword(password)
-    })
+    return this.idStoreMigrator.migratedVaultForPassword(password)
     .then((serialized) => {
       if (serialized && shouldMigrate) {
         const keyring = this.restoreKeyring(serialized)
         this.keyrings.push(keyring)
         this.configManager.setSelectedAccount(keyring.getAccounts()[0])
         return this.persistAllKeyrings()
-        .then(() => { return key })
       }
-      return key
+      return
     })
   }
 
   createNewVault (password, cb) {
-    const configManager = this.configManager
-    const salt = this.encryptor.generateSalt()
-    configManager.setSalt(salt)
-
-    return this.migrateAndGetKey(password)
+    return this.migrateOldVaultIfAny(password)
     .then(() => {
+      this.password = password
       return this.persistAllKeyrings()
     })
     .then(() => {
@@ -184,9 +174,9 @@ module.exports = class KeyringController extends EventEmitter {
   }
 
   submitPassword (password, cb) {
-    this.migrateAndGetKey(password)
-    .then((key) => {
-      return this.unlockKeyrings(key)
+    this.migrateOldVaultIfAny(password)
+    .then(() => {
+      return this.unlockKeyrings(password)
     })
     .then((keyrings) => {
       this.keyrings = keyrings
@@ -200,16 +190,6 @@ module.exports = class KeyringController extends EventEmitter {
     })
   }
 
-  loadKey (password) {
-    const salt = this.configManager.getSalt() || this.encryptor.generateSalt()
-    return this.encryptor.keyFromPassword(password + salt)
-    .then((key) => {
-      this.key = key
-      this.configManager.setSalt(salt)
-      return key
-    })
-  }
-
   addNewKeyring (type, opts, cb) {
     const Keyring = this.getKeyringClassForType(type)
     const keyring = new Keyring(opts)
@@ -285,16 +265,16 @@ module.exports = class KeyringController extends EventEmitter {
         data: keyring.serialize(),
       }
     })
-    return this.encryptor.encryptWithKey(this.key, serialized)
+    return this.encryptor.encrypt(this.password, serialized)
     .then((encryptedString) => {
       this.configManager.setVault(encryptedString)
       return true
     })
   }
 
-  unlockKeyrings (key) {
+  unlockKeyrings (password) {
     const encryptedVault = this.configManager.getVault()
-    return this.encryptor.decryptWithKey(key, encryptedVault)
+    return this.encryptor.decrypt(this.password, encryptedVault)
     .then((vault) => {
       vault.forEach(this.restoreKeyring.bind(this))
       return this.keyrings
@@ -402,7 +382,7 @@ module.exports = class KeyringController extends EventEmitter {
       })
     }
 
-    function estimateGas(txData, blockGasLimitHex, cb) {
+    function estimateGas (txData, blockGasLimitHex, cb) {
       const txParams = txData.txParams
       // check if gasLimit is already specified
       txData.gasLimitSpecified = Boolean(txParams.gas)
@@ -414,7 +394,7 @@ module.exports = class KeyringController extends EventEmitter {
       query.estimateGas(txParams, cb)
     }
 
-    function checkForGasError(txData, estimatedGasHex) {
+    function checkForGasError (txData, estimatedGasHex) {
       txData.estimatedGas = estimatedGasHex
       // all gas used - must be an error
       if (estimatedGasHex === txData.txParams.gas) {
@@ -423,7 +403,7 @@ module.exports = class KeyringController extends EventEmitter {
       cb()
     }
 
-    function setTxGas(txData, blockGasLimitHex) {
+    function setTxGas (txData, blockGasLimitHex) {
       const txParams = txData.txParams
       // if OOG, nothing more to do
       if (txData.simulationFails) {
diff --git a/app/scripts/lib/encryptor.js b/app/scripts/lib/encryptor.js
index df72b62c0..8bacab766 100644
--- a/app/scripts/lib/encryptor.js
+++ b/app/scripts/lib/encryptor.js
@@ -26,10 +26,16 @@ module.exports = {
 
 // Takes a Pojo, returns cypher text.
 function encrypt (password, dataObj) {
-  return keyFromPassword(password)
+  const salt = this.generateSalt()
+
+  return keyFromPassword(password + salt)
   .then(function (passwordDerivedKey) {
     return encryptWithKey(passwordDerivedKey, dataObj)
   })
+  .then(function (payload) {
+    payload.salt = salt
+    return JSON.stringify(payload)
+  })
 }
 
 function encryptWithKey (key, dataObj) {
@@ -44,22 +50,26 @@ function encryptWithKey (key, dataObj) {
     var buffer = new Uint8Array(buf)
     var vectorStr = encodeBufferToBase64(vector)
     var vaultStr = encodeBufferToBase64(buffer)
-    return `${vaultStr}\\${vectorStr}`
+    return {
+      data: vaultStr,
+      iv: vectorStr,
+    }
   })
 }
 
 // Takes encrypted text, returns the restored Pojo.
 function decrypt (password, text) {
-  return keyFromPassword(password)
+  const payload = JSON.parse(text)
+  const salt = payload.salt
+  return keyFromPassword(password + salt)
   .then(function (key) {
-    return decryptWithKey(key, text)
+    return decryptWithKey(key, payload)
   })
 }
 
-function decryptWithKey (key, text) {
-  const parts = text.split('\\')
-  const encryptedData = decodeBase64ToBuffer(parts[0])
-  const vector = decodeBase64ToBuffer(parts[1])
+function decryptWithKey (key, payload) {
+  const encryptedData = decodeBase64ToBuffer(payload.data)
+  const vector = decodeBase64ToBuffer(payload.iv)
   return crypto.subtle.decrypt({name: 'AES-GCM', iv: vector}, key, encryptedData)
   .then(function (result) {
     const decryptedData = new Uint8Array(result)
diff --git a/test/unit/keyring-controller-test.js b/test/unit/keyring-controller-test.js
index 437441e0e..a58043c7a 100644
--- a/test/unit/keyring-controller-test.js
+++ b/test/unit/keyring-controller-test.js
@@ -82,13 +82,16 @@ describe('KeyringController', function() {
 
   })
 
-  describe('#migrateAndGetKey', function() {
-    it('should return the key for that password', function(done) {
-      keyringController.migrateAndGetKey(password)
-      .then((key) => {
-        assert(key, 'a key is returned')
+  describe('#migrateOldVaultIfAny', function() {
+    it('should return and init a new vault', function(done) {
+      keyringController.migrateOldVaultIfAny(password)
+      .then(() => {
+        assert(keyringController.configManager.getVault(), 'now has a vault')
         done()
       })
+      .catch((reason) => {
+        assert.ifError(reason)
+      })
     })
   })
 
diff --git a/ui/lib/account-link.js b/ui/lib/account-link.js
index ff52d9c54..77db0851d 100644
--- a/ui/lib/account-link.js
+++ b/ui/lib/account-link.js
@@ -1,4 +1,4 @@
-module.exports = function(address, network) {
+module.exports = function (address, network) {
   const net = parseInt(network)
   let link