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

Migrate room encryption store to crypto store #597

Merged
merged 19 commits into from
Feb 6, 2018
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"bluebird": "^3.5.0",
"browser-request": "^0.3.3",
"content-type": "^1.0.2",
"olm": "https://matrix.org/packages/npm/olm/olm-2.2.2.tgz",
Copy link
Member

Choose a reason for hiding this comment

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

olm was deliberately omitted here for some reason involving making it possible to do without it. the details currently escape me, but what is the reason for adding it?

Copy link
Member Author

Choose a reason for hiding this comment

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

bah, because npm now adds any package you install to package.json even if you don't specify --save.

"request": "^2.53.0"
},
"devDependencies": {
Expand Down
30 changes: 18 additions & 12 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const MatrixBaseApis = require("./base-apis");
const MatrixError = httpApi.MatrixError;

import ReEmitter from './ReEmitter';
import RoomList from './crypto/RoomList';

const SCROLLBACK_DELAY_MS = 3000;
let CRYPTO_ENABLED = false;
Expand Down Expand Up @@ -181,6 +182,11 @@ function MatrixClient(opts) {
if (CRYPTO_ENABLED) {
this.olmVersion = Crypto.getOlmVersion();
}

// List of what rooms have encryption enabled: separate from crypto because
Copy link
Member

Choose a reason for hiding this comment

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

which rooms

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed :)

// we still want to know what rooms are encrypted even if crypto is disabled:
Copy link
Member

Choose a reason for hiding this comment

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

which

Copy link
Member Author

Choose a reason for hiding this comment

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

also fixed

// we don't want to start sening unencrypted events to them.
Copy link
Member

Choose a reason for hiding this comment

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

sening

Copy link
Member Author

Choose a reason for hiding this comment

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

also done

this._roomList = new RoomList(this._cryptoStore, this._sessionStore);
}
utils.inherits(MatrixClient, EventEmitter);
utils.extend(MatrixClient.prototype, MatrixBaseApis.prototype);
Expand Down Expand Up @@ -351,13 +357,6 @@ MatrixClient.prototype.initCrypto = async function() {
return;
}

if (!CRYPTO_ENABLED) {
throw new Error(
`End-to-end encryption not supported in this js-sdk build: did ` +
`you remember to load the olm library?`,
);
}

if (!this._sessionStore) {
// this is temporary, the sessionstore is supposed to be going away
throw new Error(`Cannot enable encryption: no sessionStore provided`);
Expand All @@ -367,6 +366,16 @@ MatrixClient.prototype.initCrypto = async function() {
throw new Error(`Cannot enable encryption: no cryptoStore provided`);
}

// initialise the list of encrypted rooms (whether or not crypto is enabled)
await this._roomList.init();

if (!CRYPTO_ENABLED) {
throw new Error(
`End-to-end encryption not supported in this js-sdk build: did ` +
`you remember to load the olm library?`,
);
}

const userId = this.getUserId();
if (userId === null) {
throw new Error(
Expand All @@ -387,6 +396,7 @@ MatrixClient.prototype.initCrypto = async function() {
userId, this.deviceId,
this.store,
this._cryptoStore,
this._roomList,
);

this.reEmitter.reEmit(crypto, [
Expand Down Expand Up @@ -646,11 +656,7 @@ MatrixClient.prototype.isRoomEncrypted = function(roomId) {
// we don't have an m.room.encrypted event, but that might be because
// the server is hiding it from us. Check the store to see if it was
// previously encrypted.
if (!this._sessionStore) {
return false;
}

return Boolean(this._sessionStore.getEndToEndRoom(roomId));
return this._roomList.isRoomEncrypted(roomId);
};

/**
Expand Down
81 changes: 81 additions & 0 deletions src/crypto/RoomList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
Copyright 2018 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.
*/

/**
* @module crypto/RoomList
*
* Manages the list of encrypted rooms
*/

import IndexedDBCryptoStore from './store/indexeddb-crypto-store';

/**
* @alias module:crypto/RoomList
*/
export default class RoomList {
constructor(cryptoStore, sessionStore) {
this._cryptoStore = cryptoStore;
this._sessionStore = sessionStore;

// Object of roomId -> room e2e info object
Copy link
Member

Choose a reason for hiding this comment

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

could we document the shape of said object hereabouts?

Copy link
Member Author

Choose a reason for hiding this comment

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

have added that it's the body of the m.room.encryption object

this._roomEncryption = {};
}

async init() {
let removeSessionStoreRooms = false;
await this._cryptoStore.doTxn(
'readwrite', [IndexedDBCryptoStore.STORE_ROOMS], (txn) => {
this._cryptoStore.getEndToEndRooms(txn, (result) => {
if (result === null || Object.keys(result).length === 0) {
// migrate from session store, if there's data there
const sessStoreRooms = this._sessionStore.getAllEndToEndRooms();
if (sessStoreRooms !== null) {
for (const roomId of Object.keys(sessStoreRooms)) {
this._cryptoStore.storeEndToEndRoom(
roomId, sessStoreRooms[roomId], txn,
);
}
}
this._roomEncryption = sessStoreRooms;
removeSessionStoreRooms = true;
} else {
this._roomEncryption = result;
}
});
},
);
if (removeSessionStoreRooms) {
this._sessionStore.removeAllEndToEndRooms();
}
}

getRoomEncryption(roomId) {
return this._roomEncryption[roomId] || null;
}

isRoomEncrypted(roomId) {
return Boolean(this.getRoomEncryption(roomId));
}

async setRoomEncryption(roomId, roomInfo) {
this._roomEncryption[roomId] = roomInfo;
await this._cryptoStore.doTxn(
'readwrite', [IndexedDBCryptoStore.STORE_ROOMS], (txn) => {
this._cryptoStore.storeEndToEndRoom(roomId, roomInfo, txn);
},
);
}
}
40 changes: 21 additions & 19 deletions src/crypto/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,18 @@ import IndexedDBCryptoStore from './store/indexeddb-crypto-store';
*
* @param {module:crypto/store/base~CryptoStore} cryptoStore
* storage for the crypto layer.
*
* @param {RoomList} roomList An initialised RoomList object
*/
function Crypto(baseApis, sessionStore, userId, deviceId,
clientStore, cryptoStore) {
clientStore, cryptoStore, roomList) {
this._baseApis = baseApis;
this._sessionStore = sessionStore;
this._userId = userId;
this._deviceId = deviceId;
this._clientStore = clientStore;
this._cryptoStore = cryptoStore;
this._roomList = roomList;

this._olmDevice = new OlmDevice(sessionStore, cryptoStore);
this._deviceList = new DeviceList(
Expand Down Expand Up @@ -587,6 +590,17 @@ Crypto.prototype.getEventSenderDeviceInfo = function(event) {
return device;
};

/**
* Get the current end-to-end encryption config for a room
*
* @param {string} roomId The room ID to query
*
* @return {object} The current end-to-end encyption status, or null if
Copy link
Member

Choose a reason for hiding this comment

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

I don't really understand why you've made this async, but given you have, the return type is 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.

ah ok. I think the reason it's async was to match the rest of the crypto stuff which assumes it's going to be indexeddb backed. I'd also assumed that the Promise return type was implicit on an async function.

* the room is not stored as using end-to-end encryption.
*/
Crypto.prototype.getRoomEncryption = async function(roomId) {
return this._roomList.getRoomEncryption(roomId);
};

/**
* Configure a room to use encryption (ie, save a flag in the sessionstore).
Expand All @@ -601,21 +615,19 @@ Crypto.prototype.getEventSenderDeviceInfo = function(event) {
Crypto.prototype.setRoomEncryption = async function(roomId, config, inhibitDeviceQuery) {
// if we already have encryption in this room, we should ignore this event
// (for now at least. maybe we should alert the user somehow?)
const existingConfig = this._sessionStore.getEndToEndRoom(roomId);
if (existingConfig) {
if (JSON.stringify(existingConfig) != JSON.stringify(config)) {
console.error("Ignoring m.room.encryption event which requests " +
"a change of config in " + roomId);
return;
}
const existingConfig = await this.getRoomEncryption(roomId);
Copy link
Member

Choose a reason for hiding this comment

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

given this seems to be the only place getRoomEncryption is used, any reason not to inline it?

Copy link
Member Author

Choose a reason for hiding this comment

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

mm, point

if (existingConfig && JSON.stringify(existingConfig) != JSON.stringify(config)) {
console.error("Ignoring m.room.encryption event which requests " +
"a change of config in " + roomId);
return;
}

const AlgClass = algorithms.ENCRYPTION_CLASSES[config.algorithm];
if (!AlgClass) {
throw new Error("Unable to encrypt with " + config.algorithm);
}

this._sessionStore.storeEndToEndRoom(roomId, config);
await this._roomList.setRoomEncryption(roomId, config);

const alg = new AlgClass({
userId: this._userId,
Expand Down Expand Up @@ -693,16 +705,6 @@ Crypto.prototype.ensureOlmSessionsForUsers = function(users) {
);
};

/**
* Whether encryption is enabled for a room.
* @param {string} roomId the room id to query.
* @return {bool} whether encryption is enabled.
*/
Crypto.prototype.isRoomEncrypted = function(roomId) {
return Boolean(this._roomEncryptors[roomId]);
};


/**
* Get a list containing all of the room keys
*
Expand Down
29 changes: 28 additions & 1 deletion src/crypto/store/indexeddb-crypto-store-backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ limitations under the License.
import Promise from 'bluebird';
import utils from '../../utils';

export const VERSION = 5;
export const VERSION = 6;

/**
* Implementation of a CryptoStore which is backed by an existing
Expand Down Expand Up @@ -425,6 +425,30 @@ export class Backend {
objectStore.put(deviceData, "-");
}

storeEndToEndRoom(roomId, roomInfo, txn) {
const objectStore = txn.objectStore("rooms");
objectStore.put(roomInfo, roomId);
}

getEndToEndRooms(txn, func) {
const rooms = {};
const objectStore = txn.objectStore("rooms");
const getReq = objectStore.openCursor();
getReq.onsuccess = function() {
const cursor = getReq.result;
if (cursor) {
rooms[cursor.key] = cursor.value;
cursor.continue();
} else {
try {
func(rooms);
} catch (e) {
abortWithException(txn, e);
}
}
};
}

doTxn(mode, stores, func) {
const txn = this._db.transaction(stores, mode);
const promise = promiseifyTxn(txn);
Expand Down Expand Up @@ -460,6 +484,9 @@ export function upgradeDatabase(db, oldVersion) {
if (oldVersion < 5) {
db.createObjectStore("device_data");
}
if (oldVersion < 6) {
db.createObjectStore("rooms");
}
// Expand as needed.
}

Expand Down
22 changes: 22 additions & 0 deletions src/crypto/store/indexeddb-crypto-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,27 @@ export default class IndexedDBCryptoStore {
this._backendPromise.value().getEndToEndDeviceData(txn, func);
}

// End to End Rooms

/**
* Store the end-to-end state for a room.
* @param {string} roomId The room's ID.
* @param {object} roomInfo The end-to-end info for the room.
* @param {*} txn An active transaction. See doTxn().
*/
storeEndToEndRoom(roomId, roomInfo, txn) {
this._backendPromise.value().storeEndToEndRoom(roomId, roomInfo, txn);
}

/**
* Get an object of roomId->roomInfo for all e2e rooms in the store
* @param {*} txn An active transaction. See doTxn().
* @param {function(Object)} func Function called with the end to end encrypted rooms
*/
getEndToEndRooms(txn, func) {
this._backendPromise.value().getEndToEndRooms(txn, func);
}

/**
* Perform a transaction on the crypto store. Any store methods
* that require a transaction (txn) object to be passed in may
Expand Down Expand Up @@ -418,3 +439,4 @@ IndexedDBCryptoStore.STORE_ACCOUNT = 'account';
IndexedDBCryptoStore.STORE_SESSIONS = 'sessions';
IndexedDBCryptoStore.STORE_INBOUND_GROUP_SESSIONS = 'inbound_group_sessions';
IndexedDBCryptoStore.STORE_DEVICE_DATA = 'device_data';
IndexedDBCryptoStore.STORE_ROOMS = 'rooms';
25 changes: 25 additions & 0 deletions src/crypto/store/localStorage-crypto-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const E2E_PREFIX = "crypto.";
const KEY_END_TO_END_ACCOUNT = E2E_PREFIX + "account";
const KEY_DEVICE_DATA = E2E_PREFIX + "device_data";
const KEY_INBOUND_SESSION_PREFIX = E2E_PREFIX + "inboundgroupsessions/";
const KEY_ROOMS_PREFIX = E2E_PREFIX + "rooms/";

function keyEndToEndSessions(deviceKey) {
return E2E_PREFIX + "sessions/" + deviceKey;
Expand All @@ -40,6 +41,10 @@ function keyEndToEndInboundGroupSession(senderKey, sessionId) {
return KEY_INBOUND_SESSION_PREFIX + senderKey + "/" + sessionId;
}

function keyEndToEndRoomsPrefix(roomId) {
return KEY_ROOMS_PREFIX + roomId;
}

/**
* @implements {module:crypto/store/base~CryptoStore}
*/
Expand Down Expand Up @@ -140,6 +145,26 @@ export default class LocalStorageCryptoStore extends MemoryCryptoStore {
);
}

storeEndToEndRoom(roomId, roomInfo, txn) {
setJsonItem(
this.store, keyEndToEndRoomsPrefix(roomId), roomInfo,
);
}

getEndToEndRooms(txn, func) {
const result = {};
const prefix = keyEndToEndRoomsPrefix('');

for (let i = 0; i < this.store.length; ++i) {
const key = this.store.key(i);
if (key.startsWith(prefix)) {
const roomId = key.substr(prefix.length);
result[roomId] = getJsonItem(this.store, key);
}
}
func(result);
}

/**
* Delete all data from this store.
*
Expand Down
11 changes: 11 additions & 0 deletions src/crypto/store/memory-crypto-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export default class MemoryCryptoStore {
this._inboundGroupSessions = {};
// Opaque device data object
this._deviceData = null;
// roomId -> Opaque roomInfo object
this._rooms = {};
}

/**
Expand Down Expand Up @@ -283,6 +285,15 @@ export default class MemoryCryptoStore {
this._deviceData = deviceData;
}

// E2E rooms

storeEndToEndRoom(roomId, roomInfo, txn) {
this._rooms[roomId] = roomInfo;
}

getEndToEndRooms(txn, func) {
func(this._rooms);
}

doTxn(mode, stores, func) {
return Promise.resolve(func(null));
Expand Down
Loading