From 847877f9d2954130a73860118cb7b6fe073aafe8 Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Sat, 15 Nov 2014 09:42:40 -0500 Subject: Add auth method pinning --- .../siacs/conversations/xmpp/XmppConnection.java | 39 ++++++++++++++-------- 1 file changed, 26 insertions(+), 13 deletions(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index c34a08a8..24e42c7b 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -12,6 +12,8 @@ import android.util.Log; import android.util.SparseArray; import org.apache.http.conn.ssl.StrictHostnameVerifier; +import org.json.JSONException; +import org.json.JSONObject; import org.xmlpull.v1.XmlPullParserException; import java.io.IOException; @@ -291,6 +293,8 @@ public class XmppConnection implements Runnable { Log.e(Config.LOGTAG, String.valueOf(e)); } Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": logged in"); + account.setKey(Account.PINNED_MECHANISM_KEY, + String.valueOf(saslMechanism.getPriority())); tagReader.reset(); sendStartStream(); processStream(tagReader.readTag()); @@ -629,23 +633,32 @@ public class XmppConnection implements Runnable { .findChild("mechanisms")); final Element auth = new Element("auth"); auth.setAttribute("xmlns", "urn:ietf:params:xml:ns:xmpp-sasl"); - if (mechanisms.contains(ScramSha1.getMechanism())) { + if (mechanisms.contains("SCRAM-SHA-1")) { saslMechanism = new ScramSha1(tagWriter, account, mXmppConnectionService.getRNG()); - Log.d(Config.LOGTAG, "Authenticating with " + ScramSha1.getMechanism()); - auth.setAttribute("mechanism", ScramSha1.getMechanism()); - } else if (mechanisms.contains(DigestMd5.getMechanism())) { - Log.d(Config.LOGTAG, "Authenticating with " + DigestMd5.getMechanism()); + } else if (mechanisms.contains("DIGEST-MD5")) { saslMechanism = new DigestMd5(tagWriter, account, mXmppConnectionService.getRNG()); - auth.setAttribute("mechanism", DigestMd5.getMechanism()); - } else if (mechanisms.contains(Plain.getMechanism())) { - Log.d(Config.LOGTAG, "Authenticating with " + Plain.getMechanism()); + } else if (mechanisms.contains("PLAIN")) { saslMechanism = new Plain(tagWriter, account); - auth.setAttribute("mechanism", Plain.getMechanism()); } - if (!saslMechanism.getClientFirstMessage().isEmpty()) { - auth.setContent(saslMechanism.getClientFirstMessage()); - } - tagWriter.writeElement(auth); + final JSONObject keys = account.getKeys(); + try { + if (keys.has(Account.PINNED_MECHANISM_KEY) && + keys.getInt(Account.PINNED_MECHANISM_KEY) > saslMechanism.getPriority() ) { + Log.e(Config.LOGTAG, "Auth failed. Authentication mechanism " + saslMechanism.getMechanism() + + " has lower priority (" + String.valueOf(saslMechanism.getPriority()) + + ") than pinned priority (" + keys.getInt(Account.PINNED_MECHANISM_KEY) + + "). Possible downgrade attack?"); + disconnect(true); + } + } catch (final JSONException e) { + Log.d(Config.LOGTAG, "Parse error while checking pinned auth mechanism"); + } + Log.d(Config.LOGTAG, "Authenticating with " + saslMechanism.getMechanism()); + auth.setAttribute("mechanism", saslMechanism.getMechanism()); + if (!saslMechanism.getClientFirstMessage().isEmpty()) { + auth.setContent(saslMechanism.getClientFirstMessage()); + } + tagWriter.writeElement(auth); } else if (this.streamFeatures.hasChild("sm", "urn:xmpp:sm:" + smVersion) && streamId != null) { -- cgit v1.2.3 From d94b07c916757b351c5d85762b9441ad5e7354f0 Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Sat, 15 Nov 2014 11:09:02 -0500 Subject: Make account status an enum --- .../siacs/conversations/xmpp/XmppConnection.java | 50 +++++++++++----------- 1 file changed, 25 insertions(+), 25 deletions(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 24e42c7b..16ef1b50 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -120,15 +120,15 @@ public class XmppConnection implements Runnable { applicationContext = service.getApplicationContext(); } - protected void changeStatus(int nextStatus) { + protected void changeStatus(final Account.State nextStatus) { if (account.getStatus() != nextStatus) { - if ((nextStatus == Account.STATUS_OFFLINE) - && (account.getStatus() != Account.STATUS_CONNECTING) - && (account.getStatus() != Account.STATUS_ONLINE) - && (account.getStatus() != Account.STATUS_DISABLED)) { + if ((nextStatus == Account.State.OFFLINE) + && (account.getStatus() != Account.State.CONNECTING) + && (account.getStatus() != Account.State.ONLINE) + && (account.getStatus() != Account.State.DISABLED)) { return; - } - if (nextStatus == Account.STATUS_ONLINE) { + } + if (nextStatus == Account.State.ONLINE) { this.attempt = 0; } account.setStatus(nextStatus); @@ -151,12 +151,12 @@ public class XmppConnection implements Runnable { tagReader = new XmlReader(wakeLock); tagWriter = new TagWriter(); packetCallbacks.clear(); - this.changeStatus(Account.STATUS_CONNECTING); + this.changeStatus(Account.State.CONNECTING); Bundle result = DNSHelper.getSRVRecord(account.getServer()); ArrayList values = result.getParcelableArrayList("values"); if ("timeout".equals(result.getString("error"))) { Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": dns timeout"); - this.changeStatus(Account.STATUS_OFFLINE); + this.changeStatus(Account.State.OFFLINE); return; } else if (values != null) { int i = 0; @@ -197,7 +197,7 @@ public class XmppConnection implements Runnable { } } if (socketError) { - this.changeStatus(Account.STATUS_SERVER_NOT_FOUND); + this.changeStatus(Account.State.SERVER_NOT_FOUND); if (wakeLock.isHeld()) { try { wakeLock.release(); @@ -212,7 +212,7 @@ public class XmppConnection implements Runnable { } else { Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": timeout in DNS resolution"); - changeStatus(Account.STATUS_OFFLINE); + changeStatus(Account.State.OFFLINE); return; } OutputStream out = socket.getOutputStream(); @@ -236,7 +236,7 @@ public class XmppConnection implements Runnable { socket.close(); } } catch (UnknownHostException e) { - this.changeStatus(Account.STATUS_SERVER_NOT_FOUND); + this.changeStatus(Account.State.SERVER_NOT_FOUND); if (wakeLock.isHeld()) { try { wakeLock.release(); @@ -245,7 +245,7 @@ public class XmppConnection implements Runnable { } } catch (final IOException | XmlPullParserException e) { Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": " + e.getMessage()); - this.changeStatus(Account.STATUS_OFFLINE); + this.changeStatus(Account.State.OFFLINE); if (wakeLock.isHeld()) { try { wakeLock.release(); @@ -254,7 +254,7 @@ public class XmppConnection implements Runnable { } } catch (NoSuchAlgorithmException e) { Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": " + e.getMessage()); - this.changeStatus(Account.STATUS_OFFLINE); + this.changeStatus(Account.State.OFFLINE); Log.d(Config.LOGTAG, "compression exception " + e.getMessage()); if (wakeLock.isHeld()) { try { @@ -294,14 +294,14 @@ public class XmppConnection implements Runnable { } Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": logged in"); account.setKey(Account.PINNED_MECHANISM_KEY, - String.valueOf(saslMechanism.getPriority())); + String.valueOf(saslMechanism.getPriority())); tagReader.reset(); sendStartStream(); processStream(tagReader.readTag()); break; } else if (nextTag.isStart("failure")) { tagReader.readElement(nextTag); - changeStatus(Account.STATUS_UNAUTHORIZED); + changeStatus(Account.State.UNAUTHORIZED); } else if (nextTag.isStart("challenge")) { final String challenge = tagReader.readElement(nextTag).getContent(); final Element response = new Element("response"); @@ -377,7 +377,7 @@ public class XmppConnection implements Runnable { tagReader.readElement(nextTag); Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": resumption failed"); streamId = null; - if (account.getStatus() != Account.STATUS_ONLINE) { + if (account.getStatus() != Account.State.ONLINE) { sendBindRequest(); } } else if (nextTag.isStart("iq")) { @@ -389,8 +389,8 @@ public class XmppConnection implements Runnable { } nextTag = tagReader.readTag(); } - if (account.getStatus() == Account.STATUS_ONLINE) { - account. setStatus(Account.STATUS_OFFLINE); + if (account.getStatus() == Account.State.ONLINE) { + account. setStatus(Account.State.OFFLINE); if (statusListener != null) { statusListener.onStatusChanged(account); } @@ -408,7 +408,7 @@ public class XmppConnection implements Runnable { public void onIqPacketReceived(Account account, IqPacket packet) { Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": online with resource " + account.getResource()); - changeStatus(Account.STATUS_ONLINE); + changeStatus(Account.State.ONLINE); } }); } @@ -625,7 +625,7 @@ public class XmppConnection implements Runnable { sendRegistryRequest(); } else if (!this.streamFeatures.hasChild("register") && account.isOptionSet(Account.OPTION_REGISTER)) { - changeStatus(Account.STATUS_REGISTRATION_NOT_SUPPORTED); + changeStatus(Account.State.REGISTRATION_NOT_SUPPORTED); disconnect(true); } else if (this.streamFeatures.hasChild("mechanisms") && shouldAuthenticate && usingEncryption) { @@ -731,20 +731,20 @@ public class XmppConnection implements Runnable { if (packet.getType() == IqPacket.TYPE_RESULT) { account.setOption(Account.OPTION_REGISTER, false); - changeStatus(Account.STATUS_REGISTRATION_SUCCESSFULL); + changeStatus(Account.State.REGISTRATION_SUCCESSFUL); } else if (packet.hasChild("error") && (packet.findChild("error") .hasChild("conflict"))) { - changeStatus(Account.STATUS_REGISTRATION_CONFLICT); + changeStatus(Account.State.REGISTRATION_CONFLICT); } else { - changeStatus(Account.STATUS_REGISTRATION_FAILED); + changeStatus(Account.State.REGISTRATION_FAILED); Log.d(Config.LOGTAG, packet.toString()); } disconnect(true); } }); } else { - changeStatus(Account.STATUS_REGISTRATION_FAILED); + changeStatus(Account.State.REGISTRATION_FAILED); disconnect(true); Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": could not register. instructions are" -- cgit v1.2.3 From c09d450db815f51802caf0ea761ee01617a4ad19 Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Sat, 15 Nov 2014 11:19:42 -0500 Subject: Add security error status --- src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 16ef1b50..42a89610 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -649,7 +649,8 @@ public class XmppConnection implements Runnable { ") than pinned priority (" + keys.getInt(Account.PINNED_MECHANISM_KEY) + "). Possible downgrade attack?"); disconnect(true); - } + account.setStatus(Account.State.SECURITY_ERROR); + } } catch (final JSONException e) { Log.d(Config.LOGTAG, "Parse error while checking pinned auth mechanism"); } -- cgit v1.2.3 From 251f4d6d7f93da6d23c3422cf8eeece780789f45 Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Sat, 15 Nov 2014 11:29:58 -0500 Subject: Add incompatible server status --- src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java | 1 + 1 file changed, 1 insertion(+) (limited to 'src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 42a89610..3c911fc9 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -669,6 +669,7 @@ public class XmppConnection implements Runnable { } else if (this.streamFeatures.hasChild("bind") && shouldBind) { sendBindRequest(); } else { + account.setStatus(Account.State.INCOMPATIBLE_SERVER); Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": incompatible server. disconnecting"); disconnect(true); -- cgit v1.2.3 From 17cec63c14e15d1a78d124f718a4b51e4f4debd1 Mon Sep 17 00:00:00 2001 From: Sam Whited Date: Sat, 15 Nov 2014 11:40:41 -0500 Subject: Set security error status on TLS cert mismatch --- .../java/eu/siacs/conversations/xmpp/XmppConnection.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java') diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 3c911fc9..7b59c812 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -593,12 +593,13 @@ public class XmppConnection implements Runnable { } sslSocket.setEnabledProtocols(supportProtocols); - if (verifier != null - && !verifier.verify(account.getServer().getDomainpart(), - sslSocket.getSession())) { - sslSocket.close(); - throw new IOException("host mismatch in TLS connection"); - } + if (verifier != null + && !verifier.verify(account.getServer().getDomainpart(), + sslSocket.getSession())) { + account.setStatus(Account.State.SECURITY_ERROR); + sslSocket.close(); + throw new IOException("Host mismatch in TLS connection"); + } tagReader.setInputStream(sslSocket.getInputStream()); tagWriter.setOutputStream(sslSocket.getOutputStream()); sendStartStream(); -- cgit v1.2.3