process challenge only on secure connection

src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java

@@ -577,8 +577,16 @@ public class XmppConnection implements Runnable {
                 // two step sasl2 - we don’t support this yet
                 throw new StateChangingException(Account.State.INCOMPATIBLE_CLIENT);
             } else if (nextTag.isStart("challenge")) {
+                if (isSecure() && this.saslMechanism != null) {
                     final Element challenge = tagReader.readElement(nextTag);
                     processChallenge(challenge);
+                } else {
+                    Log.d(
+                            Config.LOGTAG,
+                            account.getJid().asBareJid()
+                                    + ": received 'challenge on an unsecure connection");
+                    throw new StateChangingException(Account.State.INCOMPATIBLE_CLIENT);
+                }
             } else if (nextTag.isStart("enabled", Namespace.STREAM_MANAGEMENT)) {
                 final Element enabled = tagReader.readElement(nextTag);
                 processEnabled(enabled);
@@ -655,7 +663,7 @@ public class XmppConnection implements Runnable {
         }
     }
 
-    private void processChallenge(Element challenge) throws IOException {
+    private void processChallenge(final Element challenge) throws IOException {
         final SaslMechanism.Version version;
         try {
             version = SaslMechanism.Version.of(challenge);
@@ -688,6 +696,10 @@ public class XmppConnection implements Runnable {
         } catch (final IllegalArgumentException e) {
             throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER);
         }
+        final SaslMechanism currentSaslMechanism = this.saslMechanism;
+        if (currentSaslMechanism == null) {
+            throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER);
+        }
         final String challenge;
         if (version == SaslMechanism.Version.SASL) {
             challenge = success.getContent();
@@ -697,7 +709,7 @@ public class XmppConnection implements Runnable {
             throw new AssertionError("Missing implementation for " + version);
         }
         try {
-            saslMechanism.getResponse(challenge, sslSocketOrNull(socket));
+            currentSaslMechanism.getResponse(challenge, sslSocketOrNull(socket));
         } catch (final SaslMechanism.AuthenticationException e) {
             Log.e(Config.LOGTAG, String.valueOf(e));
             throw new StateChangingException(Account.State.UNAUTHORIZED);
@@ -705,25 +717,10 @@ public class XmppConnection implements Runnable {
         Log.d(
                 Config.LOGTAG,
                 account.getJid().asBareJid().toString() + ": logged in (using " + version + ")");
-        if (SaslMechanism.pin(this.saslMechanism)) {
+        if (SaslMechanism.pin(currentSaslMechanism)) {
-            account.setPinnedMechanism(this.saslMechanism);
+            account.setPinnedMechanism(currentSaslMechanism);
         }
         if (version == SaslMechanism.Version.SASL_2) {
-            final Tag tag = tagReader.readTag();
-            if (tag != null && tag.isStart("features", Namespace.STREAMS)) {
-                this.streamFeatures = tagReader.readElement(tag);
-                Log.d(
-                        Config.LOGTAG,
-                        account.getJid().asBareJid()
-                                + ": processed NOP stream features after success "
-                                + XmlHelper.printElementNames(this.streamFeatures));
-            } else {
-                Log.d(
-                        Config.LOGTAG,
-                        account.getJid().asBareJid()
-                                + ": server did not send stream features after SASL2 success");
-                throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER);
-            }
             final String authorizationIdentifier =
                     success.findChildContent("authorization-identifier");
             final Jid authorizationJid;
@@ -761,6 +758,7 @@ public class XmppConnection implements Runnable {
                         account.getJid().asBareJid()
                                 + ": jid changed during SASL 2.0. updating database");
             }
+            final boolean nopStreamFeatures;
             final Element bound = success.findChild("bound", Namespace.BIND2);
             final Element resumed = success.findChild("resumed", "urn:xmpp:sm:3");
             final Element failed = success.findChild("failed", "urn:xmpp:sm:3");
@@ -773,6 +771,7 @@ public class XmppConnection implements Runnable {
                                 + ": server sent bound and resumed in SASL2 success");
                 throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER);
             }
+            final boolean processNopStreamFeatures = (resumed != null && streamId != null) || bound != null;
             if (resumed != null && streamId != null) {
                 processResumed(resumed);
             } else if (failed != null) {
@@ -801,9 +800,8 @@ public class XmppConnection implements Runnable {
                 sendPostBindInitialization(waitForDisco, carbonsEnabled != null);
             }
             final HashedToken.Mechanism tokenMechanism;
-            final SaslMechanism currentMechanism = this.saslMechanism;
+            if (SaslMechanism.hashedToken(currentSaslMechanism)) {
-            if (SaslMechanism.hashedToken(currentMechanism)) {
+                tokenMechanism = ((HashedToken) currentSaslMechanism).getTokenMechanism();
-                tokenMechanism = ((HashedToken) currentMechanism).getTokenMechanism();
             } else if (this.hashTokenRequest != null) {
                 tokenMechanism = this.hashTokenRequest;
             } else {
@@ -813,6 +811,9 @@ public class XmppConnection implements Runnable {
                 this.account.setFastToken(tokenMechanism,token);
                 Log.d(Config.LOGTAG,account.getJid().asBareJid()+": storing hashed token "+tokenMechanism);
             }
+            if (processNopStreamFeatures) {
+                processNopStreamFeatures();
+            }
         }
         mXmppConnectionService.databaseBackend.updateAccount(account);
         this.quickStartInProgress = false;
@@ -831,6 +832,25 @@ public class XmppConnection implements Runnable {
         }
     }
 
+    private void processNopStreamFeatures() throws IOException {
+        final Tag tag = tagReader.readTag();
+        if (tag != null && tag.isStart("features", Namespace.STREAMS)) {
+            this.streamFeatures = tagReader.readElement(tag);
+            Log.d(
+                    Config.LOGTAG,
+                    account.getJid().asBareJid()
+                            + ": processed NOP stream features after success: "
+                            + XmlHelper.printElementNames(this.streamFeatures));
+        } else {
+            Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": received " + tag);
+            Log.d(
+                    Config.LOGTAG,
+                    account.getJid().asBareJid()
+                            + ": server did not send stream features after SASL2 success");
+            throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER);
+        }
+    }
+
     private void processFailure(final Element failure) throws IOException {
         final SaslMechanism.Version version;
         try {
@@ -1248,8 +1268,7 @@ public class XmppConnection implements Runnable {
 
     private void processStreamFeatures(final Tag currentTag) throws IOException {
         this.streamFeatures = tagReader.readElement(currentTag);
-        final boolean isSecure =
+        final boolean isSecure = isSecure();
-                features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS || account.isOnion();
         final boolean needsBinding = !isBound && !account.isOptionSet(Account.OPTION_REGISTER);
         if (this.quickStartInProgress) {
             if (this.streamFeatures.hasChild("authentication", Namespace.SASL_2)) {
@@ -1341,8 +1360,7 @@ public class XmppConnection implements Runnable {
     }
 
     private void authenticate() throws IOException {
-        final boolean isSecure =
+        final boolean isSecure = isSecure();
-                features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS || account.isOnion();
         if (isSecure && this.streamFeatures.hasChild("authentication", Namespace.SASL_2)) {authenticate(SaslMechanism.Version.SASL_2);
         } else if (isSecure && this.streamFeatures.hasChild("mechanisms", Namespace.SASL)) {
             authenticate(SaslMechanism.Version.SASL);
@@ -1351,6 +1369,10 @@ public class XmppConnection implements Runnable {
         }
     }
 
+    private boolean isSecure() {
+        return features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS || account.isOnion();
+    }
+
     private void authenticate(final SaslMechanism.Version version) throws IOException {
         final Element authElement;
         if (version == SaslMechanism.Version.SASL) {
@@ -1658,6 +1680,7 @@ public class XmppConnection implements Runnable {
         synchronized (this.commands) {
             this.commands.clear();
         }
+        this.saslMechanism = null;
     }
 
     private void sendBindRequest() {