From 4b5d6f5b4fd29a4ee6d469f3b540dc5ba826f1a3 Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Sat, 15 Nov 2014 08:48:40 -0500 Subject: [PATCH] Improve auth error handling and state machine --- .../crypto/sasl/AuthenticationException.java | 11 -------- .../conversations/crypto/sasl/DigestMd5.java | 17 +++++------- .../crypto/sasl/SaslMechanism.java | 27 +++++++++++++++++++ .../conversations/crypto/sasl/ScramSha1.java | 13 ++------- .../conversations/xmpp/XmppConnection.java | 15 +++++------ 5 files changed, 43 insertions(+), 40 deletions(-) delete mode 100644 src/main/java/eu/siacs/conversations/crypto/sasl/AuthenticationException.java diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/AuthenticationException.java b/src/main/java/eu/siacs/conversations/crypto/sasl/AuthenticationException.java deleted file mode 100644 index 62d3ddf1a..000000000 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/AuthenticationException.java +++ /dev/null @@ -1,11 +0,0 @@ -package eu.siacs.conversations.crypto.sasl; - -public class AuthenticationException extends Exception { - public AuthenticationException(final String message) { - super(message); - } - - public AuthenticationException(final Exception inner) { - super(inner); - } -} diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java b/src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java index bef76fef8..b56d2a466 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java @@ -21,11 +21,6 @@ public class DigestMd5 extends SaslMechanism { return "DIGEST-MD5"; } - private enum State { - INITIAL, - RESPONSE_SENT, - } - private State state = State.INITIAL; @Override @@ -53,8 +48,7 @@ public class DigestMd5 extends SaslMechanism { final byte[] y = md.digest(x.getBytes(Charset.defaultCharset())); final String cNonce = new BigInteger(100, rng).toString(32); final byte[] a1 = CryptoHelper.concatenateByteArrays(y, - (":" + nonce + ":" + cNonce).getBytes(Charset - .defaultCharset())); + (":" + nonce + ":" + cNonce).getBytes(Charset.defaultCharset())); final String a2 = "AUTHENTICATE:" + digestUri; final String ha1 = CryptoHelper.bytesToHex(md.digest(a1)); final String ha2 = CryptoHelper.bytesToHex(md.digest(a2.getBytes(Charset @@ -72,13 +66,16 @@ public class DigestMd5 extends SaslMechanism { saslString.getBytes(Charset.defaultCharset()), Base64.NO_WRAP); } catch (final NoSuchAlgorithmException e) { - return ""; + throw new AuthenticationException(e); } return encodedResponse; case RESPONSE_SENT: - return ""; + state = State.VALID_SERVER_RESPONSE; + break; + default: + throw new InvalidStateException(state); } - return ""; + return null; } } diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java b/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java index 38a03c187..7dd5e99c3 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java @@ -11,6 +11,33 @@ public abstract class SaslMechanism { final protected Account account; final protected SecureRandom rng; + protected static enum State { + INITIAL, + AUTH_TEXT_SENT, + RESPONSE_SENT, + VALID_SERVER_RESPONSE, + } + + public static class AuthenticationException extends Exception { + public AuthenticationException(final String message) { + super(message); + } + + public AuthenticationException(final Exception inner) { + super(inner); + } + } + + public static class InvalidStateException extends AuthenticationException { + public InvalidStateException(final String message) { + super(message); + } + + public InvalidStateException(final State state) { + this("Invalid state: " + state.toString()); + } + } + public SaslMechanism(final TagWriter tagWriter, final Account account, final SecureRandom rng) { this.tagWriter = tagWriter; this.account = account; diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha1.java b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha1.java index e7e31e73e..2073de2d8 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha1.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha1.java @@ -33,13 +33,6 @@ public class ScramSha1 extends SaslMechanism { HMAC = new HMac(new SHA1Digest()); } - private enum State { - INITIAL, - AUTH_TEXT_SENT, - RESPONSE_SENT, - VALID_SERVER_RESPONSE, - } - private State state = State.INITIAL; public ScramSha1(final TagWriter tagWriter, final Account account, final SecureRandom rng) { @@ -56,11 +49,9 @@ public class ScramSha1 extends SaslMechanism { @Override public String getClientFirstMessage() { - if (clientFirstMessageBare.isEmpty()) { + if (clientFirstMessageBare.isEmpty() && state == State.INITIAL) { clientFirstMessageBare = "n=" + CryptoHelper.saslPrep(account.getUsername()) + ",r=" + this.clientNonce; - } - if (state == State.INITIAL) { state = State.AUTH_TEXT_SENT; } return Base64.encodeToString( @@ -157,7 +148,7 @@ public class ScramSha1 extends SaslMechanism { state = State.VALID_SERVER_RESPONSE; return ""; default: - throw new AuthenticationException("Invalid state: " + state); + throw new InvalidStateException(state); } } diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 899ebf562..c34a08a84 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -39,7 +39,6 @@ import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509TrustManager; import eu.siacs.conversations.Config; -import eu.siacs.conversations.crypto.sasl.AuthenticationException; import eu.siacs.conversations.crypto.sasl.DigestMd5; import eu.siacs.conversations.crypto.sasl.Plain; import eu.siacs.conversations.crypto.sasl.SaslMechanism; @@ -284,14 +283,14 @@ public class XmppConnection implements Runnable { } else if (nextTag.isStart("compressed")) { switchOverToZLib(nextTag); } else if (nextTag.isStart("success")) { - Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": logged in"); final String challenge = tagReader.readElement(nextTag).getContent(); try { saslMechanism.getResponse(challenge); - } catch (final AuthenticationException e) { + } catch (final SaslMechanism.AuthenticationException e) { disconnect(true); Log.e(Config.LOGTAG, String.valueOf(e)); } + Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": logged in"); tagReader.reset(); sendStartStream(); processStream(tagReader.readTag()); @@ -306,7 +305,7 @@ public class XmppConnection implements Runnable { "urn:ietf:params:xml:ns:xmpp-sasl"); try { response.setContent(saslMechanism.getResponse(challenge)); - } catch (final AuthenticationException e) { + } catch (final SaslMechanism.AuthenticationException e) { // TODO: Send auth abort tag. Log.e(Config.LOGTAG, e.toString()); } @@ -643,10 +642,10 @@ public class XmppConnection implements Runnable { saslMechanism = new Plain(tagWriter, account); auth.setAttribute("mechanism", Plain.getMechanism()); } - if (!saslMechanism.getClientFirstMessage().isEmpty()) { - auth.setContent(saslMechanism.getClientFirstMessage()); - } - tagWriter.writeElement(auth); + if (!saslMechanism.getClientFirstMessage().isEmpty()) { + auth.setContent(saslMechanism.getClientFirstMessage()); + } + tagWriter.writeElement(auth); } else if (this.streamFeatures.hasChild("sm", "urn:xmpp:sm:" + smVersion) && streamId != null) {