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.

Sign up for GitHub

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

Jump to bottom

Change crypto store transaction API #582

Merged
merged 3 commits into from
Nov 30, 2017
Merged

Change crypto store transaction API #582

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f11a58e
Change crypto store transaction API
dbkr Nov 29, 2017
fece506
delint
dbkr Nov 29, 2017
ecc1c86
Doc some more things
dbkr Nov 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member

Choose a reason for hiding this comment

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

🎉

"prepublish": "npm run clean && npm run build && git rev-parse HEAD > git-revision.txt"
},
"repository": {
Expand Down
207 changes: 123 additions & 84 deletions src/crypto/OlmDevice.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -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) {
account.unpickle(pickleKey, pickledAccount);
} else {
// Migrate from sessionStore
pickledAccount = sessionStore.getEndToEndAccount();

await cryptoStore.doTxn('readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => {
cryptoStore.getAccount(txn, (pickledAccount) => {
if (pickledAccount !== null) {
removeFromSessionStore = true;
account.unpickle(pickleKey, pickledAccount);
} else {
account.create();
pickledAccount = account.pickle(pickleKey);
// Migrate from sessionStore
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
Expand All @@ -177,36 +181,30 @@ 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.
* The `account` will be freed as soon as `func` returns - even if func returns
* a promise
* with the account object
* The `account` object is useable only within the callback passed to this
* function and will be freed as soon the callback returns. It is *not*
* useable for the rest of the lifetime of the transaction.
*
* @param {*} txn
Copy link
Member

Choose a reason for hiding this comment

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

could you document this?

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

* @param {function} func
* @return {object} result of func
* @private
*/
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.
OlmDevice.prototype._getAccount = function(txn, func) {
this._cryptoStore.getAccount(txn, (pickledAccount) => {
const account = new Olm.Account();
try {
account.unpickle(this._pickleKey, pickledAccount);

result = func(account, () => {
const pickledAccount = account.pickle(this._pickleKey);
save(pickledAccount);
});
func(account);
Copy link
Member

Choose a reason for hiding this comment

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

should we not return the result here and at line 194?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly, but as per other comment it could get quite hairy, so probably simpler to just never do it at all.

} finally {
account.free();
}
});
return result;
};

OlmDevice.prototype._storeAccount = function(txn, account) {
Copy link
Member

Choose a reason for hiding this comment

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

some docs please

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

this._cryptoStore.storeAccount(txn, account.pickle(this._pickleKey));
};

/**
* extract an OlmSession from the session store and call the given function
Expand Down Expand Up @@ -270,9 +268,16 @@ OlmDevice.prototype._getUtility = function(func) {
* @return {Promise<string>} base64-encoded signature
*/
OlmDevice.prototype.sign = async function(message) {
return await this._getAccount(function(account) {
return account.sign(message);
let result;
Copy link
Member

Choose a reason for hiding this comment

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

again, can't we pass the result up through the callbacks to save us having to fiddle with a local result?

(perhaps it actually ends up being more confusing?)

Copy link
Member Author

Choose a reason for hiding this comment

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

we can, but it could now be quite a few callbacks, and you could be doing more than one get in the transaction, at which point it's not clear which one the transaction promise would return, so I think I'd vote for just making you assign it to a captured variable.

Copy link
Member

Choose a reason for hiding this comment

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

the transaction promise would return whatever is returned by the func passed to doTxn.

I don't think I agree with you, but I don't feel super strongly about it.

await this._cryptoStore.doTxn(
'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT],
(txn) => {
this._getAccount(txn, (account) => {
result = account.sign(message);
},
);
});
return result;
};

/**
Expand All @@ -283,9 +288,17 @@ OlmDevice.prototype.sign = async function(message) {
* key.
*/
OlmDevice.prototype.getOneTimeKeys = async function() {
return await this._getAccount(function(account) {
return JSON.parse(account.one_time_keys());
});
let result;
await this._cryptoStore.doTxn(
'readonly', [IndexedDBCryptoStore.STORE_ACCOUNT],
(txn) => {
this._getAccount(txn, (account) => {
result = JSON.parse(account.one_time_keys());
});
},
);

return result;
};


Expand All @@ -302,10 +315,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) {
account.mark_keys_as_published();
save();
});
await this._cryptoStore.doTxn(
'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
(txn) => {
this._getAccount(txn, (account) => {
account.mark_keys_as_published();
this._storeAccount(txn, account);
});
},
);
};

/**
Expand All @@ -314,11 +332,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) {
return this._getAccount(function(account, save) {
account.generate_one_time_keys(numKeys);
save();
});
OlmDevice.prototype.generateOneTimeKeys = function(numKeys) {
return this._cryptoStore.doTxn(
'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
(txn) => {
this._getAccount(txn, (account) => {
account.generate_one_time_keys(numKeys);
this._storeAccount(txn, account);
});
},
);
};

/**
Expand All @@ -333,26 +356,33 @@ OlmDevice.prototype.generateOneTimeKeys = async function(numKeys) {
OlmDevice.prototype.createOutboundSession = async function(
theirIdentityKey, theirOneTimeKey,
) {
const self = this;
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 {
session.free();
}
});
let newSessionId;
await this._cryptoStore.doTxn(
'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
(txn) => {
this._getAccount(txn, (account) => {
const session = new Olm.Session();
try {
session.create_outbound(account, theirIdentityKey, theirOneTimeKey);
newSessionId = session.session_id();
this._storeAccount(txn, account);
this._saveSession(theirIdentityKey, session);
return session.session_id();
} finally {
session.free();
}
});
},
);
return newSessionId;
};


/**
* 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
Expand All @@ -362,32 +392,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) {
throw new Error("Need message_type == 0 to create inbound session");
if (messageType !== 0) {
throw new Error("Need messageType == 0 to create inbound session");
}

const self = this;
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);
save();
let result;
await this._cryptoStore.doTxn(
'readwrite', [IndexedDBCryptoStore.STORE_ACCOUNT],
(txn) => {
this._getAccount(txn, (account) => {
const session = new Olm.Session();
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 {
payload: payloadString,
session_id: session.session_id(),
};
} finally {
session.free();
}
});
result = {
payload: payloadString,
session_id: session.session_id(),
};
} finally {
session.free();
}
});
},
);

return result;
};


Expand Down Expand Up @@ -484,18 +523,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;
Expand All @@ -508,16 +547,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;
}

Expand Down
38 changes: 13 additions & 25 deletions src/crypto/store/indexeddb-crypto-store-backend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,35 +258,23 @@ 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 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");
getAccount(txn, func) {
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, "-");
},
);
func(getReq.result || null);
};
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;
});
}
Expand Down
Loading