fix: Add locking to sending encrypted to_device messages to prevent potential race conditions

Due to chunked lazy sending of megolm sessions it was in theory that we encrypted two olm
messages to the same device in different futures out-of-order. Introducing locking here should
fix this (increadibly rare, so far only theoretical?) race-condition
This commit is contained in:
Sorunome 2021-02-26 12:39:32 +01:00
parent e86353a412
commit cd5131daa5
No known key found for this signature in database
GPG Key ID: B19471D07FC9BE9C
4 changed files with 142 additions and 7 deletions

View File

@ -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<DeviceKeys> _sendToDeviceEncryptedLock = MultiLock();
/// Sends an encrypted [message] of this [eventType] to these [deviceKeys].
Future<void> sendToDeviceEncrypted(
List<DeviceKeys> 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 = <String, Map<String, Map<String, dynamic>>>{};
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 = <String, Map<String, Map<String, dynamic>>>{};
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].

View File

@ -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 {

View File

@ -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<T> {
final Map<T, Completer<void>> _completers = {};
/// Set a number of [keys] locks, awaiting them to be released previously.
Future<void> lock(Iterable<T> 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 = <Future<void>>[];
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<void>();
}
}
/// Unlock all [keys] locks. Typically these should be the same keys as called
/// in `.lock(keys)``
void unlock(Iterable<T> 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();
}
}
}
}

62
test/multilock_test.dart Normal file
View File

@ -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 <https://www.gnu.org/licenses/>.
*/
import 'package:matrix/src/utils/multilock.dart';
import 'package:test/test.dart';
void main() {
group('lock', () {
final lock = MultiLock<String>();
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);
});
});
}