From 7b6d49f329801a8f612dffba90608287610066fd Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Fri, 5 May 2017 09:33:05 +0200 Subject: [PATCH] unified all account state exceptions --- .../siacs/conversations/entities/Account.java | 12 ++- .../xmpp/OnIqPacketReceived.java | 2 +- .../conversations/xmpp/XmppConnection.java | 100 +++++++----------- src/main/res/values/strings.xml | 4 +- 4 files changed, 49 insertions(+), 69 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/entities/Account.java b/src/main/java/eu/siacs/conversations/entities/Account.java index ee8a7c55..d2297102 100644 --- a/src/main/java/eu/siacs/conversations/entities/Account.java +++ b/src/main/java/eu/siacs/conversations/entities/Account.java @@ -118,9 +118,11 @@ public class Account extends AbstractEntity { REGISTRATION_CONFLICT(true), REGISTRATION_SUCCESSFUL, REGISTRATION_NOT_SUPPORTED(true), - SECURITY_ERROR(true), + TLS_ERROR(true), INCOMPATIBLE_SERVER(true), TOR_NOT_AVAILABLE(true), + DOWNGRADE_ATTACK(true), + SESSION_FAILURE(true), BIND_FAILURE(true), HOST_UNKNOWN(true), REGISTRATION_PLEASE_WAIT(true), @@ -168,14 +170,18 @@ public class Account extends AbstractEntity { return R.string.account_status_regis_success; case REGISTRATION_NOT_SUPPORTED: return R.string.account_status_regis_not_sup; - case SECURITY_ERROR: - return R.string.account_status_security_error; + case TLS_ERROR: + return R.string.account_status_tls_error; case INCOMPATIBLE_SERVER: return R.string.account_status_incompatible_server; case TOR_NOT_AVAILABLE: return R.string.account_status_tor_unavailable; case BIND_FAILURE: return R.string.account_status_bind_failure; + case SESSION_FAILURE: + return R.string.session_failure; + case DOWNGRADE_ATTACK: + return R.string.downgrade_attack; case HOST_UNKNOWN: return R.string.account_status_host_unknown; case POLICY_VIOLATION: diff --git a/src/main/java/eu/siacs/conversations/xmpp/OnIqPacketReceived.java b/src/main/java/eu/siacs/conversations/xmpp/OnIqPacketReceived.java index a4cff986..6db24ef9 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/OnIqPacketReceived.java +++ b/src/main/java/eu/siacs/conversations/xmpp/OnIqPacketReceived.java @@ -4,5 +4,5 @@ import eu.siacs.conversations.entities.Account; import eu.siacs.conversations.xmpp.stanzas.IqPacket; public interface OnIqPacketReceived extends PacketReceived { - public void onIqPacketReceived(Account account, IqPacket packet); + void onIqPacketReceived(Account account, IqPacket packet); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index aca22569..d35d3abc 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -299,7 +299,7 @@ public class XmppConnection implements Runnable { final SSLSession session = ((SSLSocket) localSocket).getSession(); if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), session)) { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": TLS certificate verification failed"); - throw new SecurityException(); + throw new StateChangingException(Account.State.TLS_ERROR); } } catch (KeyManagementException e) { features.encryptionEnabled = false; @@ -388,7 +388,7 @@ public class XmppConnection implements Runnable { if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), ((SSLSocket) localSocket).getSession())) { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": TLS certificate verification failed"); - throw new SecurityException(); + throw new StateChangingException(Account.State.TLS_ERROR); } } if (startXmpp(localSocket)) { @@ -396,7 +396,7 @@ public class XmppConnection implements Runnable { } else { localSocket.close(); } - } catch (final SecurityException e) { + } catch (final StateChangingException e) { throw e; } catch (InterruptedException e) { Log.d(Config.LOGTAG,account.getJid().toBareJid()+": thread was interrupted before beginning stream"); @@ -410,28 +410,14 @@ public class XmppConnection implements Runnable { } } processStream(); - } catch (final java.lang.SecurityException e) { + } catch (final SecurityException e) { this.changeStatus(Account.State.MISSING_INTERNET_PERMISSION); - } catch (final RegistrationNotSupportedException e) { - this.changeStatus(Account.State.REGISTRATION_NOT_SUPPORTED); - } catch (final IncompatibleServerException e) { - this.changeStatus(Account.State.INCOMPATIBLE_SERVER); - } catch (final SecurityException e) { - this.changeStatus(Account.State.SECURITY_ERROR); - } catch (final UnauthorizedException e) { - this.changeStatus(Account.State.UNAUTHORIZED); - } catch (final PaymentRequiredException e) { - this.changeStatus(Account.State.PAYMENT_REQUIRED); + } catch(final StateChangingException e) { + this.changeStatus(e.state); } catch (final UnknownHostException | ConnectException e) { this.changeStatus(Account.State.SERVER_NOT_FOUND); } catch (final SocksSocketFactory.SocksProxyNotFoundException e) { this.changeStatus(Account.State.TOR_NOT_AVAILABLE); - } catch(final StreamErrorHostUnknown e) { - this.changeStatus(Account.State.HOST_UNKNOWN); - } catch(final StreamErrorPolicyViolation e) { - this.changeStatus(Account.State.POLICY_VIOLATION); - } catch(final StreamError e) { - this.changeStatus(Account.State.STREAM_ERROR); } catch (final IOException | XmlPullParserException | NoSuchAlgorithmException e) { Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": " + e.getMessage()); this.changeStatus(Account.State.OFFLINE); @@ -528,8 +514,8 @@ public class XmppConnection implements Runnable { try { saslMechanism.getResponse(challenge); } catch (final SaslMechanism.AuthenticationException e) { - disconnect(true); Log.e(Config.LOGTAG, String.valueOf(e)); + throw new StateChangingException(Account.State.UNAUTHORIZED); } Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": logged in"); account.setKey(Account.PINNED_MECHANISM_KEY, @@ -551,9 +537,9 @@ public class XmppConnection implements Runnable { && text.contains("renew") && Config.MAGIC_CREATE_DOMAIN != null && text.contains(Config.MAGIC_CREATE_DOMAIN)) { - throw new PaymentRequiredException(); + throw new StateChangingException(Account.State.PAYMENT_REQUIRED); } else { - throw new UnauthorizedException(); + throw new StateChangingException(Account.State.UNAUTHORIZED); } } else if (nextTag.isStart("challenge")) { final String challenge = tagReader.readElement(nextTag).getContent(); @@ -776,7 +762,11 @@ public class XmppConnection implements Runnable { } } if (callback != null) { - callback.onIqPacketReceived(account,packet); + try { + callback.onIqPacketReceived(account, packet); + } catch (StateChangingError error) { + throw new StateChangingException(error.state); + } } } } @@ -819,7 +809,7 @@ public class XmppConnection implements Runnable { if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), sslSocket.getSession())) { Log.d(Config.LOGTAG,account.getJid().toBareJid()+": TLS certificate verification failed"); - throw new SecurityException(); + throw new StateChangingException(Account.State.TLS_ERROR); } tagReader.setInputStream(sslSocket.getInputStream()); tagWriter.setOutputStream(sslSocket.getOutputStream()); @@ -835,7 +825,7 @@ public class XmppConnection implements Runnable { sslSocket.close(); } catch (final NoSuchAlgorithmException | KeyManagementException e1) { Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": TLS certificate verification failed"); - throw new SecurityException(); + throw new StateChangingException(Account.State.TLS_ERROR); } } @@ -848,10 +838,10 @@ public class XmppConnection implements Runnable { if (features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS) { sendRegistryRequest(); } else { - throw new IncompatibleServerException(); + throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER); } } else if (!this.streamFeatures.hasChild("register") && account.isOptionSet(Account.OPTION_REGISTER)) { - throw new RegistrationNotSupportedException(); + throw new StateChangingException(Account.State.REGISTRATION_NOT_SUPPORTED); } else if (this.streamFeatures.hasChild("mechanisms") && shouldAuthenticate && (features.encryptionEnabled || Config.ALLOW_NON_TLS_CONNECTIONS)) { @@ -868,7 +858,7 @@ public class XmppConnection implements Runnable { if (this.streamFeatures.hasChild("bind")) { sendBindRequest(); } else { - throw new IncompatibleServerException(); + throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER); } } } @@ -898,7 +888,7 @@ public class XmppConnection implements Runnable { " has lower priority (" + String.valueOf(saslMechanism.getPriority()) + ") than pinned priority (" + pinnedMechanism + "). Possible downgrade attack?"); - throw new SecurityException(); + throw new StateChangingException(Account.State.DOWNGRADE_ATTACK); } Log.d(Config.LOGTAG, account.getJid().toString() + ": Authenticating with " + saslMechanism.getMechanism()); auth.setAttribute("mechanism", saslMechanism.getMechanism()); @@ -907,7 +897,7 @@ public class XmppConnection implements Runnable { } tagWriter.writeElement(auth); } else { - throw new IncompatibleServerException(); + throw new StateChangingException(Account.State.INCOMPATIBLE_SERVER); } } @@ -1042,8 +1032,7 @@ public class XmppConnection implements Runnable { } else { Log.d(Config.LOGTAG, account.getJid() + ": disconnecting because of bind failure (" + packet.toString()); } - forceCloseSocket(); - changeStatus(Account.State.BIND_FAILURE); + throw new StateChangingError(Account.State.BIND_FAILURE); } }); } @@ -1085,8 +1074,7 @@ public class XmppConnection implements Runnable { if (packet.getType() == IqPacket.TYPE.RESULT) { sendPostBindInitialization(); } else if (packet.getType() != IqPacket.TYPE.TIMEOUT) { - Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": could not init sessions"); - disconnect(true); + throw new StateChangingError(Account.State.SESSION_FAILURE); } } }); @@ -1266,12 +1254,12 @@ public class XmppConnection implements Runnable { + account.getResource() + ")"); throw new IOException(); } else if (streamError.hasChild("host-unknown")) { - throw new StreamErrorHostUnknown(); + throw new StateChangingException(Account.State.HOST_UNKNOWN); } else if (streamError.hasChild("policy-violation")) { - throw new StreamErrorPolicyViolation(); + throw new StateChangingException(Account.State.POLICY_VIOLATION); } else { Log.d(Config.LOGTAG,account.getJid().toBareJid()+": stream error "+streamError.toString()); - throw new StreamError(); + throw new StateChangingException(Account.State.STREAM_ERROR); } } @@ -1561,36 +1549,20 @@ public class XmppConnection implements Runnable { return Identity.UNKNOWN; } - private class UnauthorizedException extends IOException { + private class StateChangingError extends Error { + private final Account.State state; + public StateChangingError(Account.State state) { + this.state = state; + } } - private class SecurityException extends IOException { - - } - - private class IncompatibleServerException extends IOException { - - } - - private class StreamErrorHostUnknown extends StreamError { - - } - - private class StreamErrorPolicyViolation extends StreamError { - - } - - private class StreamError extends IOException { - - } - - private class PaymentRequiredException extends IOException { - - } - - private class RegistrationNotSupportedException extends IOException { + private class StateChangingException extends IOException { + private final Account.State state; + public StateChangingException(Account.State state) { + this.state = state; + } } public enum Identity { diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index 6ab64c84..9e9660b9 100644 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -158,7 +158,7 @@ Username already in use Registration completed Server does not support registration - Security error + TLS error Policy violation Incompatible server Stream error @@ -747,4 +747,6 @@ Block entire domain online right now Retry decryption + Session failure + Downgrade attack