From cc377202da8c2ef0d6d02911ce9f52e90210d8f9 Mon Sep 17 00:00:00 2001 From: Malin Errenst Date: Mon, 8 May 2023 09:12:14 +0000 Subject: [PATCH] fix: remove deprecated sender_key occurrences --- lib/encryption/encryption.dart | 7 +- lib/encryption/key_manager.dart | 76 +++++++---------- .../encrypt_decrypt_room_message_test.dart | 20 +++++ test/encryption/key_manager_test.dart | 81 +++++++------------ test/encryption/key_request_test.dart | 45 +++++++---- test/encryption/online_key_backup_test.dart | 6 +- 6 files changed, 110 insertions(+), 125 deletions(-) diff --git a/lib/encryption/encryption.dart b/lib/encryption/encryption.dart index c46a36aa..afaf26ae 100644 --- a/lib/encryption/encryption.dart +++ b/lib/encryption/encryption.dart @@ -204,13 +204,12 @@ class Encryption { throw DecryptException(DecryptException.unknownAlgorithm); } final sessionId = content.sessionId; - final senderKey = content.senderKey; if (sessionId == null) { throw DecryptException(DecryptException.unknownSession); } final inboundGroupSession = - keyManager.getInboundGroupSession(roomId, sessionId, senderKey); + keyManager.getInboundGroupSession(roomId, sessionId); if (!(inboundGroupSession?.isValid ?? false)) { canRequestSession = true; throw DecryptException(DecryptException.unknownSession); @@ -309,14 +308,12 @@ class Encryption { .getInboundGroupSession( roomId, sessionId, - content.senderKey, ) ?.isValid ?? false)) { await keyManager.loadInboundGroupSession( roomId, sessionId, - content.senderKey, ); } event = decryptRoomEventSync(roomId, event); @@ -392,6 +389,8 @@ class Encryption { 'algorithm': AlgorithmTypes.megolmV1AesSha2, 'ciphertext': sess!.outboundGroupSession!.encrypt(json.encode(payloadContent)), + // device_id + sender_key should be removed at some point in future since + // they're deprecated. Just left here for compatibility 'device_id': client.deviceID, 'sender_key': identityKey, 'session_id': sess.outboundGroupSession!.session_id(), diff --git a/lib/encryption/key_manager.dart b/lib/encryption/key_manager.dart index 354894dd..1dc9501f 100644 --- a/lib/encryption/key_manager.dart +++ b/lib/encryption/key_manager.dart @@ -108,8 +108,10 @@ class KeyManager { senderClaimedKeys_['ed25519'] = device.ed25519Key!; } } - final oldSession = - getInboundGroupSession(roomId, sessionId, senderKey, otherRooms: false); + final oldSession = getInboundGroupSession( + roomId, + sessionId, + ); if (content['algorithm'] != AlgorithmTypes.megolmV1AesSha2) { return; } @@ -215,29 +217,14 @@ class KeyManager { return storeFuture ?? Future.value(); } - SessionKey? getInboundGroupSession( - String roomId, String sessionId, String senderKey, - {bool otherRooms = true}) { + SessionKey? getInboundGroupSession(String roomId, String sessionId) { final sess = _inboundGroupSessions[roomId]?[sessionId]; if (sess != null) { - if (sess.senderKey != senderKey && sess.senderKey.isNotEmpty) { + if (sess.sessionId != sessionId && sess.sessionId.isNotEmpty) { return null; } return sess; } - if (!otherRooms) { - return null; - } - // search if this session id is *somehow* found in another room - for (final val in _inboundGroupSessions.values) { - final sess = val[sessionId]; - if (sess != null) { - if (sess.senderKey != senderKey && sess.senderKey.isNotEmpty) { - return null; - } - return sess; - } - } return null; } @@ -245,12 +232,12 @@ class KeyManager { void maybeAutoRequest( String roomId, String sessionId, - String senderKey, { + String? senderKey, { bool tryOnlineBackup = true, bool onlineKeyBackupOnly = true, }) { final room = client.getRoomById(roomId); - final requestIdent = '$roomId|$sessionId|$senderKey'; + final requestIdent = '$roomId|$sessionId'; if (room != null && !_requestedSessionIds.contains(requestIdent) && !client.isUnknownSession) { @@ -268,11 +255,11 @@ class KeyManager { /// Loads an inbound group session Future loadInboundGroupSession( - String roomId, String sessionId, String senderKey) async { + String roomId, String sessionId) async { final sess = _inboundGroupSessions[roomId]?[sessionId]; if (sess != null) { - if (sess.senderKey != senderKey && sess.senderKey.isNotEmpty) { - return null; // sender keys do not match....better not do anything + if (sess.sessionId != sessionId && sess.sessionId.isNotEmpty) { + return null; // session_id does not match....better not do anything } return sess; // nothing to do } @@ -285,8 +272,8 @@ class KeyManager { final roomInboundGroupSessions = _inboundGroupSessions[roomId] ??= {}; if (!dbSess.isValid || - dbSess.senderKey.isEmpty || - dbSess.senderKey != senderKey) { + dbSess.sessionId.isEmpty || + dbSess.sessionId != sessionId) { return null; } roomInboundGroupSessions[sessionId] = dbSess; @@ -339,8 +326,8 @@ class KeyManager { } } - final inboundSess = await loadInboundGroupSession(room.id, - sess.outboundGroupSession!.session_id(), encryption.identityKey!); + final inboundSess = await loadInboundGroupSession( + room.id, sess.outboundGroupSession!.session_id()); if (inboundSess == null) { wipe = true; } @@ -697,14 +684,13 @@ class KeyManager { Future request( Room room, String sessionId, - String senderKey, { + String? senderKey, { bool tryOnlineBackup = true, bool onlineKeyBackupOnly = false, }) async { if (tryOnlineBackup && await isCached()) { // let's first check our online key backup store thingy... - final hadPreviously = - getInboundGroupSession(room.id, sessionId, senderKey) != null; + final hadPreviously = getInboundGroupSession(room.id, sessionId) != null; try { await loadSingleKey(room.id, sessionId); } catch (err, stacktrace) { @@ -718,7 +704,7 @@ class KeyManager { } // TODO: also don't request from others if we have an index of 0 now if (!hadPreviously && - getInboundGroupSession(room.id, sessionId, senderKey) != null) { + getInboundGroupSession(room.id, sessionId) != null) { return; // we managed to load the session from online backup, no need to care about it now } } @@ -735,7 +721,6 @@ class KeyManager { devices: devices, room: room, sessionId: sessionId, - senderKey: senderKey, ); final userList = await room.requestParticipants(); await client.sendToDevicesOfUserIds( @@ -746,8 +731,8 @@ class KeyManager { 'body': { 'algorithm': AlgorithmTypes.megolmV1AesSha2, 'room_id': room.id, - 'sender_key': senderKey, 'session_id': sessionId, + if (senderKey != null) 'sender_key': senderKey, }, 'request_id': requestId, 'requesting_device_id': client.deviceID, @@ -866,10 +851,8 @@ class KeyManager { return; // unknown room } final sessionId = event.content['body']['session_id']; - final senderKey = event.content['body']['sender_key']; // okay, let's see if we have this session at all - final session = - await loadInboundGroupSession(room.id, sessionId, senderKey); + final session = await loadInboundGroupSession(room.id, sessionId); if (session == null) { Logs().i('[KeyManager] Unknown session, ignoring'); return; // we don't have this session anyways @@ -879,7 +862,6 @@ class KeyManager { devices: [device], room: room, sessionId: sessionId, - senderKey: senderKey, ); if (incomingShareRequests.containsKey(request.requestId)) { Logs().i('[KeyManager] Already processed this request, ignoring'); @@ -935,8 +917,7 @@ class KeyManager { } final request = outgoingShareRequests.values.firstWhereOrNull((r) => r.room.id == event.content['room_id'] && - r.sessionId == event.content['session_id'] && - r.senderKey == event.content['sender_key']); + r.sessionId == event.content['session_id']); if (request == null || request.canceled) { return; // no associated request found or it got canceled } @@ -954,8 +935,8 @@ class KeyManager { .add(encryptedContent['sender_key']); // TODO: verify that the keys work to decrypt a message // alright, all checks out, let's go ahead and store this session - await setInboundGroupSession( - request.room.id, request.sessionId, request.senderKey, event.content, + await setInboundGroupSession(request.room.id, request.sessionId, + device.curve25519Key!, event.content, forwarded: true, senderClaimedKeys: { 'ed25519': event.content['sender_claimed_ed25519_key'], @@ -1022,7 +1003,6 @@ class KeyManagerKeyShareRequest { final List devices; final Room room; final String sessionId; - final String senderKey; bool canceled; KeyManagerKeyShareRequest( @@ -1030,7 +1010,6 @@ class KeyManagerKeyShareRequest { List? devices, required this.room, required this.sessionId, - required this.senderKey, this.canceled = false}) : devices = devices ?? []; } @@ -1056,8 +1035,8 @@ class RoomKeyRequest extends ToDeviceEvent { return; // request is canceled, don't send anything } final room = this.room; - final session = await keyManager.loadInboundGroupSession( - room.id, request.sessionId, request.senderKey); + final session = + await keyManager.loadInboundGroupSession(room.id, request.sessionId); if (session?.inboundGroupSession == null) { Logs().v("[KeyManager] Not forwarding key we don't have"); return; @@ -1067,8 +1046,9 @@ class RoomKeyRequest extends ToDeviceEvent { message['forwarding_curve25519_key_chain'] = List.from(session.forwardingCurve25519KeyChain); - message['sender_key'] = - (session.senderKey.isNotEmpty) ? session.senderKey : request.senderKey; + if (session.senderKey.isNotEmpty) { + message['sender_key'] = session.senderKey; + } message['sender_claimed_ed25519_key'] = session.senderClaimedKeys['ed25519'] ?? (session.forwardingCurve25519KeyChain.isEmpty diff --git a/test/encryption/encrypt_decrypt_room_message_test.dart b/test/encryption/encrypt_decrypt_room_message_test.dart index 9f9884c7..2f3d3703 100644 --- a/test/encryption/encrypt_decrypt_room_message_test.dart +++ b/test/encryption/encrypt_decrypt_room_message_test.dart @@ -79,6 +79,26 @@ void main() { expect(decryptedEvent.originalSource?.toJson(), encryptedEvent.toJson()); }); + test('decrypt payload without device_id', () async { + if (!olmEnabled) return; + payload.remove('device_id'); + payload.remove('sender_key'); + final encryptedEvent = Event( + type: EventTypes.Encrypted, + content: payload, + room: room, + originServerTs: now, + eventId: '\$event', + senderId: client.userID!, + ); + final decryptedEvent = + await client.encryption!.decryptRoomEvent(roomId, encryptedEvent); + expect(decryptedEvent.type, 'm.room.message'); + expect(decryptedEvent.content['msgtype'], 'm.text'); + expect(decryptedEvent.content['text'], 'Hello foxies!'); + expect(decryptedEvent.originalSource?.toJson(), encryptedEvent.toJson()); + }); + test('decrypt payload nocache', () async { if (!olmEnabled) return; client.encryption!.keyManager.clearInboundGroupSessions(); diff --git a/test/encryption/key_manager_test.dart b/test/encryption/key_manager_test.dart index d9f0e77f..dbdf12c1 100644 --- a/test/encryption/key_manager_test.dart +++ b/test/encryption/key_manager_test.dart @@ -69,7 +69,7 @@ void main() { await client.encryption!.keyManager.handleToDeviceEvent(event); expect( client.encryption!.keyManager.getInboundGroupSession( - '!726s6s6q:example.com', validSessionId, validSenderKey) != + '!726s6s6q:example.com', validSessionId) != null, true); @@ -89,7 +89,7 @@ void main() { await client.encryption!.keyManager.handleToDeviceEvent(event); expect( client.encryption!.keyManager.getInboundGroupSession( - '!726s6s6q:example.com', validSessionId, validSenderKey) != + '!726s6s6q:example.com', validSessionId) != null, false); }); @@ -111,7 +111,7 @@ void main() { client.encryption!.keyManager.getOutboundGroupSession(roomId) != null, true); var inbound = client.encryption!.keyManager.getInboundGroupSession( - roomId, sess.outboundGroupSession!.session_id(), client.identityKey); + roomId, sess.outboundGroupSession!.session_id()); expect(inbound != null, true); expect( inbound!.allowedAtIndex['@alice:example.com'] @@ -220,7 +220,7 @@ void main() { client.encryption!.keyManager.getOutboundGroupSession(roomId) != null, true); inbound = client.encryption!.keyManager.getInboundGroupSession( - roomId, sess.outboundGroupSession!.session_id(), client.identityKey); + roomId, sess.outboundGroupSession!.session_id()); expect( inbound!.allowedAtIndex['@alice:example.com'] ?['L+4+JCl8MD63dgo8z5Ta+9QAHXiANyOVSfgbHA5d3H8'], @@ -284,68 +284,41 @@ void main() { client.encryption!.keyManager.clearInboundGroupSessions(); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) != + .getInboundGroupSession(roomId, sessionId) != null, false); await client.encryption!.keyManager .setInboundGroupSession(roomId, sessionId, senderKey, sessionContent); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) != + .getInboundGroupSession(roomId, sessionId) != null, true); - expect( - client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, 'invalid') != - null, - false); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) != - null, - true); - expect( - client.encryption!.keyManager - .getInboundGroupSession('otherroom', sessionId, senderKey) != - null, - true); - expect( - client.encryption!.keyManager - .getInboundGroupSession('otherroom', sessionId, 'invalid') != - null, - false); - expect( - client.encryption!.keyManager - .getInboundGroupSession('otherroom', 'invalid', senderKey) != - null, - false); - - client.encryption!.keyManager.clearInboundGroupSessions(); - expect( - client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) != - null, - false); - await client.encryption!.keyManager - .loadInboundGroupSession(roomId, sessionId, senderKey); - expect( - client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) != + .getInboundGroupSession(roomId, sessionId) != null, true); client.encryption!.keyManager.clearInboundGroupSessions(); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) != + .getInboundGroupSession(roomId, sessionId) != null, false); await client.encryption!.keyManager - .loadInboundGroupSession(roomId, sessionId, 'invalid'); + .loadInboundGroupSession(roomId, sessionId); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, 'invalid') != + .getInboundGroupSession(roomId, sessionId) != + null, + true); + + client.encryption!.keyManager.clearInboundGroupSessions(); + expect( + client.encryption!.keyManager + .getInboundGroupSession(roomId, sessionId) != null, false); }); @@ -398,13 +371,13 @@ void main() { forwarded: true); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.inboundGroupSession ?.first_known_index(), 1); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.forwardingCurve25519KeyChain .length, 1); @@ -424,13 +397,13 @@ void main() { forwarded: true); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.inboundGroupSession ?.first_known_index(), 1); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.forwardingCurve25519KeyChain .length, 1); @@ -450,13 +423,13 @@ void main() { forwarded: true); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.inboundGroupSession ?.first_known_index(), 0); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.forwardingCurve25519KeyChain .length, 1); @@ -476,13 +449,13 @@ void main() { forwarded: true); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.inboundGroupSession ?.first_known_index(), 0); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.forwardingCurve25519KeyChain .length, 1); @@ -502,13 +475,13 @@ void main() { forwarded: true); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.inboundGroupSession ?.first_known_index(), 0); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) + .getInboundGroupSession(roomId, sessionId) ?.forwardingCurve25519KeyChain .length, 0); diff --git a/test/encryption/key_request_test.dart b/test/encryption/key_request_test.dart index 4470e1b0..b788a06f 100644 --- a/test/encryption/key_request_test.dart +++ b/test/encryption/key_request_test.dart @@ -72,7 +72,6 @@ void main() { final content = payload['messages']['@alice:example.com']['*']; if (content['action'] == 'request' && content['body']['room_id'] == '!726s6s6q:example.com' && - content['body']['sender_key'] == validSenderKey && content['body']['session_id'] == 'sessionId') { foundEvent = true; break; @@ -94,8 +93,7 @@ void main() { .userDeviceKeys['@alice:example.com']!.deviceKeys['OTHERDEVICE']! .setVerified(true); final session = await matrix.encryption!.keyManager - .loadInboundGroupSession( - '!726s6s6q:example.com', validSessionId, validSenderKey); + .loadInboundGroupSession('!726s6s6q:example.com', validSessionId); // test a successful share var event = ToDeviceEvent( sender: '@alice:example.com', @@ -287,8 +285,7 @@ void main() { tryOnlineBackup: false); final session = (await matrix.encryption!.keyManager - .loadInboundGroupSession( - requestRoom.id, validSessionId, validSenderKey))!; + .loadInboundGroupSession(requestRoom.id, validSessionId))!; final sessionKey = session.inboundGroupSession! .export_session(session.inboundGroupSession!.first_known_index()); matrix.encryption!.keyManager.clearInboundGroupSessions(); @@ -310,11 +307,28 @@ void main() { }); await matrix.encryption!.keyManager.handleToDeviceEvent(event); expect( - matrix.encryption!.keyManager.getInboundGroupSession( - requestRoom.id, validSessionId, validSenderKey) != + matrix.encryption!.keyManager + .getInboundGroupSession(requestRoom.id, validSessionId) != null, true); + // test ToDeviceEvent without sender_key in content + event = ToDeviceEvent( + sender: '@alice:example.com', + type: 'm.forwarded_room_key', + content: { + 'algorithm': AlgorithmTypes.megolmV1AesSha2, + 'room_id': '!726s6s6q:example.com', + 'session_id': validSessionId, + 'session_key': sessionKey, + 'forwarding_curve25519_key_chain': [], + 'sender_claimed_ed25519_key': + 'L+4+JCl8MD63dgo8z5Ta+9QAHXiANyOVSfgbHA5d3H8', + }, + encryptedContent: { + 'sender_key': 'L+4+JCl8MD63dgo8z5Ta+9QAHXiANyOVSfgbHA5d3H8', + }); + // now test a few invalid scenarios // request not found @@ -337,15 +351,14 @@ void main() { }); await matrix.encryption!.keyManager.handleToDeviceEvent(event); expect( - matrix.encryption!.keyManager.getInboundGroupSession( - requestRoom.id, validSessionId, validSenderKey) != + matrix.encryption!.keyManager + .getInboundGroupSession(requestRoom.id, validSessionId) != null, false); // unknown device - await matrix.encryption!.keyManager.request( - requestRoom, validSessionId, validSenderKey, - tryOnlineBackup: false); + await matrix.encryption!.keyManager + .request(requestRoom, validSessionId, null, tryOnlineBackup: false); matrix.encryption!.keyManager.clearInboundGroupSessions(); event = ToDeviceEvent( sender: '@alice:example.com', @@ -365,8 +378,8 @@ void main() { }); await matrix.encryption!.keyManager.handleToDeviceEvent(event); expect( - matrix.encryption!.keyManager.getInboundGroupSession( - requestRoom.id, validSessionId, validSenderKey) != + matrix.encryption!.keyManager + .getInboundGroupSession(requestRoom.id, validSessionId) != null, false); @@ -390,8 +403,8 @@ void main() { }); await matrix.encryption!.keyManager.handleToDeviceEvent(event); expect( - matrix.encryption!.keyManager.getInboundGroupSession( - requestRoom.id, validSessionId, validSenderKey) != + matrix.encryption!.keyManager + .getInboundGroupSession(requestRoom.id, validSessionId) != null, false); diff --git a/test/encryption/online_key_backup_test.dart b/test/encryption/online_key_backup_test.dart index aa46ecfd..124b1932 100644 --- a/test/encryption/online_key_backup_test.dart +++ b/test/encryption/online_key_backup_test.dart @@ -67,7 +67,7 @@ void main() { .request(client.getRoomById(roomId)!, sessionId, senderKey); expect( client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey) != + .getInboundGroupSession(roomId, sessionId) != null, true); }); @@ -108,11 +108,11 @@ void main() { final onlineKeys = RoomKeys.fromJson(json.decode(payload)); client.encryption!.keyManager.clearInboundGroupSessions(); var ret = client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey); + .getInboundGroupSession(roomId, sessionId); expect(ret, null); await client.encryption!.keyManager.loadFromResponse(onlineKeys); ret = client.encryption!.keyManager - .getInboundGroupSession(roomId, sessionId, senderKey); + .getInboundGroupSession(roomId, sessionId); expect(ret != null, true); });