From 619b8e6f4ab6e2f2b3b94525a68680f65068dfff Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 2 Jun 2019 19:56:56 +0200 Subject: [PATCH] Add secure(OnlineAttackSafe|Unique|OfflineAttackSafe)RandomString() and replace usages of java.util.UUID in Smack with secureUniqueRandomString() because it uses a thread-local secure random number generator. --- .../jivesoftware/smack/util/StringUtils.java | 92 +++++++++++++++---- .../jivesoftware/smackx/mam/MamManager.java | 10 +- .../sid/element/StableAndUniqueIdElement.java | 4 +- .../smackx/sid/StableUniqueStanzaIdTest.java | 2 +- .../ibb/InBandBytestreamManager.java | 7 +- .../socks5/Socks5BytestreamManager.java | 7 +- .../jivesoftware/smack/chat/ChatManager.java | 4 +- .../smackx/ox/util/SecretKeyBackupHelper.java | 23 +---- 8 files changed, 89 insertions(+), 60 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/StringUtils.java b/smack-core/src/main/java/org/jivesoftware/smack/util/StringUtils.java index 3e172f539..0798c1ab0 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/StringUtils.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/StringUtils.java @@ -17,6 +17,8 @@ package org.jivesoftware.smack.util; +import java.io.IOException; +import java.nio.CharBuffer; import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.Iterator; @@ -264,13 +266,9 @@ public class StringUtils { } /** - * Array of numbers and letters of mixed case. Numbers appear in the list - * twice so that there is a more equal chance that a number will be picked. - * We can use the array to get a random number or letter by picking a random - * array index. + * 24 upper case characters from the latin alphabet and numbers without '0' and 'O'. */ - private static final char[] numbersAndLetters = ("0123456789abcdefghijklmnopqrstuvwxyz" + - "ABCDEFGHIJKLMNOPQRSTUVWXYZ").toCharArray(); + private static final char[] UNAMBIGUOUS_NUMBERS_AND_LETTER = "123456789ABCDEFGHIJKLMNPQRSTUVWXYZ".toCharArray(); /** * Returns a random String of numbers and letters (lower and upper case) @@ -289,6 +287,74 @@ public class StringUtils { return randomString(length, RandomUtil.RANDOM.get()); } + public static String secureOnlineAttackSafeRandomString() { + // 34^10 = 2.06e15 possible combinations. Which is enough to protect against online brute force attacks. + // See also https://www.grc.com/haystack.htm + final int REQUIRED_LENGTH = 10; + + return randomString(RandomUtil.SECURE_RANDOM.get(), UNAMBIGUOUS_NUMBERS_AND_LETTER, REQUIRED_LENGTH); + } + + public static String secureUniqueRandomString() { + // 34^13 = 8.11e19 possible combinations, which is > 2^64. + final int REQUIRED_LENGTH = 13; + + return randomString(RandomUtil.SECURE_RANDOM.get(), UNAMBIGUOUS_NUMBERS_AND_LETTER, REQUIRED_LENGTH); + } + + /** + * Generate a secure random string with is human readable. The resulting string consists of 24 upper case characters + * from the Latin alphabet and numbers without '0' and 'O', grouped into 4-characters chunks, e.g. + * "TWNK-KD5Y-MT3T-E1GS-DRDB-KVTW". The characters are randomly selected by a cryptographically secure pseudorandom + * number generator (CSPRNG). + *

+ * The string can be used a backup "code" for secrets, and is in fact the same as the one backup code specified in + * XEP-0373 and the one used by the Backup + * Format v2 of OpenKeychain. + *

+ * + * @see XEP-0373 §5.4 Encrypting the Secret + * Key Backup + * @return a human readable secure random string. + */ + public static String secureOfflineAttackSafeRandomString() { + // 34^24 = 2^122.10 possible combinations. Which is enough to protect against offline brute force attacks. + // See also https://www.grc.com/haystack.htm + final int REQUIRED_LENGTH = 24; + + return randomString(RandomUtil.SECURE_RANDOM.get(), UNAMBIGUOUS_NUMBERS_AND_LETTER, REQUIRED_LENGTH); + } + + private static final int RANDOM_STRING_CHUNK_SIZE = 4; + + private static String randomString(Random random, char[] alphabet, int numRandomChars) { + // The buffer most hold the size of the requested number of random chars and the chunk separators ('-'). + int bufferSize = numRandomChars + ((numRandomChars - 1) / RANDOM_STRING_CHUNK_SIZE); + CharBuffer charBuffer = CharBuffer.allocate(bufferSize); + + try { + randomString(charBuffer, random, alphabet, numRandomChars); + } catch (IOException e) { + // This should never happen if we calcuate the buffer size correctly. + throw new AssertionError(e); + } + + return charBuffer.flip().toString(); + } + + private static void randomString(Appendable appendable, Random random, char[] alphabet, int numRandomChars) + throws IOException { + for (int randomCharNum = 1; randomCharNum <= numRandomChars; randomCharNum++) { + int randomIndex = random.nextInt(alphabet.length); + char randomChar = alphabet[randomIndex]; + appendable.append(randomChar); + + if (randomCharNum % RANDOM_STRING_CHUNK_SIZE == 0 && randomCharNum < numRandomChars) { + appendable.append('-'); + } + } + } + public static String randomString(final int length) { return randomString(length, RandomUtil.SECURE_RANDOM.get()); } @@ -298,24 +364,14 @@ public class StringUtils { return ""; } - byte[] randomBytes = new byte[length]; - random.nextBytes(randomBytes); char[] randomChars = new char[length]; for (int i = 0; i < length; i++) { - randomChars[i] = getPrintableChar(randomBytes[i]); + int index = random.nextInt(UNAMBIGUOUS_NUMBERS_AND_LETTER.length); + randomChars[i] = UNAMBIGUOUS_NUMBERS_AND_LETTER[index]; } return new String(randomChars); } - private static char getPrintableChar(byte indexByte) { - assert (numbersAndLetters.length < Byte.MAX_VALUE * 2); - - // Convert indexByte as it where an unsigned byte by promoting it to int - // and masking it with 0xff. Yields results from 0 - 254. - int index = indexByte & 0xff; - return numbersAndLetters[index % numbersAndLetters.length]; - } - /** * Returns true if CharSequence is not null and is not empty, false otherwise. * Examples: diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java index 0e58c3574..3aa0c614f 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/mam/MamManager.java @@ -1,6 +1,6 @@ /** * - * Copyright © 2017-2018 Florian Schmaus, 2016-2017 Fernando Ramirez + * Copyright © 2017-2019 Florian Schmaus, 2016-2017 Fernando Ramirez * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,7 +23,6 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.UUID; import java.util.WeakHashMap; import org.jivesoftware.smack.ConnectionCreationListener; @@ -456,7 +455,7 @@ public final class MamManager extends Manager { public MamQuery queryArchive(MamQueryArgs mamQueryArgs) throws NoResponseException, XMPPErrorException, NotConnectedException, NotLoggedInException, InterruptedException { - String queryId = UUID.randomUUID().toString(); + String queryId = StringUtils.secureUniqueRandomString(); String node = mamQueryArgs.node; DataForm dataForm = mamQueryArgs.getDataForm(); @@ -515,7 +514,7 @@ public final class MamManager extends Manager { public List retrieveFormFields(String node) throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException, NotLoggedInException { - String queryId = UUID.randomUUID().toString(); + String queryId = StringUtils.secureUniqueRandomString(); MamQueryIQ mamQueryIq = new MamQueryIQ(queryId, node, null); mamQueryIq.setTo(archiveAddress); @@ -601,7 +600,8 @@ public final class MamManager extends Manager { private List page(RSMSet requestRsmSet) throws NoResponseException, XMPPErrorException, NotConnectedException, NotLoggedInException, InterruptedException { - MamQueryIQ mamQueryIQ = new MamQueryIQ(UUID.randomUUID().toString(), node, form); + String queryId = StringUtils.secureUniqueRandomString(); + MamQueryIQ mamQueryIQ = new MamQueryIQ(queryId, node, form); mamQueryIQ.setType(IQ.Type.set); mamQueryIQ.setTo(archiveAddress); mamQueryIQ.addExtension(requestRsmSet); diff --git a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/element/StableAndUniqueIdElement.java b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/element/StableAndUniqueIdElement.java index 7c87f4592..3b681a259 100644 --- a/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/element/StableAndUniqueIdElement.java +++ b/smack-experimental/src/main/java/org/jivesoftware/smackx/sid/element/StableAndUniqueIdElement.java @@ -16,8 +16,6 @@ */ package org.jivesoftware.smackx.sid.element; -import java.util.UUID; - import org.jivesoftware.smack.packet.ExtensionElement; import org.jivesoftware.smack.util.StringUtils; @@ -28,7 +26,7 @@ public abstract class StableAndUniqueIdElement implements ExtensionElement { private final String id; public StableAndUniqueIdElement() { - this.id = UUID.randomUUID().toString(); + this.id = StringUtils.secureUniqueRandomString(); } public String getId() { diff --git a/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java b/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java index edafc0b9a..a344b2bd6 100644 --- a/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java +++ b/smack-experimental/src/test/java/org/jivesoftware/smackx/sid/StableUniqueStanzaIdTest.java @@ -65,7 +65,7 @@ public class StableUniqueStanzaIdTest extends SmackTestSuite { OriginIdElement element = new OriginIdElement(); assertNotNull(element); assertEquals(StableUniqueStanzaIdManager.NAMESPACE, element.getNamespace()); - assertEquals(36, element.getId().length()); + assertEquals(16, element.getId().length()); } @Test diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java index 63dbadf58..148a6a676 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/ibb/InBandBytestreamManager.java @@ -20,7 +20,6 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Random; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; @@ -36,6 +35,7 @@ import org.jivesoftware.smack.XMPPException; import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.StanzaError; +import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smackx.bytestreams.BytestreamListener; import org.jivesoftware.smackx.bytestreams.BytestreamManager; @@ -138,9 +138,6 @@ public final class InBandBytestreamManager extends Manager implements Bytestream /* prefix used to generate session IDs */ private static final String SESSION_ID_PREFIX = "jibb_"; - /* random generator to create session IDs */ - private static final Random randomGenerator = new Random(); - /* stores one InBandBytestreamManager for each XMPP connection */ private static final Map managers = new WeakHashMap<>(); @@ -490,7 +487,7 @@ public final class InBandBytestreamManager extends Manager implements Bytestream private static String getNextSessionID() { StringBuilder buffer = new StringBuilder(); buffer.append(SESSION_ID_PREFIX); - buffer.append(randomGenerator.nextInt(Integer.MAX_VALUE) + randomGenerator.nextInt(Integer.MAX_VALUE)); + buffer.append(StringUtils.secureOnlineAttackSafeRandomString()); return buffer.toString(); } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java index 732578c35..fb0b3fa7b 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/bytestreams/socks5/Socks5BytestreamManager.java @@ -24,7 +24,6 @@ import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Random; import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; @@ -44,6 +43,7 @@ import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.Stanza; import org.jivesoftware.smack.packet.StanzaError; +import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smackx.bytestreams.BytestreamListener; import org.jivesoftware.smackx.bytestreams.BytestreamManager; @@ -114,9 +114,6 @@ public final class Socks5BytestreamManager extends Manager implements Bytestream /* prefix used to generate session IDs */ private static final String SESSION_ID_PREFIX = "js5_"; - /* random generator to create session IDs */ - private static final Random randomGenerator = new Random(); - /* stores one Socks5BytestreamManager for each XMPP connection */ private static final Map managers = new WeakHashMap<>(); @@ -759,7 +756,7 @@ public final class Socks5BytestreamManager extends Manager implements Bytestream private static String getNextSessionID() { StringBuilder buffer = new StringBuilder(); buffer.append(SESSION_ID_PREFIX); - buffer.append(randomGenerator.nextInt(Integer.MAX_VALUE) + randomGenerator.nextInt(Integer.MAX_VALUE)); + buffer.append(StringUtils.secureOnlineAttackSafeRandomString()); return buffer.toString(); } diff --git a/smack-im/src/main/java/org/jivesoftware/smack/chat/ChatManager.java b/smack-im/src/main/java/org/jivesoftware/smack/chat/ChatManager.java index 1537c8152..d2571358b 100644 --- a/smack-im/src/main/java/org/jivesoftware/smack/chat/ChatManager.java +++ b/smack-im/src/main/java/org/jivesoftware/smack/chat/ChatManager.java @@ -20,7 +20,6 @@ package org.jivesoftware.smack.chat; import java.util.Collections; import java.util.Map; import java.util.Set; -import java.util.UUID; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArraySet; @@ -42,6 +41,7 @@ import org.jivesoftware.smack.filter.ThreadFilter; import org.jivesoftware.smack.packet.Message; import org.jivesoftware.smack.packet.Message.Type; import org.jivesoftware.smack.packet.Stanza; +import org.jivesoftware.smack.util.StringUtils; import org.jxmpp.jid.EntityBareJid; import org.jxmpp.jid.EntityJid; @@ -407,7 +407,7 @@ public final class ChatManager extends Manager{ * @return the next id. */ private static String nextID() { - return UUID.randomUUID().toString(); + return StringUtils.secureUniqueRandomString(); } public static void setDefaultMatchMode(MatchMode mode) { diff --git a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/SecretKeyBackupHelper.java b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/SecretKeyBackupHelper.java index 4681f0be9..577f0c686 100644 --- a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/SecretKeyBackupHelper.java +++ b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/SecretKeyBackupHelper.java @@ -18,9 +18,9 @@ package org.jivesoftware.smackx.ox.util; import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.security.SecureRandom; import java.util.Set; +import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smack.util.stringencoder.Base64; import org.jivesoftware.smackx.ox.crypto.OpenPgpProvider; @@ -52,26 +52,7 @@ public class SecretKeyBackupHelper { * @return backup code */ public static String generateBackupPassword() { - final String alphabet = "123456789ABCDEFGHIJKLMNPQRSTUVWXYZ"; - final int len = alphabet.length(); - SecureRandom random = new SecureRandom(); - StringBuilder code = new StringBuilder(29); - - // 6 blocks - for (int i = 0; i < 6; i++) { - - // of 4 chars - for (int j = 0; j < 4; j++) { - char c = alphabet.charAt(random.nextInt(len)); - code.append(c); - } - - // dash after every block except the last one - if (i != 5) { - code.append('-'); - } - } - return code.toString(); + return StringUtils.secureOfflineAttackSafeRandomString(); } /**