Merge pull request #582 from matrix-org/dbkr/crypto_store_txn_api

Change crypto store transaction API

package.json

@@ -15,7 +15,7 @@
     "build": "babel -s -d lib src && rimraf dist && mkdir dist && browserify -d browser-index.js | exorcist dist/browser-matrix.js.map > dist/browser-matrix.js && uglifyjs -c -m -o dist/browser-matrix.min.js --source-map dist/browser-matrix.min.js.map --in-source-map dist/browser-matrix.js.map dist/browser-matrix.js",
     "dist": "npm run build",
     "watch": "watchify -d browser-index.js -o 'exorcist dist/browser-matrix.js.map > dist/browser-matrix.js' -v",
-    "lint": "eslint --max-warnings 109 src spec",
+    "lint": "eslint --max-warnings 102 src spec",
     "prepublish": "npm run clean && npm run build && git rev-parse HEAD > git-revision.txt"
   },
   "repository": {

src/crypto/OlmDevice.js

@@ -14,7 +14,8 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 See the License for the specific language governing permissions and
 limitations under the License.
 */
-"use strict";
+
+import IndexedDBCryptoStore from './store/indexeddb-crypto-store';
 
 /**
  * olm.js wrapper
@@ -127,7 +128,7 @@ OlmDevice.prototype.init = async function() {
     let e2eKeys;
     const account = new Olm.Account();
     try {
-        await _initialise_account(
+        await _initialiseAccount(
             this._sessionStore, this._cryptoStore, this._pickleKey, account,
         );
         e2eKeys = JSON.parse(account.identity_keys());
@@ -142,23 +143,26 @@ OlmDevice.prototype.init = async function() {
 };
 
 
-async function _initialise_account(sessionStore, cryptoStore, pickleKey, account) {
+async function _initialiseAccount(sessionStore, cryptoStore, pickleKey, account) {
     let removeFromSessionStore = false;
-    await cryptoStore.endToEndAccountTransaction((pickledAccount, save) => {
+
-        if (pickledAccount !== null) {
+    await cryptoStore.doTxn('readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => {
-            account.unpickle(pickleKey, pickledAccount);
+        cryptoStore.getAccount(txn, (pickledAccount) => {
-        } else {
-            // Migrate from sessionStore
-            pickledAccount = sessionStore.getEndToEndAccount();
             if (pickledAccount !== null) {
-                removeFromSessionStore = true;
                 account.unpickle(pickleKey, pickledAccount);
             } else {
-                account.create();
+                // Migrate from sessionStore
-                pickledAccount = account.pickle(pickleKey);
+                pickledAccount = sessionStore.getEndToEndAccount();
+                if (pickledAccount !== null) {
+                    removeFromSessionStore = true;
+                    account.unpickle(pickleKey, pickledAccount);
+                } else {
+                    account.create();
+                    pickledAccount = account.pickle(pickleKey);
+                }
+                cryptoStore.storeAccount(txn, pickledAccount);
             }
-            save(pickledAccount);
+        });
-        }
     });
 
     // only remove this once it's safely saved to the crypto store
@@ -177,36 +181,41 @@ OlmDevice.getOlmVersion = function() {
 
 /**
  * extract our OlmAccount from the crypto store and call the given function
- * with the account object and a 'save' function which returns a promise.
+ * with the account object
- * The `account` will be freed as soon as `func` returns - even if func returns
+ * The `account` object is useable only within the callback passed to this
- * a promise
+ * function and will be freed as soon the callback returns. It is *not*
+ * useable for the rest of the lifetime of the transaction.
+ * This function requires a live transaction object from cryptoStore.doTxn()
+ * and therefore may only be called in a doTxn() callback.
  *
+ * @param {*} txn Opaque transaction object from cryptoStore.doTxn()
  * @param {function} func
- * @return {object} result of func
  * @private
  */
-OlmDevice.prototype._getAccount = async function(func) {
+OlmDevice.prototype._getAccount = function(txn, func) {
-    let result;
+    this._cryptoStore.getAccount(txn, (pickledAccount) => {
-
-    await this._cryptoStore.endToEndAccountTransaction((pickledAccount, save) => {
-        // Olm has a limited heap size so we must tightly control the number of
-        // Olm account objects in existence at any given time: once created, it
-        // must be destroyed again before we await.
         const account = new Olm.Account();
         try {
             account.unpickle(this._pickleKey, pickledAccount);
-
+            func(account);
-            result = func(account, () => {
-                const pickledAccount = account.pickle(this._pickleKey);
-                save(pickledAccount);
-            });
         } finally {
             account.free();
         }
     });
-    return result;
 };
 
+/*
+ * Saves an account to the crypto store.
+ * This function requires a live transaction object from cryptoStore.doTxn()
+ * and therefore may only be called in a doTxn() callback.
+ *
+ * @param {*} txn Opaque transaction object from cryptoStore.doTxn()
+ * @param {object} Olm.Account object
+ * @private
+ */
+OlmDevice.prototype._storeAccount = function(txn, account) {
+    this._cryptoStore.storeAccount(txn, account.pickle(this._pickleKey));
+};
 
 /**
  * extract an OlmSession from the session store and call the given function
@@ -270,9 +279,16 @@ OlmDevice.prototype._getUtility = function(func) {
  * @return {Promise<string>} base64-encoded signature
  */
 OlmDevice.prototype.sign = async function(message) {
-    return await this._getAccount(function(account) {
+    let result;
-        return account.sign(message);
+    await this._cryptoStore.doTxn(
+        'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT],
+        (txn) => {
+            this._getAccount(txn, (account) => {
+                result = account.sign(message);
+            },
+        );
     });
+    return result;
 };
 
 /**
@@ -283,9 +299,17 @@ OlmDevice.prototype.sign = async function(message) {
  * key.
  */
 OlmDevice.prototype.getOneTimeKeys = async function() {
-    return await this._getAccount(function(account) {
+    let result;
-        return JSON.parse(account.one_time_keys());
+    await this._cryptoStore.doTxn(
-    });
+        'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT],
+        (txn) => {
+            this._getAccount(txn, (account) => {
+                result = JSON.parse(account.one_time_keys());
+            });
+        },
+    );
+
+    return result;
 };
 
 
@@ -302,10 +326,15 @@ OlmDevice.prototype.maxNumberOfOneTimeKeys = function() {
  * Marks all of the one-time keys as published.
  */
 OlmDevice.prototype.markKeysAsPublished = async function() {
-    await this._getAccount(function(account, save) {
+    await this._cryptoStore.doTxn(
-        account.mark_keys_as_published();
+        'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
-        save();
+        (txn) => {
-    });
+            this._getAccount(txn, (account) => {
+                account.mark_keys_as_published();
+                this._storeAccount(txn, account);
+            });
+        },
+    );
 };
 
 /**
@@ -314,11 +343,16 @@ OlmDevice.prototype.markKeysAsPublished = async function() {
  * @param {number} numKeys number of keys to generate
  * @return {Promise} Resolved once the account is saved back having generated the keys
  */
-OlmDevice.prototype.generateOneTimeKeys = async function(numKeys) {
+OlmDevice.prototype.generateOneTimeKeys = function(numKeys) {
-    return this._getAccount(function(account, save) {
+    return this._cryptoStore.doTxn(
-        account.generate_one_time_keys(numKeys);
+        'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
-        save();
+        (txn) => {
-    });
+            this._getAccount(txn, (account) => {
+                account.generate_one_time_keys(numKeys);
+                this._storeAccount(txn, account);
+            });
+        },
+    );
 };
 
 /**
@@ -333,18 +367,25 @@ OlmDevice.prototype.generateOneTimeKeys = async function(numKeys) {
 OlmDevice.prototype.createOutboundSession = async function(
     theirIdentityKey, theirOneTimeKey,
 ) {
-    const self = this;
+    let newSessionId;
-    return await this._getAccount(function(account, save) {
+    await this._cryptoStore.doTxn(
-        const session = new Olm.Session();
+        'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
-        try {
+        (txn) => {
-            session.create_outbound(account, theirIdentityKey, theirOneTimeKey);
+            this._getAccount(txn, (account) => {
-            save();
+                const session = new Olm.Session();
-            self._saveSession(theirIdentityKey, session);
+                try {
-            return session.session_id();
+                    session.create_outbound(account, theirIdentityKey, theirOneTimeKey);
-        } finally {
+                    newSessionId = session.session_id();
-            session.free();
+                    this._storeAccount(txn, account);
-        }
+                    this._saveSession(theirIdentityKey, session);
-    });
+                    return session.session_id();
+                } finally {
+                    session.free();
+                }
+            });
+        },
+    );
+    return newSessionId;
 };
 
 
@@ -352,7 +393,7 @@ OlmDevice.prototype.createOutboundSession = async function(
  * Generate a new inbound session, given an incoming message
  *
  * @param {string} theirDeviceIdentityKey remote user's Curve25519 identity key
- * @param {number} message_type  message_type field from the received message (must be 0)
+ * @param {number} messageType  messageType field from the received message (must be 0)
  * @param {string} ciphertext base64-encoded body from the received message
  *
  * @return {{payload: string, session_id: string}} decrypted payload, and
@@ -362,32 +403,41 @@ OlmDevice.prototype.createOutboundSession = async function(
  *     didn't use a valid one-time key).
  */
 OlmDevice.prototype.createInboundSession = async function(
-    theirDeviceIdentityKey, message_type, ciphertext,
+    theirDeviceIdentityKey, messageType, ciphertext,
 ) {
-    if (message_type !== 0) {
+    if (messageType !== 0) {
-        throw new Error("Need message_type == 0 to create inbound session");
+        throw new Error("Need messageType == 0 to create inbound session");
     }
 
-    const self = this;
+    let result;
-    return await this._getAccount(function(account, save) {
+    await this._cryptoStore.doTxn(
-        const session = new Olm.Session();
+        'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
-        try {
+        (txn) => {
-            session.create_inbound_from(account, theirDeviceIdentityKey, ciphertext);
+            this._getAccount(txn, (account) => {
-            account.remove_one_time_keys(session);
+                const session = new Olm.Session();
-            save();
+                try {
+                    session.create_inbound_from(
+                        account, theirDeviceIdentityKey, ciphertext,
+                    );
+                    account.remove_one_time_keys(session);
+                    this._storeAccount(txn, account);
 
-            const payloadString = session.decrypt(message_type, ciphertext);
+                    const payloadString = session.decrypt(messageType, ciphertext);
 
-            self._saveSession(theirDeviceIdentityKey, session);
+                    this._saveSession(theirDeviceIdentityKey, session);
 
-            return {
+                    result = {
-                payload: payloadString,
+                        payload: payloadString,
-                session_id: session.session_id(),
+                        session_id: session.session_id(),
-            };
+                    };
-        } finally {
+                } finally {
-            session.free();
+                    session.free();
-        }
+                }
-    });
+            });
+        },
+    );
+
+    return result;
 };
 
 
@@ -484,18 +534,18 @@ OlmDevice.prototype.encryptMessage = async function(
  * @param {string} theirDeviceIdentityKey Curve25519 identity key for the
  *     remote device
  * @param {string} sessionId  the id of the active session
- * @param {number} message_type  message_type field from the received message
+ * @param {number} messageType  messageType field from the received message
  * @param {string} ciphertext base64-encoded body from the received message
  *
  * @return {Promise<string>} decrypted payload.
  */
 OlmDevice.prototype.decryptMessage = async function(
-    theirDeviceIdentityKey, sessionId, message_type, ciphertext,
+    theirDeviceIdentityKey, sessionId, messageType, ciphertext,
 ) {
     const self = this;
 
     return this._getSession(theirDeviceIdentityKey, sessionId, function(session) {
-        const payloadString = session.decrypt(message_type, ciphertext);
+        const payloadString = session.decrypt(messageType, ciphertext);
         self._saveSession(theirDeviceIdentityKey, session);
 
         return payloadString;
@@ -508,16 +558,16 @@ OlmDevice.prototype.decryptMessage = async function(
  * @param {string} theirDeviceIdentityKey Curve25519 identity key for the
  *     remote device
  * @param {string} sessionId  the id of the active session
- * @param {number} message_type  message_type field from the received message
+ * @param {number} messageType  messageType field from the received message
  * @param {string} ciphertext base64-encoded body from the received message
  *
  * @return {Promise<boolean>} true if the received message is a prekey message which matches
  *    the given session.
  */
 OlmDevice.prototype.matchesSession = async function(
-    theirDeviceIdentityKey, sessionId, message_type, ciphertext,
+    theirDeviceIdentityKey, sessionId, messageType, ciphertext,
 ) {
-    if (message_type !== 0) {
+    if (messageType !== 0) {
         return false;
     }
 

src/crypto/store/indexeddb-crypto-store-backend.js

@@ -258,35 +258,23 @@ export class Backend {
         return promiseifyTxn(txn);
     }
 
-    /**
+    getAccount(txn, func) {
-     * Load the end to end account for the logged-in user. Once the account
-     * is retrieved, the given function is executed and passed the pickled
-     * account string and a method for saving the pickle
-     * back to the database. This allows the account to be read and writen
-     * atomically.
-     * @param {function(string, function())} func Function called with the
-     *     picked account and a save function
-     * @return {Promise} Resolves with the return value of `func` once
-     *     the transaction is complete (ie. once data is written back if the
-     *     save function is called.)
-     */
-    endToEndAccountTransaction(func) {
-        const txn = this._db.transaction("account", "readwrite");
         const objectStore = txn.objectStore("account");
-
-        const txnPromise = promiseifyTxn(txn);
-
         const getReq = objectStore.get("-");
-        let result;
         getReq.onsuccess = function() {
-            result = func(
+            func(getReq.result || null);
-                getReq.result || null,
-                (newData) => {
-                    objectStore.put(newData, "-");
-                },
-            );
         };
-        return txnPromise.then(() => {
+    }
+
+    storeAccount(txn, newData) {
+        const objectStore = txn.objectStore("account");
+        objectStore.put(newData, "-");
+    }
+
+    doTxn(mode, stores, func) {
+        const txn = this._db.transaction(stores, mode);
+        const result = func(txn);
+        return promiseifyTxn(txn).then(() => {
             return result;
         });
     }

src/crypto/store/indexeddb-crypto-store.js

@@ -227,21 +227,50 @@ export default class IndexedDBCryptoStore {
         });
     }
 
-    /**
+    /*
-     * Load the end to end account for the logged-in user. Once the account
+     * Get the account pickle from the store.
-     * is retrieved, the given function is executed and passed the pickled
+     * This requires an active transaction. See doTxn().
-     * account string and a method for saving the pickle
+     *
-     * back to the database. This allows the account to be read and writen
+     * @param {*} txn An active transaction. See doTxn().
-     * atomically.
+     * @param {function(string)} func Called with the account pickle
-     * @param {function(string, function())} func Function called with the
-     *     account data and a save function
-     * @return {Promise} Resolves with the return value of `func` once
-     *     the transaction is complete (ie. once data is written back if the
-     *     save function is called.)
      */
-    endToEndAccountTransaction(func) {
+    getAccount(txn, func) {
+        this._backendPromise.value().getAccount(txn, func);
+    }
+
+    /*
+     * Write the account pickle to the store.
+     * This requires an active transaction. See doTxn().
+     *
+     * @param {*} txn An active transaction. See doTxn().
+     * @param {string} newData The new account pickle to store.
+     */
+    storeAccount(txn, newData) {
+        this._backendPromise.value().storeAccount(txn, newData);
+    }
+
+    /**
+     * Perform a transaction on the crypto store. Any store methods
+     * that require a transaction (txn) object to be passed in may
+     * only be called within a callback of either this function or
+     * one of the store functions operating on the same transaction.
+     *
+     * @param {string} mode 'readwrite' if you need to call setter
+     *     functions with this transaction. Otherwise, 'readonly'.
+     * @param {string[]} stores List IndexedDBCryptoStore.STORE_*
+     *     options representing all types of object that will be
+     *     accessed or written to with this transaction.
+     * @param {function(*)} func Function called with the
+     *     transaction object: an opaque object that should be passed
+     *     to store functions.
+     * @return {Promise} Promise that resolves with the result of the `func`
+     *     when the transaction is complete
+     */
+    doTxn(mode, stores, func) {
         return this._connect().then((backend) => {
-            return backend.endToEndAccountTransaction(func);
+            return backend.doTxn(mode, stores, func);
         });
     }
 }
+
+IndexedDBCryptoStore.STORE_ACCOUNT = 'account';

src/crypto/store/localStorage-crypto-store.js

@@ -49,10 +49,16 @@ export default class LocalStorageCryptoStore extends MemoryCryptoStore {
         return Promise.resolve();
     }
 
-    endToEndAccountTransaction(func) {
+    getAccount(txn, func) {
         const account = this.store.getItem(KEY_END_TO_END_ACCOUNT);
-        return Promise.resolve(func(account, (newData) => {
+        func(account);
-            this.store.setItem(KEY_END_TO_END_ACCOUNT, newData);
+    }
-        }));
+
+    storeAccount(txn, newData) {
+        this.store.setItem(KEY_END_TO_END_ACCOUNT, newData);
+    }
+
+    doTxn(mode, stores, func) {
+        return Promise.resolve(func(null));
     }
 }

src/crypto/store/memory-crypto-store.js

@@ -198,20 +198,15 @@ export default class MemoryCryptoStore {
         return Promise.resolve(null);
     }
 
-    /**
+    getAccount(txn, func) {
-     * Load the end to end account for the logged-in user. Once the account
+        func(this._account);
-     * is retrieved, the given function is executed and passed the base64
+    }
-     * encoded account string and a method for saving the account string
+
-     * back to the database. This allows the account to be read and writen
+    storeAccount(txn, newData) {
-     * atomically.
+        this._account = newData;
-     * @param {func} func Function called with the account data and a save function
+    }
-     * @return {Promise} Resolves with the return value of the function once
+
-     *     the transaction is complete (ie. once data is written back if the
+    doTxn(mode, stores, func) {
-     *     save function is called.
+        return Promise.resolve(func(null));
-     */
-    endToEndAccountTransaction(func) {
-        return Promise.resolve(func(this._account, (newData) => {
-            this._account = newData;
-        }));
     }
 }