From f0ec9006f0257fcfa1998cec6e8dd2cb12e1abec Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 2 Jun 2024 17:01:17 +0200 Subject: [PATCH] fix rare NPE race condition --- .../conversations/xmpp/XmppConnection.java | 42 +++++++++---------- .../xmpp/model/AuthenticationRequest.java | 13 ++++++ .../android/xmpp/model/sasl/Auth.java | 9 +++- .../xmpp/model/sasl2/Authenticate.java | 10 ++++- 4 files changed, 49 insertions(+), 25 deletions(-) create mode 100644 src/main/java/im/conversations/android/xmpp/model/AuthenticationRequest.java diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 9a51f9e51550624e70e07c2e20232be3140513d1..38194d5cd9674932a2ad0dce1992c1fe3fa63093 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -63,6 +63,7 @@ import eu.siacs.conversations.xmpp.bind.Bind2; import eu.siacs.conversations.xmpp.forms.Data; import eu.siacs.conversations.xmpp.jingle.OnJinglePacketReceived; +import im.conversations.android.xmpp.model.AuthenticationRequest; import im.conversations.android.xmpp.model.AuthenticationStreamFeature; import im.conversations.android.xmpp.model.StreamElement; import im.conversations.android.xmpp.model.bind2.Bind; @@ -1514,14 +1515,15 @@ public class XmppConnection implements Runnable { final String firstMessage = saslMechanism.getClientFirstMessage(sslSocketOrNull(this.socket)); final boolean usingFast = SaslMechanism.hashedToken(saslMechanism); - final StreamElement authenticate; + final AuthenticationRequest authenticate; + final LoginInfo loginInfo; if (version == SaslMechanism.Version.SASL) { authenticate = new Auth(); if (!Strings.isNullOrEmpty(firstMessage)) { authenticate.setContent(firstMessage); } quickStartAvailable = false; - this.loginInfo = new LoginInfo(saslMechanism, version, Collections.emptyList()); + loginInfo = new LoginInfo(saslMechanism, version, Collections.emptyList()); } else if (version == SaslMechanism.Version.SASL_2) { final Authentication authentication = (Authentication) authElement; final var inline = authentication.getInline(); @@ -1552,7 +1554,7 @@ public class XmppConnection implements Runnable { return; } } - this.loginInfo = new LoginInfo(saslMechanism, version, bindFeatures); + loginInfo = new LoginInfo(saslMechanism, version, bindFeatures); this.hashTokenRequest = hashTokenRequest; authenticate = generateAuthenticationRequest( @@ -1560,7 +1562,7 @@ public class XmppConnection implements Runnable { } else { throw new AssertionError("Missing implementation for " + version); } - + this.loginInfo = loginInfo; if (account.setOption(Account.OPTION_QUICKSTART_AVAILABLE, quickStartAvailable)) { mXmppConnectionService.databaseBackend.updateAccount(account); } @@ -1571,8 +1573,8 @@ public class XmppConnection implements Runnable { + ": Authenticating with " + version + "/" - + LoginInfo.mechanism(this.loginInfo).getMechanism()); - authenticate.setAttribute("mechanism", LoginInfo.mechanism(this.loginInfo).getMechanism()); + + LoginInfo.mechanism(loginInfo).getMechanism()); + authenticate.setMechanism(LoginInfo.mechanism(loginInfo)); synchronized (this.mStanzaQueue) { this.stanzasSentBeforeAuthentication = this.stanzasSent; tagWriter.writeElement(authenticate); @@ -1650,13 +1652,13 @@ public class XmppConnection implements Runnable { } } - private Authenticate generateAuthenticationRequest( + private AuthenticationRequest generateAuthenticationRequest( final String firstMessage, final boolean usingFast) { return generateAuthenticationRequest( firstMessage, usingFast, null, Bind2.QUICKSTART_FEATURES, true); } - private Authenticate generateAuthenticationRequest( + private AuthenticationRequest generateAuthenticationRequest( final String firstMessage, final boolean usingFast, final HashedToken.Mechanism hashedTokenRequest, @@ -1909,13 +1911,15 @@ public class XmppConnection implements Runnable { } clearIqCallbacks(); if (account.getJid().isBareJid()) { - account.setResource(this.createNewResource()); + account.setResource(createNewResource()); } else { fixResource(mXmppConnectionService, account); } final Iq iq = new Iq(Iq.Type.SET); final String resource = - Config.USE_RANDOM_RESOURCE_ON_EVERY_BIND ? nextRandomId() : account.getResource(); + Config.USE_RANDOM_RESOURCE_ON_EVERY_BIND + ? CryptoHelper.random(9) + : account.getResource(); iq.addExtension(new im.conversations.android.xmpp.model.bind.Bind()).setResource(resource); this.sendUnmodifiedIqPacket( iq, @@ -2381,11 +2385,11 @@ public class XmppConnection implements Runnable { SaslMechanism.Version.SASL_2, Bind2.QUICKSTART_FEATURES); final boolean usingFast = quickStartMechanism instanceof HashedToken; - final StreamElement authenticate = + final AuthenticationRequest authenticate = generateAuthenticationRequest( quickStartMechanism.getClientFirstMessage(sslSocketOrNull(this.socket)), usingFast); - authenticate.setAttribute("mechanism", quickStartMechanism.getMechanism()); + authenticate.setMechanism(quickStartMechanism); sendStartStream(true, false); synchronized (this.mStanzaQueue) { this.stanzasSentBeforeAuthentication = this.stanzasSent; @@ -2416,16 +2420,8 @@ public class XmppConnection implements Runnable { tagWriter.writeTag(stream, flush); } - private String createNewResource() { - return mXmppConnectionService.getString(R.string.app_name) + '.' + nextRandomId(true); - } - - private String nextRandomId() { - return nextRandomId(false); - } - - private String nextRandomId(final boolean s) { - return CryptoHelper.random(s ? 3 : 9); + private static String createNewResource() { + return String.format("%s.%s", BuildConfig.APP_NAME, CryptoHelper.random(3)); } public String sendIqPacket(final Iq packet, final Consumer callback) { @@ -2437,7 +2433,7 @@ public class XmppConnection implements Runnable { final Iq packet, final Consumer callback, boolean force) { // TODO if callback != null verify that type is get or set if (packet.getId() == null) { - packet.setAttribute("id", nextRandomId()); + packet.setId(CryptoHelper.random(9)); } if (callback != null) { synchronized (this.packetCallbacks) { diff --git a/src/main/java/im/conversations/android/xmpp/model/AuthenticationRequest.java b/src/main/java/im/conversations/android/xmpp/model/AuthenticationRequest.java new file mode 100644 index 0000000000000000000000000000000000000000..b2122ab863637c257b1b957b6e200c2f27aa5f5d --- /dev/null +++ b/src/main/java/im/conversations/android/xmpp/model/AuthenticationRequest.java @@ -0,0 +1,13 @@ +package im.conversations.android.xmpp.model; + +import eu.siacs.conversations.crypto.sasl.SaslMechanism; + +public abstract class AuthenticationRequest extends StreamElement{ + + + protected AuthenticationRequest(Class clazz) { + super(clazz); + } + + public abstract void setMechanism(final SaslMechanism mechanism); +} diff --git a/src/main/java/im/conversations/android/xmpp/model/sasl/Auth.java b/src/main/java/im/conversations/android/xmpp/model/sasl/Auth.java index 668d10d80f8bcb56971968d41d500615d129a987..e9dd801f233ef6db23d3781b6724a6ab1e536c3a 100644 --- a/src/main/java/im/conversations/android/xmpp/model/sasl/Auth.java +++ b/src/main/java/im/conversations/android/xmpp/model/sasl/Auth.java @@ -1,12 +1,19 @@ package im.conversations.android.xmpp.model.sasl; +import eu.siacs.conversations.crypto.sasl.SaslMechanism; import im.conversations.android.annotation.XmlElement; +import im.conversations.android.xmpp.model.AuthenticationRequest; import im.conversations.android.xmpp.model.StreamElement; @XmlElement -public class Auth extends StreamElement { +public class Auth extends AuthenticationRequest { public Auth() { super(Auth.class); } + + @Override + public void setMechanism(final SaslMechanism mechanism) { + this.setAttribute("mechanism", mechanism.getMechanism()); + } } diff --git a/src/main/java/im/conversations/android/xmpp/model/sasl2/Authenticate.java b/src/main/java/im/conversations/android/xmpp/model/sasl2/Authenticate.java index 7869bf010600d2525add357171a4f57601977db7..c709a779dd4614b9ec0f5e5dc2e6adf549004819 100644 --- a/src/main/java/im/conversations/android/xmpp/model/sasl2/Authenticate.java +++ b/src/main/java/im/conversations/android/xmpp/model/sasl2/Authenticate.java @@ -1,12 +1,20 @@ package im.conversations.android.xmpp.model.sasl2; +import eu.siacs.conversations.crypto.sasl.SaslMechanism; +import eu.siacs.conversations.xmpp.XmppConnection; import im.conversations.android.annotation.XmlElement; +import im.conversations.android.xmpp.model.AuthenticationRequest; import im.conversations.android.xmpp.model.StreamElement; @XmlElement -public class Authenticate extends StreamElement { +public class Authenticate extends AuthenticationRequest { public Authenticate() { super(Authenticate.class); } + + @Override + public void setMechanism(final SaslMechanism mechanism) { + this.setAttribute("mechanism", mechanism.getMechanism()); + } }