From a210568a9ce00cbafc36563b0aa70bf10ef90047 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 6 Sep 2022 09:25:09 +0200 Subject: [PATCH] refactor SASL choice into factory; remove unused TagWriter --- .../crypto/axolotl/AxolotlService.java | 4 +- .../conversations/crypto/sasl/Anonymous.java | 7 +- .../conversations/crypto/sasl/DigestMd5.java | 72 +++++--- .../conversations/crypto/sasl/External.java | 10 +- .../conversations/crypto/sasl/Plain.java | 15 +- .../crypto/sasl/SaslMechanism.java | 102 +++++++---- .../crypto/sasl/ScramMechanism.java | 168 ++++++++++-------- .../conversations/crypto/sasl/ScramSha1.java | 11 +- .../crypto/sasl/ScramSha256.java | 11 +- .../crypto/sasl/ScramSha512.java | 11 +- .../conversations/crypto/sasl/Tokenizer.java | 17 +- .../http/HttpConnectionManager.java | 4 +- .../http/HttpUploadConnection.java | 4 +- .../services/MessageArchiveService.java | 4 +- .../services/XmppConnectionService.java | 11 +- .../ui/CreatePublicChannelDialog.java | 3 +- .../conversations/utils/CryptoHelper.java | 14 +- .../eu/siacs/conversations/utils/Random.java | 13 ++ .../conversations/xmpp/XmppConnection.java | 38 ++-- 19 files changed, 288 insertions(+), 231 deletions(-) create mode 100644 src/main/java/eu/siacs/conversations/utils/Random.java diff --git a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java index faef2e098..3d4f23360 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java +++ b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java @@ -1,5 +1,7 @@ package eu.siacs.conversations.crypto.axolotl; +import static eu.siacs.conversations.utils.Random.SECURE_RANDOM; + import android.os.Bundle; import android.security.KeyChain; import android.util.Log; @@ -499,7 +501,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded { PrivateKey x509PrivateKey = KeyChain.getPrivateKey(mXmppConnectionService, account.getPrivateKeyAlias()); X509Certificate[] chain = KeyChain.getCertificateChain(mXmppConnectionService, account.getPrivateKeyAlias()); Signature verifier = Signature.getInstance("sha256WithRSA"); - verifier.initSign(x509PrivateKey, mXmppConnectionService.getRNG()); + verifier.initSign(x509PrivateKey, SECURE_RANDOM); verifier.update(axolotlPublicKey.serialize()); byte[] signature = verifier.sign(); IqPacket packet = mXmppConnectionService.getIqGenerator().publishVerification(signature, chain, getOwnDeviceId()); diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/Anonymous.java b/src/main/java/eu/siacs/conversations/crypto/sasl/Anonymous.java index a9abb2bf8..22cf80e65 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/Anonymous.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/Anonymous.java @@ -1,16 +1,13 @@ package eu.siacs.conversations.crypto.sasl; -import java.security.SecureRandom; - import eu.siacs.conversations.entities.Account; -import eu.siacs.conversations.xml.TagWriter; public class Anonymous extends SaslMechanism { public static final String MECHANISM = "ANONYMOUS"; - public Anonymous(TagWriter tagWriter, Account account, SecureRandom rng) { - super(tagWriter, account, rng); + public Anonymous(final Account account) { + super(account); } @Override 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 74d4463d5..7229299ef 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/DigestMd5.java @@ -5,18 +5,17 @@ import android.util.Base64; 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 static final String MECHANISM = "DIGEST-MD5"; + private State state = State.INITIAL; - public DigestMd5(final TagWriter tagWriter, final Account account, final SecureRandom rng) { - super(tagWriter, account, rng); + public DigestMd5(final Account account) { + super(account); } @Override @@ -29,8 +28,6 @@ public class DigestMd5 extends SaslMechanism { return MECHANISM; } - private State state = State.INITIAL; - @Override public String getResponse(final String challenge) throws AuthenticationException { switch (state) { @@ -38,7 +35,8 @@ public class DigestMd5 extends SaslMechanism { state = State.RESPONSE_SENT; final String encodedResponse; try { - final Tokenizer tokenizer = new Tokenizer(Base64.decode(challenge, Base64.DEFAULT)); + final Tokenizer tokenizer = + new Tokenizer(Base64.decode(challenge, Base64.DEFAULT)); String nonce = ""; for (final String token : tokenizer) { final String[] parts = token.split("=", 2); @@ -50,29 +48,49 @@ public class DigestMd5 extends SaslMechanism { } final String digestUri = "xmpp/" + account.getServer(); final String nonceCount = "00000001"; - final String x = account.getUsername() + ":" + account.getServer() + ":" - + account.getPassword(); + 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 = CryptoHelper.random(100, rng); - final byte[] a1 = CryptoHelper.concatenateByteArrays(y, - (":" + nonce + ":" + cNonce).getBytes(Charset.defaultCharset())); + final String cNonce = CryptoHelper.random(100); + 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); + 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) { throw new AuthenticationException(e); } @@ -83,7 +101,7 @@ public class DigestMd5 extends SaslMechanism { break; case VALID_SERVER_RESPONSE: if (challenge == null) { - return null; //everything is fine + return null; // everything is fine } default: throw new InvalidStateException(state); diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/External.java b/src/main/java/eu/siacs/conversations/crypto/sasl/External.java index 6e0ed4390..06323f039 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/External.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/External.java @@ -2,17 +2,14 @@ package eu.siacs.conversations.crypto.sasl; import android.util.Base64; -import java.security.SecureRandom; - import eu.siacs.conversations.entities.Account; -import eu.siacs.conversations.xml.TagWriter; public class External extends SaslMechanism { public static final String MECHANISM = "EXTERNAL"; - public External(TagWriter tagWriter, Account account, SecureRandom rng) { - super(tagWriter, account, rng); + public External(final Account account) { + super(account); } @Override @@ -27,6 +24,7 @@ public class External extends SaslMechanism { @Override public String getClientFirstMessage() { - return Base64.encodeToString(account.getJid().asBareJid().toEscapedString().getBytes(), Base64.NO_WRAP); + return Base64.encodeToString( + account.getJid().asBareJid().toEscapedString().getBytes(), Base64.NO_WRAP); } } diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/Plain.java b/src/main/java/eu/siacs/conversations/crypto/sasl/Plain.java index d5cc037e1..875538bec 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/Plain.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/Plain.java @@ -5,14 +5,18 @@ 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 static final String MECHANISM = "PLAIN"; - public Plain(final TagWriter tagWriter, final Account account) { - super(tagWriter, account, null); + public Plain(final Account account) { + super(account); + } + + public static String getMessage(String username, String password) { + final String message = '\u0000' + username + '\u0000' + password; + return Base64.encodeToString(message.getBytes(Charset.defaultCharset()), Base64.NO_WRAP); } @Override @@ -29,9 +33,4 @@ public class Plain extends SaslMechanism { public String getClientFirstMessage() { return getMessage(account.getUsername(), account.getPassword()); } - - public static String getMessage(String username, String password) { - final String message = '\u0000' + username + '\u0000' + password; - return Base64.encodeToString(message.getBytes(Charset.defaultCharset()), Base64.NO_WRAP); - } } 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 b255b6f42..ce2d5cd6a 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/SaslMechanism.java @@ -2,18 +2,38 @@ package eu.siacs.conversations.crypto.sasl; import com.google.common.base.Strings; -import java.security.SecureRandom; +import java.util.Collection; import eu.siacs.conversations.entities.Account; import eu.siacs.conversations.xml.Element; import eu.siacs.conversations.xml.Namespace; -import eu.siacs.conversations.xml.TagWriter; public abstract class SaslMechanism { - final protected TagWriter tagWriter; - final protected Account account; - final protected SecureRandom rng; + protected final Account account; + + protected SaslMechanism(final Account account) { + this.account = account; + } + + /** + * The priority is used to pin the authentication mechanism. If authentication fails, it MAY be + * retried with another mechanism of the same priority, but MUST NOT be tried with a mechanism + * of lower priority (to prevent downgrade attacks). + * + * @return An arbitrary int representing the priority + */ + public abstract int getPriority(); + + public abstract String getMechanism(); + + public String getClientFirstMessage() { + return ""; + } + + public String getResponse(final String challenge) throws AuthenticationException { + return ""; + } protected enum State { INITIAL, @@ -22,6 +42,22 @@ public abstract class SaslMechanism { VALID_SERVER_RESPONSE, } + public enum Version { + SASL, + SASL_2; + + public static Version of(final Element element) { + switch (Strings.nullToEmpty(element.getNamespace())) { + case Namespace.SASL: + return SASL; + case Namespace.SASL_2: + return SASL_2; + default: + throw new IllegalArgumentException("Unrecognized SASL namespace"); + } + } + } + public static class AuthenticationException extends Exception { public AuthenticationException(final String message) { super(message); @@ -46,42 +82,32 @@ public abstract class SaslMechanism { } } - public SaslMechanism(final TagWriter tagWriter, final Account account, final SecureRandom rng) { - this.tagWriter = tagWriter; - this.account = account; - this.rng = rng; - } + public static final class Factory { - /** - * The priority is used to pin the authentication mechanism. If authentication fails, it MAY be retried with another - * mechanism of the same priority, but MUST NOT be tried with a mechanism of lower priority (to prevent downgrade - * attacks). - * - * @return An arbitrary int representing the priority - */ - public abstract int getPriority(); + private final Account account; - public abstract String getMechanism(); + public Factory(final Account account) { + this.account = account; + } - public String getClientFirstMessage() { - return ""; - } - - public String getResponse(final String challenge) throws AuthenticationException { - return ""; - } - - public enum Version { - SASL, SASL_2; - - public static Version of(final Element element) { - switch ( Strings.nullToEmpty(element.getNamespace())) { - case Namespace.SASL: - return SASL; - case Namespace.SASL_2: - return SASL_2; - default: - throw new IllegalArgumentException("Unrecognized SASL namespace"); + public SaslMechanism of(final Collection mechanisms) { + if (mechanisms.contains(External.MECHANISM) && account.getPrivateKeyAlias() != null) { + return new External(account); + } else if (mechanisms.contains(ScramSha512.MECHANISM)) { + return new ScramSha512(account); + } else if (mechanisms.contains(ScramSha256.MECHANISM)) { + return new ScramSha256(account); + } else if (mechanisms.contains(ScramSha1.MECHANISM)) { + return new ScramSha1(account); + } else if (mechanisms.contains(Plain.MECHANISM) + && !account.getServer().equals("nimbuzz.com")) { + return new Plain(account); + } else if (mechanisms.contains(DigestMd5.MECHANISM)) { + return new DigestMd5(account); + } else if (mechanisms.contains(Anonymous.MECHANISM)) { + return new Anonymous(account); + } else { + return null; } } } 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 807056bf8..0fe7434a8 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramMechanism.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramMechanism.java @@ -12,78 +12,53 @@ import org.bouncycastle.crypto.params.KeyParameter; import java.nio.charset.Charset; import java.security.InvalidKeyException; -import java.security.SecureRandom; import java.util.concurrent.ExecutionException; import eu.siacs.conversations.entities.Account; import eu.siacs.conversations.utils.CryptoHelper; -import eu.siacs.conversations.xml.TagWriter; abstract class ScramMechanism extends SaslMechanism { - // TODO: When channel binding (SCRAM-SHA1-PLUS) is supported in future, generalize this to indicate support and/or usage. - private final static String GS2_HEADER = "n,,"; + // TODO: When channel binding (SCRAM-SHA1-PLUS) is supported in future, generalize this to + // indicate support and/or usage. + private static final String GS2_HEADER = "n,,"; private static final byte[] CLIENT_KEY_BYTES = "Client Key".getBytes(); private static final byte[] SERVER_KEY_BYTES = "Server Key".getBytes(); - - protected abstract HMac getHMAC(); - - protected abstract Digest getDigest(); - - private static final Cache CACHE = CacheBuilder.newBuilder().maximumSize(10).build(); - - private static class CacheKey { - final String algorithm; - final String password; - final String salt; - final int iterations; - - private CacheKey(String algorithm, String password, String salt, int iterations) { - this.algorithm = algorithm; - this.password = password; - this.salt = salt; - this.iterations = iterations; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CacheKey cacheKey = (CacheKey) o; - return iterations == cacheKey.iterations && - Objects.equal(algorithm, cacheKey.algorithm) && - Objects.equal(password, cacheKey.password) && - Objects.equal(salt, cacheKey.salt); - } - - @Override - public int hashCode() { - return Objects.hashCode(algorithm, password, salt, iterations); - } - } - - private KeyPair getKeyPair(final String password, final String salt, final int iterations) throws ExecutionException { - return CACHE.get(new CacheKey(getHMAC().getAlgorithmName(), password, salt, iterations), () -> { - final byte[] saltedPassword, serverKey, clientKey; - saltedPassword = hi(password.getBytes(), Base64.decode(salt, Base64.DEFAULT), iterations); - serverKey = hmac(saltedPassword, SERVER_KEY_BYTES); - clientKey = hmac(saltedPassword, CLIENT_KEY_BYTES); - return new KeyPair(clientKey, serverKey); - }); - } - + private static final Cache CACHE = + CacheBuilder.newBuilder().maximumSize(10).build(); private final String clientNonce; protected State state = State.INITIAL; private String clientFirstMessageBare; private byte[] serverSignature = null; - ScramMechanism(final TagWriter tagWriter, final Account account, final SecureRandom rng) { - super(tagWriter, account, rng); + ScramMechanism(final Account account) { + super(account); // This nonce should be different for each authentication attempt. - clientNonce = CryptoHelper.random(100, rng); + this.clientNonce = CryptoHelper.random(100); clientFirstMessageBare = ""; } + protected abstract HMac getHMAC(); + + protected abstract Digest getDigest(); + + private KeyPair getKeyPair(final String password, final String salt, final int iterations) + throws ExecutionException { + return CACHE.get( + new CacheKey(getHMAC().getAlgorithmName(), password, salt, iterations), + () -> { + final byte[] saltedPassword, serverKey, clientKey; + saltedPassword = + hi( + password.getBytes(), + Base64.decode(salt, Base64.DEFAULT), + iterations); + serverKey = hmac(saltedPassword, SERVER_KEY_BYTES); + clientKey = hmac(saltedPassword, CLIENT_KEY_BYTES); + return new KeyPair(clientKey, serverKey); + }); + } + private byte[] hmac(final byte[] key, final byte[] input) throws InvalidKeyException { final HMac hMac = getHMAC(); hMac.init(new KeyParameter(key)); @@ -123,8 +98,11 @@ abstract class ScramMechanism extends SaslMechanism { @Override public String getClientFirstMessage() { if (clientFirstMessageBare.isEmpty() && state == State.INITIAL) { - clientFirstMessageBare = "n=" + CryptoHelper.saslEscape(CryptoHelper.saslPrep(account.getUsername())) + - ",r=" + this.clientNonce; + clientFirstMessageBare = + "n=" + + CryptoHelper.saslEscape(CryptoHelper.saslPrep(account.getUsername())) + + ",r=" + + this.clientNonce; state = State.AUTH_TEXT_SENT; } return Base64.encodeToString( @@ -173,7 +151,8 @@ abstract class ScramMechanism extends SaslMechanism { * MUST cause authentication failure when the attribute is parsed by * the other end. */ - throw new AuthenticationException("Server sent reserved token: `m'"); + throw new AuthenticationException( + "Server sent reserved token: `m'"); } } } @@ -182,20 +161,33 @@ abstract class ScramMechanism extends SaslMechanism { throw new AuthenticationException("Server did not send iteration count"); } if (nonce.isEmpty() || !nonce.startsWith(clientNonce)) { - throw new AuthenticationException("Server nonce does not contain client nonce: " + nonce); + throw new AuthenticationException( + "Server nonce does not contain client nonce: " + nonce); } if (salt.isEmpty()) { throw new AuthenticationException("Server sent empty salt"); } - final String clientFinalMessageWithoutProof = "c=" + Base64.encodeToString( - GS2_HEADER.getBytes(), Base64.NO_WRAP) + ",r=" + nonce; - final byte[] authMessage = (clientFirstMessageBare + ',' + new String(serverFirstMessage) + ',' - + clientFinalMessageWithoutProof).getBytes(); + final String clientFinalMessageWithoutProof = + "c=" + + Base64.encodeToString(GS2_HEADER.getBytes(), Base64.NO_WRAP) + + ",r=" + + nonce; + final byte[] authMessage = + (clientFirstMessageBare + + ',' + + new String(serverFirstMessage) + + ',' + + clientFinalMessageWithoutProof) + .getBytes(); final KeyPair keys; try { - keys = getKeyPair(CryptoHelper.saslPrep(account.getPassword()), salt, iterationCount); + keys = + getKeyPair( + CryptoHelper.saslPrep(account.getPassword()), + salt, + iterationCount); } catch (ExecutionException e) { throw new AuthenticationException("Invalid keys generated"); } @@ -213,35 +205,69 @@ abstract class ScramMechanism extends SaslMechanism { final byte[] clientProof = new byte[keys.clientKey.length]; if (clientSignature.length < keys.clientKey.length) { - throw new AuthenticationException("client signature was shorter than clientKey"); + throw new AuthenticationException( + "client signature was shorter than clientKey"); } for (int i = 0; i < clientProof.length; i++) { clientProof[i] = (byte) (keys.clientKey[i] ^ clientSignature[i]); } - - final String clientFinalMessage = clientFinalMessageWithoutProof + ",p=" + - Base64.encodeToString(clientProof, Base64.NO_WRAP); + final String clientFinalMessage = + clientFinalMessageWithoutProof + + ",p=" + + Base64.encodeToString(clientProof, Base64.NO_WRAP); state = State.RESPONSE_SENT; return Base64.encodeToString(clientFinalMessage.getBytes(), Base64.NO_WRAP); case RESPONSE_SENT: try { - final String clientCalculatedServerFinalMessage = "v=" + - Base64.encodeToString(serverSignature, Base64.NO_WRAP); - if (!clientCalculatedServerFinalMessage.equals(new String(Base64.decode(challenge, Base64.DEFAULT)))) { + final String clientCalculatedServerFinalMessage = + "v=" + Base64.encodeToString(serverSignature, Base64.NO_WRAP); + if (!clientCalculatedServerFinalMessage.equals( + new String(Base64.decode(challenge, Base64.DEFAULT)))) { throw new Exception(); } state = State.VALID_SERVER_RESPONSE; return ""; } catch (Exception e) { - throw new AuthenticationException("Server final message does not match calculated final message"); + throw new AuthenticationException( + "Server final message does not match calculated final message"); } default: throw new InvalidStateException(state); } } + private static class CacheKey { + final String algorithm; + final String password; + final String salt; + final int iterations; + + private CacheKey(String algorithm, String password, String salt, int iterations) { + this.algorithm = algorithm; + this.password = password; + this.salt = salt; + this.iterations = iterations; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CacheKey cacheKey = (CacheKey) o; + return iterations == cacheKey.iterations + && Objects.equal(algorithm, cacheKey.algorithm) + && Objects.equal(password, cacheKey.password) + && Objects.equal(salt, cacheKey.salt); + } + + @Override + public int hashCode() { + return Objects.hashCode(algorithm, password, salt, iterations); + } + } + private static class KeyPair { final byte[] clientKey; final byte[] serverKey; 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 c58dd147c..472c4dea1 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha1.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha1.java @@ -4,15 +4,16 @@ import org.bouncycastle.crypto.Digest; import org.bouncycastle.crypto.digests.SHA1Digest; import org.bouncycastle.crypto.macs.HMac; -import java.security.SecureRandom; - import eu.siacs.conversations.entities.Account; -import eu.siacs.conversations.xml.TagWriter; public class ScramSha1 extends ScramMechanism { public static final String MECHANISM = "SCRAM-SHA-1"; + public ScramSha1(final Account account) { + super(account); + } + @Override protected HMac getHMAC() { return new HMac(new SHA1Digest()); @@ -23,10 +24,6 @@ public class ScramSha1 extends ScramMechanism { return new SHA1Digest(); } - public ScramSha1(final TagWriter tagWriter, final Account account, final SecureRandom rng) { - super(tagWriter, account, rng); - } - @Override public int getPriority() { return 20; diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha256.java b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha256.java index d5dc42b07..f3f6cab57 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha256.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha256.java @@ -4,15 +4,16 @@ import org.bouncycastle.crypto.Digest; import org.bouncycastle.crypto.digests.SHA256Digest; import org.bouncycastle.crypto.macs.HMac; -import java.security.SecureRandom; - import eu.siacs.conversations.entities.Account; -import eu.siacs.conversations.xml.TagWriter; public class ScramSha256 extends ScramMechanism { public static final String MECHANISM = "SCRAM-SHA-256"; + public ScramSha256(final Account account) { + super(account); + } + @Override protected HMac getHMAC() { return new HMac(new SHA256Digest()); @@ -23,10 +24,6 @@ public class ScramSha256 extends ScramMechanism { return new SHA256Digest(); } - public ScramSha256(final TagWriter tagWriter, final Account account, final SecureRandom rng) { - super(tagWriter, account, rng); - } - @Override public int getPriority() { return 25; diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha512.java b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha512.java index dbd30945c..9a2f1a82d 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha512.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/ScramSha512.java @@ -4,15 +4,16 @@ import org.bouncycastle.crypto.Digest; import org.bouncycastle.crypto.digests.SHA512Digest; import org.bouncycastle.crypto.macs.HMac; -import java.security.SecureRandom; - import eu.siacs.conversations.entities.Account; -import eu.siacs.conversations.xml.TagWriter; public class ScramSha512 extends ScramMechanism { public static final String MECHANISM = "SCRAM-SHA-512"; + public ScramSha512(final Account account) { + super(account); + } + @Override protected HMac getHMAC() { return new HMac(new SHA512Digest()); @@ -23,10 +24,6 @@ public class ScramSha512 extends ScramMechanism { return new SHA512Digest(); } - public ScramSha512(final TagWriter tagWriter, final Account account, final SecureRandom rng) { - super(tagWriter, account, rng); - } - @Override public int getPriority() { return 30; diff --git a/src/main/java/eu/siacs/conversations/crypto/sasl/Tokenizer.java b/src/main/java/eu/siacs/conversations/crypto/sasl/Tokenizer.java index f9ba24f09..3038fb060 100644 --- a/src/main/java/eu/siacs/conversations/crypto/sasl/Tokenizer.java +++ b/src/main/java/eu/siacs/conversations/crypto/sasl/Tokenizer.java @@ -6,9 +6,7 @@ import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; -/** - * A tokenizer for GS2 header strings - */ +/** A tokenizer for GS2 header strings */ public final class Tokenizer implements Iterator, Iterable { private final List parts; private int index; @@ -50,18 +48,19 @@ public final class Tokenizer implements Iterator, Iterable { } /** - * Removes the last object returned by {@code next} from the collection. - * This method can only be called once between each call to {@code next}. + * Removes the last object returned by {@code next} from the collection. This method can only be + * called once between each call to {@code next}. * * @throws UnsupportedOperationException if removing is not supported by the collection being - * iterated. - * @throws IllegalStateException if {@code next} has not been called, or {@code remove} has - * already been called after the last call to {@code next}. + * iterated. + * @throws IllegalStateException if {@code next} has not been called, or {@code remove} has + * already been called after the last call to {@code next}. */ @Override public void remove() { if (index <= 0) { - throw new IllegalStateException("You can't delete an element before first next() method call"); + throw new IllegalStateException( + "You can't delete an element before first next() method call"); } parts.remove(--index); } diff --git a/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java b/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java index 566ce1e6a..27f5c3fc7 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java +++ b/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java @@ -1,5 +1,7 @@ package eu.siacs.conversations.http; +import static eu.siacs.conversations.utils.Random.SECURE_RANDOM; + import android.os.Build; import android.util.Log; @@ -147,7 +149,7 @@ public class HttpConnectionManager extends AbstractConnectionManager { trustManager = mXmppConnectionService.getMemorizingTrustManager().getNonInteractive(); } try { - final SSLSocketFactory sf = new TLSSocketFactory(new X509TrustManager[]{trustManager}, mXmppConnectionService.getRNG()); + final SSLSocketFactory sf = new TLSSocketFactory(new X509TrustManager[]{trustManager}, SECURE_RANDOM); builder.sslSocketFactory(sf, trustManager); builder.hostnameVerifier(new StrictHostnameVerifier()); } catch (final KeyManagementException | NoSuchAlgorithmException ignored) { diff --git a/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java b/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java index 3e478dd0f..e2366dd48 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java +++ b/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java @@ -1,5 +1,7 @@ package eu.siacs.conversations.http; +import static eu.siacs.conversations.utils.Random.SECURE_RANDOM; + import android.util.Log; import androidx.annotation.NonNull; @@ -124,7 +126,7 @@ public class HttpUploadConnection implements Transferable, AbstractConnectionMan || message.getEncryption() == Message.ENCRYPTION_AXOLOTL || message.getEncryption() == Message.ENCRYPTION_OTR) { this.key = new byte[44]; - mXmppConnectionService.getRNG().nextBytes(this.key); + SECURE_RANDOM.nextBytes(this.key); this.file.setKeyAndIv(this.key); } this.file.setExpectedSize(originalFileSize + (file.getKey() != null ? 16 : 0)); diff --git a/src/main/java/eu/siacs/conversations/services/MessageArchiveService.java b/src/main/java/eu/siacs/conversations/services/MessageArchiveService.java index 79cad9520..e74af3773 100644 --- a/src/main/java/eu/siacs/conversations/services/MessageArchiveService.java +++ b/src/main/java/eu/siacs/conversations/services/MessageArchiveService.java @@ -1,5 +1,7 @@ package eu.siacs.conversations.services; +import static eu.siacs.conversations.utils.Random.SECURE_RANDOM; + import android.util.Log; import org.jetbrains.annotations.NotNull; @@ -502,7 +504,7 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded { this.start = start.getTimestamp(); } this.end = end; - this.queryId = new BigInteger(50, mXmppConnectionService.getRNG()).toString(32); + this.queryId = new BigInteger(50, SECURE_RANDOM).toString(32); this.version = version; } diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index ba2c8514e..586b717ff 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java @@ -1,6 +1,7 @@ package eu.siacs.conversations.services; import static eu.siacs.conversations.utils.Compatibility.s; +import static eu.siacs.conversations.utils.Random.SECURE_RANDOM; import android.Manifest; import android.annotation.SuppressLint; @@ -379,7 +380,6 @@ public class XmppConnectionService extends Service { } }; private final AtomicLong mLastExpiryRun = new AtomicLong(0); - private SecureRandom mRandom; private final LruCache, ServiceDiscoveryResult> discoCache = new LruCache<>(20); private final OnStatusChanged statusListener = new OnStatusChanged() { @@ -451,7 +451,7 @@ public class XmppConnectionService extends Service { Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": went into offline state during low ping mode. reconnecting now"); reconnectAccount(account, true, false); } else { - int timeToReconnect = mRandom.nextInt(10) + 2; + final int timeToReconnect = SECURE_RANDOM.nextInt(10) + 2; scheduleWakeUpCall(timeToReconnect, account.getUuid().hashCode()); } } else if (account.getStatus() == Account.State.REGISTRATION_SUCCESSFUL) { @@ -1143,7 +1143,6 @@ public class XmppConnectionService extends Service { Log.e(Config.LOGTAG, "unable to initialize security provider", throwable); } Resolver.init(this); - this.mRandom = new SecureRandom(); updateMemorizingTrustmanager(); final int maxMemory = (int) (Runtime.getRuntime().maxMemory() / 1024); final int cacheSize = maxMemory / 8; @@ -3269,7 +3268,7 @@ public class XmppConnectionService extends Service { } return false; } - final Jid jid = Jid.of(CryptoHelper.pronounceable(getRNG()), server, null); + final Jid jid = Jid.of(CryptoHelper.pronounceable(), server, null); final Conversation conversation = findOrCreateConversation(account, jid, true, false, true); joinMuc(conversation, new OnConferenceJoined() { @Override @@ -4366,10 +4365,6 @@ public class XmppConnectionService extends Service { } } - public SecureRandom getRNG() { - return this.mRandom; - } - public MemorizingTrustManager getMemorizingTrustManager() { return this.mMemorizingTrustManager; } diff --git a/src/main/java/eu/siacs/conversations/ui/CreatePublicChannelDialog.java b/src/main/java/eu/siacs/conversations/ui/CreatePublicChannelDialog.java index 3c23b06eb..8f5e2e6d2 100644 --- a/src/main/java/eu/siacs/conversations/ui/CreatePublicChannelDialog.java +++ b/src/main/java/eu/siacs/conversations/ui/CreatePublicChannelDialog.java @@ -43,7 +43,6 @@ public class CreatePublicChannelDialog extends DialogFragment implements OnBacke private boolean jidWasModified = false; private boolean nameEntered = false; private boolean skipTetxWatcher = false; - private static final SecureRandom RANDOM = new SecureRandom(); public static CreatePublicChannelDialog newInstance(List accounts) { CreatePublicChannelDialog dialog = new CreatePublicChannelDialog(); @@ -158,7 +157,7 @@ public class CreatePublicChannelDialog extends DialogFragment implements OnBacke try { return Jid.of(localpart, domain, null).toEscapedString(); } catch (IllegalArgumentException e) { - return Jid.of(CryptoHelper.pronounceable(RANDOM), domain, null).toEscapedString(); + return Jid.of(CryptoHelper.pronounceable(), domain, null).toEscapedString(); } } } diff --git a/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java b/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java index a92d48825..85ea2bb86 100644 --- a/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java +++ b/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java @@ -1,5 +1,7 @@ package eu.siacs.conversations.utils; +import static eu.siacs.conversations.utils.Random.SECURE_RANDOM; + import android.os.Bundle; import android.util.Base64; import android.util.Pair; @@ -59,12 +61,12 @@ public final class CryptoHelper { return builder.toString(); } - public static String pronounceable(SecureRandom random) { - final int rand = random.nextInt(4); + public static String pronounceable() { + final int rand = SECURE_RANDOM.nextInt(4); char[] output = new char[rand * 2 + (5 - rand)]; - boolean vowel = random.nextBoolean(); + boolean vowel = SECURE_RANDOM.nextBoolean(); for (int i = 0; i < output.length; ++i) { - output[i] = vowel ? VOWELS[random.nextInt(VOWELS.length)] : CONSONANTS[random.nextInt(CONSONANTS.length)]; + output[i] = vowel ? VOWELS[SECURE_RANDOM.nextInt(VOWELS.length)] : CONSONANTS[SECURE_RANDOM.nextInt(CONSONANTS.length)]; vowel = !vowel; } return String.valueOf(output); @@ -117,9 +119,9 @@ public final class CryptoHelper { return Normalizer.normalize(s, Normalizer.Form.NFKC); } - public static String random(int length, SecureRandom random) { + public static String random(final int length) { final byte[] bytes = new byte[length]; - random.nextBytes(bytes); + SECURE_RANDOM.nextBytes(bytes); return Base64.encodeToString(bytes, Base64.NO_PADDING | Base64.NO_WRAP | Base64.URL_SAFE); } diff --git a/src/main/java/eu/siacs/conversations/utils/Random.java b/src/main/java/eu/siacs/conversations/utils/Random.java new file mode 100644 index 000000000..792c1fce1 --- /dev/null +++ b/src/main/java/eu/siacs/conversations/utils/Random.java @@ -0,0 +1,13 @@ +package eu.siacs.conversations.utils; + +import java.security.SecureRandom; + +public final class Random { + + public static final SecureRandom SECURE_RANDOM = new SecureRandom(); + + private Random() { + + } + +} diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index 70bc347b3..a1336e22c 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -1,5 +1,7 @@ package eu.siacs.conversations.xmpp; +import static eu.siacs.conversations.utils.Random.SECURE_RANDOM; + import android.content.Context; import android.graphics.Bitmap; import android.graphics.BitmapFactory; @@ -521,7 +523,7 @@ public class XmppConnection implements Runnable { ? trustManager.getInteractive(domain) : trustManager.getNonInteractive(domain) }, - mXmppConnectionService.getRNG()); + SECURE_RANDOM); return sc.getSocketFactory(); } @@ -1216,23 +1218,11 @@ public class XmppConnection implements Runnable { } private void authenticate(final SaslMechanism.Version version) throws IOException { - final List mechanisms = extractMechanisms(streamFeatures.findChild("mechanisms")); - if (mechanisms.contains(External.MECHANISM) && account.getPrivateKeyAlias() != null) { - saslMechanism = new External(tagWriter, account, mXmppConnectionService.getRNG()); - } else if (mechanisms.contains(ScramSha512.MECHANISM)) { - saslMechanism = new ScramSha512(tagWriter, account, mXmppConnectionService.getRNG()); - } else if (mechanisms.contains(ScramSha256.MECHANISM)) { - saslMechanism = new ScramSha256(tagWriter, account, mXmppConnectionService.getRNG()); - } else if (mechanisms.contains(ScramSha1.MECHANISM)) { - saslMechanism = new ScramSha1(tagWriter, account, mXmppConnectionService.getRNG()); - } else if (mechanisms.contains(Plain.MECHANISM) - && !account.getJid().getDomain().toEscapedString().equals("nimbuzz.com")) { - saslMechanism = new Plain(tagWriter, account); - } else if (mechanisms.contains(DigestMd5.MECHANISM)) { - saslMechanism = new DigestMd5(tagWriter, account, mXmppConnectionService.getRNG()); - } else if (mechanisms.contains(Anonymous.MECHANISM)) { - saslMechanism = new Anonymous(tagWriter, account, mXmppConnectionService.getRNG()); - } + final Element element = streamFeatures.findChild("mechanisms"); + final Collection mechanisms = Collections2.transform(element.getChildren(), c -> c == null ? null : c.getContent()); + final SaslMechanism.Factory factory = new SaslMechanism.Factory(account); + this.saslMechanism = factory.of(mechanisms); + if (saslMechanism == null) { Log.d( Config.LOGTAG, @@ -1317,12 +1307,8 @@ public class XmppConnection implements Runnable { return bind; } - private static List extractMechanisms(final Element stream) { - final ArrayList mechanisms = new ArrayList<>(stream.getChildren().size()); - for (final Element child : stream.getChildren()) { - mechanisms.add(child.getContent()); - } - return mechanisms; + private static Collection extractMechanisms(final Element stream) { + return Collections2.transform(stream.getChildren(), c -> c == null ? null : c.getContent()); } private void register() { @@ -1963,8 +1949,8 @@ public class XmppConnection implements Runnable { return nextRandomId(false); } - private String nextRandomId(boolean s) { - return CryptoHelper.random(s ? 3 : 9, mXmppConnectionService.getRNG()); + private String nextRandomId(final boolean s) { + return CryptoHelper.random(s ? 3 : 9); } public String sendIqPacket(final IqPacket packet, final OnIqPacketReceived callback) {