Skip to content
Permalink
Browse files

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 8e75091

Fixes SMACK-739
  • Loading branch information...
Flowdalic committed Nov 12, 2016
1 parent fca2f59 commit 059ee99ba0d5ff7758829acf5a9aeede09ec820b
@@ -362,6 +362,7 @@ public synchronized AbstractXMPPConnection connect() throws SmackException, IOEx

// Perform the actual connection to the XMPP service
connectInternal();

return this;
}

@@ -28,10 +28,9 @@
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.ConnectionException;
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;
@@ -857,6 +856,14 @@ protected void connectInternal() throws SmackException, IOException, XMPPExcepti
// 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) {
shutdown();
throw new SecurityRequiredByClientException();
}

// Make note of the fact that we're now connected.
connected = true;
callConnectionConnectedListener();
@@ -897,7 +904,7 @@ protected void setWriter(Writer writer) {
}

@Override
protected void afterFeaturesReceived() throws SecurityRequiredException, NotConnectedException {
protected void afterFeaturesReceived() throws NotConnectedException {
StartTls startTlsFeature = getFeature(StartTls.ELEMENT, StartTls.NAMESPACE);
if (startTlsFeature != null) {
if (startTlsFeature.required() && config.getSecurityMode() == SecurityMode.disabled) {
@@ -909,13 +916,6 @@ protected void afterFeaturesReceived() throws SecurityRequiredException, NotConn
send(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

0 comments on commit 059ee99

Please sign in to comment.
You can’t perform that action at this time.