Verify group ids on peer-to-peer group changes.

This commit is contained in:
Cody Henthorne 2024-11-26 12:24:47 -05:00 committed by Greyson Parrelli
parent 878900c09c
commit 0913b84657
9 changed files with 69 additions and 19 deletions

View file

@ -90,6 +90,7 @@
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_MICROPHONE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_CAMERA" />
<!-- For vestigial telecom integration service -->
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_PHONE_CALL" />

View file

@ -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);

View file

@ -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)
}
}
}

View file

@ -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));
}

View file

@ -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.
* <p>
* 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.
* <p>
* 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<DecryptedGroupChange> decryptChange(GroupChange groupChange, boolean verifySignature)
public Optional<DecryptedGroupChange> 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));
}

View file

@ -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;

View file

@ -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;

View file

@ -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

View file

@ -82,7 +82,7 @@ public final class GroupsV2Operations_decrypt_change_Test {
.changeEpoch(GroupsV2Operations.HIGHEST_KNOWN_EPOCH + 1)
.build();
Optional<DecryptedGroupChange> decryptedGroupChangeOptional = groupOperations.decryptChange(change, false);
Optional<DecryptedGroupChange> 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);
}