diff --git a/lib/src/client.dart b/lib/src/client.dart index 577b67ec..70479dab 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -38,6 +38,7 @@ import 'utils/matrix_file.dart'; import 'utils/room_update.dart'; import 'utils/to_device_event.dart'; import 'utils/uia_request.dart'; +import 'utils/multilock.dart'; typedef RoomSorter = int Function(Room a, Room b); @@ -1857,6 +1858,8 @@ sort order of ${prevState.sortOrder}. This should never happen...'''); return; } + final MultiLock _sendToDeviceEncryptedLock = MultiLock(); + /// Sends an encrypted [message] of this [eventType] to these [deviceKeys]. Future sendToDeviceEncrypted( List deviceKeys, @@ -1876,13 +1879,27 @@ sort order of ${prevState.sortOrder}. This should never happen...'''); if (deviceKeys.isEmpty) return; } - // Send with send-to-device messaging - var data = >>{}; - data = - await encryption.encryptToDeviceMessage(deviceKeys, eventType, message); - eventType = EventTypes.Encrypted; - await sendToDevice( - eventType, messageId ?? generateUniqueTransactionId(), data); + // So that we can guarantee order of encrypted to_device messages to be preserved we + // must ensure that we don't attempt to encrypt multiple concurrent to_device messages + // to the same device at the same time. + // A failure to do so can result in edge-cases where encryption and sending order of + // said to_device messages does not match up, resulting in an olm session corruption. + // As we send to multiple devices at the same time, we may only proceed here if the lock for + // *all* of them is freed and lock *all* of them while sending. + + try { + await _sendToDeviceEncryptedLock.lock(deviceKeys); + + // Send with send-to-device messaging + var data = >>{}; + data = await encryption.encryptToDeviceMessage( + deviceKeys, eventType, message); + eventType = EventTypes.Encrypted; + await sendToDevice( + eventType, messageId ?? generateUniqueTransactionId(), data); + } finally { + _sendToDeviceEncryptedLock.unlock(deviceKeys); + } } /// Sends an encrypted [message] of this [eventType] to these [deviceKeys]. diff --git a/lib/src/utils/device_keys_list.dart b/lib/src/utils/device_keys_list.dart index 4e6d6a5f..bc8088f1 100644 --- a/lib/src/utils/device_keys_list.dart +++ b/lib/src/utils/device_keys_list.dart @@ -333,6 +333,11 @@ abstract class SignableKey extends MatrixSignableKey { @override String toString() => json.encode(toJson()); + + @override + bool operator ==(dynamic other) => (other is SignableKey && + other.userId == userId && + other.identifier == identifier); } class CrossSigningKey extends SignableKey { diff --git a/lib/src/utils/multilock.dart b/lib/src/utils/multilock.dart new file mode 100644 index 00000000..30eb5890 --- /dev/null +++ b/lib/src/utils/multilock.dart @@ -0,0 +1,51 @@ +import 'dart:async'; + +/// Lock management class. It allows to lock and unlock multiple keys at once. The keys have +/// the type [T] +class MultiLock { + final Map> _completers = {}; + + /// Set a number of [keys] locks, awaiting them to be released previously. + Future lock(Iterable keys) async { + // An iterable might have duplicate entries. A set is guaranteed not to, and we need + // unique entries, as else a lot of things might go bad. + final uniqueKeys = keys.toSet(); + // we want to make sure that there are no existing completers for any of the locks + // we are trying to set. So, we await all the completers until they are all gone. + // We can't just assume they are all gone after one go, due to rare race conditions + // which could then result in a deadlock. + while (_completers.keys.any((k) => uniqueKeys.contains(k))) { + // Here we try to build all the futures to wait for single completers and then await + // them at the same time, in parallel + final futures = >[]; + for (final key in uniqueKeys) { + if (_completers[key] != null) { + futures.add(() async { + while (_completers[key] != null) { + await _completers[key].future; + } + }()); + } + } + await Future.wait(futures); + } + // And finally set all the completers + for (final key in uniqueKeys) { + _completers[key] = Completer(); + } + } + + /// Unlock all [keys] locks. Typically these should be the same keys as called + /// in `.lock(keys)`` + void unlock(Iterable keys) { + final uniqueKeys = keys.toSet(); + // we just have to simply unlock all the completers + for (final key in uniqueKeys) { + if (_completers[key] != null) { + final completer = _completers[key]; + _completers.remove(key); + completer.complete(); + } + } + } +} diff --git a/test/multilock_test.dart b/test/multilock_test.dart new file mode 100644 index 00000000..292b1e4e --- /dev/null +++ b/test/multilock_test.dart @@ -0,0 +1,62 @@ +/* + * Famedly Matrix SDK + * Copyright (C) 2021 Famedly GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import 'package:matrix/src/utils/multilock.dart'; +import 'package:test/test.dart'; + +void main() { + group('lock', () { + final lock = MultiLock(); + test('lock and unlock', () async { + // lock and unlock + await lock.lock(['fox']); + lock.unlock(['fox']); + expect(true, true); // we were able to reach this line of code! + }); + test('lock the same lock', () async { + var counter = 0; + await lock.lock(['fox']); + final future = lock.lock(['fox']).then((_) { + counter++; + lock.unlock(['fox']); + }); + await Future.delayed(Duration(milliseconds: 50)); + expect(counter, 0); + lock.unlock(['fox']); + await future; + expect(counter, 1); + }); + test('multilock', () async { + var counter = 0; + await lock.lock(['fox']); + final future1 = lock.lock(['fox', 'raccoon']).then((_) { + counter++; + lock.unlock(['fox', 'raccoon']); + }); + await Future.delayed(Duration(milliseconds: 50)); + expect(counter, 0); + await lock.lock(['raccoon']); + lock.unlock(['fox']); + await Future.delayed(Duration(milliseconds: 50)); + expect(counter, 0); + lock.unlock(['raccoon']); + await future1; + expect(counter, 1); + }); + }); +}