From f832a36a5e87bb5408699d7983cefd0bdac6cb16 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 4 Dec 2019 23:54:56 -0500 Subject: [PATCH] Prevent possible UUID-only recipient creations. --- .../api/crypto/SignalServiceCipher.java | 13 +----- .../api/push/SignalServiceAddress.java | 10 +++++ .../storage/TextSecureSessionStore.java | 41 +++++++++++++------ 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/SignalServiceCipher.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/SignalServiceCipher.java index 1c8aafb2ca..3f53c40b7b 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/SignalServiceCipher.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/crypto/SignalServiceCipher.java @@ -293,18 +293,7 @@ public class SignalServiceCipher { } else if (e164Address != null && store.containsSession(e164Address)) { return e164Address; } else { - // TODO [greyson][uuid] We should switch to preferring the UUID once we allow UUID-only recipients - String preferred; - - if (address.getNumber().isPresent()) { - preferred = address.getNumber().get(); - } else if (address.getUuid().isPresent()) { - preferred = address.getUuid().get().toString(); - } else { - throw new AssertionError("Given the constrains of creating a SignalServiceAddress, this should not be possible."); - } - - return new SignalProtocolAddress(preferred, sourceDevice); + return new SignalProtocolAddress(address.getLegacyIdentifier(), sourceDevice); } } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/SignalServiceAddress.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/SignalServiceAddress.java index 8231723101..fc912b606f 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/SignalServiceAddress.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/push/SignalServiceAddress.java @@ -69,6 +69,16 @@ public class SignalServiceAddress { } } + public String getLegacyIdentifier() { + if (e164.isPresent()) { + return e164.get(); + } else if (uuid.isPresent()) { + return uuid.get().toString(); + } else { + throw new AssertionError("Given the checks in the constructor, this should not be possible."); + } + } + public Optional getRelay() { return relay; } diff --git a/src/org/thoughtcrime/securesms/crypto/storage/TextSecureSessionStore.java b/src/org/thoughtcrime/securesms/crypto/storage/TextSecureSessionStore.java index bffcd2df07..d41453b244 100644 --- a/src/org/thoughtcrime/securesms/crypto/storage/TextSecureSessionStore.java +++ b/src/org/thoughtcrime/securesms/crypto/storage/TextSecureSessionStore.java @@ -15,6 +15,8 @@ import org.whispersystems.libsignal.state.SessionRecord; import org.whispersystems.libsignal.state.SessionStore; import org.whispersystems.signalservice.api.util.UuidUtil; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.logging.Logger; @@ -72,37 +74,52 @@ public class TextSecureSessionStore implements SessionStore { @Override public void deleteSession(SignalProtocolAddress address) { synchronized (FILE_LOCK) { - RecipientId recipientId = Recipient.external(context, address.getName()).getId(); - DatabaseFactory.getSessionDatabase(context).delete(recipientId, address.getDeviceId()); + if (DatabaseFactory.getRecipientDatabase(context).containsPhoneOrUuid(address.getName())) { + RecipientId recipientId = Recipient.external(context, address.getName()).getId(); + DatabaseFactory.getSessionDatabase(context).delete(recipientId, address.getDeviceId()); + } else { + Log.w(TAG, "Tried to delete session for " + address.toString() + ", but none existed!"); + } } } @Override public void deleteAllSessions(String name) { synchronized (FILE_LOCK) { - RecipientId recipientId = Recipient.external(context, name).getId(); - DatabaseFactory.getSessionDatabase(context).deleteAllFor(recipientId); + if (DatabaseFactory.getRecipientDatabase(context).containsPhoneOrUuid(name)) { + RecipientId recipientId = Recipient.external(context, name).getId(); + DatabaseFactory.getSessionDatabase(context).deleteAllFor(recipientId); + } } } @Override public List getSubDeviceSessions(String name) { synchronized (FILE_LOCK) { - RecipientId recipientId = Recipient.external(context, name).getId(); - return DatabaseFactory.getSessionDatabase(context).getSubDevices(recipientId); + if (DatabaseFactory.getRecipientDatabase(context).containsPhoneOrUuid(name)) { + RecipientId recipientId = Recipient.external(context, name).getId(); + return DatabaseFactory.getSessionDatabase(context).getSubDevices(recipientId); + } else { + Log.w(TAG, "Tried to get sub device sessions for " + name + ", but none existed!"); + return Collections.emptyList(); + } } } public void archiveSiblingSessions(@NonNull SignalProtocolAddress address) { synchronized (FILE_LOCK) { - RecipientId recipientId = Recipient.external(context, address.getName()).getId(); - List sessions = DatabaseFactory.getSessionDatabase(context).getAllFor(recipientId); + if (DatabaseFactory.getRecipientDatabase(context).containsPhoneOrUuid(address.getName())) { + RecipientId recipientId = Recipient.external(context, address.getName()).getId(); + List sessions = DatabaseFactory.getSessionDatabase(context).getAllFor(recipientId); - for (SessionDatabase.SessionRow row : sessions) { - if (row.getDeviceId() != address.getDeviceId()) { - row.getRecord().archiveCurrentState(); - storeSession(new SignalProtocolAddress(Recipient.resolved(row.getRecipientId()).requireServiceId(), row.getDeviceId()), row.getRecord()); + for (SessionDatabase.SessionRow row : sessions) { + if (row.getDeviceId() != address.getDeviceId()) { + row.getRecord().archiveCurrentState(); + storeSession(new SignalProtocolAddress(Recipient.resolved(row.getRecipientId()).requireServiceId(), row.getDeviceId()), row.getRecord()); + } } + } else { + Log.w(TAG, "Tried to archive sibling sessions for " + address.toString() + ", but none existed!"); } } }