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.
This commit is contained in:
Daniel Gultsch 2019-09-15 11:49:55 +02:00
parent be4953b1e4
commit 4cd652884c
2 changed files with 40 additions and 15 deletions

View file

@ -48,7 +48,6 @@ import eu.siacs.conversations.services.XmppConnectionService;
import eu.siacs.conversations.utils.CryptoHelper; import eu.siacs.conversations.utils.CryptoHelper;
import eu.siacs.conversations.utils.SerialSingleThreadExecutor; import eu.siacs.conversations.utils.SerialSingleThreadExecutor;
import eu.siacs.conversations.xml.Element; import eu.siacs.conversations.xml.Element;
import eu.siacs.conversations.xml.Namespace;
import eu.siacs.conversations.xmpp.OnAdvancedStreamFeaturesLoaded; import eu.siacs.conversations.xmpp.OnAdvancedStreamFeaturesLoaded;
import eu.siacs.conversations.xmpp.OnIqPacketReceived; import eu.siacs.conversations.xmpp.OnIqPacketReceived;
import eu.siacs.conversations.xmpp.pep.PublishOptions; 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 String LOGPREFIX = "AxolotlService";
public static final int NUM_KEYS_TO_PUBLISH = 100; private static final int NUM_KEYS_TO_PUBLISH = 100;
public static final int publishTriesThreshold = 3; private static final int publishTriesThreshold = 3;
private final Account account; private final Account account;
private final XmppConnectionService mXmppConnectionService; private final XmppConnectionService mXmppConnectionService;
@ -1469,9 +1468,11 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
} else { } else {
Log.d(Config.LOGTAG,account.getJid().asBareJid()+": nothing to flush. Not republishing key"); Log.d(Config.LOGTAG,account.getJid().asBareJid()+": nothing to flush. Not republishing key");
} }
if (trustedOrPreviouslyResponded(session)) {
completeSession(session); completeSession(session);
} }
} }
}
public void processPostponed() { public void processPostponed() {
if (postponedSessions.size() > 0) { if (postponedSessions.size() > 0) {
@ -1479,23 +1480,44 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
publishBundlesIfNeeded(false, false); publishBundlesIfNeeded(false, false);
} }
} }
Iterator<XmppAxolotlSession> iterator = postponedSessions.iterator(); final Iterator<XmppAxolotlSession> iterator = postponedSessions.iterator();
while (iterator.hasNext()) { while (iterator.hasNext()) {
final XmppAxolotlSession session = iterator.next();
if (trustedOrPreviouslyResponded(session)) {
completeSession(iterator.next()); completeSession(iterator.next());
}
iterator.remove(); iterator.remove();
} }
Iterator<SignalProtocolAddress> postponedHealingAttemptsIterator = postponedHealing.iterator(); final Iterator<SignalProtocolAddress> postponedHealingAttemptsIterator = postponedHealing.iterator();
while (postponedHealingAttemptsIterator.hasNext()) { while (postponedHealingAttemptsIterator.hasNext()) {
notifyRequiresHealing(postponedHealingAttemptsIterator.next()); notifyRequiresHealing(postponedHealingAttemptsIterator.next());
postponedHealingAttemptsIterator.remove(); 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) { private void completeSession(XmppAxolotlSession session) {
final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId()); final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId());
axolotlMessage.addDevice(session, true); axolotlMessage.addDevice(session, true);
try { try {
Jid jid = Jid.of(session.getRemoteAddress().getName()); final Jid jid = Jid.of(session.getRemoteAddress().getName());
MessagePacket packet = mXmppConnectionService.getMessageGenerator().generateKeyTransportMessage(jid, axolotlMessage); MessagePacket packet = mXmppConnectionService.getMessageGenerator().generateKeyTransportMessage(jid, axolotlMessage);
mXmppConnectionService.sendMessagePacket(account, packet); mXmppConnectionService.sendMessagePacket(account, packet);
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
@ -1505,9 +1527,8 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
public XmppAxolotlMessage.XmppAxolotlKeyTransportMessage processReceivingKeyTransportMessage(XmppAxolotlMessage message, final boolean postponePreKeyMessageHandling) { public XmppAxolotlMessage.XmppAxolotlKeyTransportMessage processReceivingKeyTransportMessage(XmppAxolotlMessage message, final boolean postponePreKeyMessageHandling) {
XmppAxolotlMessage.XmppAxolotlKeyTransportMessage keyTransportMessage; final XmppAxolotlMessage.XmppAxolotlKeyTransportMessage keyTransportMessage;
final XmppAxolotlSession session = getReceivingSession(message);
XmppAxolotlSession session = getReceivingSession(message);
try { try {
keyTransportMessage = message.getParameters(session, getOwnDeviceId()); keyTransportMessage = message.getParameters(session, getOwnDeviceId());
Integer preKeyId = session.getPreKeyIdAndReset(); Integer preKeyId = session.getPreKeyIdAndReset();
@ -1516,7 +1537,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
} }
} catch (CryptoFailedException e) { } catch (CryptoFailedException e) {
Log.d(Config.LOGTAG, "could not decrypt keyTransport message " + e.getMessage()); Log.d(Config.LOGTAG, "could not decrypt keyTransport message " + e.getMessage());
keyTransportMessage = null; return null;
} }
if (session.isFresh() && keyTransportMessage != null) { if (session.isFresh() && keyTransportMessage != null) {
@ -1527,7 +1548,6 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
} }
private void putFreshSession(XmppAxolotlSession session) { private void putFreshSession(XmppAxolotlSession session) {
Log.d(Config.LOGTAG, "put fresh session");
sessions.put(session); sessions.put(session);
if (Config.X509_VERIFICATION) { if (Config.X509_VERIFICATION) {
if (session.getIdentityKey() != null) { if (session.getIdentityKey() != null) {

View file

@ -125,8 +125,13 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
plaintextMessage = service.processReceivingPayloadMessage(xmppAxolotlMessage, postpone); plaintextMessage = service.processReceivingPayloadMessage(xmppAxolotlMessage, postpone);
} catch (BrokenSessionException e) { } catch (BrokenSessionException e) {
if (checkedForDuplicates) { if (checkedForDuplicates) {
if (service.trustedOrPreviouslyResponded(from.asBareJid())) {
service.reportBrokenSessionException(e, postpone); service.reportBrokenSessionException(e, postpone);
return new Message(conversation, "", Message.ENCRYPTION_AXOLOTL_FAILED, status); 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 { } else {
Log.d(Config.LOGTAG,"ignoring broken session exception because checkForDuplicates failed"); Log.d(Config.LOGTAG,"ignoring broken session exception because checkForDuplicates failed");
//TODO should be still emit a failed message? //TODO should be still emit a failed message?