Business logic for handling read-only devices

This commit is contained in:
Paul Schaub 2018-10-10 14:05:45 +02:00
parent afb432dcee
commit 378aa4b262
5 changed files with 123 additions and 83 deletions

View File

@ -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 <a href="TODO: Add URL">Blog Post explaining the danger of read-only devices.</a>
*/
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 <a href="TODO: Add URL">Blog Post explaining the danger of read-only devices.</a>
*/
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 <a href="TODO: Add URL">Blog Post explaining the danger of read-only devices.</a>
*/
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 <a href="TODO: Add URL">Blog Post explaining the danger of read-only devices.</a>
*/
public static int getMaxReadOnlyMessageCount() {
return MAX_READ_ONLY_MESSAGE_COUNT;
}
/**

View File

@ -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<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
throw new AssertionError("We MUST have an identityKey for " + contactsDevice + " since we built a session." + e);
}
// Note: We don't need to update our message counter for a ratchet update message.
return builder.finish();
}
@ -386,28 +387,20 @@ public abstract class OmemoService<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
}
}
// Ignore own stale devices
if (contactsDevice.getJid().equals(userDevice.getJid()) && OmemoConfiguration.getIgnoreStaleDevices()) {
int messageCounter = omemoStore.loadOmemoMessageCounter(userDevice, contactsDevice);
Date lastMessageDate = getOmemoStoreBackend().getDateOfLastReceivedMessage(userDevice, contactsDevice);
if (lastMessageDate == null) {
lastMessageDate = new Date();
getOmemoStoreBackend().setDateOfLastReceivedMessage(userDevice, contactsDevice, lastMessageDate);
}
// Ignore read-only devices
if (OmemoConfiguration.getIgnoreReadOnlyDevices()) {
Date lastPublicationDate = getOmemoStoreBackend().getDateOfLastDeviceIdPublication(userDevice, contactsDevice);
if (lastPublicationDate == null) {
lastPublicationDate = new Date();
getOmemoStoreBackend().setDateOfLastDeviceIdPublication(userDevice, contactsDevice, lastPublicationDate);
}
boolean readOnly = messageCounter >= 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<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
LOGGER.log(Level.WARNING, "Device " + contactsDevice + " is untrusted. Message is not encrypted for it.");
skippedRecipients.put(contactsDevice, e);
}
// Increment the message counter of the device
omemoStore.storeOmemoMessageCounter(userDevice, contactsDevice,
messageCounter + 1);
}
OmemoElement element = builder.finish();
@ -466,6 +463,9 @@ public abstract class OmemoService<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
throw new AssertionError("Cannot retrieve OmemoFingerprint of sender although decryption was successful: " + e);
}
// Reset the message counter.
omemoStore.storeOmemoMessageCounter(manager.getOwnDevice(), senderDevice, 0);
if (omemoElement.isMessageElement()) {
// Use symmetric message key to decrypt message payload.
String plaintext = OmemoRatchet.decryptMessageElement(omemoElement, cipherAndAuthTag);
@ -1009,24 +1009,6 @@ public abstract class OmemoService<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
return removeStaleDevicesFromDeviceList(userDevice, userDevice.getJid(), deviceList, maxAgeHours);
}
/**
* Return a copy of the contactsDeviceList, but with stale devices marked as inactive.
* Never mark our own device as stale.
* This method ignores {@link OmemoConfiguration#getIgnoreStaleDevices()}!
*
* In this case, a stale device is one of our devices, from which we haven't received an OMEMO message from
* for more than {@link OmemoConfiguration#IGNORE_STALE_DEVICE_AFTER_HOURS} hours.
*
* @param userDevice our OmemoDevice
* @param contact subjects BareJid
* @param contactsDeviceList subjects deviceList
* @return
*/
private OmemoCachedDeviceList ignoreStaleDevices(OmemoDevice userDevice, BareJid contact, OmemoCachedDeviceList contactsDeviceList) {
int maxAgeHours = OmemoConfiguration.getIgnoreStaleDevicesAfterHours();
return removeStaleDevicesFromDeviceList(userDevice, contact, contactsDeviceList, maxAgeHours);
}
/**
* Return a copy of the given deviceList of user contact, but with stale devices marked as inactive.
* Never mark our own device as stale. If we haven't yet received a message from a device, store the current date
@ -1086,12 +1068,14 @@ public abstract class OmemoService<T_IdKeyPair, T_IdKey, T_PreKey, T_SigPreKey,
}
/**
* Determine, whether another one of *our* devices is stale or not.
*
* @param userDevice
* @param subject
* @param lastReceipt
* @param maxAgeHours
* @return
* @param userDevice our omemoDevice
* @param subject another one of our devices
* @param lastReceipt date of last received message from that device
* @param maxAgeHours threshold
*
* @return staleness
*/
static boolean isStale(OmemoDevice userDevice, OmemoDevice subject, Date lastReceipt, int maxAgeHours) {
if (userDevice.equals(subject)) {

View File

@ -0,0 +1,52 @@
/**
*
* Copyright 2018 Paul Schaub
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smackx.omemo.exceptions;
import org.jivesoftware.smackx.omemo.OmemoConfiguration;
import org.jivesoftware.smackx.omemo.internal.OmemoDevice;
/**
* Exception that signals, that a device is considered read-only.
* Read-only devices are devices that receive OMEMO messages, but do not send any.
* Those devices are weakening forward secrecy. For that reason, read-only devices are ignored after n messages have
* been sent without getting a reply back.
*/
public class ReadOnlyDeviceException extends Exception {
private static final long serialVersionUID = 1L;
private final OmemoDevice device;
/**
* Constructor.
* We do not need to hand over the current value of the message counter, as that value will always be equal to
* {@link OmemoConfiguration#getMaxReadOnlyMessageCount()}. Therefore providing the {@link OmemoDevice} should be
* enough.
*
* @param device device which is considered read-only.
*/
public ReadOnlyDeviceException(OmemoDevice device) {
this.device = device;
}
/**
* Return the device in question.
* @return device
*/
public OmemoDevice getDevice() {
return device;
}
}

View File

@ -52,20 +52,6 @@ public class OmemoConfigurationTest {
// Expected.
}
// Ignore stale device
OmemoConfiguration.setIgnoreStaleDevices(false);
assertEquals(false, OmemoConfiguration.getIgnoreStaleDevices());
OmemoConfiguration.setIgnoreStaleDevices(true);
assertEquals(true, OmemoConfiguration.getIgnoreStaleDevices());
OmemoConfiguration.setIgnoreStaleDevicesAfterHours(44);
assertEquals(44, OmemoConfiguration.getIgnoreStaleDevicesAfterHours());
try {
OmemoConfiguration.setIgnoreStaleDevicesAfterHours(-5);
TestCase.fail("OmemoConfiguration.setIgnoreStaleDevicesAfterHours should not accept values <= 0.");
} catch (IllegalArgumentException e) {
// Expected
}
// Renew signedPreKeys
OmemoConfiguration.setRenewOldSignedPreKeys(false);
assertEquals(false, OmemoConfiguration.getRenewOldSignedPreKeys());

View File

@ -32,7 +32,6 @@ import org.jxmpp.stringprep.XmppStringprepException;
public class OmemoServiceTest extends SmackTestSuite {
private static final long ONE_HOUR = 1000L * 60 * 60;
private static final int IGNORE_STALE = OmemoConfiguration.getIgnoreStaleDevicesAfterHours();
private static final int DELETE_STALE = OmemoConfiguration.getDeleteStaleDevicesAfterHours();
@Test(expected = IllegalStateException.class)
@ -55,15 +54,9 @@ public class OmemoServiceTest extends SmackTestSuite {
OmemoDevice other = new OmemoDevice(JidCreate.bareFrom("bob@builder.tv"), 444);
Date now = new Date();
Date ignoreMe = new Date(now.getTime() - ((IGNORE_STALE + 1) * ONE_HOUR));
Date deleteMe = new Date(now.getTime() - ((DELETE_STALE + 1) * ONE_HOUR));
Date imFine = new Date(now.getTime() - ONE_HOUR);
// One hour "old" devices are (probably) not not stale
assertFalse(OmemoService.isStale(user, other, imFine, IGNORE_STALE));
// Devices one hour "older" than max ages are stale
assertTrue(OmemoService.isStale(user, other, ignoreMe, IGNORE_STALE));
assertTrue(OmemoService.isStale(user, other, deleteMe, DELETE_STALE));
// Own device is never stale, no matter how old