Merge branch 'soru/e2ee-fixes' into 'main'

fix: Various small e2ee fixes

See merge request famedly/famedlysdk!588

lib/encryption/key_manager.dart

@@ -30,6 +30,7 @@ import '../matrix_api/utils/logs.dart';
 import '../src/utils/run_in_background.dart';
 import '../src/utils/run_in_root.dart';
 import '../matrix_api/utils/try_get_map_extension.dart';
+import '../matrix_api/utils/map_copy_extension.dart';
 
 const MEGOLM_KEY = EventTypes.MegolmBackup;
 
@@ -743,7 +744,8 @@ class KeyManager {
       }
       if (event.content['action'] == 'request') {
         // we are *receiving* a request
-        Logs().i('[KeyManager] Received key sharing request...');
+        Logs().i(
+            '[KeyManager] Received key sharing request from ${event.sender}:${event.content['requesting_device_id']}...');
         if (!event.content.containsKey('body')) {
           Logs().i('[KeyManager] No body, doing nothing');
           return; // no body
@@ -954,10 +956,10 @@ class RoomKeyRequest extends ToDeviceEvent {
       keyManager.incomingShareRequests.remove(request.requestId);
       return; // request is canceled, don't send anything
     }
-    var room = this.room;
+    final room = this.room;
     final session = await keyManager.loadInboundGroupSession(
         room.id, request.sessionId, request.senderKey);
-    var message = session.content;
+    final message = session.content.copy();
     message['forwarding_curve25519_key_chain'] =
         List<String>.from(session.forwardingCurve25519KeyChain);
 

lib/encryption/olm_manager.dart

@@ -227,9 +227,7 @@ class OlmManager {
     if (client.database == null) {
       return;
     }
-    if (!_olmSessions.containsKey(session.identityKey)) {
+    _olmSessions[session.identityKey] ??= [];
-      _olmSessions[session.identityKey] = [];
-    }
     final ix = _olmSessions[session.identityKey]
         .indexWhere((s) => s.sessionId == session.sessionId);
     if (ix == -1) {
@@ -264,19 +262,21 @@ class OlmManager {
     if (type != 0 && type != 1) {
       throw ('Unknown message type');
     }
-    var existingSessions = olmSessions[senderKey];
+    final existingSessions = olmSessions[senderKey];
+    final updateSessionUsage = (OlmSession session) => runInRoot(() {
+          session.lastReceived = DateTime.now();
+          storeOlmSession(session);
+        });
     if (existingSessions != null) {
       for (var session in existingSessions) {
         if (type == 0 && session.session.matches_inbound(body) == true) {
           plaintext = session.session.decrypt(type, body);
-          session.lastReceived = DateTime.now();
+          updateSessionUsage(session);
-          storeOlmSession(session);
           break;
         } else if (type == 1) {
           try {
             plaintext = session.session.decrypt(type, body);
-            session.lastReceived = DateTime.now();
+            updateSessionUsage(session);
-            storeOlmSession(session);
             break;
           } catch (_) {
             plaintext = null;
@@ -295,13 +295,13 @@ class OlmManager {
         _olmAccount.remove_one_time_keys(newSession);
         client.database?.updateClientKeys(pickledOlmAccount, client.id);
         plaintext = newSession.decrypt(type, body);
-        storeOlmSession(OlmSession(
+        runInRoot(() => storeOlmSession(OlmSession(
               key: client.userID,
               identityKey: senderKey,
               sessionId: newSession.session_id(),
               session: newSession,
               lastReceived: DateTime.now(),
-        ));
+            )));
       } catch (_) {
         newSession?.free();
         rethrow;
@@ -345,6 +345,21 @@ class OlmManager {
     return res;
   }
 
+  Future<List<OlmSession>> getOlmSessions(String senderKey) async {
+    var sess = olmSessions[senderKey];
+    if (sess == null || sess.isEmpty) {
+      final sessions = await getOlmSessionsFromDatabase(senderKey);
+      if (sessions.isEmpty) {
+        return [];
+      }
+      sess = _olmSessions[senderKey] = sessions;
+    }
+    sess.sort((a, b) => a.lastReceived == b.lastReceived
+        ? a.sessionId.compareTo(b.sessionId)
+        : b.lastReceived.compareTo(a.lastReceived));
+    return sess;
+  }
+
   final Map<String, DateTime> _restoredOlmSessionsTime = {};
 
   Future<void> restoreOlmSession(String userId, String senderKey) async {
@@ -375,15 +390,8 @@ class OlmManager {
     }
     final senderKey = event.content['sender_key'];
     final loadFromDb = () async {
-      if (client.database == null) {
+      final sessions = await getOlmSessions(senderKey);
-        return false;
+      return sessions.isNotEmpty;
-      }
-      final sessions = await getOlmSessionsFromDatabase(senderKey);
-      if (sessions.isEmpty) {
-        return false; // okay, can't do anything
-      }
-      _olmSessions[senderKey] = sessions;
-      return true;
     };
     if (!_olmSessions.containsKey(senderKey)) {
       await loadFromDb();
@@ -406,6 +414,9 @@ class OlmManager {
   }
 
   Future<void> startOutgoingOlmSessions(List<DeviceKeys> deviceKeys) async {
+    Logs().v(
+        '[OlmManager] Starting session with ${deviceKeys.length} devices...');
+
     var requestingKeysFrom = <String, Map<String, String>>{};
     for (var device in deviceKeys) {
       if (requestingKeysFrom[device.userId] == null) {
@@ -429,6 +440,7 @@ class OlmManager {
           if (!deviceKey.checkJsonSignature(fingerprintKey, userId, deviceId)) {
             continue;
           }
+          Logs().v('[OlmManager] Starting session with ${userId}:${deviceId}');
           var session = olm.Session();
           try {
             session.create_outbound(_olmAccount, identityKey, deviceKey['key']);
@@ -452,17 +464,11 @@ class OlmManager {
 
   Future<Map<String, dynamic>> encryptToDeviceMessagePayload(
       DeviceKeys device, String type, Map<String, dynamic> payload) async {
-    var sess = olmSessions[device.curve25519Key];
+    final sess = await getOlmSessions(device.curve25519Key);
-    if (sess == null || sess.isEmpty) {
+    if (sess.isEmpty) {
-      final sessions = await getOlmSessionsFromDatabase(device.curve25519Key);
+      throw ('No olm session found for ${device.userId}:${device.deviceId}');
-      if (sessions.isEmpty) {
-        throw ('No olm session found');
     }
-      sess = _olmSessions[device.curve25519Key] = sessions;
+
-    }
-    sess.sort((a, b) => a.lastReceived == b.lastReceived
-        ? a.sessionId.compareTo(b.sessionId)
-        : b.lastReceived.compareTo(a.lastReceived));
     final fullPayload = {
       'type': type,
       'content': payload,
@@ -493,18 +499,13 @@ class OlmManager {
     // first check if any of our sessions we want to encrypt for are in the database
     if (client.database != null) {
       for (final device in deviceKeys) {
-        if (!olmSessions.containsKey(device.curve25519Key)) {
+        await getOlmSessions(device.curve25519Key);
-          final sessions =
-              await getOlmSessionsFromDatabase(device.curve25519Key);
-          if (sessions.isNotEmpty) {
-            _olmSessions[device.curve25519Key] = sessions;
-          }
-        }
       }
     }
     final deviceKeysWithoutSession = List<DeviceKeys>.from(deviceKeys);
     deviceKeysWithoutSession.removeWhere((DeviceKeys deviceKeys) =>
-        olmSessions.containsKey(deviceKeys.curve25519Key));
+        olmSessions.containsKey(deviceKeys.curve25519Key) &&
+        olmSessions[deviceKeys.curve25519Key].isNotEmpty);
     if (deviceKeysWithoutSession.isNotEmpty) {
       await startOutgoingOlmSessions(deviceKeysWithoutSession);
     }

lib/src/client.dart

@@ -1428,16 +1428,6 @@ sort order of ${prevState.sortOrder}. This should never happen...''');
                   // Always trust the own device
                   entry.setDirectVerified(true);
                 }
-              } else {
-                // This shouldn't ever happen. The same device ID has gotten
-                // a new public key. So we ignore the update. TODO: ask krille
-                // if we should instead use the new key with unknown verified / blocked status
-                _userDeviceKeys[userId].deviceKeys[deviceId] =
-                    oldKeys[deviceId];
-              }
-            } else {
-              Logs().w('Invalid device ${entry.userId}:${entry.deviceId}');
-            }
                 if (database != null) {
                   dbActions.add(() => database.storeUserDeviceKey(
                         id,
@@ -1448,6 +1438,16 @@ sort order of ${prevState.sortOrder}. This should never happen...''');
                         entry.blocked,
                       ));
                 }
+              } else if (oldKeys.containsKey(deviceId)) {
+                // This shouldn't ever happen. The same device ID has gotten
+                // a new public key. So we ignore the update. TODO: ask krille
+                // if we should instead use the new key with unknown verified / blocked status
+                _userDeviceKeys[userId].deviceKeys[deviceId] =
+                    oldKeys[deviceId];
+              }
+            } else {
+              Logs().w('Invalid device ${entry.userId}:${entry.deviceId}');
+            }
           }
           // delete old/unused entries
           if (database != null) {
@@ -1571,8 +1571,7 @@ sort order of ${prevState.sortOrder}. This should never happen...''');
     return;
   }
 
-  /// Sends an encrypted [message] of this [type] to these [deviceKeys]. To send
+  /// Sends an encrypted [message] of this [type] to these [deviceKeys].
-  /// the request to all devices of the current user, pass an empty list to [deviceKeys].
   Future<void> sendToDeviceEncrypted(
     List<DeviceKeys> deviceKeys,
     String eventType,
@@ -1586,7 +1585,7 @@ sort order of ${prevState.sortOrder}. This should never happen...''');
     if (deviceKeys.isNotEmpty) {
       deviceKeys.removeWhere((DeviceKeys deviceKeys) =>
           deviceKeys.blocked ||
-          deviceKeys.deviceId == deviceID ||
+          (deviceKeys.userId == userID && deviceKeys.deviceId == deviceID) ||
           (onlyVerified && !deviceKeys.verified));
       if (deviceKeys.isEmpty) return;
     }