From 31b64a6631bdc83f1c95d1d899969c40597d92eb Mon Sep 17 00:00:00 2001 From: Christian Pauly Date: Thu, 27 Feb 2020 08:41:49 +0000 Subject: [PATCH] [Room] Clear outbound session only if devices changed --- lib/src/client.dart | 28 ++++++-------------------- lib/src/room.dart | 42 ++++++++++++++++++++++++++++++++++----- test/client_test.dart | 4 ++-- test/fake_matrix_api.dart | 18 +++++++++++++++++ test/room_test.dart | 2 +- 5 files changed, 64 insertions(+), 30 deletions(-) diff --git a/lib/src/client.dart b/lib/src/client.dart index 13c81c6..ab8dc4b 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -755,7 +755,7 @@ class Client { sessions.forEach((olm.Session session) => session?.free()); }); rooms.forEach((Room room) { - room.clearOutboundGroupSession(); + room.clearOutboundGroupSession(wipe: true); room.sessionKeys.values.forEach((SessionKey sessionKey) { sessionKey.inboundGroupSession?.free(); }); @@ -986,22 +986,6 @@ class Client { } } - /// Clears the outboundGroupSession from all rooms where this user is - /// participating. Should be called when the user's devices list has changed. - void _clearOutboundGroupSessionsByUserId(String userId) { - for (Room room in rooms) { - if (!room.encrypted) continue; - room.requestParticipants().then((List users) { - if (users.indexWhere((u) => - u.id == userId && - [Membership.join, Membership.invite].contains(u.membership)) != - -1) { - room.clearOutboundGroupSession(); - } - }); - } - } - void _handleDeviceListsEvents(Map deviceLists) { if (deviceLists["changed"] is List) { for (final userId in deviceLists["changed"]) { @@ -1010,7 +994,6 @@ class Client { } } for (final userId in deviceLists["left"]) { - _clearOutboundGroupSessionsByUserId(userId); if (_userDeviceKeys.containsKey(userId)) { _userDeviceKeys.remove(userId); } @@ -1489,13 +1472,14 @@ class Client { } } _userDeviceKeys[userId].outdated = false; - if (_userDeviceKeys[userId].deviceKeys.toString() != - oldKeys.toString()) { - _clearOutboundGroupSessionsByUserId(userId); - } } } await this.storeAPI?.storeUserDeviceKeys(userDeviceKeys); + rooms.forEach((Room room) { + if (room.encrypted) { + room.clearOutboundGroupSession(); + } + }); } catch (e) { print("[LibOlm] Unable to update user device keys: " + e.toString()); } diff --git a/lib/src/room.dart b/lib/src/room.dart index 13c783a..68c6977 100644 --- a/lib/src/room.dart +++ b/lib/src/room.dart @@ -84,12 +84,20 @@ class Room { olm.OutboundGroupSession get outboundGroupSession => _outboundGroupSession; olm.OutboundGroupSession _outboundGroupSession; + List _outboundGroupSessionDevices; + /// Clears the existing outboundGroupSession, tries to create a new one and /// stores it as an ingoingGroupSession in the [sessionKeys]. Then sends the /// new session encrypted with olm to all non-blocked devices using /// to-device-messaging. Future createOutboundGroupSession() async { - await clearOutboundGroupSession(); + await clearOutboundGroupSession(wipe: true); + List deviceKeys = await getUserDeviceKeys(); + _outboundGroupSessionDevices = []; + for (DeviceKeys keys in deviceKeys) { + _outboundGroupSessionDevices.add(keys.deviceId); + } + _outboundGroupSessionDevices.sort(); try { _outboundGroupSession = olm.OutboundGroupSession(); _outboundGroupSession.create(); @@ -110,7 +118,6 @@ class Room { "session_key": _outboundGroupSession.session_key(), }; setSessionKey(rawSession["session_id"], rawSession); - List deviceKeys = await getUserDeviceKeys(); try { await client.sendToDevice(deviceKeys, "m.room_key", rawSession); } catch (e) { @@ -127,17 +134,35 @@ class Room { await client.storeAPI?.setItem( "/clients/${client.deviceID}/rooms/${this.id}/outbound_group_session", _outboundGroupSession.pickle(client.userID)); + await client.storeAPI?.setItem( + "/clients/${client.deviceID}/rooms/${this.id}/outbound_group_session_devices", + json.encode(_outboundGroupSessionDevices)); return; } - /// Clears the existing outboundGroupSession. - Future clearOutboundGroupSession() async { + /// Clears the existing outboundGroupSession but first checks if the participating + /// devices have been changed. Returns false if the session has not been cleared because + /// it wasn't necessary. + Future clearOutboundGroupSession({bool wipe = false}) async { + if (!wipe && this._outboundGroupSessionDevices != null) { + List deviceKeys = await getUserDeviceKeys(); + List outboundGroupSessionDevices = []; + for (DeviceKeys keys in deviceKeys) { + outboundGroupSessionDevices.add(keys.deviceId); + } + outboundGroupSessionDevices.sort(); + if (outboundGroupSessionDevices.toString() == + this._outboundGroupSessionDevices.toString()) { + return false; + } + } + this._outboundGroupSessionDevices == null; await client.storeAPI?.setItem( "/clients/${client.deviceID}/rooms/${this.id}/outbound_group_session", null); this._outboundGroupSession?.free(); this._outboundGroupSession = null; - return; + return true; } /// Key-Value store of session ids to the session keys. Only m.megolm.v1.aes-sha2 @@ -861,6 +886,13 @@ class Room { e.toString()); } } + final String outboundGroupSessionDevicesString = await client.storeAPI + .getItem( + "/clients/${client.deviceID}/rooms/${this.id}/outbound_group_session_devices"); + if (outboundGroupSessionDevicesString != null) { + this._outboundGroupSessionDevices = + List.from(json.decode(outboundGroupSessionDevicesString)); + } final String sessionKeysPickle = await client.storeAPI .getItem("/clients/${client.deviceID}/rooms/${this.id}/session_keys"); if (sessionKeysPickle?.isNotEmpty ?? false) { diff --git a/test/client_test.dart b/test/client_test.dart index 7f81e0c..c46d596 100644 --- a/test/client_test.dart +++ b/test/client_test.dart @@ -563,7 +563,7 @@ void main() { }); test('Test invalidate outboundGroupSessions', () async { if (matrix.encryptionEnabled) { - await matrix.rooms[1].clearOutboundGroupSession(); + await matrix.rooms[1].clearOutboundGroupSession(wipe: true); expect(matrix.rooms[1].outboundGroupSession == null, true); await matrix.rooms[1].createOutboundGroupSession(); expect(matrix.rooms[1].outboundGroupSession != null, true); @@ -589,7 +589,7 @@ void main() { } }); await Future.delayed(Duration(milliseconds: 50)); - expect(matrix.rooms[1].outboundGroupSession == null, true); + expect(matrix.rooms[1].outboundGroupSession != null, true); } }); DeviceKeys deviceKeys = DeviceKeys.fromJson({ diff --git a/test/fake_matrix_api.dart b/test/fake_matrix_api.dart index a562d54..1528d77 100644 --- a/test/fake_matrix_api.dart +++ b/test/fake_matrix_api.dart @@ -603,6 +603,24 @@ class FakeMatrixApi extends MockClient { {"type": "m.login.password"} ] }, + "/client/r0/rooms/!696r7674:example.com/members": (var req) => { + "chunk": [ + { + "content": { + "membership": "join", + "avatar_url": "mxc://example.org/SEsfnsuifSDFSSEF", + "displayname": "Alice Margatroid" + }, + "type": "m.room.member", + "event_id": "ยง143273582443PhrSn:example.org", + "room_id": "!636q39766251:example.com", + "sender": "@alice:example.com", + "origin_server_ts": 1432735824653, + "unsigned": {"age": 1234}, + "state_key": "@alice:example.com" + } + ] + }, "/client/r0/rooms/!726s6s6q:example.com/members": (var req) => { "chunk": [ { diff --git a/test/room_test.dart b/test/room_test.dart index 822b2f9..03807b0 100644 --- a/test/room_test.dart +++ b/test/room_test.dart @@ -406,7 +406,7 @@ void main() { test('clearOutboundGroupSession', () async { if (!room.client.encryptionEnabled) return; - await room.clearOutboundGroupSession(); + await room.clearOutboundGroupSession(wipe: true); expect(room.outboundGroupSession == null, true); });