Simplify Encryptor API Surface

At least, the portion of it that we use.

Moved salting within the encryptor, so it does not need to be managed externally.

KeyringController now caches the password instead of a passwordDerivedKey, since it is ignorant of the salt.

Encryptor payload is now in a JSON format, so its portions are both base64 encoded *and* labeled appropriately.  The format is `{ "data": "0x0", "iv": "0x0", "salt": "string" }`.

Fixes #843
Fixes #859

app/scripts/keyring-controller.js

@@ -113,34 +113,25 @@ module.exports = class KeyringController extends EventEmitter {
     })
   }
 
-  migrateAndGetKey (password) {
+  migrateOldVaultIfAny (password) {
-    let key
     const shouldMigrate = !!this.configManager.getWallet() && !this.configManager.getVault()
-    return this.loadKey(password)
-    .then((derivedKey) => {
-      key = derivedKey
-      this.key = key
     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 })
+        .then(() => { return })
       }
-      return key
+      return
     })
   }
 
   createNewVault (password, cb) {
-    const configManager = this.configManager
+    return this.migrateOldVaultIfAny(password)
-    const salt = this.getSalt()
-    configManager.setSalt(salt)
-
-    return this.migrateAndGetKey(password)
     .then(() => {
+      this.password = password
       return this.persistAllKeyrings()
     })
     .then(() => {
@@ -184,8 +175,8 @@ module.exports = class KeyringController extends EventEmitter {
   }
 
   submitPassword (password, cb) {
-    this.migrateAndGetKey(password)
+    this.migrateOldVaultIfAny(password)
-    .then((key) => {
+    .then(() => {
       return this.unlockKeyrings(password)
     })
     .then((keyrings) => {
@@ -200,21 +191,6 @@ module.exports = class KeyringController extends EventEmitter {
     })
   }
 
-  loadKey (password) {
-    const salt = this.getSalt()
-    return this.encryptor.keyFromPassword(password + salt)
-    .then((key) => {
-      this.key = key
-      this.configManager.setSalt(salt)
-      return key
-    })
-  }
-
-  getSalt () {
-    const vault = this.configManager.getVault()
-    const salt = vault.salt || this.encryptor.generateSalt()
-  }
-
   addNewKeyring (type, opts, cb) {
     const Keyring = this.getKeyringClassForType(type)
     const keyring = new Keyring(opts)
@@ -290,7 +266,7 @@ 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
@@ -299,7 +275,7 @@ module.exports = class KeyringController extends EventEmitter {
 
   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

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) {
+function decryptWithKey (key, payload) {
-  const parts = text.split('\\')
+  const encryptedData = decodeBase64ToBuffer(payload.data)
-  const encryptedData = decodeBase64ToBuffer(parts[0])
+  const vector = decodeBase64ToBuffer(payload.iv)
-  const vector = decodeBase64ToBuffer(parts[1])
   return crypto.subtle.decrypt({name: 'AES-GCM', iv: vector}, key, encryptedData)
   .then(function (result) {
     const decryptedData = new Uint8Array(result)

test/unit/keyring-controller-test.js

@@ -82,13 +82,15 @@ describe('KeyringController', function() {
 
   })
 
-  describe('#migrateAndGetKey', function() {
+  describe('#migrateOldVaultIfAny', function() {
     it('should return the key for that password', function(done) {
-      keyringController.migrateAndGetKey(password)
+      keyringController.migrateOldVaultIfAny(password)
-      .then((key) => {
+      .then(() => {
-        assert(key, 'a key is returned')
         done()
       })
+      .catch((reason) => {
+        assert.ifError(reason)
+      })
     })
   })