From 4cd652884c31d65f4261f1812c36af84e81ef244 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 15 Sep 2019 11:49:55 +0200 Subject: [PATCH] do not finish or repair sessions for untrusted senders finishing (sending a key transport message in response to pre key message) as well as reparing sessions will leak resource and availability and might in certain situations in group chat leak the Jabber ID. Therefor we disable that. Leaking resource might not be considered harmful by a lot of people however we have always doing similar things with receipts. --- .../crypto/axolotl/AxolotlService.java | 46 +++++++++++++------ .../conversations/parser/MessageParser.java | 9 +++- 2 files changed, 40 insertions(+), 15 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 bbc96811b8173158992e75a7228d0617883448c8..e44e5dcd2c6dc5c11c50056dfaf6167ce0f440b7 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java +++ b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java @@ -48,7 +48,6 @@ import eu.siacs.conversations.services.XmppConnectionService; import eu.siacs.conversations.utils.CryptoHelper; import eu.siacs.conversations.utils.SerialSingleThreadExecutor; import eu.siacs.conversations.xml.Element; -import eu.siacs.conversations.xml.Namespace; import eu.siacs.conversations.xmpp.OnAdvancedStreamFeaturesLoaded; import eu.siacs.conversations.xmpp.OnIqPacketReceived; import eu.siacs.conversations.xmpp.pep.PublishOptions; @@ -67,8 +66,8 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { public static final String LOGPREFIX = "AxolotlService"; - public static final int NUM_KEYS_TO_PUBLISH = 100; - public static final int publishTriesThreshold = 3; + private static final int NUM_KEYS_TO_PUBLISH = 100; + private static final int publishTriesThreshold = 3; private final Account account; private final XmppConnectionService mXmppConnectionService; @@ -1469,7 +1468,9 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { } else { Log.d(Config.LOGTAG,account.getJid().asBareJid()+": nothing to flush. Not republishing key"); } - completeSession(session); + if (trustedOrPreviouslyResponded(session)) { + completeSession(session); + } } } @@ -1479,23 +1480,44 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { publishBundlesIfNeeded(false, false); } } - Iterator iterator = postponedSessions.iterator(); + final Iterator iterator = postponedSessions.iterator(); while (iterator.hasNext()) { - completeSession(iterator.next()); + final XmppAxolotlSession session = iterator.next(); + if (trustedOrPreviouslyResponded(session)) { + completeSession(iterator.next()); + } iterator.remove(); } - Iterator postponedHealingAttemptsIterator = postponedHealing.iterator(); + final Iterator postponedHealingAttemptsIterator = postponedHealing.iterator(); while (postponedHealingAttemptsIterator.hasNext()) { notifyRequiresHealing(postponedHealingAttemptsIterator.next()); postponedHealingAttemptsIterator.remove(); } } + + private boolean trustedOrPreviouslyResponded(XmppAxolotlSession session) { + try { + return trustedOrPreviouslyResponded(Jid.of(session.getRemoteAddress().getName())); + } catch (IllegalArgumentException e) { + return false; + } + } + + public boolean trustedOrPreviouslyResponded(Jid jid) { + final Contact contact = account.getRoster().getContact(jid); + if (contact.showInRoster() || contact.isSelf()) { + return true; + } + final Conversation conversation = mXmppConnectionService.find(account, jid); + return conversation != null && conversation.sentMessagesCount() > 0; + } + private void completeSession(XmppAxolotlSession session) { final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId()); axolotlMessage.addDevice(session, true); try { - Jid jid = Jid.of(session.getRemoteAddress().getName()); + final Jid jid = Jid.of(session.getRemoteAddress().getName()); MessagePacket packet = mXmppConnectionService.getMessageGenerator().generateKeyTransportMessage(jid, axolotlMessage); mXmppConnectionService.sendMessagePacket(account, packet); } catch (IllegalArgumentException e) { @@ -1505,9 +1527,8 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { public XmppAxolotlMessage.XmppAxolotlKeyTransportMessage processReceivingKeyTransportMessage(XmppAxolotlMessage message, final boolean postponePreKeyMessageHandling) { - XmppAxolotlMessage.XmppAxolotlKeyTransportMessage keyTransportMessage; - - XmppAxolotlSession session = getReceivingSession(message); + final XmppAxolotlMessage.XmppAxolotlKeyTransportMessage keyTransportMessage; + final XmppAxolotlSession session = getReceivingSession(message); try { keyTransportMessage = message.getParameters(session, getOwnDeviceId()); Integer preKeyId = session.getPreKeyIdAndReset(); @@ -1516,7 +1537,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { } } catch (CryptoFailedException e) { Log.d(Config.LOGTAG, "could not decrypt keyTransport message " + e.getMessage()); - keyTransportMessage = null; + return null; } if (session.isFresh() && keyTransportMessage != null) { @@ -1527,7 +1548,6 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { } private void putFreshSession(XmppAxolotlSession session) { - Log.d(Config.LOGTAG, "put fresh session"); sessions.put(session); if (Config.X509_VERIFICATION) { if (session.getIdentityKey() != null) { diff --git a/src/main/java/eu/siacs/conversations/parser/MessageParser.java b/src/main/java/eu/siacs/conversations/parser/MessageParser.java index 738b8fffae91d357666bc1925e5cc62d8eba4fd6..972ce419580a9d36dc09e44df0b95ccf3f9ebef2 100644 --- a/src/main/java/eu/siacs/conversations/parser/MessageParser.java +++ b/src/main/java/eu/siacs/conversations/parser/MessageParser.java @@ -125,8 +125,13 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece plaintextMessage = service.processReceivingPayloadMessage(xmppAxolotlMessage, postpone); } catch (BrokenSessionException e) { if (checkedForDuplicates) { - service.reportBrokenSessionException(e, postpone); - return new Message(conversation, "", Message.ENCRYPTION_AXOLOTL_FAILED, status); + if (service.trustedOrPreviouslyResponded(from.asBareJid())) { + service.reportBrokenSessionException(e, postpone); + return new Message(conversation, "", Message.ENCRYPTION_AXOLOTL_FAILED, status); + } else { + Log.d(Config.LOGTAG, "ignoring broken session exception because contact was not trusted"); + return new Message(conversation, "", Message.ENCRYPTION_AXOLOTL_FAILED, status); + } } else { Log.d(Config.LOGTAG,"ignoring broken session exception because checkForDuplicates failed"); //TODO should be still emit a failed message?