From ed0701bdcdf1511bedcb9068bbd2bd55c358544c Mon Sep 17 00:00:00 2001 From: "Alexander \"PapaTutuWawa" Date: Sat, 17 Jun 2023 17:47:07 +0200 Subject: [PATCH] feat: Remove locking from the BTBV trust manager --- lib/src/omemo/omemo.dart | 16 +++- lib/src/trust/base.dart | 2 + lib/src/trust/btbv.dart | 187 +++++++++++++++++---------------------- test/omemo_test.dart | 11 ++- 4 files changed, 102 insertions(+), 114 deletions(-) diff --git a/lib/src/omemo/omemo.dart b/lib/src/omemo/omemo.dart index fb73572..1d8232d 100644 --- a/lib/src/omemo/omemo.dart +++ b/lib/src/omemo/omemo.dart @@ -169,7 +169,6 @@ class OmemoManager { /// The OmemoManager's trust management final TrustManager _trustManager; - TrustManager get trustManager => _trustManager; /// Our own keys... final Lock _deviceLock = Lock(); @@ -200,7 +199,7 @@ class OmemoManager { _ratchetMap.addAll(result.ratchets); // Load trust data - await trustManager.loadTrustData(jid); + await _trustManager.loadTrustData(jid); } Future> _decryptAndVerifyHmac( @@ -449,7 +448,7 @@ class OmemoManager { } // Notify the trust manager - await trustManager.onNewSession( + await _trustManager.onNewSession( stanza.bareSenderJid, stanza.senderDeviceId, ); @@ -653,7 +652,7 @@ class OmemoManager { addedRatchetKeys.add(ratchetKey); // Initiate trust - await trustManager.onNewSession(jid, bundle.id); + await _trustManager.onNewSession(jid, bundle.id); // Track the KEX for later final ik = await ownDevice.ik.pk.getBytes(); @@ -940,4 +939,13 @@ class OmemoManager { @visibleForTesting OmemoDoubleRatchet? getRatchet(RatchetMapKey key) => _ratchetMap[key]; + + /// Trust management functions + Future withTrustManager( + String jid, Future Function(TrustManager) callback) async { + await _ratchetQueue.synchronized( + [jid], + () => callback(_trustManager), + ); + } } diff --git a/lib/src/trust/base.dart b/lib/src/trust/base.dart index cd0e72f..8d2da8a 100644 --- a/lib/src/trust/base.dart +++ b/lib/src/trust/base.dart @@ -9,6 +9,7 @@ abstract class TrustManager { /// Called by the OmemoSessionManager when a new session has been built. Should set /// a default trust state to [jid]'s device with identifier [deviceId]. + @internal Future onNewSession(String jid, int deviceId); /// Return true if the device with id [deviceId] of Jid [jid] should be used for encryption. @@ -20,6 +21,7 @@ abstract class TrustManager { Future setEnabled(String jid, int deviceId, bool enabled); /// Removes all trust decisions for [jid]. + @internal Future removeTrustDecisionsForJid(String jid); // ignore: comment_references diff --git a/lib/src/trust/btbv.dart b/lib/src/trust/btbv.dart index 496fbd2..4eb1365 100644 --- a/lib/src/trust/btbv.dart +++ b/lib/src/trust/btbv.dart @@ -2,7 +2,6 @@ import 'package:meta/meta.dart'; import 'package:omemo_dart/src/helpers.dart'; import 'package:omemo_dart/src/omemo/ratchet_map_key.dart'; import 'package:omemo_dart/src/trust/base.dart'; -import 'package:synchronized/synchronized.dart'; @immutable class BTBVTrustData { @@ -83,9 +82,6 @@ class BlindTrustBeforeVerificationTrustManager extends TrustManager { @protected final Map> devices = {}; - /// The lock for devices and trustCache - final Lock _lock = Lock(); - /// Callback for loading trust data. final BTBVLoadDataCallback loadData; @@ -108,76 +104,63 @@ class BlindTrustBeforeVerificationTrustManager extends TrustManager { @override Future isTrusted(String jid, int deviceId) async { - var returnValue = false; - await _lock.synchronized(() async { - final trustCacheValue = trustCache[RatchetMapKey(jid, deviceId)]; - if (trustCacheValue == BTBVTrustState.notTrusted) { - returnValue = false; - return; - } else if (trustCacheValue == BTBVTrustState.verified) { - // The key is verified, so it's safe. - returnValue = true; - return; + final trustCacheValue = trustCache[RatchetMapKey(jid, deviceId)]; + if (trustCacheValue == BTBVTrustState.notTrusted) { + return false; + } else if (trustCacheValue == BTBVTrustState.verified) { + // The key is verified, so it's safe. + return true; + } else { + if (_hasAtLeastOneVerifiedDevice(jid)) { + // Do not trust if there is at least one device with full trust + return false; } else { - if (_hasAtLeastOneVerifiedDevice(jid)) { - // Do not trust if there is at least one device with full trust - returnValue = false; - return; - } else { - // We have not verified a key from [jid], so it is blind trust all the way. - returnValue = true; - return; - } + // We have not verified a key from [jid], so it is blind trust all the way. + return true; } - }); - - return returnValue; + } } @override Future onNewSession(String jid, int deviceId) async { - await _lock.synchronized(() async { - final key = RatchetMapKey(jid, deviceId); - if (_hasAtLeastOneVerifiedDevice(jid)) { - trustCache[key] = BTBVTrustState.notTrusted; - enablementCache[key] = false; - } else { - trustCache[key] = BTBVTrustState.blindTrust; - enablementCache[key] = true; - } + final key = RatchetMapKey(jid, deviceId); + if (_hasAtLeastOneVerifiedDevice(jid)) { + trustCache[key] = BTBVTrustState.notTrusted; + enablementCache[key] = false; + } else { + trustCache[key] = BTBVTrustState.blindTrust; + enablementCache[key] = true; + } - if (devices.containsKey(jid)) { - devices[jid]!.add(deviceId); - } else { - devices[jid] = List.from([deviceId]); - } + if (devices.containsKey(jid)) { + devices[jid]!.add(deviceId); + } else { + devices[jid] = List.from([deviceId]); + } - // Commit the state - await commit( - BTBVTrustData( - jid, - deviceId, - trustCache[key]!, - enablementCache[key]!, - ), - ); - }); + // Commit the state + await commit( + BTBVTrustData( + jid, + deviceId, + trustCache[key]!, + enablementCache[key]!, + ), + ); } /// Returns a mapping from the device identifiers of [jid] to their trust state. If /// there are no devices known for [jid], then an empty map is returned. Future> getDevicesTrust(String jid) async { - return _lock.synchronized(() async { - final map = {}; + final map = {}; - if (!devices.containsKey(jid)) return map; + if (!devices.containsKey(jid)) return map; - for (final deviceId in devices[jid]!) { - map[deviceId] = trustCache[RatchetMapKey(jid, deviceId)]!; - } + for (final deviceId in devices[jid]!) { + map[deviceId] = trustCache[RatchetMapKey(jid, deviceId)]!; + } - return map; - }); + return map; } /// Sets the trust of [jid]'s device with identifier [deviceId] to [state]. @@ -186,76 +169,66 @@ class BlindTrustBeforeVerificationTrustManager extends TrustManager { int deviceId, BTBVTrustState state, ) async { - await _lock.synchronized(() async { - final key = RatchetMapKey(jid, deviceId); - trustCache[key] = state; + final key = RatchetMapKey(jid, deviceId); + trustCache[key] = state; - // Commit the state - await commit( - BTBVTrustData( - jid, - deviceId, - state, - enablementCache[key]!, - ), - ); - }); + // Commit the state + await commit( + BTBVTrustData( + jid, + deviceId, + state, + enablementCache[key]!, + ), + ); } @override Future isEnabled(String jid, int deviceId) async { - return _lock.synchronized(() async { - final value = enablementCache[RatchetMapKey(jid, deviceId)]; + final value = enablementCache[RatchetMapKey(jid, deviceId)]; - if (value == null) return false; - return value; - }); + if (value == null) return false; + return value; } @override Future setEnabled(String jid, int deviceId, bool enabled) async { final key = RatchetMapKey(jid, deviceId); - await _lock.synchronized(() async { - enablementCache[key] = enabled; + enablementCache[key] = enabled; - // Commit the state - await commit( - BTBVTrustData( - jid, - deviceId, - trustCache[key]!, - enabled, - ), - ); - }); + // Commit the state + await commit( + BTBVTrustData( + jid, + deviceId, + trustCache[key]!, + enabled, + ), + ); } @override Future removeTrustDecisionsForJid(String jid) async { - await _lock.synchronized(() async { - // Clear the caches - for (final device in devices[jid]!) { - final key = RatchetMapKey(jid, device); - trustCache.remove(key); - enablementCache.remove(key); - } - devices.remove(jid); + // Clear the caches + for (final device in devices[jid]!) { + final key = RatchetMapKey(jid, device); + trustCache.remove(key); + enablementCache.remove(key); + } + devices.remove(jid); - // Commit the state - await removeTrust(jid); - }); + // Commit the state + await removeTrust(jid); } @override Future loadTrustData(String jid) async { - await _lock.synchronized(() async { - for (final result in await loadData(jid)) { - final key = RatchetMapKey(jid, result.device); - trustCache[key] = result.state; - enablementCache[key] = result.enabled; - devices.appendOrCreate(jid, result.device); - } - }); + for (final result in await loadData(jid)) { + final key = RatchetMapKey(jid, result.device); + trustCache[key] = result.state; + enablementCache[key] = result.enabled; + devices.appendOrCreate(jid, result.device); + } } @visibleForTesting diff --git a/test/omemo_test.dart b/test/omemo_test.dart index f0bdf25..8d37052 100644 --- a/test/omemo_test.dart +++ b/test/omemo_test.dart @@ -1529,9 +1529,14 @@ void main() { expect(bobResult1.error, null); // Bob should have some trust state - expect( - (bobManager.trustManager as TestingTrustManager).devices[aliceJid], - await aliceManager.getDeviceId(), + await bobManager.withTrustManager( + bobJid, + (tm) async { + expect( + (tm as TestingTrustManager).devices[aliceJid], + await aliceManager.getDeviceId(), + ); + }, ); });