From 6cbd68fe9f8e1254b1be633cb541b773a7dd8e95 Mon Sep 17 00:00:00 2001 From: Alan Evans Date: Wed, 25 Nov 2020 14:21:33 -0400 Subject: [PATCH] Clean up any invalid group V1 ids in database. --- .../securesms/database/ThreadDatabase.java | 2 +- .../database/helpers/SQLCipherOpenHelper.java | 73 +++++++++++++++++-- .../securesms/groups/GroupId.java | 7 -- .../storage/GroupV1ConflictMerger.java | 2 +- .../securesms/util/GroupUtil.java | 3 +- .../securesms/groups/GroupIdTest.java | 4 +- 6 files changed, 71 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java index 5457a6f5c6..02e17b834e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java @@ -1107,7 +1107,7 @@ public class ThreadDatabase extends Database { pinnedRecipient = Recipient.externalPush(context, pinned.getContact().get()); } else if (pinned.getGroupV1Id().isPresent()) { try { - pinnedRecipient = Recipient.externalGroupExact(context, GroupId.v1Exact(pinned.getGroupV1Id().get())); + pinnedRecipient = Recipient.externalGroupExact(context, GroupId.v1(pinned.getGroupV1Id().get())); } catch (BadGroupIdException e) { Log.w(TAG, "Failed to parse pinned groupV1 ID!", e); pinnedRecipient = null; diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java index 3271b51b4e..1a650ccbf3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SQLCipherOpenHelper.java @@ -22,12 +22,6 @@ import net.sqlcipher.database.SQLiteDatabaseHook; import net.sqlcipher.database.SQLiteOpenHelper; import org.thoughtcrime.securesms.contacts.avatars.ContactColorsLegacy; -import org.thoughtcrime.securesms.database.MentionDatabase; -import org.thoughtcrime.securesms.database.RemappedRecordsDatabase; -import org.thoughtcrime.securesms.profiles.AvatarHelper; -import org.thoughtcrime.securesms.profiles.ProfileName; -import org.thoughtcrime.securesms.recipients.RecipientId; -import org.thoughtcrime.securesms.storage.StorageSyncHelper; import org.thoughtcrime.securesms.crypto.DatabaseSecret; import org.thoughtcrime.securesms.crypto.MasterSecret; import org.thoughtcrime.securesms.database.AttachmentDatabase; @@ -38,10 +32,12 @@ import org.thoughtcrime.securesms.database.IdentityDatabase; import org.thoughtcrime.securesms.database.JobDatabase; import org.thoughtcrime.securesms.database.KeyValueDatabase; import org.thoughtcrime.securesms.database.MegaphoneDatabase; +import org.thoughtcrime.securesms.database.MentionDatabase; import org.thoughtcrime.securesms.database.MmsDatabase; import org.thoughtcrime.securesms.database.OneTimePreKeyDatabase; import org.thoughtcrime.securesms.database.PushDatabase; import org.thoughtcrime.securesms.database.RecipientDatabase; +import org.thoughtcrime.securesms.database.RemappedRecordsDatabase; import org.thoughtcrime.securesms.database.SearchDatabase; import org.thoughtcrime.securesms.database.SessionDatabase; import org.thoughtcrime.securesms.database.SignedPreKeyDatabase; @@ -55,10 +51,15 @@ import org.thoughtcrime.securesms.jobs.RefreshPreKeysJob; import org.thoughtcrime.securesms.logging.Log; import org.thoughtcrime.securesms.notifications.NotificationChannels; import org.thoughtcrime.securesms.phonenumbers.PhoneNumberFormatter; +import org.thoughtcrime.securesms.profiles.AvatarHelper; +import org.thoughtcrime.securesms.profiles.ProfileName; +import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.service.KeyCachingService; +import org.thoughtcrime.securesms.storage.StorageSyncHelper; import org.thoughtcrime.securesms.util.Base64; import org.thoughtcrime.securesms.util.CursorUtil; import org.thoughtcrime.securesms.util.FileUtils; +import org.thoughtcrime.securesms.util.Hex; import org.thoughtcrime.securesms.util.ServiceUtil; import org.thoughtcrime.securesms.util.SqlUtil; import org.thoughtcrime.securesms.util.TextSecurePreferences; @@ -72,6 +73,7 @@ import java.io.IOException; import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Locale; import java.util.Set; public class SQLCipherOpenHelper extends SQLiteOpenHelper { @@ -161,8 +163,9 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper { private static final int NOTIFIED_TIMESTAMP = 81; private static final int GV1_MIGRATION_LAST_SEEN = 82; private static final int VIEWED_RECEIPTS = 83; + private static final int CLEAN_UP_GV1_IDS = 84; - private static final int DATABASE_VERSION = 83; + private static final int DATABASE_VERSION = 84; private static final String DATABASE_NAME = "signal.db"; private final Context context; @@ -1175,6 +1178,62 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper { db.execSQL("ALTER TABLE mms ADD COLUMN viewed_receipt_count INTEGER DEFAULT 0"); } + if (oldVersion < CLEAN_UP_GV1_IDS) { + List deletableRecipients = new LinkedList<>(); + try (Cursor cursor = db.rawQuery("SELECT _id, group_id FROM recipient\n" + + "WHERE group_id NOT IN (SELECT group_id FROM groups)\n" + + "AND group_id LIKE '__textsecure_group__!%' AND length(group_id) <> 53\n" + + "AND (_id NOT IN (SELECT recipient_ids FROM thread) OR _id IN (SELECT recipient_ids FROM thread WHERE message_count = 0))", null)) + { + while (cursor.moveToNext()) { + String recipientId = cursor.getString(cursor.getColumnIndexOrThrow("_id")); + String groupIdV1 = cursor.getString(cursor.getColumnIndexOrThrow("group_id")); + deletableRecipients.add(recipientId); + Log.d(TAG, String.format(Locale.US, "Found invalid GV1 on %s with no or empty thread %s length %d", recipientId, groupIdV1, groupIdV1.length())); + } + } + + for (String recipientId : deletableRecipients) { + db.delete("recipient", "_id = ?", new String[]{recipientId}); + Log.d(TAG, "Deleted recipient " + recipientId); + } + + List orphanedThreads = new LinkedList<>(); + try (Cursor cursor = db.rawQuery("SELECT _id FROM thread WHERE message_count = 0 AND recipient_ids NOT IN (SELECT _id FROM recipient)", null)) { + while (cursor.moveToNext()) { + orphanedThreads.add(cursor.getString(cursor.getColumnIndexOrThrow("_id"))); + } + } + + for (String orphanedThreadId : orphanedThreads) { + db.delete("thread", "_id = ?", new String[]{orphanedThreadId}); + Log.d(TAG, "Deleted orphaned thread " + orphanedThreadId); + } + + List remainingInvalidGV1Recipients = new LinkedList<>(); + try (Cursor cursor = db.rawQuery("SELECT _id, group_id FROM recipient\n" + + "WHERE group_id NOT IN (SELECT group_id FROM groups)\n" + + "AND group_id LIKE '__textsecure_group__!%' AND length(group_id) <> 53\n" + + "AND _id IN (SELECT recipient_ids FROM thread)", null)) + { + while (cursor.moveToNext()) { + String recipientId = cursor.getString(cursor.getColumnIndexOrThrow("_id")); + String groupIdV1 = cursor.getString(cursor.getColumnIndexOrThrow("group_id")); + remainingInvalidGV1Recipients.add(recipientId); + Log.d(TAG, String.format(Locale.US, "Found invalid GV1 on %s with non-empty thread %s length %d", recipientId, groupIdV1, groupIdV1.length())); + } + } + + for (String recipientId : remainingInvalidGV1Recipients) { + String newId = "__textsecure_group__!" + Hex.toStringCondensed(Util.getSecretBytes(16)); + ContentValues values = new ContentValues(1); + values.put("group_id", newId); + + db.update("recipient", values, "_id = ?", new String[] { String.valueOf(recipientId) }); + Log.d(TAG, String.format("Replaced group id on recipient %s now %s", recipientId, newId)); + } + } + db.setTransactionSuccessful(); } finally { db.endTransaction(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupId.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupId.java index de0d20740f..db2618b803 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupId.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupId.java @@ -43,13 +43,6 @@ public abstract class GroupId { } public static @NonNull GroupId.V1 v1(byte[] gv1GroupIdBytes) throws BadGroupIdException { - if (gv1GroupIdBytes.length == V2_BYTE_LENGTH) { - throw new BadGroupIdException(); - } - return new GroupId.V1(gv1GroupIdBytes); - } - - public static @NonNull GroupId.V1 v1Exact(byte[] gv1GroupIdBytes) throws BadGroupIdException { if (gv1GroupIdBytes.length != V1_BYTE_LENGTH) { throw new BadGroupIdException(); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMerger.java b/app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMerger.java index c19839e584..dbfd234e50 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMerger.java +++ b/app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMerger.java @@ -35,7 +35,7 @@ final class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger { try { - GroupId.V1 id = GroupId.v1Exact(record.getGroupId()); + GroupId.V1 id = GroupId.v1(record.getGroupId()); return groupExistenceChecker.exists(id.deriveV2MigrationGroupId()); } catch (BadGroupIdException e) { return true; diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/GroupUtil.java b/app/src/main/java/org/thoughtcrime/securesms/util/GroupUtil.java index 1c290a4a55..56fa3123d2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/GroupUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/GroupUtil.java @@ -25,7 +25,6 @@ import org.whispersystems.signalservice.api.messages.SignalServiceDataMessage; import org.whispersystems.signalservice.api.messages.SignalServiceGroup; import org.whispersystems.signalservice.api.messages.SignalServiceGroupContext; import org.whispersystems.signalservice.api.messages.SignalServiceGroupV2; -import org.whispersystems.signalservice.internal.push.SignalServiceProtos; import org.whispersystems.signalservice.internal.push.SignalServiceProtos.GroupContext; import java.io.IOException; @@ -46,7 +45,7 @@ public final class GroupUtil { throws BadGroupIdException { if (groupContext.getGroupV1().isPresent()) { - return GroupId.v1Exact(groupContext.getGroupV1().get().getGroupId()); + return GroupId.v1(groupContext.getGroupV1().get().getGroupId()); } else if (groupContext.getGroupV2().isPresent()) { return GroupId.v2(groupContext.getGroupV2().get().getMasterKey()); } else { diff --git a/app/src/test/java/org/thoughtcrime/securesms/groups/GroupIdTest.java b/app/src/test/java/org/thoughtcrime/securesms/groups/GroupIdTest.java index a1e4ca3384..fc02fcb4a1 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/groups/GroupIdTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/groups/GroupIdTest.java @@ -273,7 +273,7 @@ public final class GroupIdTest { @Test(expected = BadGroupIdException.class) public void cannot_create_v1_with_wrong_length() throws IOException, BadGroupIdException { - GroupId.v1Exact(Hex.fromStringCondensed("000102030405060708090a0b0c0d0e")); + GroupId.v1(Hex.fromStringCondensed("000102030405060708090a0b0c0d0e")); } @Test @@ -302,7 +302,7 @@ public final class GroupIdTest { @Test public void v1Exact_static_factory() throws BadGroupIdException { - GroupId.V1 v1 = GroupId.v1Exact(new byte[]{ 9, 10, 11, 12, 13, 14, 15, 0, 1, 2, 3, 4, 5, 6, 7, 8 }); + GroupId.V1 v1 = GroupId.v1(new byte[]{9, 10, 11, 12, 13, 14, 15, 0, 1, 2, 3, 4, 5, 6, 7, 8}); assertEquals("__textsecure_group__!090a0b0c0d0e0f000102030405060708", v1.toString()); assertTrue(v1.isV1());