From 23282484d65107d13e0a47c0d85cd5e7f24c27ae Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 3 Oct 2018 22:03:40 +0200 Subject: [PATCH] prevent race condition when fetching device ids --- .../crypto/axolotl/AxolotlService.java | 86 +++++++++---------- .../crypto/axolotl/FingerprintStatus.java | 4 + 2 files changed, 47 insertions(+), 43 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java index 93626e634..5c28219c6 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java +++ b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java @@ -1021,28 +1021,33 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { } if (packet != null) { mXmppConnectionService.sendIqPacket(account, packet, (account, response) -> { - synchronized (fetchDeviceIdsMap) { - List callbacks = fetchDeviceIdsMap.remove(jid); - if (response.getType() == IqPacket.TYPE.RESULT) { - fetchDeviceListStatus.put(jid, true); - Element item = mXmppConnectionService.getIqParser().getItem(response); - Set deviceIds = mXmppConnectionService.getIqParser().deviceIds(item); - registerDevices(jid, deviceIds); - if (callbacks != null) { - for (OnDeviceIdsFetched c : callbacks) { - c.fetched(jid, deviceIds); - } + if (response.getType() == IqPacket.TYPE.RESULT) { + fetchDeviceListStatus.put(jid, true); + Element item = mXmppConnectionService.getIqParser().getItem(response); + Set deviceIds = mXmppConnectionService.getIqParser().deviceIds(item); + registerDevices(jid, deviceIds); + final List callbacks; + synchronized (fetchDeviceIdsMap) { + callbacks = fetchDeviceIdsMap.remove(jid); + } + if (callbacks != null) { + for (OnDeviceIdsFetched c : callbacks) { + c.fetched(jid, deviceIds); } + } + } else { + if (response.getType() == IqPacket.TYPE.TIMEOUT) { + fetchDeviceListStatus.remove(jid); } else { - if (response.getType() == IqPacket.TYPE.TIMEOUT) { - fetchDeviceListStatus.remove(jid); - } else { - fetchDeviceListStatus.put(jid, false); - } - if (callbacks != null) { - for (OnDeviceIdsFetched c : callbacks) { - c.fetched(jid, null); - } + fetchDeviceListStatus.put(jid, false); + } + final List callbacks; + synchronized (fetchDeviceIdsMap) { + callbacks = fetchDeviceIdsMap.remove(jid); + } + if (callbacks != null) { + for (OnDeviceIdsFetched c : callbacks) { + c.fetched(jid, null); } } } @@ -1157,8 +1162,9 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { Set addresses = new HashSet<>(); for (Jid jid : getCryptoTargets(conversation)) { Log.d(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Finding devices without session for " + jid); - if (deviceIds.get(jid) != null) { - for (Integer foreignId : this.deviceIds.get(jid)) { + Set ids = deviceIds.get(jid); + if (deviceIds.get(jid) != null && !ids.isEmpty()) { + for (Integer foreignId : ids) { SignalProtocolAddress address = new SignalProtocolAddress(jid.toString(), foreignId); if (sessions.get(address) == null) { IdentityKey identityKey = axolotlStore.loadSession(address).getSessionState().getRemoteIdentityKey(); @@ -1181,22 +1187,21 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { Log.w(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Have no target devices in PEP!"); } } - if (deviceIds.get(account.getJid().asBareJid()) != null) { - for (Integer ownId : this.deviceIds.get(account.getJid().asBareJid())) { - SignalProtocolAddress address = new SignalProtocolAddress(account.getJid().asBareJid().toString(), ownId); - if (sessions.get(address) == null) { - IdentityKey identityKey = axolotlStore.loadSession(address).getSessionState().getRemoteIdentityKey(); - if (identityKey != null) { - Log.d(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Already have session for " + address.toString() + ", adding to cache..."); - XmppAxolotlSession session = new XmppAxolotlSession(account, axolotlStore, address, identityKey); - sessions.put(address, session); + Set ownIds = this.deviceIds.get(account.getJid().asBareJid()); + for (Integer ownId : (ownIds != null ? ownIds : new HashSet())) { + SignalProtocolAddress address = new SignalProtocolAddress(account.getJid().asBareJid().toString(), ownId); + if (sessions.get(address) == null) { + IdentityKey identityKey = axolotlStore.loadSession(address).getSessionState().getRemoteIdentityKey(); + if (identityKey != null) { + Log.d(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Already have session for " + address.toString() + ", adding to cache..."); + XmppAxolotlSession session = new XmppAxolotlSession(account, axolotlStore, address, identityKey); + sessions.put(address, session); + } else { + Log.d(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Found device " + account.getJid().asBareJid() + ":" + ownId); + if (fetchStatusMap.get(address) != FetchStatus.ERROR) { + addresses.add(address); } else { - Log.d(Config.LOGTAG, AxolotlService.getLogprefix(account) + "Found device " + account.getJid().asBareJid() + ":" + ownId); - if (fetchStatusMap.get(address) != FetchStatus.ERROR) { - addresses.add(address); - } else { - Log.d(Config.LOGTAG, getLogprefix(account) + "skipping over " + address + " because it's broken"); - } + Log.d(Config.LOGTAG, getLogprefix(account) + "skipping over " + address + " because it's broken"); } } } @@ -1215,12 +1220,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { } Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": createSessionsIfNeeded() - jids with empty device list: " + jidsWithEmptyDeviceList); if (jidsWithEmptyDeviceList.size() > 0) { - fetchDeviceIds(jidsWithEmptyDeviceList, new OnMultipleDeviceIdFetched() { - @Override - public void fetched() { - createSessionsIfNeededActual(conversation); - } - }); + fetchDeviceIds(jidsWithEmptyDeviceList, () -> createSessionsIfNeededActual(conversation)); return true; } else { return createSessionsIfNeededActual(conversation); diff --git a/src/main/java/eu/siacs/conversations/crypto/axolotl/FingerprintStatus.java b/src/main/java/eu/siacs/conversations/crypto/axolotl/FingerprintStatus.java index 56f4a5d2d..2f1856d09 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/FingerprintStatus.java +++ b/src/main/java/eu/siacs/conversations/crypto/axolotl/FingerprintStatus.java @@ -78,6 +78,10 @@ public class FingerprintStatus implements Comparable { return status; } + public static FingerprintStatus createActive(Boolean trusted) { + return createActive(trusted != null && trusted); + } + public static FingerprintStatus createActive(boolean trusted) { final FingerprintStatus status = new FingerprintStatus(); status.trust = trusted ? Trust.TRUSTED : Trust.UNTRUSTED;