From 378aa4b2629ec18e945dd7d0ed12b5598a68de3b Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Wed, 10 Oct 2018 14:05:45 +0200 Subject: [PATCH] Business logic for handling read-only devices --- .../smackx/omemo/OmemoConfiguration.java | 57 ++++++++++---- .../smackx/omemo/OmemoService.java | 76 ++++++++----------- .../exceptions/ReadOnlyDeviceException.java | 52 +++++++++++++ .../smackx/omemo/OmemoConfigurationTest.java | 14 ---- .../smackx/omemo/OmemoServiceTest.java | 7 -- 5 files changed, 123 insertions(+), 83 deletions(-) create mode 100644 smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/exceptions/ReadOnlyDeviceException.java diff --git a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoConfiguration.java b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoConfiguration.java index a529162fd..8756ef37e 100644 --- a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoConfiguration.java +++ b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoConfiguration.java @@ -23,31 +23,56 @@ package org.jivesoftware.smackx.omemo; */ public final class OmemoConfiguration { + private static boolean IGNORE_READ_ONLY_DEVICES = true; + private static int MAX_READ_ONLY_MESSAGE_COUNT = 400; + /** - * Ignore own other stale devices that we did not receive a message from for a period of time. - * Ignoring means do not encrypt messages for them. This helps to mitigate stale devices that threaten - * forward secrecy by never advancing ratchets. + * Set to true, in order to ignore read-only devices. + * + * @param ignore ignore read-only devices + * @see Blog Post explaining the danger of read-only devices. */ - private static boolean IGNORE_STALE_DEVICES = true; - private static int IGNORE_STALE_DEVICE_AFTER_HOURS = 24 * 7; //One week - - public static void setIgnoreStaleDevices(boolean ignore) { - IGNORE_STALE_DEVICES = ignore; + public static void setIgnoreReadOnlyDevices(boolean ignore) { + IGNORE_READ_ONLY_DEVICES = ignore; } - public static boolean getIgnoreStaleDevices() { - return IGNORE_STALE_DEVICES; + /** + * Return true, if the client should stop encrypting messages to a read-only device. + * + * @return true if read-only devices should get ignored after a certain amount of unanswered messages. + * @see Blog Post explaining the danger of read-only devices. + */ + public static boolean getIgnoreReadOnlyDevices() { + return IGNORE_READ_ONLY_DEVICES; } - public static void setIgnoreStaleDevicesAfterHours(int hours) { - if (hours <= 0) { - throw new IllegalArgumentException("Hours must be greater than 0."); + /** + * Set the maximum amount of messages that the client is allowed to send to a read-only device without getting a + * response. Once the message counter of a device reaches that value, the client will stop encrypting messages for + * the device (given that {@link #getIgnoreReadOnlyDevices()} is true). + * This threshold is used to prevent read-only devices from weakening forward secrecy. + * + * @param maxReadOnlyMessageCount maximum number of allowed messages to a read-only device. + * @see Blog Post explaining the danger of read-only devices. + */ + public static void setMaxReadOnlyMessageCount(int maxReadOnlyMessageCount) { + if (maxReadOnlyMessageCount <= 0) { + throw new IllegalArgumentException("maxReadOnlyMessageCount MUST be greater than 0."); } - IGNORE_STALE_DEVICE_AFTER_HOURS = hours; + MAX_READ_ONLY_MESSAGE_COUNT = maxReadOnlyMessageCount; } - public static int getIgnoreStaleDevicesAfterHours() { - return IGNORE_STALE_DEVICE_AFTER_HOURS; + /** + * Get the maximum amount of messages that the client is allowed to send to a read-only device without getting a + * response. Once the message counter of a device reaches that value, the client will stop encrypting messages for + * the device (given that {@link #getIgnoreReadOnlyDevices()} is true). + * This threshold is used to prevent read-only devices from weakening forward secrecy. + * + * @return maximum number of allowed messages to a read-only device. + * @see Blog Post explaining the danger of read-only devices. + */ + public static int getMaxReadOnlyMessageCount() { + return MAX_READ_ONLY_MESSAGE_COUNT; } /** diff --git a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java index d29d3339c..f8d38706d 100644 --- a/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java +++ b/smack-omemo/src/main/java/org/jivesoftware/smackx/omemo/OmemoService.java @@ -19,6 +19,9 @@ package org.jivesoftware.smackx.omemo; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.Crypto.KEYLENGTH; import static org.jivesoftware.smackx.omemo.util.OmemoConstants.Crypto.KEYTYPE; +import javax.crypto.BadPaddingException; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.NoSuchPaddingException; import java.io.UnsupportedEncodingException; import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; @@ -36,14 +39,10 @@ import java.util.Random; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; -import javax.crypto.BadPaddingException; -import javax.crypto.IllegalBlockSizeException; -import javax.crypto.NoSuchPaddingException; import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPException; - import org.jivesoftware.smack.packet.Message; import org.jivesoftware.smack.packet.Stanza; import org.jivesoftware.smack.packet.StanzaError; @@ -62,7 +61,7 @@ import org.jivesoftware.smackx.omemo.exceptions.CorruptedOmemoKeyException; import org.jivesoftware.smackx.omemo.exceptions.CryptoFailedException; import org.jivesoftware.smackx.omemo.exceptions.NoIdentityKeyException; import org.jivesoftware.smackx.omemo.exceptions.NoRawSessionException; -import org.jivesoftware.smackx.omemo.exceptions.StaleDeviceException; +import org.jivesoftware.smackx.omemo.exceptions.ReadOnlyDeviceException; import org.jivesoftware.smackx.omemo.exceptions.UndecidedOmemoIdentityException; import org.jivesoftware.smackx.omemo.exceptions.UntrustedOmemoIdentityException; import org.jivesoftware.smackx.omemo.internal.CipherAndAuthTag; @@ -323,6 +322,8 @@ public abstract class OmemoService= OmemoConfiguration.getMaxReadOnlyMessageCount(); - boolean stale = isStale(userDevice, contactsDevice, lastPublicationDate, OmemoConfiguration.getIgnoreStaleDevicesAfterHours()); - stale &= isStale(userDevice, contactsDevice, lastMessageDate, OmemoConfiguration.getIgnoreStaleDevicesAfterHours()); + if (readOnly) { + LOGGER.log(Level.FINE, "Device " + contactsDevice + " seems to be read-only (We sent " + + messageCounter + " messages without getting a reply back (max allowed is " + + OmemoConfiguration.getMaxReadOnlyMessageCount() + "). Ignoring the device."); + skippedRecipients.put(contactsDevice, new ReadOnlyDeviceException(contactsDevice)); - if (stale) { - LOGGER.log(Level.FINE, "Device " + contactsDevice + " seems to be stale (last message received " - + lastMessageDate + ", last publication of deviceId: " + lastPublicationDate + "). Ignore it."); - skippedRecipients.put(contactsDevice, new StaleDeviceException(contactsDevice, lastMessageDate, lastPublicationDate)); + // Skip this device and handle next device continue; } } @@ -428,6 +421,10 @@ public abstract class OmemoService