Merge pull request #656 from SamWhited/authrefactor

Daniel Gultsch created

Refactor authentication code

Change summary

src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java     | 72 
src/main/java/eu/siacs/conversations/crypto/sasl/Plain.java         | 25 
src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java | 27 
src/main/java/eu/siacs/conversations/utils/CryptoHelper.java        | 60 
src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java       | 49 
5 files changed, 145 insertions(+), 88 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java 🔗

@@ -0,0 +1,72 @@
+package eu.siacs.conversations.crypto.sasl;
+
+import android.util.Base64;
+
+import java.math.BigInteger;
+import java.nio.charset.Charset;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+
+import eu.siacs.conversations.entities.Account;
+import eu.siacs.conversations.utils.CryptoHelper;
+import eu.siacs.conversations.xml.TagWriter;
+
+public class DigestMd5 extends SaslMechanism {
+    public DigestMd5(final TagWriter tagWriter, final Account account, final SecureRandom rng) {
+        super(tagWriter, account, rng);
+    }
+
+    @Override
+    public String getMechanism() {
+        return "DIGEST-MD5";
+    }
+
+    @Override
+    public String getResponse(final String challenge) {
+        final String encodedResponse;
+        try {
+            final String[] challengeParts = new String(Base64.decode(challenge,
+                    Base64.DEFAULT)).split(",");
+            String nonce = "";
+            for (int i = 0; i < challengeParts.length; ++i) {
+                String[] parts = challengeParts[i].split("=");
+                if (parts[0].equals("nonce")) {
+                    nonce = parts[1].replace("\"", "");
+                } else if (parts[0].equals("rspauth")) {
+                    return "";
+                }
+            }
+            final String digestUri = "xmpp/" + account.getServer();
+            final String nonceCount = "00000001";
+            final String x = account.getUsername() + ":" + account.getServer() + ":"
+                    + account.getPassword();
+            final MessageDigest md = MessageDigest.getInstance("MD5");
+            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()));
+            final String a2 = "AUTHENTICATE:" + digestUri;
+            final String ha1 = CryptoHelper.bytesToHex(md.digest(a1));
+            final String ha2 = CryptoHelper.bytesToHex(md.digest(a2.getBytes(Charset
+                    .defaultCharset())));
+            final String kd = ha1 + ":" + nonce + ":" + nonceCount + ":" + cNonce
+                    + ":auth:" + ha2;
+            final String response = CryptoHelper.bytesToHex(md.digest(kd.getBytes(Charset
+                    .defaultCharset())));
+            final String saslString = "username=\"" + account.getUsername()
+                    + "\",realm=\"" + account.getServer() + "\",nonce=\""
+                    + nonce + "\",cnonce=\"" + cNonce + "\",nc=" + nonceCount
+                    + ",qop=auth,digest-uri=\"" + digestUri + "\",response="
+                    + response + ",charset=utf-8";
+            encodedResponse = Base64.encodeToString(
+                    saslString.getBytes(Charset.defaultCharset()),
+                    Base64.NO_WRAP);
+        } catch (final NoSuchAlgorithmException e) {
+            return "";
+        }
+
+        return encodedResponse;
+    }
+}

src/main/java/eu/siacs/conversations/crypto/sasl/Plain.java 🔗

@@ -0,0 +1,25 @@
+package eu.siacs.conversations.crypto.sasl;
+
+import android.util.Base64;
+
+import java.nio.charset.Charset;
+
+import eu.siacs.conversations.entities.Account;
+import eu.siacs.conversations.xml.TagWriter;
+
+public class Plain extends SaslMechanism {
+    public Plain(final TagWriter tagWriter, final Account account) {
+        super(tagWriter, account, null);
+    }
+
+    @Override
+    public String getMechanism() {
+        return "PLAIN";
+    }
+
+    @Override
+    public String getStartAuth() {
+        final String sasl = '\u0000' + account.getUsername() + '\u0000' + account.getPassword();
+        return Base64.encodeToString(sasl.getBytes(Charset.defaultCharset()), Base64.NO_WRAP);
+    }
+}

src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java 🔗

@@ -0,0 +1,27 @@
+package eu.siacs.conversations.crypto.sasl;
+
+import java.security.SecureRandom;
+
+import eu.siacs.conversations.entities.Account;
+import eu.siacs.conversations.xml.TagWriter;
+
+public abstract class SaslMechanism {
+
+    final protected TagWriter tagWriter;
+    final protected Account account;
+    final protected SecureRandom rng;
+
+    public SaslMechanism(final TagWriter tagWriter, final Account account, final SecureRandom rng) {
+        this.tagWriter = tagWriter;
+        this.account = account;
+        this.rng = rng;
+    }
+
+    public abstract String getMechanism();
+    public String getStartAuth() {
+        return "";
+    }
+    public String getResponse(final String challenge) {
+        return "";
+    }
+}

src/main/java/eu/siacs/conversations/utils/CryptoHelper.java 🔗

@@ -1,14 +1,7 @@
 package eu.siacs.conversations.utils;
 
-import java.math.BigInteger;
-import java.nio.charset.Charset;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 
-import eu.siacs.conversations.entities.Account;
-import android.util.Base64;
-
 public class CryptoHelper {
 	public static final String FILETRANSFER = "?FILETRANSFERv1:";
 	final protected static char[] hexArray = "0123456789abcdef".toCharArray();
@@ -36,64 +29,13 @@ public class CryptoHelper {
 		return array;
 	}
 
-	public static String saslPlain(String username, String password) {
-		String sasl = '\u0000' + username + '\u0000' + password;
-		return Base64.encodeToString(sasl.getBytes(Charset.defaultCharset()),
-				Base64.NO_WRAP);
-	}
-
-	private static byte[] concatenateByteArrays(byte[] a, byte[] b) {
+	public static byte[] concatenateByteArrays(byte[] a, byte[] b) {
 		byte[] result = new byte[a.length + b.length];
 		System.arraycopy(a, 0, result, 0, a.length);
 		System.arraycopy(b, 0, result, a.length, b.length);
 		return result;
 	}
 
-	public static String saslDigestMd5(Account account, String challenge,
-			SecureRandom random) {
-		try {
-			String[] challengeParts = new String(Base64.decode(challenge,
-					Base64.DEFAULT)).split(",");
-			String nonce = "";
-			for (int i = 0; i < challengeParts.length; ++i) {
-				String[] parts = challengeParts[i].split("=");
-				if (parts[0].equals("nonce")) {
-					nonce = parts[1].replace("\"", "");
-				} else if (parts[0].equals("rspauth")) {
-					return null;
-				}
-			}
-			String digestUri = "xmpp/" + account.getServer();
-			String nonceCount = "00000001";
-			String x = account.getUsername() + ":" + account.getServer() + ":"
-					+ account.getPassword();
-			MessageDigest md = MessageDigest.getInstance("MD5");
-			byte[] y = md.digest(x.getBytes(Charset.defaultCharset()));
-			String cNonce = new BigInteger(100, random).toString(32);
-			byte[] a1 = concatenateByteArrays(y,
-					(":" + nonce + ":" + cNonce).getBytes(Charset
-							.defaultCharset()));
-			String a2 = "AUTHENTICATE:" + digestUri;
-			String ha1 = bytesToHex(md.digest(a1));
-			String ha2 = bytesToHex(md.digest(a2.getBytes(Charset
-					.defaultCharset())));
-			String kd = ha1 + ":" + nonce + ":" + nonceCount + ":" + cNonce
-					+ ":auth:" + ha2;
-			String response = bytesToHex(md.digest(kd.getBytes(Charset
-					.defaultCharset())));
-			String saslString = "username=\"" + account.getUsername()
-					+ "\",realm=\"" + account.getServer() + "\",nonce=\""
-					+ nonce + "\",cnonce=\"" + cNonce + "\",nc=" + nonceCount
-					+ ",qop=auth,digest-uri=\"" + digestUri + "\",response="
-					+ response + ",charset=utf-8";
-			return Base64.encodeToString(
-					saslString.getBytes(Charset.defaultCharset()),
-					Base64.NO_WRAP);
-		} catch (NoSuchAlgorithmException e) {
-			return null;
-		}
-	}
-
 	public static String randomMucName(SecureRandom random) {
 		return randomWord(3, random) + "." + randomWord(7, random);
 	}

src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java 🔗

@@ -39,9 +39,11 @@ import javax.net.ssl.SSLSocketFactory;
 import javax.net.ssl.X509TrustManager;
 
 import eu.siacs.conversations.Config;
+import eu.siacs.conversations.crypto.sasl.DigestMd5;
+import eu.siacs.conversations.crypto.sasl.Plain;
+import eu.siacs.conversations.crypto.sasl.SaslMechanism;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.services.XmppConnectionService;
-import eu.siacs.conversations.utils.CryptoHelper;
 import eu.siacs.conversations.utils.DNSHelper;
 import eu.siacs.conversations.utils.zlib.ZLibInputStream;
 import eu.siacs.conversations.utils.zlib.ZLibOutputStream;
@@ -104,6 +106,8 @@ public class XmppConnection implements Runnable {
 	private OnMessageAcknowledged acknowledgedListener = null;
 	private XmppConnectionService mXmppConnectionService = null;
 
+    private SaslMechanism saslMechanism;
+
 	public XmppConnection(Account account, XmppConnectionService service) {
 		this.account = account;
 		this.wakeLock = service.getPowerManager().newWakeLock(
@@ -287,12 +291,11 @@ public class XmppConnection implements Runnable {
 				tagReader.readElement(nextTag);
 				changeStatus(Account.STATUS_UNAUTHORIZED);
 			} else if (nextTag.isStart("challenge")) {
-				String challange = tagReader.readElement(nextTag).getContent();
-				Element response = new Element("response");
+				final String challenge = tagReader.readElement(nextTag).getContent();
+				final Element response = new Element("response");
 				response.setAttribute("xmlns",
 						"urn:ietf:params:xml:ns:xmpp-sasl");
-				response.setContent(CryptoHelper.saslDigestMd5(account,
-						challange, mXmppConnectionService.getRNG()));
+				response.setContent(saslMechanism.getResponse(challenge));
 				tagWriter.writeElement(response);
 			} else if (nextTag.isStart("enabled")) {
 				Element enabled = tagReader.readElement(nextTag);
@@ -592,23 +595,6 @@ public class XmppConnection implements Runnable {
 		}
     }
 
-	private void sendSaslAuthPlain() throws IOException {
-		String saslString = CryptoHelper.saslPlain(account.getUsername(),
-				account.getPassword());
-		Element auth = new Element("auth");
-		auth.setAttribute("xmlns", "urn:ietf:params:xml:ns:xmpp-sasl");
-		auth.setAttribute("mechanism", "PLAIN");
-		auth.setContent(saslString);
-		tagWriter.writeElement(auth);
-	}
-
-	private void sendSaslAuthDigestMd5() throws IOException {
-		Element auth = new Element("auth");
-		auth.setAttribute("xmlns", "urn:ietf:params:xml:ns:xmpp-sasl");
-		auth.setAttribute("mechanism", "DIGEST-MD5");
-		tagWriter.writeElement(auth);
-	}
-
 	private void processStreamFeatures(Tag currentTag)
 			throws XmlPullParserException, IOException {
 		this.streamFeatures = tagReader.readElement(currentTag);
@@ -626,14 +612,19 @@ public class XmppConnection implements Runnable {
 			disconnect(true);
 		} else if (this.streamFeatures.hasChild("mechanisms")
 				&& shouldAuthenticate && usingEncryption) {
-			List<String> mechanisms = extractMechanisms(streamFeatures
+			final List<String> mechanisms = extractMechanisms(streamFeatures
 					.findChild("mechanisms"));
-			if (mechanisms.contains("PLAIN")) {
-				sendSaslAuthPlain();
-			} else if (mechanisms.contains("DIGEST-MD5")) {
-				sendSaslAuthDigestMd5();
-			}
-		} else if (this.streamFeatures.hasChild("sm", "urn:xmpp:sm:"
+            final Element auth = new Element("auth");
+            auth.setAttribute("xmlns", "urn:ietf:params:xml:ns:xmpp-sasl");
+            if (mechanisms.contains("DIGEST-MD5")) {
+                saslMechanism = new DigestMd5(tagWriter, account, mXmppConnectionService.getRNG());
+			} else if (mechanisms.contains("PLAIN")) {
+                saslMechanism = new Plain(tagWriter, account);
+                auth.setContent(((Plain)saslMechanism).getStartAuth());
+            }
+            auth.setAttribute("mechanism", saslMechanism.getMechanism());
+            tagWriter.writeElement(auth);
+        } else if (this.streamFeatures.hasChild("sm", "urn:xmpp:sm:"
 				+ smVersion)
 				&& streamId != null) {
 			ResumePacket resume = new ResumePacket(this.streamId,