diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index 1e65ade2ca..b33459568e 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -90,6 +90,7 @@
+
diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java
index 7ca5faeb4c..26957f44a3 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java
@@ -50,8 +50,8 @@ import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.recipients.RecipientId;
import org.thoughtcrime.securesms.sms.MessageSender;
import org.thoughtcrime.securesms.util.ProfileUtil;
+import org.whispersystems.signalservice.api.groupsv2.DecryptChangeVerificationMode;
import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupResponse;
-import org.whispersystems.signalservice.api.groupsv2.ReceivedGroupSendEndorsements;
import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil;
import org.whispersystems.signalservice.api.groupsv2.GroupCandidate;
import org.whispersystems.signalservice.api.groupsv2.GroupChangeReconstruct;
@@ -61,6 +61,7 @@ import org.whispersystems.signalservice.api.groupsv2.GroupsV2Api;
import org.whispersystems.signalservice.api.groupsv2.GroupsV2Operations;
import org.whispersystems.signalservice.api.groupsv2.InvalidGroupStateException;
import org.whispersystems.signalservice.api.groupsv2.NotAbleToApplyGroupV2ChangeException;
+import org.whispersystems.signalservice.api.groupsv2.ReceivedGroupSendEndorsements;
import org.whispersystems.signalservice.api.push.ServiceId;
import org.whispersystems.signalservice.api.push.ServiceId.ACI;
import org.whispersystems.signalservice.api.push.ServiceId.PNI;
@@ -678,8 +679,7 @@ final class GroupManagerV2 {
GroupChangeResponse changeResponse = commitToServer(changeActions);
GroupChange signedGroupChange = changeResponse.groupChange;
try {
- //noinspection OptionalGetWithoutIsPresent
- decryptedChange = groupOperations.decryptChange(signedGroupChange, false).get();
+ decryptedChange = groupOperations.decryptChange(signedGroupChange, DecryptChangeVerificationMode.alreadyTrusted()).get();
decryptedGroupState = DecryptedGroupUtil.apply(previousGroupState, decryptedChange);
} catch (VerificationFailedException | InvalidGroupStateException | NotAbleToApplyGroupV2ChangeException e) {
Log.w(TAG, e);
@@ -766,7 +766,7 @@ final class GroupManagerV2 {
GroupsV2Operations.GroupOperations groupOperations = groupsV2Operations.forGroup(GroupSecretParams.deriveFromMasterKey(groupMasterKey));
try {
- return groupOperations.decryptChange(GroupChange.ADAPTER.decode(signedGroupChange), true)
+ return groupOperations.decryptChange(GroupChange.ADAPTER.decode(signedGroupChange), DecryptChangeVerificationMode.verify(GroupId.getIdentifierForMasterKey(groupMasterKey)))
.orElse(null);
} catch (VerificationFailedException | InvalidGroupStateException | IOException e) {
Log.w(TAG, "Unable to verify supplied group change", e);
@@ -989,7 +989,7 @@ final class GroupManagerV2 {
{
try {
//noinspection OptionalGetWithoutIsPresent
- return groupOperations.decryptChange(signedGroupChange, false).get();
+ return groupOperations.decryptChange(signedGroupChange, DecryptChangeVerificationMode.alreadyTrusted()).get();
} catch (VerificationFailedException | InvalidGroupStateException | IOException e) {
Log.w(TAG, e);
throw new GroupChangeFailedException(e);
@@ -1159,7 +1159,7 @@ final class GroupManagerV2 {
try {
//noinspection OptionalGetWithoutIsPresent
- DecryptedGroupChange decryptedChange = groupOperations.decryptChange(signedGroupChange, false).get();
+ DecryptedGroupChange decryptedChange = groupOperations.decryptChange(signedGroupChange, DecryptChangeVerificationMode.alreadyTrusted()).get();
DecryptedGroup newGroup = DecryptedGroupUtil.applyWithoutRevisionCheck(decryptedGroup, decryptedChange);
groupDatabase.update(groupId, resetRevision(newGroup, decryptedGroup.revision), null);
diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptChangeVerificationMode.kt b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptChangeVerificationMode.kt
new file mode 100644
index 0000000000..2fc1b5840e
--- /dev/null
+++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptChangeVerificationMode.kt
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2024 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.whispersystems.signalservice.api.groupsv2
+
+import org.signal.libsignal.zkgroup.groups.GroupIdentifier
+
+/**
+ * Details what verification should take place when decrypting a group change.
+ *
+ * @param verify Should perform group change verification during decryption
+ * @param groupId The id this change should apply to and will be verified is set in change payload
+ */
+class DecryptChangeVerificationMode private constructor(
+ @get:JvmName("verify") val verify: Boolean,
+ @get:JvmName("groupId") val groupId: GroupIdentifier?
+) {
+ companion object {
+
+ /**
+ * Use when the changes are already trusted. This would be during group creation or when fetching
+ * group changes directly from the server.
+ */
+ @JvmStatic
+ @get:JvmName("alreadyTrusted")
+ val alreadyTrusted by lazy { DecryptChangeVerificationMode(verify = false, groupId = null) }
+
+ /**
+ * Use when the changes are from an untrusted source like a P2P message of a group change. This
+ * will verify the signature and group id match.
+ */
+ @JvmStatic
+ fun verify(groupId: GroupIdentifier): DecryptChangeVerificationMode {
+ return DecryptChangeVerificationMode(verify = true, groupId = groupId)
+ }
+ }
+}
diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java
index 0ce4797ccb..40f7726311 100644
--- a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java
+++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java
@@ -124,7 +124,7 @@ public class GroupsV2Api {
for (GroupChanges.GroupChangeState change : group.getGroupChanges().groupChanges) {
DecryptedGroup decryptedGroup = change.groupState != null ? groupOperations.decryptGroup(change.groupState) : null;
- DecryptedGroupChange decryptedChange = change.groupChange != null ? groupOperations.decryptChange(change.groupChange, false).orElse(null) : null;
+ DecryptedGroupChange decryptedChange = change.groupChange != null ? groupOperations.decryptChange(change.groupChange, DecryptChangeVerificationMode.alreadyTrusted()).orElse(null) : null;
result.add(new DecryptedGroupChangeLog(decryptedGroup, decryptedChange));
}
diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java
index 4019528390..0f71acf647 100644
--- a/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java
+++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations.java
@@ -7,6 +7,7 @@ import org.signal.libsignal.zkgroup.ServerPublicParams;
import org.signal.libsignal.zkgroup.VerificationFailedException;
import org.signal.libsignal.zkgroup.auth.ClientZkAuthOperations;
import org.signal.libsignal.zkgroup.groups.ClientZkGroupCipher;
+import org.signal.libsignal.zkgroup.groups.GroupIdentifier;
import org.signal.libsignal.zkgroup.groups.GroupSecretParams;
import org.signal.libsignal.zkgroup.groups.ProfileKeyCiphertext;
import org.signal.libsignal.zkgroup.groups.UuidCiphertext;
@@ -45,6 +46,7 @@ import org.whispersystems.signalservice.api.util.UuidUtil;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
@@ -486,14 +488,13 @@ public final class GroupsV2Operations {
}
/**
- * @param verifySignature You might want to avoid verification if you already know it's correct, or you
- * are not going to pass to other clients.
- *
- * Also, if you know it's version 0, do not verify because changes for version 0
- * are not signed, but should be empty.
+ * @param verification You might want to avoid verification if you already know it's correct, or you are not going to pass to other clients.
+ *
+ * Also, if you know it's version 0, do not verify because changes for version 0 are not signed, but should be empty.
+ *
* @return {@link Optional#empty()} if the epoch for the change is higher that this code can decrypt.
*/
- public Optional decryptChange(GroupChange groupChange, boolean verifySignature)
+ public Optional decryptChange(GroupChange groupChange, @Nonnull DecryptChangeVerificationMode verification)
throws IOException, VerificationFailedException, InvalidGroupStateException
{
if (groupChange.changeEpoch > HIGHEST_KNOWN_EPOCH) {
@@ -501,7 +502,14 @@ public final class GroupsV2Operations {
return Optional.empty();
}
- GroupChange.Actions actions = verifySignature ? getVerifiedActions(groupChange) : getActions(groupChange);
+ GroupChange.Actions actions = verification.verify() ? getVerifiedActions(groupChange) : getActions(groupChange);
+
+ if (verification.verify()) {
+ GroupIdentifier groupId = verification.groupId();
+ if (groupId == null || !Arrays.equals(groupId.serialize(), actions.groupId.toByteArray())) {
+ throw new VerificationFailedException("Invalid group id");
+ }
+ }
return Optional.of(decryptChange(actions));
}
diff --git a/libsignal-service/src/main/protowire/DecryptedGroups.proto b/libsignal-service/src/main/protowire/DecryptedGroups.proto
index fb8263ecb3..86b0595438 100644
--- a/libsignal-service/src/main/protowire/DecryptedGroups.proto
+++ b/libsignal-service/src/main/protowire/DecryptedGroups.proto
@@ -75,6 +75,7 @@ message DecryptedGroup {
// Keep field numbers in step
message DecryptedGroupChange {
bytes editorServiceIdBytes = 1;
+ reserved 25; // groupId used only during verification + decrypt, only provided by server
uint32 revision = 2;
repeated DecryptedMember newMembers = 3;
repeated bytes deleteMembers = 4;
diff --git a/libsignal-service/src/main/protowire/Groups.proto b/libsignal-service/src/main/protowire/Groups.proto
index 63970c7741..e202eb2aaf 100644
--- a/libsignal-service/src/main/protowire/Groups.proto
+++ b/libsignal-service/src/main/protowire/Groups.proto
@@ -183,6 +183,7 @@ message GroupChange {
}
bytes sourceServiceId = 1;
+ bytes groupId = 25; // Only set when receiving from server
uint32 revision = 2;
repeated AddMemberAction addMembers = 3;
repeated DeleteMemberAction deleteMembers = 4;
diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java
index 24ff9e4f9a..7a58d848ca 100644
--- a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java
+++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_changeIsEmpty_Test.java
@@ -22,7 +22,7 @@ public final class GroupChangeUtil_changeIsEmpty_Test {
int maxFieldFound = getMaxDeclaredFieldNumber(GroupChange.Actions.class);
assertEquals("GroupChangeUtil and its tests need updating to account for new fields on " + GroupChange.Actions.class.getName(),
- 24, maxFieldFound);
+ 25, maxFieldFound);
}
@Test
diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java
index 1b963514f9..1ba884d1e7 100644
--- a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java
+++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Operations_decrypt_change_Test.java
@@ -82,7 +82,7 @@ public final class GroupsV2Operations_decrypt_change_Test {
.changeEpoch(GroupsV2Operations.HIGHEST_KNOWN_EPOCH + 1)
.build();
- Optional decryptedGroupChangeOptional = groupOperations.decryptChange(change, false);
+ Optional decryptedGroupChangeOptional = groupOperations.decryptChange(change, DecryptChangeVerificationMode.alreadyTrusted());
assertFalse(decryptedGroupChangeOptional.isPresent());
}
@@ -159,7 +159,7 @@ public final class GroupsV2Operations_decrypt_change_Test {
actions.addMembers(List.of(new GroupChange.Actions.AddMemberAction.Builder().added(new Member.Builder().role(Member.Role.DEFAULT)
.presentation(ByteString.of(randomPresentation)).build()).build()));
- groupOperations.decryptChange(new GroupChange.Builder().actions(actions.build().encodeByteString()).build(), false);
+ groupOperations.decryptChange(new GroupChange.Builder().actions(actions.build().encodeByteString()).build(), DecryptChangeVerificationMode.alreadyTrusted());
}
@Test
@@ -180,7 +180,7 @@ public final class GroupsV2Operations_decrypt_change_Test {
actions.deleteMembers(List.of(new GroupChange.Actions.DeleteMemberAction.Builder().deletedUserId(ByteString.of(randomPresentation)).build()));
- groupOperations.decryptChange(new GroupChange.Builder().actions(actions.build().encodeByteString()).build(), false);
+ groupOperations.decryptChange(new GroupChange.Builder().actions(actions.build().encodeByteString()).build(), DecryptChangeVerificationMode.alreadyTrusted());
}
@Test
@@ -518,7 +518,7 @@ public final class GroupsV2Operations_decrypt_change_Test {
private DecryptedGroupChange decrypt(GroupChange build) {
try {
- return groupOperations.decryptChange(build, false).get();
+ return groupOperations.decryptChange(build, DecryptChangeVerificationMode.alreadyTrusted()).get();
} catch (IOException | VerificationFailedException | InvalidGroupStateException e) {
throw new AssertionError(e);
}