From 2d1bf339029065c87590a388c300a9852a709237 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 20 Oct 2020 17:13:00 -0400 Subject: [PATCH] Update group table schema to support GV1->GV2 migration. Also puts in protections to make sure we don't insert bad recipients or groups. --- .../securesms/database/GroupDatabase.java | 154 +++++++++++++----- .../securesms/database/RecipientDatabase.java | 55 +++++-- .../database/helpers/SQLCipherOpenHelper.java | 23 ++- .../securesms/groups/GroupManager.java | 2 +- 4 files changed, 177 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java index f1b6a7fea7..305997278a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/GroupDatabase.java @@ -27,6 +27,7 @@ import org.thoughtcrime.securesms.groups.GroupId; import org.thoughtcrime.securesms.logging.Log; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.RecipientId; +import org.thoughtcrime.securesms.util.CursorUtil; import org.thoughtcrime.securesms.util.SqlUtil; import org.whispersystems.libsignal.util.guava.Optional; import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil; @@ -62,6 +63,9 @@ public final class GroupDatabase extends Database { private static final String TIMESTAMP = "timestamp"; static final String ACTIVE = "active"; static final String MMS = "mms"; + private static final String EXPECTED_V2_ID = "expected_v2_id"; + private static final String FORMER_V1_MEMBERS = "former_v1_members"; + /* V2 Group columns */ /** 32 bytes serialized {@link GroupMasterKey} */ @@ -71,32 +75,33 @@ public final class GroupDatabase extends Database { /** Serialized {@link DecryptedGroup} protobuf */ private static final String V2_DECRYPTED_GROUP = "decrypted_group"; - public static final String CREATE_TABLE = - "CREATE TABLE " + TABLE_NAME + - " (" + ID + " INTEGER PRIMARY KEY, " + - GROUP_ID + " TEXT, " + - RECIPIENT_ID + " INTEGER, " + - TITLE + " TEXT, " + - MEMBERS + " TEXT, " + - AVATAR_ID + " INTEGER, " + - AVATAR_KEY + " BLOB, " + - AVATAR_CONTENT_TYPE + " TEXT, " + - AVATAR_RELAY + " TEXT, " + - TIMESTAMP + " INTEGER, " + - ACTIVE + " INTEGER DEFAULT 1, " + - AVATAR_DIGEST + " BLOB, " + - MMS + " INTEGER DEFAULT 0, " + - V2_MASTER_KEY + " BLOB, " + - V2_REVISION + " BLOB, " + - V2_DECRYPTED_GROUP + " BLOB);"; + public static final String CREATE_TABLE = "CREATE TABLE " + TABLE_NAME + " (" + ID + " INTEGER PRIMARY KEY, " + + GROUP_ID + " TEXT, " + + RECIPIENT_ID + " INTEGER, " + + TITLE + " TEXT, " + + MEMBERS + " TEXT, " + + AVATAR_ID + " INTEGER, " + + AVATAR_KEY + " BLOB, " + + AVATAR_CONTENT_TYPE + " TEXT, " + + AVATAR_RELAY + " TEXT, " + + TIMESTAMP + " INTEGER, " + + ACTIVE + " INTEGER DEFAULT 1, " + + AVATAR_DIGEST + " BLOB, " + + MMS + " INTEGER DEFAULT 0, " + + V2_MASTER_KEY + " BLOB, " + + V2_REVISION + " BLOB, " + + V2_DECRYPTED_GROUP + " BLOB, " + + EXPECTED_V2_ID + " TEXT DEFAULT NULL, " + + FORMER_V1_MEMBERS + " TEXT DEFAULT NULL);"; public static final String[] CREATE_INDEXS = { "CREATE UNIQUE INDEX IF NOT EXISTS group_id_index ON " + TABLE_NAME + " (" + GROUP_ID + ");", "CREATE UNIQUE INDEX IF NOT EXISTS group_recipient_id_index ON " + TABLE_NAME + " (" + RECIPIENT_ID + ");", + "CREATE UNIQUE INDEX IF NOT EXISTS expected_v2_id_index ON " + TABLE_NAME + " (" + EXPECTED_V2_ID + ");" }; private static final String[] GROUP_PROJECTION = { - GROUP_ID, RECIPIENT_ID, TITLE, MEMBERS, AVATAR_ID, AVATAR_KEY, AVATAR_CONTENT_TYPE, AVATAR_RELAY, AVATAR_DIGEST, + GROUP_ID, RECIPIENT_ID, TITLE, MEMBERS, FORMER_V1_MEMBERS, AVATAR_ID, AVATAR_KEY, AVATAR_CONTENT_TYPE, AVATAR_RELAY, AVATAR_DIGEST, TIMESTAMP, ACTIVE, MMS, V2_MASTER_KEY, V2_REVISION, V2_DECRYPTED_GROUP }; @@ -129,7 +134,7 @@ public final class GroupDatabase extends Database { } } - public boolean findGroup(@NonNull GroupId groupId) { + public boolean groupExists(@NonNull GroupId groupId) { try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, null, GROUP_ID + " = ?", new String[] {groupId.toString()}, null, null, null)) @@ -138,6 +143,27 @@ public final class GroupDatabase extends Database { } } + /** + * @return A gv1 group whose expected v2 ID matches the one provided. + */ + public Optional getGroupV1ByExpectedV2(@NonNull GroupId.V2 gv2Id) { + SQLiteDatabase db = databaseHelper.getReadableDatabase(); + try (Cursor cursor = db.query(TABLE_NAME, GROUP_PROJECTION, EXPECTED_V2_ID + " = ?", SqlUtil.buildArgs(gv2Id), null, null, null)) { + if (cursor.moveToFirst()) { + return getGroup(cursor); + } else { + return Optional.absent(); + } + } + } + + public void clearFormerV1Members(@NonNull GroupId.V2 id) { + ContentValues values = new ContentValues(); + values.putNull(FORMER_V1_MEMBERS); + + databaseHelper.getWritableDatabase().update(TABLE_NAME, values, GROUP_ID + " = ?", SqlUtil.buildArgs(id)); + } + Optional getGroup(Cursor cursor) { Reader reader = new Reader(cursor); return Optional.fromNullable(reader.getCurrent()); @@ -320,6 +346,9 @@ public final class GroupDatabase extends Database { @Nullable SignalServiceAttachmentPointer avatar, @Nullable String relay) { + if (groupExists(groupId.deriveV2MigrationGroupId())) { + throw new LegacyGroupInsertException(groupId); + } create(groupId, title, members, avatar, relay, null, null); } @@ -334,6 +363,10 @@ public final class GroupDatabase extends Database { { GroupId.V2 groupId = GroupId.v2(groupMasterKey); + if (getGroupV1ByExpectedV2(groupId).isPresent()) { + throw new MissedGroupMigrationInsertException(groupId); + } + create(groupId, groupState.getTitle(), Collections.emptyList(), null, null, groupMasterKey, groupState); return groupId; @@ -378,6 +411,7 @@ public final class GroupDatabase extends Database { contentValues.put(ACTIVE, groupState != null && gv2GroupActive(groupState) ? 1 : 0); } else { contentValues.put(ACTIVE, 1); + contentValues.put(EXPECTED_V2_ID, groupId.requireV1().deriveV2MigrationGroupId().toString()); } contentValues.put(MMS, groupId.isMms()); @@ -630,20 +664,21 @@ public final class GroupDatabase extends Database { return null; } - return new GroupRecord(GroupId.parseOrThrow(cursor.getString(cursor.getColumnIndexOrThrow(GROUP_ID))), - RecipientId.from(cursor.getLong(cursor.getColumnIndexOrThrow(RECIPIENT_ID))), - cursor.getString(cursor.getColumnIndexOrThrow(TITLE)), - cursor.getString(cursor.getColumnIndexOrThrow(MEMBERS)), - cursor.getLong(cursor.getColumnIndexOrThrow(AVATAR_ID)), - cursor.getBlob(cursor.getColumnIndexOrThrow(AVATAR_KEY)), - cursor.getString(cursor.getColumnIndexOrThrow(AVATAR_CONTENT_TYPE)), - cursor.getString(cursor.getColumnIndexOrThrow(AVATAR_RELAY)), - cursor.getInt(cursor.getColumnIndexOrThrow(ACTIVE)) == 1, - cursor.getBlob(cursor.getColumnIndexOrThrow(AVATAR_DIGEST)), - cursor.getInt(cursor.getColumnIndexOrThrow(MMS)) == 1, - cursor.getBlob(cursor.getColumnIndexOrThrow(V2_MASTER_KEY)), - cursor.getInt(cursor.getColumnIndexOrThrow(V2_REVISION)), - cursor.getBlob(cursor.getColumnIndexOrThrow(V2_DECRYPTED_GROUP))); + return new GroupRecord(GroupId.parseOrThrow(CursorUtil.requireString(cursor, GROUP_ID)), + RecipientId.from(CursorUtil.requireString(cursor, RECIPIENT_ID)), + CursorUtil.requireString(cursor, TITLE), + CursorUtil.requireString(cursor, MEMBERS), + CursorUtil.requireString(cursor, FORMER_V1_MEMBERS), + CursorUtil.requireLong(cursor, AVATAR_ID), + CursorUtil.requireBlob(cursor, AVATAR_KEY), + CursorUtil.requireString(cursor, AVATAR_CONTENT_TYPE), + CursorUtil.requireString(cursor, AVATAR_RELAY), + CursorUtil.requireBoolean(cursor, ACTIVE), + CursorUtil.requireBlob(cursor, AVATAR_DIGEST), + CursorUtil.requireBoolean(cursor, MMS), + CursorUtil.requireBlob(cursor, V2_MASTER_KEY), + CursorUtil.requireInt(cursor, V2_REVISION), + CursorUtil.requireBlob(cursor, V2_DECRYPTED_GROUP)); } @Override @@ -659,6 +694,7 @@ public final class GroupDatabase extends Database { private final RecipientId recipientId; private final String title; private final List members; + private final List formerV1Members; private final long avatarId; private final byte[] avatarKey; private final byte[] avatarDigest; @@ -668,10 +704,21 @@ public final class GroupDatabase extends Database { private final boolean mms; @Nullable private final V2GroupProperties v2GroupProperties; - public GroupRecord(@NonNull GroupId id, @NonNull RecipientId recipientId, String title, String members, - long avatarId, byte[] avatarKey, String avatarContentType, - String relay, boolean active, byte[] avatarDigest, boolean mms, - @Nullable byte[] groupMasterKeyBytes, int groupRevision, @Nullable byte[] decryptedGroupBytes) + public GroupRecord(@NonNull GroupId id, + @NonNull RecipientId recipientId, + String title, + String members, + String formerV1Members, + long avatarId, + byte[] avatarKey, + String avatarContentType, + String relay, + boolean active, + byte[] avatarDigest, + boolean mms, + @Nullable byte[] groupMasterKeyBytes, + int groupRevision, + @Nullable byte[] decryptedGroupBytes) { this.id = id; this.recipientId = recipientId; @@ -696,8 +743,17 @@ public final class GroupDatabase extends Database { } this.v2GroupProperties = v2GroupProperties; - if (!TextUtils.isEmpty(members)) this.members = RecipientId.fromSerializedList(members); - else this.members = new LinkedList<>(); + if (!TextUtils.isEmpty(members)) { + this.members = RecipientId.fromSerializedList(members); + } else { + this.members = Collections.emptyList(); + } + + if (!TextUtils.isEmpty(formerV1Members)) { + this.formerV1Members = RecipientId.fromSerializedList(formerV1Members); + } else { + this.formerV1Members = Collections.emptyList(); + } } public GroupId getId() { @@ -712,10 +768,14 @@ public final class GroupDatabase extends Database { return title; } - public List getMembers() { + public @NonNull List getMembers() { return members; } + public @NonNull List getFormerV1Members() { + return formerV1Members; + } + public boolean hasAvatar() { return avatarId != 0; } @@ -952,4 +1012,16 @@ public final class GroupDatabase extends Database { return inGroup; } } + + public static class LegacyGroupInsertException extends IllegalStateException { + public LegacyGroupInsertException(@Nullable GroupId id) { + super("Tried to create a new GV1 entry when we already had a migrated GV2! " + id); + } + } + + public static class MissedGroupMigrationInsertException extends IllegalStateException { + public MissedGroupMigrationInsertException(@Nullable GroupId id) { + super("Tried to create a new GV2 entry when we already had a V1 group that mapped to the new ID! " + id); + } + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java index 75bb660ef1..6da868fa5c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java @@ -554,27 +554,54 @@ public class RecipientDatabase extends Database { } public @NonNull RecipientId getOrInsertFromGroupId(@NonNull GroupId groupId) { - GetOrInsertResult result = getOrInsertByColumn(GROUP_ID, groupId.toString()); + Optional existing = getByColumn(GROUP_ID, groupId.toString()); - if (result.neededInsert) { + if (existing.isPresent()) { + return existing.get(); + } else if (groupId.isV1() && DatabaseFactory.getGroupDatabase(context).groupExists(groupId.requireV1().deriveV2MigrationGroupId())) { + throw new GroupDatabase.LegacyGroupInsertException(groupId); + } else if (groupId.isV2() && DatabaseFactory.getGroupDatabase(context).getGroupV1ByExpectedV2(groupId.requireV2()).isPresent()) { + throw new GroupDatabase.MissedGroupMigrationInsertException(groupId); + } else { ContentValues values = new ContentValues(); + values.put(GROUP_ID, groupId.toString()); - if (groupId.isMms()) { - values.put(GROUP_TYPE, GroupType.MMS.getId()); - } else { - if (groupId.isV2()) { - values.put(GROUP_TYPE, GroupType.SIGNAL_V2.getId()); + long id = databaseHelper.getWritableDatabase().insert(TABLE_NAME, null, values); + + if (id < 0) { + existing = getByColumn(GROUP_ID, groupId.toString()); + + if (existing.isPresent()) { + return existing.get(); + } else if (groupId.isV1() && DatabaseFactory.getGroupDatabase(context).groupExists(groupId.requireV1().deriveV2MigrationGroupId())) { + throw new GroupDatabase.LegacyGroupInsertException(groupId); + } else if (groupId.isV2() && DatabaseFactory.getGroupDatabase(context).getGroupV1ByExpectedV2(groupId.requireV2()).isPresent()) { + throw new GroupDatabase.MissedGroupMigrationInsertException(groupId); } else { - values.put(GROUP_TYPE, GroupType.SIGNAL_V1.getId()); + throw new AssertionError("Failed to insert recipient!"); } - values.put(DIRTY, DirtyState.INSERT.getId()); - values.put(STORAGE_SERVICE_ID, Base64.encodeBytes(StorageSyncHelper.generateKey())); + } else { + ContentValues groupUpdates = new ContentValues(); + + if (groupId.isMms()) { + groupUpdates.put(GROUP_TYPE, GroupType.MMS.getId()); + } else { + if (groupId.isV2()) { + groupUpdates.put(GROUP_TYPE, GroupType.SIGNAL_V2.getId()); + } else { + groupUpdates.put(GROUP_TYPE, GroupType.SIGNAL_V1.getId()); + } + groupUpdates.put(DIRTY, DirtyState.INSERT.getId()); + groupUpdates.put(STORAGE_SERVICE_ID, Base64.encodeBytes(StorageSyncHelper.generateKey())); + } + + RecipientId recipientId = RecipientId.from(id); + + update(recipientId, groupUpdates); + + return recipientId; } - - update(result.recipientId, values); } - - return result.recipientId; } public Cursor getBlocked() { 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 47d749fef8..9b5a95d347 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 @@ -158,8 +158,9 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper { private static final int MENTION_CLEANUP_V2 = 77; private static final int REACTION_CLEANUP = 78; private static final int CAPABILITIES_REFACTOR = 79; + private static final int GV1_MIGRATION = 80; - private static final int DATABASE_VERSION = 79; + private static final int DATABASE_VERSION = 80; private static final String DATABASE_NAME = "signal.db"; private final Context context; @@ -1139,6 +1140,26 @@ public class SQLCipherOpenHelper extends SQLiteOpenHelper { db.execSQL("UPDATE recipient SET capabilities = 2 WHERE gv2_capability = -1"); } + if (oldVersion < GV1_MIGRATION) { + db.execSQL("ALTER TABLE groups ADD COLUMN expected_v2_id TEXT DEFAULT NULL"); + db.execSQL("ALTER TABLE groups ADD COLUMN former_v1_members TEXT DEFAULT NULL"); + db.execSQL("CREATE UNIQUE INDEX IF NOT EXISTS expected_v2_id_index ON groups (expected_v2_id)"); + + int count = 0; + try (Cursor cursor = db.rawQuery("SELECT * FROM groups WHERE group_id LIKE '__textsecure_group__!%' AND LENGTH(group_id) = 53", null)) { + while (cursor.moveToNext()) { + String gv1 = CursorUtil.requireString(cursor, "group_id"); + String gv2 = GroupId.parseOrThrow(gv1).requireV1().deriveV2MigrationGroupId().toString(); + + ContentValues values = new ContentValues(); + values.put("expected_v2_id", gv2); + count += db.update("groups", values, "group_id = ?", SqlUtil.buildArgs(gv1)); + } + } + + Log.i(TAG, "Updated " + count + " GV1 groups with expected GV2 IDs."); + } + db.setTransactionSuccessful(); } finally { db.endTransaction(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java index 4f57d551ea..ff6236ed9f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManager.java @@ -202,7 +202,7 @@ public final class GroupManager { public static void updateSelfProfileKeyInGroup(@NonNull Context context, @NonNull GroupId.V2 groupId) throws IOException, GroupChangeBusyException, GroupInsufficientRightsException, GroupNotAMemberException, GroupChangeFailedException { - if (!DatabaseFactory.getGroupDatabase(context).findGroup(groupId)) { + if (!DatabaseFactory.getGroupDatabase(context).groupExists(groupId)) { Log.i(TAG, "Group is not available locally " + groupId); return; }