1
0
Fork 0
mirror of https://github.com/vanitasvitae/Smack.git synced 2024-11-25 13:32:07 +01:00

Propagate stream errors on connect/login to the caller

Before this, if there was a stream error response by the server to our
stream open, that error response would only be handled in the reader
thread, and the user would get a message like:

"org.jivesoftware.smack.SmackException$NoResponseException: No
response received within reply timeout. Timeout was
5000ms (~5s). While waiting for SASL mechanisms stream feature from
server"

while the server may actually sent something like

<stream:stream
  xmlns='jabber:client'
  xmlns:stream='http://etherx.jabber.org/streams'
  id='6785787028201586334'
  from='jabbim.com'
  version='1.0'
  xml:lang='en'>
  <stream:error>
    <policy-violation xmlns='urn:ietf:params:xml:ns:xmpp-streams'>
	</policy-violation>
	<text xml:lang='en' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>
	  Too many (2) failed authentications from this IP
      address (1xx.66.xx.xxx). The address will be unblocked at 04:24:00
      06.01.2017 UTC
    </text>
  </stream:error>
</stream:stream>

It was necessary to change saslFeatureReceived from SmackException to
XMPPException in order to return the StreamErrorException at this sync
point. But this change in return required the introduction of a
tlsHandled sync point for SmackException (which just acts as a wrapper
for the various exception types that could occurn when establishing
TLS). The tlsHandled sync point is marked successful even if no TLS
was established in case none was required and/or if not supported by
the server.
This commit is contained in:
Florian Schmaus 2017-01-06 15:03:28 +01:00
parent cff91f5a92
commit bfc14227ca
4 changed files with 30 additions and 7 deletions

View file

@ -198,6 +198,9 @@ public class XMPPBOSHConnection extends AbstractXMPPConnection {
+ getHost() + ":" + getPort() + "."; + getHost() + ":" + getPort() + ".";
throw new SmackException(errorMessage); throw new SmackException(errorMessage);
} }
tlsHandled.reportSuccess();
saslFeatureReceived.reportSuccess();
} }
public boolean isSecureConnection() { public boolean isSecureConnection() {

View file

@ -188,6 +188,8 @@ public abstract class AbstractXMPPConnection implements XMPPConnection {
*/ */
protected Writer writer; protected Writer writer;
protected final SynchronizationPoint<SmackException> tlsHandled = new SynchronizationPoint<>(this, "establishing TLS");
/** /**
* Set to success if the last features stanza from the server has been parsed. A XMPP connection * Set to success if the last features stanza from the server has been parsed. A XMPP connection
* handshake can invoke multiple features stanzas, e.g. when TLS is activated a second feature * handshake can invoke multiple features stanzas, e.g. when TLS is activated a second feature
@ -198,9 +200,9 @@ public abstract class AbstractXMPPConnection implements XMPPConnection {
AbstractXMPPConnection.this, "last stream features received from server"); AbstractXMPPConnection.this, "last stream features received from server");
/** /**
* Set to success if the sasl feature has been received. * Set to success if the SASL feature has been received.
*/ */
protected final SynchronizationPoint<SmackException> saslFeatureReceived = new SynchronizationPoint<SmackException>( protected final SynchronizationPoint<XMPPException> saslFeatureReceived = new SynchronizationPoint<>(
AbstractXMPPConnection.this, "SASL mechanisms stream feature from server"); AbstractXMPPConnection.this, "SASL mechanisms stream feature from server");
/** /**
@ -368,11 +370,15 @@ public abstract class AbstractXMPPConnection implements XMPPConnection {
saslAuthentication.init(); saslAuthentication.init();
saslFeatureReceived.init(); saslFeatureReceived.init();
lastFeaturesReceived.init(); lastFeaturesReceived.init();
tlsHandled.init();
streamId = null; streamId = null;
// Perform the actual connection to the XMPP service // Perform the actual connection to the XMPP service
connectInternal(); connectInternal();
// TLS handled will be successful either if TLS was established, or if it was not mandatory.
tlsHandled.checkIfSuccessOrWaitOrThrow();
// Wait with SASL auth until the SASL mechanisms have been received // Wait with SASL auth until the SASL mechanisms have been received
saslFeatureReceived.checkIfSuccessOrWaitOrThrow(); saslFeatureReceived.checkIfSuccessOrWaitOrThrow();
@ -1407,6 +1413,7 @@ public abstract class AbstractXMPPConnection implements XMPPConnection {
// Only proceed with SASL auth if TLS is disabled or if the server doesn't announce it // Only proceed with SASL auth if TLS is disabled or if the server doesn't announce it
if (!hasFeature(StartTls.ELEMENT, StartTls.NAMESPACE) if (!hasFeature(StartTls.ELEMENT, StartTls.NAMESPACE)
|| config.getSecurityMode() == SecurityMode.disabled) { || config.getSecurityMode() == SecurityMode.disabled) {
tlsHandled.reportSuccess();
saslFeatureReceived.reportSuccess(); saslFeatureReceived.reportSuccess();
} }
} }

View file

@ -91,6 +91,7 @@ public class DummyConnection extends AbstractXMPPConnection {
protected void connectInternal() { protected void connectInternal() {
connected = true; connected = true;
saslFeatureReceived.reportSuccess(); saslFeatureReceived.reportSuccess();
tlsHandled.reportSuccess();
streamId = "dummy-" + new Random(new Date().getTime()).nextInt(); streamId = "dummy-" + new Random(new Date().getTime()).nextInt();
if (reconnect) { if (reconnect) {

View file

@ -46,6 +46,7 @@ import org.jivesoftware.smack.packet.StreamOpen;
import org.jivesoftware.smack.packet.Stanza; import org.jivesoftware.smack.packet.Stanza;
import org.jivesoftware.smack.packet.Presence; import org.jivesoftware.smack.packet.Presence;
import org.jivesoftware.smack.packet.StartTls; import org.jivesoftware.smack.packet.StartTls;
import org.jivesoftware.smack.packet.StreamError;
import org.jivesoftware.smack.sasl.packet.SaslStreamElements; import org.jivesoftware.smack.sasl.packet.SaslStreamElements;
import org.jivesoftware.smack.sasl.packet.SaslStreamElements.Challenge; import org.jivesoftware.smack.sasl.packet.SaslStreamElements.Challenge;
import org.jivesoftware.smack.sasl.packet.SaslStreamElements.SASLFailure; import org.jivesoftware.smack.sasl.packet.SaslStreamElements.SASLFailure;
@ -923,13 +924,19 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
StartTls startTlsFeature = getFeature(StartTls.ELEMENT, StartTls.NAMESPACE); StartTls startTlsFeature = getFeature(StartTls.ELEMENT, StartTls.NAMESPACE);
if (startTlsFeature != null) { if (startTlsFeature != null) {
if (startTlsFeature.required() && config.getSecurityMode() == SecurityMode.disabled) { if (startTlsFeature.required() && config.getSecurityMode() == SecurityMode.disabled) {
notifyConnectionError(new SecurityRequiredByServerException()); SmackException smackException = new SecurityRequiredByServerException();
tlsHandled.reportFailure(smackException);
notifyConnectionError(smackException);
return; return;
} }
if (config.getSecurityMode() != ConnectionConfiguration.SecurityMode.disabled) { if (config.getSecurityMode() != ConnectionConfiguration.SecurityMode.disabled) {
sendNonza(new StartTls()); sendNonza(new StartTls());
} else {
tlsHandled.reportSuccess();
} }
} else {
tlsHandled.reportSuccess();
} }
if (getSASLAuthentication().authenticationSuccessful()) { if (getSASLAuthentication().authenticationSuccessful()) {
@ -1028,7 +1035,13 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
} }
break; break;
case "error": case "error":
throw new StreamErrorException(PacketParserUtils.parseStreamError(parser)); StreamError streamError = PacketParserUtils.parseStreamError(parser);
saslFeatureReceived.reportFailure(new StreamErrorException(streamError));
// Mark the tlsHandled sync point as success, we will use the saslFeatureReceived sync
// point to report the error, which is checked immediately after tlsHandled in
// connectInternal().
tlsHandled.reportSuccess();
throw new StreamErrorException(streamError);
case "features": case "features":
parseFeatures(parser); parseFeatures(parser);
break; break;
@ -1040,9 +1053,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
openStream(); openStream();
} }
catch (Exception e) { catch (Exception e) {
// We report any failure regarding TLS in the second stage of XMPP SmackException smackException = new SmackException(e);
// connection establishment, namely the SASL authentication tlsHandled.reportFailure(smackException);
saslFeatureReceived.reportFailure(new SmackException(e));
throw e; throw e;
} }
break; break;