Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Olm account to IndexedDB #579

Merged
merged 16 commits into from
Nov 27, 2017
101 changes: 66 additions & 35 deletions src/crypto/OlmDevice.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,15 @@ function checkPayloadLength(payloadString) {
* @alias module:crypto/OlmDevice
*
* @param {Object} sessionStore A store to be used for data in end-to-end
* crypto
* crypto. This is deprecated and being replaced by cryptoStore.
* @param {Object} cryptoStore A store for crypto data
*
* @property {string} deviceCurve25519Key Curve25519 key for the account
* @property {string} deviceEd25519Key Ed25519 key for the account
*/
function OlmDevice(sessionStore) {
function OlmDevice(sessionStore, cryptoStore) {
this._sessionStore = sessionStore;
this._cryptoStore = cryptoStore;
this._pickleKey = "DEFAULT_KEY";

// don't know these until we load the account from storage in init()
Expand Down Expand Up @@ -124,7 +126,9 @@ OlmDevice.prototype.init = async function() {
let e2eKeys;
const account = new Olm.Account();
try {
_initialise_account(this._sessionStore, this._pickleKey, account);
await _initialise_account(
this._sessionStore, this._cryptoStore, this._pickleKey, account,
);
e2eKeys = JSON.parse(account.identity_keys());

this._maxOneTimeKeys = account.max_number_of_one_time_keys();
Expand All @@ -137,16 +141,29 @@ OlmDevice.prototype.init = async function() {
};


function _initialise_account(sessionStore, pickleKey, account) {
const e2eAccount = sessionStore.getEndToEndAccount();
if (e2eAccount !== null) {
account.unpickle(pickleKey, e2eAccount);
return;
}
async function _initialise_account(sessionStore, cryptoStore, pickleKey, account) {
let removeFromSessionStore = false;
await cryptoStore.endToEndAccountTransaction((pickledAccount, save) => {
if (pickledAccount !== null) {
account.unpickle(pickleKey, pickledAccount);
} else {
// Migrate from sessionStore
pickledAccount = sessionStore.getEndToEndAccount();
if (pickledAccount !== null) {
removeFromSessionStore = true;
account.unpickle(pickleKey, pickledAccount);
} else {
account.create();
pickledAccount = account.pickle(pickleKey);
}
save(pickledAccount);
}
});

account.create();
const pickled = account.pickle(pickleKey);
sessionStore.storeEndToEndAccount(pickled);
// only remove this once it's safely saved to the crypto store
if (removeFromSessionStore) {
sessionStore.removeEndToEndAccount();
}
}

/**
Expand All @@ -158,21 +175,35 @@ OlmDevice.getOlmVersion = function() {


/**
* extract our OlmAccount from the session store and call the given 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.
* The function will not be awaited upon and the save function must be
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is maybe a bit misleading. The function will be awaited upon, by virtue of any promise it returns being returned by this async function - so the promise returned by func must resolve before the promise returned by _getAccount will resolve.

It would probably be clearer to say "The account will be freed as soon as func returns - even if func returns a promise".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, agreed. done.

* called before the function returns, or not at all.
*
* @param {function} func
* @return {object} result of func
* @private
*/
OlmDevice.prototype._getAccount = function(func) {
const account = new Olm.Account();
try {
const pickledAccount = this._sessionStore.getEndToEndAccount();
account.unpickle(this._pickleKey, pickledAccount);
return func(account);
} finally {
account.free();
}
OlmDevice.prototype._getAccount = async function(func) {
let result;

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);

result = func(account, () => {
const pickledAccount = account.pickle(this._pickleKey);
save(pickledAccount);
});
} finally {
account.free();
}
});
return result;
};


Expand All @@ -182,9 +213,9 @@ OlmDevice.prototype._getAccount = function(func) {
* @param {OlmAccount} account
* @private
*/
OlmDevice.prototype._saveAccount = function(account) {
OlmDevice.prototype._saveAccount = async function(account) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is presumably no longer used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, it is not.

const pickledAccount = account.pickle(this._pickleKey);
this._sessionStore.storeEndToEndAccount(pickledAccount);
await this._cryptoStore.storeEndToEndAccount(pickledAccount);
};


Expand Down Expand Up @@ -250,7 +281,7 @@ OlmDevice.prototype._getUtility = function(func) {
* @return {Promise<string>} base64-encoded signature
*/
OlmDevice.prototype.sign = async function(message) {
return this._getAccount(function(account) {
return await this._getAccount(function(account) {
return account.sign(message);
});
};
Expand All @@ -263,7 +294,7 @@ OlmDevice.prototype.sign = async function(message) {
* key.
*/
OlmDevice.prototype.getOneTimeKeys = async function() {
return this._getAccount(function(account) {
return await this._getAccount(function(account) {
return JSON.parse(account.one_time_keys());
});
};
Expand All @@ -282,23 +313,22 @@ OlmDevice.prototype.maxNumberOfOneTimeKeys = function() {
* Marks all of the one-time keys as published.
*/
OlmDevice.prototype.markKeysAsPublished = async function() {
const self = this;
this._getAccount(function(account) {
await this._getAccount(function(account, save) {
account.mark_keys_as_published();
self._saveAccount(account);
save();
});
};

/**
* Generate some new one-time keys
*
* @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) {
const self = this;
this._getAccount(function(account) {
return this._getAccount(function(account, save) {
account.generate_one_time_keys(numKeys);
self._saveAccount(account);
save();
});
};

Expand All @@ -315,10 +345,11 @@ OlmDevice.prototype.createOutboundSession = async function(
theirIdentityKey, theirOneTimeKey,
) {
const self = this;
return this._getAccount(function(account) {
return await this._getAccount(function(account, save) {
const session = new Olm.Session();
try {
session.create_outbound(account, theirIdentityKey, theirOneTimeKey);
save();
self._saveSession(theirIdentityKey, session);
return session.session_id();
} finally {
Expand Down Expand Up @@ -349,12 +380,12 @@ OlmDevice.prototype.createInboundSession = async function(
}

const self = this;
return this._getAccount(function(account) {
return await this._getAccount(function(account, save) {
const session = new Olm.Session();
try {
session.create_inbound_from(account, theirDeviceIdentityKey, ciphertext);
account.remove_one_time_keys(session);
self._saveAccount(account);
save();

const payloadString = session.decrypt(message_type, ciphertext);

Expand Down
2 changes: 1 addition & 1 deletion src/crypto/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function Crypto(baseApis, sessionStore, userId, deviceId,
this._clientStore = clientStore;
this._cryptoStore = cryptoStore;

this._olmDevice = new OlmDevice(sessionStore);
this._olmDevice = new OlmDevice(sessionStore, cryptoStore);
this._deviceList = new DeviceList(baseApis, sessionStore, this._olmDevice);

// the last time we did a check for the number of one-time-keys on the
Expand Down
41 changes: 40 additions & 1 deletion src/crypto/store/indexeddb-crypto-store-backend.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Promise from 'bluebird';
import utils from '../../utils';

export const VERSION = 1;
export const VERSION = 2;

/**
* Implementation of a CryptoStore which is backed by an existing
Expand Down Expand Up @@ -257,6 +257,38 @@ export class Backend {
};
return promiseifyTxn(txn);
}

/**
* Load the end to end account for the logged-in user. Once the account
* is retrieved, the given function is executed and passed the base64
* encoded account string and a method for saving the account string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/base64 encoded/pickled/? And s/account string/pickle/?

Might be more consistent. just a thought.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, wfm

* back to the database. This allows the account to be read and writen
* atomically.
* @param {func} func Function called with the account data and a save function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@param {function(string, function())} func ....

would be the correct annotation here I think

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

* @return {Promise} Resolves with the return value of the function once
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/the function/func/, possibly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

* the transaction is complete (ie. once data is written back if the
* save function is called.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dangling (

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

*/
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(
getReq.result || null,
(newData) => {
objectStore.put(newData, "-");
},
);
};
return txnPromise.then(() => {
return result;
});
}
}

export function upgradeDatabase(db, oldVersion) {
Expand All @@ -267,6 +299,9 @@ export function upgradeDatabase(db, oldVersion) {
if (oldVersion < 1) { // The database did not previously exist.
createDatabase(db);
}
if (oldVersion < 2) {
createV2Tables(db);
}
// Expand as needed.
}

Expand All @@ -283,6 +318,10 @@ function createDatabase(db) {
outgoingRoomKeyRequestsStore.createIndex("state", "state");
}

function createV2Tables(db) {
db.createObjectStore("account");
}

function promiseifyTxn(txn) {
return new Promise((resolve, reject) => {
txn.oncomplete = resolve;
Expand Down
25 changes: 24 additions & 1 deletion src/crypto/store/indexeddb-crypto-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.

import Promise from 'bluebird';

import LocalStorageCryptoStore from './localStorage-crypto-store';
import MemoryCryptoStore from './memory-crypto-store';
import * as IndexedDBCryptoStoreBackend from './indexeddb-crypto-store-backend';

Expand Down Expand Up @@ -93,7 +94,12 @@ export default class IndexedDBCryptoStore {
}).catch((e) => {
console.warn(
`unable to connect to indexeddb ${this._dbName}` +
`: falling back to in-memory store: ${e}`,
`: falling back to localStorage store: ${e}`,
);
return new LocalStorageCryptoStore();
}).catch((e) => {
console.warn(
`unable to open localStorage: falling back to in-memory store: ${e}`,
);
return new MemoryCryptoStore();
});
Expand Down Expand Up @@ -220,4 +226,21 @@ export default class IndexedDBCryptoStore {
return backend.deleteOutgoingRoomKeyRequest(requestId, expectedState);
});
}

/**
* Load the end to end account for the logged-in user. Once the 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
* atomically.
* @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
* save function is called.
*/
endToEndAccountTransaction(func) {
return this._connect().then((backend) => {
return backend.endToEndAccountTransaction(func);
});
}
}
58 changes: 58 additions & 0 deletions src/crypto/store/localStorage-crypto-store.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright 2017 New Vector Ltd

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
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.
*/

import Promise from 'bluebird';
import MemoryCryptoStore from './memory-crypto-store.js';

/**
* Internal module. Partial localStorage backed storage for e2e.
* This is not a full crypto store, just the in-memory store with
* some things backed by localStorage. It exists because indexedDB
* is broken in Firefox private mode or set to, "will not remember
* history".
*
* @module
*/

const E2E_PREFIX = "crypto.";
const KEY_END_TO_END_ACCOUNT = E2E_PREFIX + "account";

/**
* @implements {module:crypto/store/base~CryptoStore}
*/
export default class LocalStorageCryptoStore extends MemoryCryptoStore {
constructor() {
super();
this.store = global.localStorage;
}

/**
* Delete all data from this store.
*
* @returns {Promise} Promise which resolves when the store has been cleared.
*/
deleteAllData() {
this.store.removeItem(KEY_END_TO_END_ACCOUNT);
return Promise.resolve();
}

endToEndAccountTransaction(func) {
const account = this.store.getItem(KEY_END_TO_END_ACCOUNT);
return Promise.resolve(func(account, (newData) => {
this.store.setItem(KEY_END_TO_END_ACCOUNT, newData);
}));
}
}
Loading