From e11277c70f84cb2feceb556f221f910bdf2a11e1 Mon Sep 17 00:00:00 2001
From: Daniel Gultsch <daniel@gultsch.de>
Date: Sun, 30 Jul 2017 11:39:47 +0200
Subject: [PATCH] use base64 instead of base36 when creating random strings

---
 .../siacs/conversations/crypto/sasl/SaslMechanism.java   | 4 ++++
 .../siacs/conversations/crypto/sasl/ScramMechanism.java  | 9 +++++++--
 .../java/eu/siacs/conversations/utils/CryptoHelper.java  | 8 ++++++++
 .../java/eu/siacs/conversations/xmpp/XmppConnection.java | 3 ++-
 4 files changed, 21 insertions(+), 3 deletions(-)

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 5b4b99ef..5833309c 100644
--- a/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java
+++ b/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java
@@ -26,6 +26,10 @@ public abstract class SaslMechanism {
 		public AuthenticationException(final Exception inner) {
 			super(inner);
 		}
+
+		public AuthenticationException(final String message, final Exception exception) {
+			super(message,exception);
+		}
 	}
 
 	public static class InvalidStateException extends AuthenticationException {
diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramMechanism.java b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramMechanism.java
index 566835d6..638d341c 100644
--- a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramMechanism.java
+++ b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramMechanism.java
@@ -70,7 +70,7 @@ abstract class ScramMechanism extends SaslMechanism {
 		super(tagWriter, account, rng);
 
 		// This nonce should be different for each authentication attempt.
-		clientNonce = new BigInteger(100, this.rng).toString(32);
+		clientNonce = CryptoHelper.random(100,rng);
 		clientFirstMessageBare = "";
 	}
 
@@ -93,7 +93,12 @@ abstract class ScramMechanism extends SaslMechanism {
 				if (challenge == null) {
 					throw new AuthenticationException("challenge can not be null");
 				}
-				byte[] serverFirstMessage = Base64.decode(challenge, Base64.DEFAULT);
+				byte[] serverFirstMessage;
+				try {
+					serverFirstMessage = Base64.decode(challenge, Base64.DEFAULT);
+				} catch (IllegalArgumentException e) {
+					throw new AuthenticationException("Unable to decode server challenge",e);
+				}
 				final Tokenizer tokenizer = new Tokenizer(serverFirstMessage);
 				String nonce = "";
 				int iterationCount = -1;
diff --git a/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java b/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java
index c91891a7..75a1a87b 100644
--- a/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java
+++ b/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java
@@ -1,6 +1,7 @@
 package eu.siacs.conversations.utils;
 
 import android.os.Bundle;
+import android.util.Base64;
 import android.util.Pair;
 
 import org.bouncycastle.asn1.x500.X500Name;
@@ -12,6 +13,7 @@ import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
@@ -97,6 +99,12 @@ public final class CryptoHelper {
 		return Normalizer.normalize(s, Normalizer.Form.NFKC);
 	}
 
+	public static String random(int length, SecureRandom random) {
+		final byte[] bytes = new byte[length];
+		random.nextBytes(bytes);
+		return Base64.encodeToString(bytes,Base64.NO_PADDING|Base64.NO_WRAP);
+	}
+
 	public static String prettifyFingerprint(String fingerprint) {
 		if (fingerprint==null) {
 			return "";
diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java
index 22334bc5..49bddc61 100644
--- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java
+++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java
@@ -66,6 +66,7 @@ import eu.siacs.conversations.entities.ServiceDiscoveryResult;
 import eu.siacs.conversations.generator.IqGenerator;
 import eu.siacs.conversations.services.NotificationService;
 import eu.siacs.conversations.services.XmppConnectionService;
+import eu.siacs.conversations.utils.CryptoHelper;
 import eu.siacs.conversations.utils.IP;
 import eu.siacs.conversations.utils.Patterns;
 import eu.siacs.conversations.utils.Resolver;
@@ -1322,7 +1323,7 @@ public class XmppConnection implements Runnable {
 	}
 
 	private String nextRandomId() {
-		return new BigInteger(50, mXmppConnectionService.getRNG()).toString(36);
+		return CryptoHelper.random(50,mXmppConnectionService.getRNG());
 	}
 
 	public String sendIqPacket(final IqPacket packet, final OnIqPacketReceived callback) {