Skip to content

Commit d74ed50

Browse files
dbkrdbkr
committed
Restart broken Olm sessions
 * Start a new Olm sessions with a device when we get an undecryptable message on it. * Send a dummy message on that sessions such that the other end knows about it. * Re-send any outstanding keyshare requests for that device. Also includes a unit test for megolm that isn't very related but came out as a result anyway. Includes #776 Fixes element-hq/element-web#3822
1 parent 2a6a67c commit d74ed50

10 files changed

+476
-17
lines changed

‎spec/unit/crypto.spec.js

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
2-
"use strict";
31
import 'source-map-support/register';
42
import Crypto from '../../lib/crypto';
53
import expect from 'expect';
64

5+
import WebStorageSessionStore from '../../lib/store/session/webstorage';
6+
import MemoryCryptoStore from '../../lib/crypto/store/memory-crypto-store.js';
7+
import MockStorageApi from '../MockStorageApi';
8+
9+
const EventEmitter = require("events").EventEmitter;
10+
711
const sdk = require("../..");
812

913
const Olm = global.Olm;
@@ -20,4 +24,96 @@ describe("Crypto", function() {
2024
it("Crypto exposes the correct olm library version", function() {
2125
expect(Crypto.getOlmVersion()[0]).toEqual(3);
2226
});
27+
28+
29+
describe('Session management', function() {
30+
const otkResponse = {
31+
one_time_keys: {
32+
'@alice:home.server': {
33+
aliceDevice: {
34+
'signed_curve25519:FLIBBLE': {
35+
key: 'YmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmI',
36+
signatures: {
37+
'@alice:home.server': {
38+
'ed25519:aliceDevice': 'totally a valid signature',
39+
},
40+
},
41+
},
42+
},
43+
},
44+
},
45+
};
46+
let crypto;
47+
let mockBaseApis;
48+
let mockRoomList;
49+
50+
let fakeEmitter;
51+
52+
beforeEach(async function() {
53+
const mockStorage = new MockStorageApi();
54+
const sessionStore = new WebStorageSessionStore(mockStorage);
55+
const cryptoStore = new MemoryCryptoStore(mockStorage);
56+
57+
cryptoStore.storeEndToEndDeviceData({
58+
devices: {
59+
'@bob:home.server': {
60+
'BOBDEVICE': {
61+
keys: {
62+
'curve25519:BOBDEVICE': 'this is a key',
63+
},
64+
},
65+
},
66+
},
67+
trackingStatus: {},
68+
});
69+
70+
mockBaseApis = {
71+
sendToDevice: expect.createSpy(),
72+
};
73+
mockRoomList = {};
74+
75+
fakeEmitter = new EventEmitter();
76+
77+
crypto = new Crypto(
78+
mockBaseApis,
79+
sessionStore,
80+
"@alice:home.server",
81+
"FLIBBLE",
82+
sessionStore,
83+
cryptoStore,
84+
mockRoomList,
85+
);
86+
crypto.registerEventHandlers(fakeEmitter);
87+
await crypto.init();
88+
});
89+
90+
afterEach(async function() {
91+
await crypto.stop();
92+
});
93+
94+
it("restarts wedged Olm sessions", async function() {
95+
const prom = new Promise((resolve) => {
96+
mockBaseApis.claimOneTimeKeys = function() {
97+
resolve();
98+
return otkResponse;
99+
};
100+
});
101+
102+
fakeEmitter.emit('toDeviceEvent', {
103+
getType: expect.createSpy().andReturn('m.room.message'),
104+
getContent: expect.createSpy().andReturn({
105+
msgtype: 'm.bad.encrypted',
106+
}),
107+
getWireContent: expect.createSpy().andReturn({
108+
algorithm: 'm.olm.v1.curve25519-aes-sha2',
109+
sender_key: 'this is a key',
110+
}),
111+
getSender: expect.createSpy().andReturn('@bob:home.server'),
112+
});
113+
114+
console.log("waiting");
115+
await prom;
116+
console.log("done");
117+
});
118+
});
23119
});

‎spec/unit/crypto/algorithms/megolm.spec.js

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import Crypto from '../../../../lib/crypto';
1818

1919
const MatrixEvent = sdk.MatrixEvent;
2020
const MegolmDecryption = algorithms.DECRYPTION_CLASSES['m.megolm.v1.aes-sha2'];
21+
const MegolmEncryption = algorithms.ENCRYPTION_CLASSES['m.megolm.v1.aes-sha2'];
2122

2223
const ROOM_ID = '!ROOM:ID';
2324

@@ -34,9 +35,11 @@ describe("MegolmDecryption", function() {
3435
let mockCrypto;
3536
let mockBaseApis;
3637

37-
beforeEach(function() {
38+
beforeEach(async function() {
3839
testUtils.beforeEach(this); // eslint-disable-line no-invalid-this
3940

41+
await Olm.init();
42+
4043
mockCrypto = testUtils.mock(Crypto, 'Crypto');
4144
mockBaseApis = {};
4245

@@ -66,7 +69,6 @@ describe("MegolmDecryption", function() {
6669
describe('receives some keys:', function() {
6770
let groupSession;
6871
beforeEach(async function() {
69-
await Olm.init();
7072
groupSession = new global.Olm.OutboundGroupSession();
7173
groupSession.create();
7274

@@ -263,5 +265,92 @@ describe("MegolmDecryption", function() {
263265
// test is successful if no exception is thrown
264266
});
265267
});
268+
269+
it("re-uses sessions for sequential messages", async function() {
270+
const mockStorage = new MockStorageApi();
271+
const sessionStore = new WebStorageSessionStore(mockStorage);
272+
const cryptoStore = new MemoryCryptoStore(mockStorage);
273+
274+
const olmDevice = new OlmDevice(sessionStore, cryptoStore);
275+
olmDevice.verifySignature = expect.createSpy();
276+
await olmDevice.init();
277+
278+
mockBaseApis.claimOneTimeKeys = expect.createSpy().andReturn(Promise.resolve({
279+
one_time_keys: {
280+
'@alice:home.server': {
281+
aliceDevice: {
282+
'signed_curve25519:flooble': {
283+
key: 'YmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmJiYmI',
284+
signatures: {
285+
'@alice:home.server': {
286+
'ed25519:aliceDevice': 'totally valid',
287+
},
288+
},
289+
},
290+
},
291+
},
292+
},
293+
}));
294+
mockBaseApis.sendToDevice = expect.createSpy().andReturn(Promise.resolve());
295+
296+
mockCrypto.downloadKeys.andReturn(Promise.resolve({
297+
'@alice:home.server': {
298+
aliceDevice: {
299+
deviceId: 'aliceDevice',
300+
isBlocked: expect.createSpy().andReturn(false),
301+
isUnverified: expect.createSpy().andReturn(false),
302+
getIdentityKey: expect.createSpy().andReturn(
303+
'YWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWE',
304+
),
305+
getFingerprint: expect.createSpy().andReturn(''),
306+
},
307+
},
308+
}));
309+
310+
const megolmEncryption = new MegolmEncryption({
311+
userId: '@user:id',
312+
crypto: mockCrypto,
313+
olmDevice: olmDevice,
314+
baseApis: mockBaseApis,
315+
roomId: ROOM_ID,
316+
config: {
317+
rotation_period_ms: 9999999999999,
318+
},
319+
});
320+
const mockRoom = {
321+
getEncryptionTargetMembers: expect.createSpy().andReturn(
322+
[{userId: "@alice:home.server"}],
323+
),
324+
getBlacklistUnverifiedDevices: expect.createSpy().andReturn(false),
325+
};
326+
const ct1 = await megolmEncryption.encryptMessage(mockRoom, "a.fake.type", {
327+
body: "Some text",
328+
});
329+
expect(mockRoom.getEncryptionTargetMembers).toHaveBeenCalled();
330+
331+
// this should have claimed a key for alice as it's starting a new session
332+
expect(mockBaseApis.claimOneTimeKeys).toHaveBeenCalled(
333+
[['@alice:home.server', 'aliceDevice']], 'signed_curve25519',
334+
);
335+
expect(mockCrypto.downloadKeys).toHaveBeenCalledWith(
336+
['@alice:home.server'], false,
337+
);
338+
expect(mockBaseApis.sendToDevice).toHaveBeenCalled();
339+
expect(mockBaseApis.claimOneTimeKeys).toHaveBeenCalled(
340+
[['@alice:home.server', 'aliceDevice']], 'signed_curve25519',
341+
);
342+
343+
mockBaseApis.claimOneTimeKeys.reset();
344+
345+
const ct2 = await megolmEncryption.encryptMessage(mockRoom, "a.fake.type", {
346+
body: "Some more text",
347+
});
348+
349+
// this should *not* have claimed a key as it should be using the same session
350+
expect(mockBaseApis.claimOneTimeKeys).toNotHaveBeenCalled();
351+
352+
// likewise they should show the same session ID
353+
expect(ct2.session_id).toEqual(ct1.session_id);
354+
});
266355
});
267356
});

‎src/crypto/OlmDevice.js

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,12 @@ OlmDevice.prototype.createOutboundSession = async function(
461461
session.create_outbound(account, theirIdentityKey, theirOneTimeKey);
462462
newSessionId = session.session_id();
463463
this._storeAccount(txn, account);
464+
// Pretend we've received a message at this point, otherwise
465+
// if we try to send a message to the device, it won't use
466+
// this session (storing the creation time separately would
467+
// make the pickle longer and would not be useful otherwise).
468+
session.set_last_received_message_ts(Date.now());
469+
464470
this._saveSession(theirIdentityKey, session, txn);
465471
} finally {
466472
session.free();
@@ -725,7 +731,7 @@ OlmDevice.prototype._saveOutboundGroupSession = function(session) {
725731
*/
726732
OlmDevice.prototype._getOutboundGroupSession = function(sessionId, func) {
727733
const pickled = this._outboundGroupSessionStore[sessionId];
728-
if (pickled === null) {
734+
if (pickled === undefined) {
729735
throw new Error("Unknown outbound group session " + sessionId);
730736
}
731737

@@ -1059,16 +1065,21 @@ OlmDevice.prototype.hasInboundSessionKeys = async function(roomId, senderKey, se
10591065
* @param {string} roomId room in which the message was received
10601066
* @param {string} senderKey base64-encoded curve25519 key of the sender
10611067
* @param {string} sessionId session identifier
1068+
* @param {integer} chainIndex The chain index at which to export the session.
1069+
* If omitted, export at the first index we know about.
10621070
*
10631071
* @returns {Promise<{chain_index: number, key: string,
10641072
* forwarding_curve25519_key_chain: Array<string>,
10651073
* sender_claimed_ed25519_key: string
10661074
* }>}
10671075
* details of the session key. The key is a base64-encoded megolm key in
10681076
* export format.
1077+
*
1078+
* @throws Error If the given chain index could not be obtained from the known
1079+
* index (ie. the given chain index is before the first we have).
10691080
*/
10701081
OlmDevice.prototype.getInboundGroupSessionKey = async function(
1071-
roomId, senderKey, sessionId,
1082+
roomId, senderKey, sessionId, chainIndex,
10721083
) {
10731084
let result;
10741085
await this._cryptoStore.doTxn(
@@ -1079,14 +1090,19 @@ OlmDevice.prototype.getInboundGroupSessionKey = async function(
10791090
result = null;
10801091
return;
10811092
}
1082-
const messageIndex = session.first_known_index();
1093+
1094+
if (chainIndex === undefined) {
1095+
chainIndex = session.first_known_index();
1096+
}
1097+
1098+
const exportedSession = session.export_session(chainIndex);
10831099

10841100
const claimedKeys = sessionData.keysClaimed || {};
10851101
const senderEd25519Key = claimedKeys.ed25519 || null;
10861102

10871103
result = {
1088-
"chain_index": messageIndex,
1089-
"key": session.export_session(messageIndex),
1104+
"chain_index": chainIndex,
1105+
"key": exportedSession,
10901106
"forwarding_curve25519_key_chain":
10911107
sessionData.forwardingCurve25519KeyChain || [],
10921108
"sender_claimed_ed25519_key": senderEd25519Key,

‎src/crypto/OutgoingRoomKeyRequestManager.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,21 @@ export default class OutgoingRoomKeyRequestManager {
244244
});
245245
}
246246

247+
/**
248+
* Look for room key requests by target device and state
249+
*
250+
* @param {string} userId Target user ID
251+
* @param {string} deviceId Target device ID
252+
*
253+
* @return {Promise} resolves to a list of all the
254+
* {@link module:crypto/store/base~OutgoingRoomKeyRequest}
255+
*/
256+
getOutgoingSentRoomKeyRequest(userId, deviceId) {
257+
return this._cryptoStore.getOutgoingRoomKeyRequestsByTarget(
258+
userId, deviceId, [ROOM_KEY_REQUEST_STATES.SENT],
259+
);
260+
}
261+
247262
// start the background timer to send queued requests, if the timer isn't
248263
// already running
249264
_startTimer() {

0 commit comments

Comments
 (0)