From 61997912fd8191153c67c38af436980460af7933 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 4 Apr 2016 20:06:07 +0200 Subject: made sure the disco#items query has returned before finalizing the bind --- .../siacs/conversations/xmpp/XmppConnection.java | 36 ++++++++++++++-------- 1 file changed, 24 insertions(+), 12 deletions(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 6371f115..b3fd122f 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -39,6 +39,8 @@ import java.util.Hashtable; import java.util.Iterator; import java.util.List; import java.util.Map.Entry; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.KeyManager; @@ -114,7 +116,8 @@ public class XmppConnection implements Runnable { private long lastConnect = 0; private long lastSessionStarted = 0; private long lastDiscoStarted = 0; - private int mPendingServiceDiscoveries = 0; + private AtomicInteger mPendingServiceDiscoveries = new AtomicInteger(0); + private AtomicBoolean mIsServiceItemsDiscoveryPending = new AtomicBoolean(true); private final ArrayList mPendingServiceDiscoveriesIds = new ArrayList<>(); private boolean mInteractive = false; private int attempt = 0; @@ -923,7 +926,7 @@ public class XmppConnection implements Runnable { disconnect(true); } } else { - Log.d(Config.LOGTAG, account.getJid() + ": disconnecting because of bind failure ("+packet.toString()); + Log.d(Config.LOGTAG, account.getJid() + ": disconnecting because of bind failure (" + packet.toString()); disconnect(true); } } @@ -1008,7 +1011,8 @@ public class XmppConnection implements Runnable { synchronized (this.disco) { this.disco.clear(); } - mPendingServiceDiscoveries = mServerIdentity == Identity.NIMBUZZ ? 1 : 0; + mPendingServiceDiscoveries.set(mServerIdentity == Identity.NIMBUZZ ? 1 : 0); + mIsServiceItemsDiscoveryPending.set(true); lastDiscoStarted = SystemClock.elapsedRealtime(); Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": starting service discovery"); mXmppConnectionService.scheduleWakeUpCall(Config.CONNECT_DISCO_TIMEOUT, account.getUuid().hashCode()); @@ -1032,7 +1036,7 @@ public class XmppConnection implements Runnable { private void sendServiceDiscoveryInfo(final Jid jid) { if (mServerIdentity != Identity.NIMBUZZ) { - mPendingServiceDiscoveries++; + mPendingServiceDiscoveries.incrementAndGet(); } final IqPacket iq = new IqPacket(IqPacket.TYPE.GET); iq.setTo(jid); @@ -1077,14 +1081,8 @@ public class XmppConnection implements Runnable { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": could not query disco info for " + jid.toString()); } if (packet.getType() != IqPacket.TYPE.TIMEOUT) { - mPendingServiceDiscoveries--; - if (mPendingServiceDiscoveries == 0) { - Log.d(Config.LOGTAG,account.getJid().toBareJid()+": done with service discovery"); - Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": online with resource " + account.getResource()); - if (bindListener != null) { - bindListener.onBind(account); - } - changeStatus(Account.State.ONLINE); + if (mPendingServiceDiscoveries.decrementAndGet() == 0 && !mIsServiceItemsDiscoveryPending.get()) { + finalizeBind(); } } } @@ -1094,6 +1092,14 @@ public class XmppConnection implements Runnable { } } + private void finalizeBind() { + Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": online with resource " + account.getResource()); + if (bindListener != null) { + bindListener.onBind(account); + } + changeStatus(Account.State.ONLINE); + } + private void enableAdvancedStreamFeatures() { if (getFeatures().carbons() && !features.carbonsEnabled) { sendEnableCarbons(); @@ -1128,6 +1134,12 @@ public class XmppConnection implements Runnable { } else { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": could not query disco items of " + server); } + if (packet.getType() != IqPacket.TYPE.TIMEOUT) { + mIsServiceItemsDiscoveryPending.set(false); + if (mPendingServiceDiscoveries.get() == 0) { + finalizeBind(); + } + } } }); } -- cgit v1.2.3 From 0385e3a8d62ebe68a8c40a75ea95d3726d3839f3 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 4 Apr 2016 20:35:40 +0200 Subject: switched around info and items query to avoid race condition --- src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index b3fd122f..58e510e1 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -1016,7 +1016,6 @@ public class XmppConnection implements Runnable { lastDiscoStarted = SystemClock.elapsedRealtime(); Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": starting service discovery"); mXmppConnectionService.scheduleWakeUpCall(Config.CONNECT_DISCO_TIMEOUT, account.getUuid().hashCode()); - sendServiceDiscoveryItems(account.getServer()); Element caps = streamFeatures.findChild("c"); final String hash = caps == null ? null : caps.getAttribute("hash"); final String ver = caps == null ? null : caps.getAttribute("ver"); @@ -1031,6 +1030,7 @@ public class XmppConnection implements Runnable { disco.put(account.getServer(), discoveryResult); } sendServiceDiscoveryInfo(account.getJid().toBareJid()); + sendServiceDiscoveryItems(account.getServer()); this.lastSessionStarted = SystemClock.elapsedRealtime(); } -- cgit v1.2.3 From a8ebc5fafcd0c200b960015f7a33ce71d1689a7f Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Fri, 8 Apr 2016 20:20:37 +0200 Subject: add required disco#items query to timeout list --- src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 58e510e1..19d4fcba 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -1117,7 +1117,7 @@ public class XmppConnection implements Runnable { final IqPacket iq = new IqPacket(IqPacket.TYPE.GET); iq.setTo(server.toDomainJid()); iq.query("http://jabber.org/protocol/disco#items"); - this.sendIqPacket(iq, new OnIqPacketReceived() { + String id = this.sendIqPacket(iq, new OnIqPacketReceived() { @Override public void onIqPacketReceived(final Account account, final IqPacket packet) { @@ -1142,6 +1142,9 @@ public class XmppConnection implements Runnable { } } }); + synchronized (this.mPendingServiceDiscoveriesIds) { + this.mPendingServiceDiscoveriesIds.add(id); + } } private void sendEnableCarbons() { -- cgit v1.2.3 From 14b46c3ee7aa860d349bcf8418b6ad35715c9ec4 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 9 Apr 2016 08:53:58 +0200 Subject: transform nimbuzz workaround into a more general 'waitForDisco' condition --- .../eu/siacs/conversations/xmpp/XmppConnection.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 19d4fcba..6427c347 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -118,6 +118,7 @@ public class XmppConnection implements Runnable { private long lastDiscoStarted = 0; private AtomicInteger mPendingServiceDiscoveries = new AtomicInteger(0); private AtomicBoolean mIsServiceItemsDiscoveryPending = new AtomicBoolean(true); + private boolean mWaitForDisco = true; private final ArrayList mPendingServiceDiscoveriesIds = new ArrayList<>(); private boolean mInteractive = false; private int attempt = 0; @@ -1011,8 +1012,9 @@ public class XmppConnection implements Runnable { synchronized (this.disco) { this.disco.clear(); } - mPendingServiceDiscoveries.set(mServerIdentity == Identity.NIMBUZZ ? 1 : 0); + mPendingServiceDiscoveries.set(0); mIsServiceItemsDiscoveryPending.set(true); + mWaitForDisco = mServerIdentity != Identity.NIMBUZZ; lastDiscoStarted = SystemClock.elapsedRealtime(); Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": starting service discovery"); mXmppConnectionService.scheduleWakeUpCall(Config.CONNECT_DISCO_TIMEOUT, account.getUuid().hashCode()); @@ -1031,13 +1033,14 @@ public class XmppConnection implements Runnable { } sendServiceDiscoveryInfo(account.getJid().toBareJid()); sendServiceDiscoveryItems(account.getServer()); + if (!mWaitForDisco) { + finalizeBind(); + } this.lastSessionStarted = SystemClock.elapsedRealtime(); } private void sendServiceDiscoveryInfo(final Jid jid) { - if (mServerIdentity != Identity.NIMBUZZ) { - mPendingServiceDiscoveries.incrementAndGet(); - } + mPendingServiceDiscoveries.incrementAndGet(); final IqPacket iq = new IqPacket(IqPacket.TYPE.GET); iq.setTo(jid); iq.query("http://jabber.org/protocol/disco#info"); @@ -1081,7 +1084,9 @@ public class XmppConnection implements Runnable { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": could not query disco info for " + jid.toString()); } if (packet.getType() != IqPacket.TYPE.TIMEOUT) { - if (mPendingServiceDiscoveries.decrementAndGet() == 0 && !mIsServiceItemsDiscoveryPending.get()) { + if (mPendingServiceDiscoveries.decrementAndGet() == 0 + && !mIsServiceItemsDiscoveryPending.get() + && mWaitForDisco) { finalizeBind(); } } @@ -1136,7 +1141,7 @@ public class XmppConnection implements Runnable { } if (packet.getType() != IqPacket.TYPE.TIMEOUT) { mIsServiceItemsDiscoveryPending.set(false); - if (mPendingServiceDiscoveries.get() == 0) { + if (mPendingServiceDiscoveries.get() == 0 && mWaitForDisco) { finalizeBind(); } } -- cgit v1.2.3 From 607b7d159319e7591f9a1dac07e187abdb4cf278 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 10 Apr 2016 00:18:14 +0200 Subject: moved authentication into seperate method. force close socket before changing status --- .../siacs/conversations/xmpp/XmppConnection.java | 82 ++++++++++++---------- 1 file changed, 44 insertions(+), 38 deletions(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 6427c347..30cb96dc 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -183,16 +183,18 @@ public class XmppConnection implements Runnable { if (packet.getType() == IqPacket.TYPE.RESULT) { account.setOption(Account.OPTION_REGISTER, false); + forceCloseSocket(); changeStatus(Account.State.REGISTRATION_SUCCESSFUL); } else if (packet.hasChild("error") && (packet.findChild("error") .hasChild("conflict"))) { + forceCloseSocket(); changeStatus(Account.State.REGISTRATION_CONFLICT); } else { + forceCloseSocket(); changeStatus(Account.State.REGISTRATION_FAILED); Log.d(Config.LOGTAG, packet.toString()); } - disconnect(true); } }; } @@ -739,47 +741,12 @@ public class XmppConnection implements Runnable { } } else if (!this.streamFeatures.hasChild("register") && account.isOptionSet(Account.OPTION_REGISTER)) { + forceCloseSocket(); changeStatus(Account.State.REGISTRATION_NOT_SUPPORTED); - disconnect(true); } else if (this.streamFeatures.hasChild("mechanisms") && shouldAuthenticate && (features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS)) { - final List mechanisms = extractMechanisms(streamFeatures - .findChild("mechanisms")); - final Element auth = new Element("auth"); - auth.setAttribute("xmlns", "urn:ietf:params:xml:ns:xmpp-sasl"); - if (mechanisms.contains("EXTERNAL") && account.getPrivateKeyAlias() != null) { - saslMechanism = new External(tagWriter, account, mXmppConnectionService.getRNG()); - } else if (mechanisms.contains("SCRAM-SHA-1")) { - saslMechanism = new ScramSha1(tagWriter, account, mXmppConnectionService.getRNG()); - } else if (mechanisms.contains("PLAIN")) { - saslMechanism = new Plain(tagWriter, account); - } else if (mechanisms.contains("DIGEST-MD5")) { - saslMechanism = new DigestMd5(tagWriter, account, mXmppConnectionService.getRNG()); - } - if (saslMechanism != null) { - final JSONObject keys = account.getKeys(); - try { - if (keys.has(Account.PINNED_MECHANISM_KEY) && - keys.getInt(Account.PINNED_MECHANISM_KEY) > saslMechanism.getPriority()) { - Log.e(Config.LOGTAG, "Auth failed. Authentication mechanism " + saslMechanism.getMechanism() + - " has lower priority (" + String.valueOf(saslMechanism.getPriority()) + - ") than pinned priority (" + keys.getInt(Account.PINNED_MECHANISM_KEY) + - "). Possible downgrade attack?"); - throw new SecurityException(); - } - } catch (final JSONException e) { - Log.d(Config.LOGTAG, "Parse error while checking pinned auth mechanism"); - } - Log.d(Config.LOGTAG, account.getJid().toString() + ": Authenticating with " + saslMechanism.getMechanism()); - auth.setAttribute("mechanism", saslMechanism.getMechanism()); - if (!saslMechanism.getClientFirstMessage().isEmpty()) { - auth.setContent(saslMechanism.getClientFirstMessage()); - } - tagWriter.writeElement(auth); - } else { - throw new IncompatibleServerException(); - } + authenticate(); } else if (this.streamFeatures.hasChild("sm", "urn:xmpp:sm:" + smVersion) && streamId != null) { if (Config.EXTENDED_SM_LOGGING) { Log.d(Config.LOGTAG,account.getJid().toBareJid()+": resuming after stanza #"+stanzasReceived); @@ -795,6 +762,45 @@ public class XmppConnection implements Runnable { } } + private void authenticate() throws IOException { + final List mechanisms = extractMechanisms(streamFeatures + .findChild("mechanisms")); + final Element auth = new Element("auth"); + auth.setAttribute("xmlns", "urn:ietf:params:xml:ns:xmpp-sasl"); + if (mechanisms.contains("EXTERNAL") && account.getPrivateKeyAlias() != null) { + saslMechanism = new External(tagWriter, account, mXmppConnectionService.getRNG()); + } else if (mechanisms.contains("SCRAM-SHA-1")) { + saslMechanism = new ScramSha1(tagWriter, account, mXmppConnectionService.getRNG()); + } else if (mechanisms.contains("PLAIN")) { + saslMechanism = new Plain(tagWriter, account); + } else if (mechanisms.contains("DIGEST-MD5")) { + saslMechanism = new DigestMd5(tagWriter, account, mXmppConnectionService.getRNG()); + } + if (saslMechanism != null) { + final JSONObject keys = account.getKeys(); + try { + if (keys.has(Account.PINNED_MECHANISM_KEY) && + keys.getInt(Account.PINNED_MECHANISM_KEY) > saslMechanism.getPriority()) { + Log.e(Config.LOGTAG, "Auth failed. Authentication mechanism " + saslMechanism.getMechanism() + + " has lower priority (" + String.valueOf(saslMechanism.getPriority()) + + ") than pinned priority (" + keys.getInt(Account.PINNED_MECHANISM_KEY) + + "). Possible downgrade attack?"); + throw new SecurityException(); + } + } catch (final JSONException e) { + Log.d(Config.LOGTAG, "Parse error while checking pinned auth mechanism"); + } + Log.d(Config.LOGTAG, account.getJid().toString() + ": Authenticating with " + saslMechanism.getMechanism()); + auth.setAttribute("mechanism", saslMechanism.getMechanism()); + if (!saslMechanism.getClientFirstMessage().isEmpty()) { + auth.setContent(saslMechanism.getClientFirstMessage()); + } + tagWriter.writeElement(auth); + } else { + throw new IncompatibleServerException(); + } + } + private List extractMechanisms(final Element stream) { final ArrayList mechanisms = new ArrayList<>(stream .getChildren().size()); -- cgit v1.2.3 From 5786e753740065cb2a5398ee964642054074e6f2 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 10 Apr 2016 00:19:53 +0200 Subject: don't throw IO exception at end of stream --- src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java | 1 - 1 file changed, 1 deletion(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 30cb96dc..7bc08e9c 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -560,7 +560,6 @@ public class XmppConnection implements Runnable { } nextTag = tagReader.readTag(); } - throw new IOException("reached end of stream. last tag was "+nextTag); } private void acknowledgeStanzaUpTo(int serverCount) { -- cgit v1.2.3 From 7ff890e513212f1cf03667e4ea7e429431d811c4 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 11 Apr 2016 22:20:32 +0200 Subject: republish avatar if server offers non-persistent pep :-( --- .../java/eu/siacs/conversations/xmpp/XmppConnection.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 7bc08e9c..4dc5492e 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -1530,13 +1530,15 @@ public class XmppConnection implements Runnable { public boolean pep() { synchronized (XmppConnection.this.disco) { - ServiceDiscoveryResult info = disco.get(account.getServer()); - if (info != null && info.hasIdentity("pubsub", "pep")) { - return true; - } else { - info = disco.get(account.getJid().toBareJid()); - return info != null && info.hasIdentity("pubsub", "pep"); - } + ServiceDiscoveryResult info = disco.get(account.getJid().toBareJid()); + return info != null && info.hasIdentity("pubsub", "pep"); + } + } + + public boolean pepPersistent() { + synchronized (XmppConnection.this.disco) { + ServiceDiscoveryResult info = disco.get(account.getJid().toBareJid()); + return info != null && info.getFeatures().contains("http://jabber.org/protocol/pubsub#persistent-items"); } } -- cgit v1.2.3