Skip to content

Commit 20a4edf

Browse files
dbkrdbkr
authored
Merge pull request #776 from matrix-org/dbkr/use_session_last_received_message
Use the last olm session that got a message
2 parents b233ab8 + 3c85bd5 commit 20a4edf

6 files changed

+110
-42
lines changed

‎.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
language: node_js
22
node_js:
3-
- node # Latest stable version of nodejs.
3+
- "10.11.0"
44
script:
55
- ./travis.sh

‎src/crypto/OlmDevice.js

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,8 @@ OlmDevice.prototype._storeAccount = function(txn, account) {
295295
*/
296296
OlmDevice.prototype._getSession = function(deviceKey, sessionId, txn, func) {
297297
this._cryptoStore.getEndToEndSession(
298-
deviceKey, sessionId, txn, (pickledSession) => {
299-
this._unpickleSession(pickledSession, func);
298+
deviceKey, sessionId, txn, (sessionInfo) => {
299+
this._unpickleSession(sessionInfo, func);
300300
},
301301
);
302302
};
@@ -306,15 +306,17 @@ OlmDevice.prototype._getSession = function(deviceKey, sessionId, txn, func) {
306306
* function with it. The session object is destroyed once the function
307307
* returns.
308308
*
309-
* @param {string} pickledSession
309+
* @param {object} sessionInfo
310310
* @param {function} func
311311
* @private
312312
*/
313-
OlmDevice.prototype._unpickleSession = function(pickledSession, func) {
313+
OlmDevice.prototype._unpickleSession = function(sessionInfo, func) {
314314
const session = new global.Olm.Session();
315315
try {
316-
session.unpickle(this._pickleKey, pickledSession);
317-
func(session);
316+
session.unpickle(this._pickleKey, sessionInfo.session);
317+
const unpickledSessInfo = Object.assign({}, sessionInfo, {session});
318+
319+
func(unpickledSessInfo);
318320
} finally {
319321
session.free();
320322
}
@@ -324,14 +326,17 @@ OlmDevice.prototype._unpickleSession = function(pickledSession, func) {
324326
* store our OlmSession in the session store
325327
*
326328
* @param {string} deviceKey
327-
* @param {OlmSession} session
329+
* @param {object} sessionInfo {session: OlmSession, lastReceivedMessageTs: int}
328330
* @param {*} txn Opaque transaction object from cryptoStore.doTxn()
329331
* @private
330332
*/
331-
OlmDevice.prototype._saveSession = function(deviceKey, session, txn) {
332-
const pickledSession = session.pickle(this._pickleKey);
333+
OlmDevice.prototype._saveSession = function(deviceKey, sessionInfo, txn) {
334+
const sessionId = sessionInfo.session.session_id();
335+
const pickledSessionInfo = Object.assign(sessionInfo, {
336+
session: sessionInfo.session.pickle(this._pickleKey),
337+
});
333338
this._cryptoStore.storeEndToEndSession(
334-
deviceKey, session.session_id(), pickledSession, txn,
339+
deviceKey, sessionId, pickledSessionInfo, txn,
335340
);
336341
};
337342

@@ -461,7 +466,14 @@ OlmDevice.prototype.createOutboundSession = async function(
461466
session.create_outbound(account, theirIdentityKey, theirOneTimeKey);
462467
newSessionId = session.session_id();
463468
this._storeAccount(txn, account);
464-
this._saveSession(theirIdentityKey, session, txn);
469+
const sessionInfo = {
470+
session,
471+
// Pretend we've received a message at this point, otherwise
472+
// if we try to send a message to the device, it won't use
473+
// this session
474+
lastReceivedMessageTs: Date.now(),
475+
};
476+
this._saveSession(theirIdentityKey, sessionInfo, txn);
465477
} finally {
466478
session.free();
467479
}
@@ -510,7 +522,13 @@ OlmDevice.prototype.createInboundSession = async function(
510522

511523
const payloadString = session.decrypt(messageType, ciphertext);
512524

513-
this._saveSession(theirDeviceIdentityKey, session, txn);
525+
const sessionInfo = {
526+
session,
527+
// this counts as a received message: set last received message time
528+
// to now
529+
lastReceivedMessageTs: Date.now(),
530+
};
531+
this._saveSession(theirDeviceIdentityKey, sessionInfo, txn);
514532

515533
result = {
516534
payload: payloadString,
@@ -558,13 +576,30 @@ OlmDevice.prototype.getSessionIdsForDevice = async function(theirDeviceIdentityK
558576
* @return {Promise<?string>} session id, or null if no established session
559577
*/
560578
OlmDevice.prototype.getSessionIdForDevice = async function(theirDeviceIdentityKey) {
561-
const sessionIds = await this.getSessionIdsForDevice(theirDeviceIdentityKey);
562-
if (sessionIds.length === 0) {
579+
const sessionInfos = await this.getSessionInfoForDevice(theirDeviceIdentityKey);
580+
if (sessionInfos.length === 0) {
563581
return null;
564582
}
565-
// Use the session with the lowest ID.
566-
sessionIds.sort();
567-
return sessionIds[0];
583+
// Use the session that has most recently received a message
584+
let idxOfBest = 0;
585+
for (let i = 1; i < sessionInfos.length; i++) {
586+
const thisSessInfo = sessionInfos[i];
587+
const thisLastReceived = thisSessInfo.lastReceivedMessageTs === undefined ?
588+
0 : thisSessInfo.lastReceivedMessageTs;
589+
590+
const bestSessInfo = sessionInfos[idxOfBest];
591+
const bestLastReceived = bestSessInfo.lastReceivedMessageTs === undefined ?
592+
0 : bestSessInfo.lastReceivedMessageTs;
593+
if (
594+
thisLastReceived > bestLastReceived || (
595+
thisLastReceived === bestLastReceived &&
596+
thisSessInfo.sessionId < bestSessInfo.sessionId
597+
)
598+
) {
599+
idxOfBest = i;
600+
}
601+
}
602+
return sessionInfos[idxOfBest].sessionId;
568603
};
569604

570605
/**
@@ -587,9 +622,10 @@ OlmDevice.prototype.getSessionInfoForDevice = async function(deviceIdentityKey)
587622
this._cryptoStore.getEndToEndSessions(deviceIdentityKey, txn, (sessions) => {
588623
const sessionIds = Object.keys(sessions).sort();
589624
for (const sessionId of sessionIds) {
590-
this._unpickleSession(sessions[sessionId], (session) => {
625+
this._unpickleSession(sessions[sessionId], (sessInfo) => {
591626
info.push({
592-
hasReceivedMessage: session.has_received_message(),
627+
lastReceivedMessageTs: sessInfo.lastReceivedMessageTs,
628+
hasReceivedMessage: sessInfo.session.has_received_message(),
593629
sessionId: sessionId,
594630
});
595631
});
@@ -620,9 +656,9 @@ OlmDevice.prototype.encryptMessage = async function(
620656
await this._cryptoStore.doTxn(
621657
'readwrite', [IndexedDBCryptoStore.STORE_SESSIONS],
622658
(txn) => {
623-
this._getSession(theirDeviceIdentityKey, sessionId, txn, (session) => {
624-
res = session.encrypt(payloadString);
625-
this._saveSession(theirDeviceIdentityKey, session, txn);
659+
this._getSession(theirDeviceIdentityKey, sessionId, txn, (sessionInfo) => {
660+
res = sessionInfo.session.encrypt(payloadString);
661+
this._saveSession(theirDeviceIdentityKey, sessionInfo, txn);
626662
});
627663
},
628664
);
@@ -647,9 +683,10 @@ OlmDevice.prototype.decryptMessage = async function(
647683
await this._cryptoStore.doTxn(
648684
'readwrite', [IndexedDBCryptoStore.STORE_SESSIONS],
649685
(txn) => {
650-
this._getSession(theirDeviceIdentityKey, sessionId, txn, (session) => {
651-
payloadString = session.decrypt(messageType, ciphertext);
652-
this._saveSession(theirDeviceIdentityKey, session, txn);
686+
this._getSession(theirDeviceIdentityKey, sessionId, txn, (sessionInfo) => {
687+
payloadString = sessionInfo.session.decrypt(messageType, ciphertext);
688+
sessionInfo.lastReceivedMessageTs = Date.now();
689+
this._saveSession(theirDeviceIdentityKey, sessionInfo, txn);
653690
});
654691
},
655692
);
@@ -679,8 +716,8 @@ OlmDevice.prototype.matchesSession = async function(
679716
await this._cryptoStore.doTxn(
680717
'readonly', [IndexedDBCryptoStore.STORE_SESSIONS],
681718
(txn) => {
682-
this._getSession(theirDeviceIdentityKey, sessionId, txn, (session) => {
683-
matches = session.matches_inbound(ciphertext);
719+
this._getSession(theirDeviceIdentityKey, sessionId, txn, (sessionInfo) => {
720+
matches = sessionInfo.session.matches_inbound(ciphertext);
684721
});
685722
},
686723
);

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,10 @@ export class Backend {
314314
getReq.onsuccess = function() {
315315
const cursor = getReq.result;
316316
if (cursor) {
317-
results[cursor.value.sessionId] = cursor.value.session;
317+
results[cursor.value.sessionId] = {
318+
session: cursor.value.session,
319+
lastReceivedMessageTs: cursor.value.lastReceivedMessageTs,
320+
};
318321
cursor.continue();
319322
} else {
320323
try {
@@ -332,7 +335,10 @@ export class Backend {
332335
getReq.onsuccess = function() {
333336
try {
334337
if (getReq.result) {
335-
func(getReq.result.session);
338+
func({
339+
session: getReq.result.session,
340+
lastReceivedMessageTs: getReq.result.lastReceivedMessageTs,
341+
});
336342
} else {
337343
func(null);
338344
}
@@ -342,9 +348,14 @@ export class Backend {
342348
};
343349
}
344350

345-
storeEndToEndSession(deviceKey, sessionId, session, txn) {
351+
storeEndToEndSession(deviceKey, sessionId, sessionInfo, txn) {
346352
const objectStore = txn.objectStore("sessions");
347-
objectStore.put({deviceKey, sessionId, session});
353+
objectStore.put({
354+
deviceKey,
355+
sessionId,
356+
session: sessionInfo.session,
357+
lastReceivedMessageTs: sessionInfo.lastReceivedMessageTs,
358+
});
348359
}
349360

350361
// Inbound group sessions

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,10 @@ export default class IndexedDBCryptoStore {
284284
* @param {string} sessionId The ID of the session to retrieve
285285
* @param {*} txn An active transaction. See doTxn().
286286
* @param {function(object)} func Called with A map from sessionId
287-
* to Base64 end-to-end session.
287+
* to session information object with 'session' key being the
288+
* Base64 end-to-end session and lastReceivedMessageTs being the
289+
* timestamp in milliseconds at which the session last received
290+
* a message.
288291
*/
289292
getEndToEndSession(deviceKey, sessionId, txn, func) {
290293
this._backendPromise.value().getEndToEndSession(deviceKey, sessionId, txn, func);
@@ -296,7 +299,10 @@ export default class IndexedDBCryptoStore {
296299
* @param {string} deviceKey The public key of the other device.
297300
* @param {*} txn An active transaction. See doTxn().
298301
* @param {function(object)} func Called with A map from sessionId
299-
* to Base64 end-to-end session.
302+
* to session information object with 'session' key being the
303+
* Base64 end-to-end session and lastReceivedMessageTs being the
304+
* timestamp in milliseconds at which the session last received
305+
* a message.
300306
*/
301307
getEndToEndSessions(deviceKey, txn, func) {
302308
this._backendPromise.value().getEndToEndSessions(deviceKey, txn, func);
@@ -306,12 +312,12 @@ export default class IndexedDBCryptoStore {
306312
* Store a session between the logged-in user and another device
307313
* @param {string} deviceKey The public key of the other device.
308314
* @param {string} sessionId The ID for this end-to-end session.
309-
* @param {string} session Base64 encoded end-to-end session.
315+
* @param {string} sessionInfo Session information object
310316
* @param {*} txn An active transaction. See doTxn().
311317
*/
312-
storeEndToEndSession(deviceKey, sessionId, session, txn) {
318+
storeEndToEndSession(deviceKey, sessionId, sessionInfo, txn) {
313319
this._backendPromise.value().storeEndToEndSession(
314-
deviceKey, sessionId, session, txn,
320+
deviceKey, sessionId, sessionInfo, txn,
315321
);
316322
}
317323

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,21 @@ export default class LocalStorageCryptoStore extends MemoryCryptoStore {
6767
}
6868

6969
_getEndToEndSessions(deviceKey, txn, func) {
70-
return getJsonItem(this.store, keyEndToEndSessions(deviceKey));
70+
const sessions = getJsonItem(this.store, keyEndToEndSessions(deviceKey));
71+
const fixedSessions = {};
72+
73+
// fix up any old sessions to be objects rather than just the base64 pickle
74+
for (const [sid, val] of Object.entries(sessions || {})) {
75+
if (typeof val === 'string') {
76+
fixedSessions[sid] = {
77+
session: val,
78+
};
79+
} else {
80+
fixedSessions[sid] = val;
81+
}
82+
}
83+
84+
return fixedSessions;
7185
}
7286

7387
getEndToEndSession(deviceKey, sessionId, txn, func) {
@@ -79,9 +93,9 @@ export default class LocalStorageCryptoStore extends MemoryCryptoStore {
7993
func(this._getEndToEndSessions(deviceKey) || {});
8094
}
8195

82-
storeEndToEndSession(deviceKey, sessionId, session, txn) {
96+
storeEndToEndSession(deviceKey, sessionId, sessionInfo, txn) {
8397
const sessions = this._getEndToEndSessions(deviceKey) || {};
84-
sessions[sessionId] = session;
98+
sessions[sessionId] = sessionInfo;
8599
setJsonItem(
86100
this.store, keyEndToEndSessions(deviceKey), sessions,
87101
);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,13 +234,13 @@ export default class MemoryCryptoStore {
234234
func(this._sessions[deviceKey] || {});
235235
}
236236

237-
storeEndToEndSession(deviceKey, sessionId, session, txn) {
237+
storeEndToEndSession(deviceKey, sessionId, sessionInfo, txn) {
238238
let deviceSessions = this._sessions[deviceKey];
239239
if (deviceSessions === undefined) {
240240
deviceSessions = {};
241241
this._sessions[deviceKey] = deviceSessions;
242242
}
243-
deviceSessions[sessionId] = session;
243+
deviceSessions[sessionId] = sessionInfo;
244244
}
245245

246246
// Inbound Group Sessions

0 commit comments

Comments
 (0)