From 3cffaddc0ab329e6d4c531d7b0fc458ab0203bf6 Mon Sep 17 00:00:00 2001 From: Alan Evans Date: Thu, 10 Sep 2020 14:39:29 -0300 Subject: [PATCH] Validate incoming Group lengths and remote delete entries if wrong. Ignore incoming messages with bad V1 group lengths. --- .../securesms/database/RecipientDatabase.java | 12 ++-- .../securesms/groups/GroupId.java | 28 +++++---- .../storage/GroupV1ConflictMerger.java | 17 +++++- .../storage/GroupV2ConflictMerger.java | 20 ++++--- .../securesms/storage/StorageSyncHelper.java | 5 +- .../securesms/util/GroupUtil.java | 2 +- .../securesms/groups/GroupIdTest.java | 32 +++++++--- .../storage/GroupV1ConflictMergerTest.java | 37 ++++++++++-- .../storage/GroupV2ConflictMergerTest.java | 60 ++++++++++++------- .../storage/StorageSyncHelperTest.java | 52 ++++++++++++++-- .../api/storage/SignalGroupV2Record.java | 30 ++++++---- 11 files changed, 218 insertions(+), 77 deletions(-) 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 41cd650588..68d34d02f6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java @@ -859,13 +859,14 @@ public class RecipientDatabase extends Database { for (SignalGroupV2Record insert : groupV2Inserts) { db.insertOrThrow(TABLE_NAME, null, getValuesForStorageGroupV2(insert)); - GroupId.V2 groupId = GroupId.v2(insert.getMasterKey()); - Recipient recipient = Recipient.externalGroup(context, groupId); + GroupMasterKey masterKey = insert.getMasterKeyOrThrow(); + GroupId.V2 groupId = GroupId.v2(masterKey); + Recipient recipient = Recipient.externalGroup(context, groupId); Log.i(TAG, "Creating restore placeholder for " + groupId); DatabaseFactory.getGroupDatabase(context) - .create(insert.getMasterKey(), + .create(masterKey, DecryptedGroup.newBuilder() .setRevision(GroupsV2StateProcessor.RESTORE_PLACEHOLDER_REVISION) .build()); @@ -886,7 +887,8 @@ public class RecipientDatabase extends Database { throw new AssertionError("Had an update, but it didn't match any rows!"); } - Recipient recipient = Recipient.externalGroup(context, GroupId.v2(update.getOld().getMasterKey())); + GroupMasterKey masterKey = update.getOld().getMasterKeyOrThrow(); + Recipient recipient = Recipient.externalGroup(context, GroupId.v2(masterKey)); threadDatabase.setArchived(recipient.getId(), update.getNew().isArchived()); needsRefresh.add(recipient.getId()); @@ -1031,7 +1033,7 @@ public class RecipientDatabase extends Database { private static @NonNull ContentValues getValuesForStorageGroupV2(@NonNull SignalGroupV2Record groupV2) { ContentValues values = new ContentValues(); - values.put(GROUP_ID, GroupId.v2(groupV2.getMasterKey()).toString()); + values.put(GROUP_ID, GroupId.v2(groupV2.getMasterKeyOrThrow()).toString()); values.put(GROUP_TYPE, GroupType.SIGNAL_V2.getId()); values.put(PROFILE_SHARING, groupV2.isProfileSharingEnabled() ? "1" : "0"); values.put(BLOCKED, groupV2.isBlocked() ? "1" : "0"); 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 56d1c02cf5..86e285540f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupId.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupId.java @@ -19,6 +19,7 @@ public abstract class GroupId { private static final String ENCODED_MMS_GROUP_PREFIX = "__signal_mms_group__!"; private static final int MMS_BYTE_LENGTH = 16; private static final int V1_MMS_BYTE_LENGTH = 16; + private static final int V1_BYTE_LENGTH = 16; private static final int V2_BYTE_LENGTH = GroupIdentifier.SIZE; private final String encodedId; @@ -46,6 +47,13 @@ public abstract class GroupId { return new GroupId.V1(gv1GroupIdBytes); } + public static @NonNull GroupId.V1 v1Exact(byte[] gv1GroupIdBytes) throws BadGroupIdException { + if (gv1GroupIdBytes.length != V1_BYTE_LENGTH) { + throw new BadGroupIdException(); + } + return new GroupId.V1(gv1GroupIdBytes); + } + public static GroupId.V1 createV1(@NonNull SecureRandom secureRandom) { return v1orThrow(Util.getSecretBytes(secureRandom, V1_MMS_BYTE_LENGTH)); } @@ -54,15 +62,11 @@ public abstract class GroupId { return mms(Util.getSecretBytes(secureRandom, MMS_BYTE_LENGTH)); } - public static GroupId.V2 v2orThrow(@NonNull byte[] bytes) { - try { - return v2(bytes); - } catch (BadGroupIdException e) { - throw new AssertionError(e); - } - } - - public static GroupId.V2 v2(@NonNull byte[] bytes) throws BadGroupIdException { + /** + * Private because it's too easy to pass the {@link GroupMasterKey} bytes directly to this as they + * are the same length as the {@link GroupIdentifier}. + */ + private static GroupId.V2 v2(@NonNull byte[] bytes) throws BadGroupIdException { if (bytes.length != V2_BYTE_LENGTH) { throw new BadGroupIdException(); } @@ -70,7 +74,11 @@ public abstract class GroupId { } public static GroupId.V2 v2(@NonNull GroupIdentifier groupIdentifier) { - return v2orThrow(groupIdentifier.serialize()); + try { + return v2(groupIdentifier.serialize()); + } catch (BadGroupIdException e) { + throw new AssertionError(e); + } } public static GroupId.V2 v2(@NonNull GroupMasterKey masterKey) { 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 bd8f2fcff4..dc7c03db0b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMerger.java +++ b/app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMerger.java @@ -5,16 +5,16 @@ import androidx.annotation.NonNull; import com.annimon.stream.Collectors; import com.annimon.stream.Stream; +import org.thoughtcrime.securesms.groups.BadGroupIdException; import org.thoughtcrime.securesms.groups.GroupId; import org.whispersystems.libsignal.util.guava.Optional; import org.whispersystems.signalservice.api.storage.SignalGroupV1Record; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.Map; -class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger { +final class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger { private final Map localByGroupId; @@ -29,7 +29,9 @@ class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger getInvalidEntries(@NonNull Collection remoteRecords) { - return Collections.emptySet(); + return Stream.of(remoteRecords) + .filterNot(GroupV1ConflictMerger::isValidGroupId) + .toList(); } @Override @@ -54,4 +56,13 @@ class GroupV1ConflictMerger implements StorageSyncHelper.ConflictMerger { +final class GroupV2ConflictMerger implements StorageSyncHelper.ConflictMerger { - private final Map localByGroupId; + private final Map localByMasterKeyBytes; GroupV2ConflictMerger(@NonNull Collection localOnly) { - localByGroupId = Stream.of(localOnly).collect(Collectors.toMap(SignalGroupV2Record::getMasterKey, g -> g)); + localByMasterKeyBytes = Stream.of(localOnly).collect(Collectors.toMap((SignalGroupV2Record signalGroupV2Record) -> ByteString.copyFrom(signalGroupV2Record.getMasterKeyBytes()), g -> g)); } @Override public @NonNull Optional getMatching(@NonNull SignalGroupV2Record record) { - return Optional.fromNullable(localByGroupId.get(record.getMasterKey())); + return Optional.fromNullable(localByMasterKeyBytes.get(ByteString.copyFrom(record.getMasterKeyBytes()))); } @Override public @NonNull Collection getInvalidEntries(@NonNull Collection remoteRecords) { - return Collections.emptySet(); + return Stream.of(remoteRecords) + .filterNot(GroupV2ConflictMerger::isValidMasterKey) + .toList(); } @Override @@ -47,11 +49,15 @@ class GroupV2ConflictMerger implements StorageSyncHelper.ConflictMerger remoteDeletes = new HashSet<>(); remoteDeletes.addAll(contactMergeResult.remoteDeletes); remoteDeletes.addAll(groupV1MergeResult.remoteDeletes); + remoteDeletes.addAll(groupV2MergeResult.remoteDeletes); remoteDeletes.addAll(accountMergeResult.remoteDeletes); return new MergeResult(contactMergeResult.localInserts, @@ -603,8 +604,8 @@ public final class StorageSyncHelper { @Override public @NonNull String toString() { return String.format(Locale.ENGLISH, - "localContactInserts: %d, localContactUpdates: %d, localGroupV1Inserts: %d, localGroupV1Updates: %d, localGroupV2Inserts: %d, localGroupV2Updates: %d, localUnknownInserts: %d, localUnknownDeletes: %d, localAccountUpdate: %b, remoteInserts: %d, remoteUpdates: %d", - localContactInserts.size(), localContactUpdates.size(), localGroupV1Inserts.size(), localGroupV1Updates.size(), localGroupV2Inserts.size(), localGroupV2Updates.size(), localUnknownInserts.size(), localUnknownDeletes.size(), localAccountUpdate.isPresent(), remoteInserts.size(), remoteUpdates.size()); + "localContactInserts: %d, localContactUpdates: %d, localGroupV1Inserts: %d, localGroupV1Updates: %d, localGroupV2Inserts: %d, localGroupV2Updates: %d, localUnknownInserts: %d, localUnknownDeletes: %d, localAccountUpdate: %b, remoteInserts: %d, remoteUpdates: %d, remoteDeletes: %d", + localContactInserts.size(), localContactUpdates.size(), localGroupV1Inserts.size(), localGroupV1Updates.size(), localGroupV2Inserts.size(), localGroupV2Updates.size(), localUnknownInserts.size(), localUnknownDeletes.size(), localAccountUpdate.isPresent(), remoteInserts.size(), remoteUpdates.size(), remoteDeletes.size()); } } 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 73da984d02..93a7cc7ef0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/GroupUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/GroupUtil.java @@ -38,7 +38,7 @@ public final class GroupUtil { throws BadGroupIdException { if (groupContext.getGroupV1().isPresent()) { - return GroupId.v1(groupContext.getGroupV1().get().getGroupId()); + return GroupId.v1Exact(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 b919150abe..a1e4ca3384 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/groups/GroupIdTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/groups/GroupIdTest.java @@ -261,19 +261,19 @@ public final class GroupIdTest { groupId.requireV2(); } + @Test(expected = BadGroupIdException.class) + public void cannot_create_v1_with_a_v2_length() throws IOException, BadGroupIdException { + GroupId.v1(Hex.fromStringCondensed("9f475f59b2518bff6df22e820803f0e3585bd99e686fa7e7fbfc2f92fd5d953e")); + } + @Test(expected = AssertionError.class) - public void cannot_create_v1_with_a_v2_length() throws IOException { + public void cannot_create_v1_with_a_v2_length_assert() throws IOException { GroupId.v1orThrow(Hex.fromStringCondensed("9f475f59b2518bff6df22e820803f0e3585bd99e686fa7e7fbfc2f92fd5d953e")); } @Test(expected = BadGroupIdException.class) - public void cannot_create_v2_with_a_v1_length() throws IOException, BadGroupIdException { - GroupId.v2(Hex.fromStringCondensed("000102030405060708090a0b0c0d0e0f")); - } - - @Test(expected = AssertionError.class) - public void cannot_create_v2_with_a_v1_length_assert() throws IOException { - GroupId.v2orThrow(Hex.fromStringCondensed("000102030405060708090a0b0c0d0e0f")); + public void cannot_create_v1_with_wrong_length() throws IOException, BadGroupIdException { + GroupId.v1Exact(Hex.fromStringCondensed("000102030405060708090a0b0c0d0e")); } @Test @@ -292,6 +292,22 @@ public final class GroupIdTest { assertTrue(v1.isV1()); } + @Test + public void v1_static_factory() throws BadGroupIdException { + 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()); + } + + @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 }); + + assertEquals("__textsecure_group__!090a0b0c0d0e0f000102030405060708", v1.toString()); + assertTrue(v1.isV1()); + } + @Test public void parse_bytes_to_v1_via_push() throws BadGroupIdException { GroupId.V1 v1 = GroupId.push(new byte[]{ 9, 10, 11, 12, 13, 14, 15, 0, 1, 2, 3, 4, 5, 6, 7, 8 }).requireV1(); diff --git a/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMergerTest.java b/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMergerTest.java index 3eb276c446..c47b981bcf 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMergerTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV1ConflictMergerTest.java @@ -1,10 +1,11 @@ package org.thoughtcrime.securesms.storage; import org.junit.Test; -import org.thoughtcrime.securesms.storage.GroupV1ConflictMerger; import org.thoughtcrime.securesms.storage.StorageSyncHelper.KeyGenerator; import org.whispersystems.signalservice.api.storage.SignalGroupV1Record; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import static org.junit.Assert.assertArrayEquals; @@ -14,10 +15,11 @@ import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.when; import static org.thoughtcrime.securesms.testutil.TestHelpers.byteArray; -public class GroupV1ConflictMergerTest { +public final class GroupV1ConflictMergerTest { + + private static final byte[] GENERATED_KEY = byteArray(8675309); + private static final KeyGenerator KEY_GENERATOR = mock(KeyGenerator.class); - private static byte[] GENERATED_KEY = byteArray(8675309); - private static KeyGenerator KEY_GENERATOR = mock(KeyGenerator.class); static { when(KEY_GENERATOR.generate()).thenReturn(GENERATED_KEY); } @@ -78,4 +80,31 @@ public class GroupV1ConflictMergerTest { assertEquals(local, merged); } + + @Test + public void merge_excludeBadGroupId() { + SignalGroupV1Record badRemote = new SignalGroupV1Record.Builder(byteArray(1), badGroupKey(99)) + .setBlocked(false) + .setProfileSharingEnabled(true) + .setArchived(true) + .build(); + + SignalGroupV1Record goodRemote = new SignalGroupV1Record.Builder(byteArray(1), groupKey(99)) + .setBlocked(false) + .setProfileSharingEnabled(true) + .setArchived(true) + .build(); + + Collection invalid = new GroupV1ConflictMerger(Collections.emptyList()).getInvalidEntries(Arrays.asList(badRemote, goodRemote)); + + assertEquals(Collections.singletonList(badRemote), invalid); + } + + private static byte[] groupKey(int value) { + return byteArray(value, 16); + } + + private static byte[] badGroupKey(int value) { + return byteArray(value, 32); + } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV2ConflictMergerTest.java b/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV2ConflictMergerTest.java index 19ea87b226..793ace89dc 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV2ConflictMergerTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/storage/GroupV2ConflictMergerTest.java @@ -1,11 +1,11 @@ package org.thoughtcrime.securesms.storage; import org.junit.Test; -import org.signal.zkgroup.InvalidInputException; -import org.signal.zkgroup.groups.GroupMasterKey; import org.thoughtcrime.securesms.storage.StorageSyncHelper.KeyGenerator; import org.whispersystems.signalservice.api.storage.SignalGroupV2Record; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import static org.junit.Assert.assertArrayEquals; @@ -15,10 +15,11 @@ import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.when; import static org.thoughtcrime.securesms.testutil.TestHelpers.byteArray; -public class GroupV2ConflictMergerTest { +public final class GroupV2ConflictMergerTest { + + private static final byte[] GENERATED_KEY = byteArray(8675309); + private static final KeyGenerator KEY_GENERATOR = mock(KeyGenerator.class); - private static byte[] GENERATED_KEY = byteArray(8675309); - private static KeyGenerator KEY_GENERATOR = mock(KeyGenerator.class); static { when(KEY_GENERATOR.generate()).thenReturn(GENERATED_KEY); } @@ -26,20 +27,20 @@ public class GroupV2ConflictMergerTest { @Test public void merge_alwaysPreferRemote_exceptProfileSharingIsEitherOr() { SignalGroupV2Record remote = new SignalGroupV2Record.Builder(byteArray(1), groupKey(100)) - .setBlocked(false) - .setProfileSharingEnabled(false) - .setArchived(false) - .build(); + .setBlocked(false) + .setProfileSharingEnabled(false) + .setArchived(false) + .build(); SignalGroupV2Record local = new SignalGroupV2Record.Builder(byteArray(2), groupKey(100)) - .setBlocked(true) - .setProfileSharingEnabled(true) - .setArchived(true) - .build(); + .setBlocked(true) + .setProfileSharingEnabled(true) + .setArchived(true) + .build(); SignalGroupV2Record merged = new GroupV2ConflictMerger(Collections.singletonList(local)).merge(remote, local, KEY_GENERATOR); assertArrayEquals(GENERATED_KEY, merged.getId().getRaw()); - assertEquals(groupKey(100), merged.getMasterKey()); + assertArrayEquals(groupKey(100), merged.getMasterKeyBytes()); assertFalse(merged.isBlocked()); assertFalse(merged.isArchived()); } @@ -80,11 +81,30 @@ public class GroupV2ConflictMergerTest { assertEquals(local, merged); } - private static GroupMasterKey groupKey(int value) { - try { - return new GroupMasterKey(byteArray(value, 32)); - } catch (InvalidInputException e) { - throw new AssertionError(e); - } + @Test + public void merge_excludeBadGroupId() { + SignalGroupV2Record badRemote = new SignalGroupV2Record.Builder(byteArray(1), badGroupKey(99)) + .setBlocked(false) + .setProfileSharingEnabled(true) + .setArchived(true) + .build(); + + SignalGroupV2Record goodRemote = new SignalGroupV2Record.Builder(byteArray(1), groupKey(99)) + .setBlocked(false) + .setProfileSharingEnabled(true) + .setArchived(true) + .build(); + + Collection invalid = new GroupV2ConflictMerger(Collections.emptyList()).getInvalidEntries(Arrays.asList(badRemote, goodRemote)); + + assertEquals(Collections.singletonList(badRemote), invalid); + } + + private static byte[] groupKey(int value) { + return byteArray(value, 32); + } + + private static byte[] badGroupKey(int value) { + return byteArray(value, 16); } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java b/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java index 21a385aad6..0088160a16 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java +++ b/app/src/test/java/org/thoughtcrime/securesms/storage/StorageSyncHelperTest.java @@ -129,6 +129,34 @@ public final class StorageSyncHelperTest { assertEquals(setOf(remote1), result.getRemoteDeletes()); } + @Test + public void resolveConflict_contact_deleteBadGv1() { + SignalGroupV1Record remote1 = badGroupV1(1, 1, true, false); + SignalGroupV1Record local1 = groupV1(2, 1, true, true); + + MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1)); + + assertTrue(result.getLocalContactInserts().isEmpty()); + assertTrue(result.getLocalContactUpdates().isEmpty()); + assertEquals(setOf(record(local1)), result.getRemoteInserts()); + assertTrue(result.getRemoteUpdates().isEmpty()); + assertEquals(setOf(remote1), result.getRemoteDeletes()); + } + + @Test + public void resolveConflict_contact_deleteBadGv2() { + SignalGroupV2Record remote1 = badGroupV2(1, 2, true, false); + SignalGroupV2Record local1 = groupV2(2, 2, true, false); + + MergeResult result = StorageSyncHelper.resolveConflict(recordSetOf(remote1), recordSetOf(local1)); + + assertTrue(result.getLocalContactInserts().isEmpty()); + assertTrue(result.getLocalContactUpdates().isEmpty()); + assertEquals(setOf(record(local1)), result.getRemoteInserts()); + assertTrue(result.getRemoteUpdates().isEmpty()); + assertEquals(setOf(remote1), result.getRemoteDeletes()); + } + @Test public void resolveConflict_contact_sameAsRemote() { SignalContactRecord remote1 = contact(1, UUID_A, E164_A, "a"); @@ -417,7 +445,15 @@ public final class StorageSyncHelperTest { boolean blocked, boolean profileSharing) { - return new SignalGroupV1Record.Builder(byteArray(key), byteArray(groupId)).setBlocked(blocked).setProfileSharingEnabled(profileSharing).build(); + return new SignalGroupV1Record.Builder(byteArray(key), byteArray(groupId, 16)).setBlocked(blocked).setProfileSharingEnabled(profileSharing).build(); + } + + private static SignalGroupV1Record badGroupV1(int key, + int groupId, + boolean blocked, + boolean profileSharing) + { + return new SignalGroupV1Record.Builder(byteArray(key), byteArray(groupId, 42)).setBlocked(blocked).setProfileSharingEnabled(profileSharing).build(); } private static SignalGroupV2Record groupV2(int key, @@ -425,11 +461,15 @@ public final class StorageSyncHelperTest { boolean blocked, boolean profileSharing) { - try { - return new SignalGroupV2Record.Builder(byteArray(key), new GroupMasterKey(byteArray(groupId, 32))).setBlocked(blocked).setProfileSharingEnabled(profileSharing).build(); - } catch (InvalidInputException e) { - throw new AssertionError(e); - } + return new SignalGroupV2Record.Builder(byteArray(key), byteArray(groupId, 32)).setBlocked(blocked).setProfileSharingEnabled(profileSharing).build(); + } + + private static SignalGroupV2Record badGroupV2(int key, + int groupId, + boolean blocked, + boolean profileSharing) + { + return new SignalGroupV2Record.Builder(byteArray(key), byteArray(groupId, 42)).setBlocked(blocked).setProfileSharingEnabled(profileSharing).build(); } private static StorageSyncHelper.RecordUpdate update(E oldRecord, E newRecord) { diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java index cb23c0afe1..da81ba2e74 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalGroupV2Record.java @@ -11,20 +11,16 @@ import java.util.Objects; public final class SignalGroupV2Record implements SignalRecord { - private final StorageId id; - private final GroupV2Record proto; - private final GroupMasterKey masterKey; - private final boolean hasUnknownFields; + private final StorageId id; + private final GroupV2Record proto; + private final byte[] masterKey; + private final boolean hasUnknownFields; public SignalGroupV2Record(StorageId id, GroupV2Record proto) { this.id = id; this.proto = proto; this.hasUnknownFields = ProtoUtil.hasUnknownFields(proto); - try { - this.masterKey = new GroupMasterKey(proto.getMasterKey().toByteArray()); - } catch (InvalidInputException e) { - throw new AssertionError(e); - } + this.masterKey = proto.getMasterKey().toByteArray(); } @Override @@ -40,10 +36,18 @@ public final class SignalGroupV2Record implements SignalRecord { return hasUnknownFields ? proto.toByteArray() : null; } - public GroupMasterKey getMasterKey() { + public byte[] getMasterKeyBytes() { return masterKey; } + public GroupMasterKey getMasterKeyOrThrow() { + try { + return new GroupMasterKey(masterKey); + } catch (InvalidInputException e) { + throw new AssertionError(e); + } + } + public boolean isBlocked() { return proto.getBlocked(); } @@ -81,10 +85,14 @@ public final class SignalGroupV2Record implements SignalRecord { private byte[] unknownFields; public Builder(byte[] rawId, GroupMasterKey masterKey) { + this(rawId, masterKey.serialize()); + } + + public Builder(byte[] rawId, byte[] masterKey) { this.id = StorageId.forGroupV2(rawId); this.builder = GroupV2Record.newBuilder(); - builder.setMasterKey(ByteString.copyFrom(masterKey.serialize())); + builder.setMasterKey(ByteString.copyFrom(masterKey)); } public Builder setUnknownFields(byte[] serializedUnknowns) {