From a9d5cd4a611f47123f9561bc5a81a4555fe7cb04 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sat, 12 Nov 2016 11:12:50 +0100 Subject: [PATCH] Move TLS Required check at the end of connect() It was a *very* bad idea to perform the SecurityMode.Required check in the connection's reader thread and not at the end of AbstractXMPPConnectin's connect(). :/ This behavior dates back to 8e750912a765f77a4f178a4f307a8b42c2afb5ae Fixes SMACK-739 --- .../jivesoftware/smack/AbstractXMPPConnection.java | 8 ++++++++ .../org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 11 +---------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java index c0a6d2ba1..9868d49cb 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java @@ -46,6 +46,7 @@ import org.jivesoftware.smack.SmackException.AlreadyLoggedInException; import org.jivesoftware.smack.SmackException.NoResponseException; import org.jivesoftware.smack.SmackException.NotConnectedException; import org.jivesoftware.smack.SmackException.ResourceBindingNotOfferedException; +import org.jivesoftware.smack.SmackException.SecurityRequiredByClientException; import org.jivesoftware.smack.SmackException.SecurityRequiredException; import org.jivesoftware.smack.XMPPException.StreamErrorException; import org.jivesoftware.smack.XMPPException.XMPPErrorException; @@ -373,6 +374,13 @@ public abstract class AbstractXMPPConnection implements XMPPConnection { // Wait with SASL auth until the SASL mechanisms have been received saslFeatureReceived.checkIfSuccessOrWaitOrThrow(); + // If TLS is required but the server doesn't offer it, disconnect + // from the server and throw an error. First check if we've already negotiated TLS + // and are secure, however (features get parsed a second time after TLS is established). + if (!isSecureConnection() && getConfiguration().getSecurityMode() == SecurityMode.required) { + throw new SecurityRequiredByClientException(); + } + // Make note of the fact that we're now connected. connected = true; callConnectionConnectedListener(); diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index 951abd602..b9444a8ed 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -29,9 +29,7 @@ import org.jivesoftware.smack.SmackException.AlreadyLoggedInException; import org.jivesoftware.smack.SmackException.NoResponseException; import org.jivesoftware.smack.SmackException.NotConnectedException; import org.jivesoftware.smack.SmackException.ConnectionException; -import org.jivesoftware.smack.SmackException.SecurityRequiredByClientException; import org.jivesoftware.smack.SmackException.SecurityRequiredByServerException; -import org.jivesoftware.smack.SmackException.SecurityRequiredException; import org.jivesoftware.smack.SynchronizationPoint; import org.jivesoftware.smack.XMPPException.StreamErrorException; import org.jivesoftware.smack.XMPPConnection; @@ -917,7 +915,7 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } @Override - protected void afterFeaturesReceived() throws SecurityRequiredException, NotConnectedException, InterruptedException { + protected void afterFeaturesReceived() throws NotConnectedException, InterruptedException { StartTls startTlsFeature = getFeature(StartTls.ELEMENT, StartTls.NAMESPACE); if (startTlsFeature != null) { if (startTlsFeature.required() && config.getSecurityMode() == SecurityMode.disabled) { @@ -929,13 +927,6 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { sendNonza(new StartTls()); } } - // If TLS is required but the server doesn't offer it, disconnect - // from the server and throw an error. First check if we've already negotiated TLS - // and are secure, however (features get parsed a second time after TLS is established). - if (!isSecureConnection() && startTlsFeature == null - && getConfiguration().getSecurityMode() == SecurityMode.required) { - throw new SecurityRequiredByClientException(); - } if (getSASLAuthentication().authenticationSuccessful()) { // If we have received features after the SASL has been successfully completed, then we