From 67669d536e2e8caeca6e8edb3c7d195c07f97a11 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Mon, 27 Jul 2026 00:50:49 +0200 Subject: [PATCH] fix: harden private group membership --- .../android/groups/GroupCoordinator.kt | 34 ++++- .../com/bitchat/android/ui/ChatViewModel.kt | 9 ++ .../android/groups/GroupCoordinatorTest.kt | 124 ++++++++++++++++++ 3 files changed, 166 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/bitchat/android/groups/GroupCoordinator.kt b/app/src/main/java/com/bitchat/android/groups/GroupCoordinator.kt index 30a0c59a..12b69dcf 100644 --- a/app/src/main/java/com/bitchat/android/groups/GroupCoordinator.kt +++ b/app/src/main/java/com/bitchat/android/groups/GroupCoordinator.kt @@ -2,6 +2,7 @@ package com.bitchat.android.groups import com.bitchat.android.model.BitchatMessage import com.bitchat.android.model.DeliveryStatus +import com.bitchat.android.model.PeerCapabilities import java.util.Date import java.util.UUID @@ -15,6 +16,23 @@ data class GroupCommandResult( val message: String ) +enum class PeerGroupCapability { + SUPPORTED, + UNSUPPORTED, + UNKNOWN; + + companion object { + fun fromPeerState( + capabilities: PeerCapabilities?, + hasVerifiedAnnouncement: Boolean + ): PeerGroupCapability = when { + capabilities?.contains(PeerCapabilities.GROUPS) == true -> SUPPORTED + capabilities != null || hasVerifiedAnnouncement -> UNSUPPORTED + else -> UNKNOWN + } + } +} + interface GroupCoordinatorContext { val groupStore: GroupStore val nickname: String @@ -27,6 +45,7 @@ interface GroupCoordinatorContext { fun peerIDForNickname(nickname: String): String? fun isPeerConnected(peerID: String): Boolean + fun peerGroupCapability(peerID: String): PeerGroupCapability fun peerNickname(peerID: String): String? fun peerIdentity(peerID: String): GroupPeerIdentity? fun connectedPeerID(fingerprint: String): String? @@ -76,6 +95,13 @@ class GroupCoordinator(private val context: GroupCoordinatorContext) { val peerID = context.peerIDForNickname(nickname) ?: return error("user '$nickname' was not found") if (!context.isPeerConnected(peerID)) return error("$nickname is not connected") + when (context.peerGroupCapability(peerID)) { + PeerGroupCapability.SUPPORTED -> Unit + PeerGroupCapability.UNSUPPORTED -> + return error("$nickname does not support private groups") + PeerGroupCapability.UNKNOWN -> + return error("private-group support for $nickname is not confirmed yet; try again") + } val identity = context.peerIdentity(peerID) ?: return error("$nickname does not have a verified mesh identity") if (group.isMember(identity.fingerprint)) return error("$nickname is already a member") @@ -124,6 +150,9 @@ class GroupCoordinator(private val context: GroupCoordinatorContext) { fun leaveGroup(): GroupCommandResult { val group = selectedGroup() ?: return error("open a private group first") + if (isCreator(group) && group.members.size > 1) { + return error("remove all other members before leaving this group") + } context.closeGroupConversation() context.removeGroupConversation(group.peerID) context.groupStore.removeGroup(group.groupID) @@ -260,6 +289,10 @@ class GroupCoordinator(private val context: GroupCoordinatorContext) { val ownFingerprint = context.myNoiseFingerprint() val existing = context.groupStore.group(state.groupID) + // Reject stale state before interpreting a missing-self roster as a + // removal. Otherwise an old, valid removal notice could delete a + // membership restored by a later creator-signed re-invite. + if (existing != null && state.epoch < existing.epoch) return if (state.members.none { it.fingerprint == ownFingerprint }) { if (existing != null) { if (context.selectedConversationID == existing.peerID) { @@ -271,7 +304,6 @@ class GroupCoordinator(private val context: GroupCoordinatorContext) { } return } - if (existing != null && state.epoch < existing.epoch) return if (!context.groupStore.upsert(state.asGroup(), state.key)) return if (existing == null) { diff --git a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt index fc720fd5..565119b4 100644 --- a/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt +++ b/app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt @@ -37,6 +37,7 @@ import com.bitchat.android.groups.GroupCoordinatorContext import com.bitchat.android.groups.GroupIds import com.bitchat.android.groups.GroupPeerIdentity import com.bitchat.android.groups.GroupStore +import com.bitchat.android.groups.PeerGroupCapability /** * Refactored ChatViewModel - Main coordinator for bitchat functionality @@ -150,6 +151,14 @@ class ChatViewModel( override fun isPeerConnected(peerID: String): Boolean = mesh.getPeerInfo(peerID)?.isConnected == true && mesh.hasEstablishedSession(peerID) + override fun peerGroupCapability(peerID: String): PeerGroupCapability { + val peerInfo = mesh.getPeerInfo(peerID) ?: return PeerGroupCapability.UNKNOWN + return PeerGroupCapability.fromPeerState( + peerInfo.capabilities, + peerInfo.hasVerifiedAnnouncement + ) + } + override fun peerNickname(peerID: String): String? = mesh.getPeerNicknames()[peerID] diff --git a/app/src/test/kotlin/com/bitchat/android/groups/GroupCoordinatorTest.kt b/app/src/test/kotlin/com/bitchat/android/groups/GroupCoordinatorTest.kt index 679d7520..c7d6b24c 100644 --- a/app/src/test/kotlin/com/bitchat/android/groups/GroupCoordinatorTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/groups/GroupCoordinatorTest.kt @@ -1,6 +1,7 @@ package com.bitchat.android.groups import com.bitchat.android.model.BitchatMessage +import com.bitchat.android.model.PeerCapabilities import java.security.MessageDigest import java.util.Date import org.bouncycastle.crypto.params.Ed25519PrivateKeyParameters @@ -14,6 +15,32 @@ import org.junit.Assert.assertTrue import org.junit.Test class GroupCoordinatorTest { + @Test + fun `peer group capability distinguishes unknown unsupported and supported state`() { + assertEquals( + PeerGroupCapability.UNKNOWN, + PeerGroupCapability.fromPeerState(null, hasVerifiedAnnouncement = false) + ) + assertEquals( + PeerGroupCapability.UNSUPPORTED, + PeerGroupCapability.fromPeerState(null, hasVerifiedAnnouncement = true) + ) + assertEquals( + PeerGroupCapability.UNSUPPORTED, + PeerGroupCapability.fromPeerState( + PeerCapabilities.PRIVATE_MEDIA, + hasVerifiedAnnouncement = false + ) + ) + assertEquals( + PeerGroupCapability.SUPPORTED, + PeerGroupCapability.fromPeerState( + PeerCapabilities.GROUPS, + hasVerifiedAnnouncement = false + ) + ) + } + @Test fun `creator invite rotates epoch and sends creator-signed state`() { val localKey = privateKey(0x11) @@ -47,6 +74,38 @@ class GroupCoordinatorTest { assertArrayEquals(updatedKey, state.key) } + @Test + fun `invite requires confirmed group capability`() { + val localKey = privateKey(0x12) + val inviteeKey = privateKey(0x13) + val peerID = "13".repeat(8) + val context = FakeGroupContext(localKey, "12".repeat(32)) + context.peerIDs["alice"] = peerID + context.connected += peerID + context.peerNames[peerID] = "alice" + context.identities[peerID] = GroupPeerIdentity( + "13".repeat(32), + inviteeKey.generatePublicKey().encoded + ) + val coordinator = GroupCoordinator(context) + assertTrue(coordinator.createGroup("trail crew").success) + val original = context.groupStore.groups.value.single() + + context.groupCapabilities[peerID] = PeerGroupCapability.UNSUPPORTED + val unsupported = coordinator.inviteMember("@alice") + assertFalse(unsupported.success) + assertTrue(unsupported.message.contains("does not support")) + assertEquals(original, context.groupStore.groups.value.single()) + assertTrue(context.invites.isEmpty()) + + context.groupCapabilities[peerID] = PeerGroupCapability.UNKNOWN + val unknown = coordinator.inviteMember("@alice") + assertFalse(unknown.success) + assertTrue(unknown.message.contains("not confirmed")) + assertEquals(original, context.groupStore.groups.value.single()) + assertTrue(context.invites.isEmpty()) + } + @Test fun `invite is accepted only from authenticated creator`() { val creatorKey = privateKey(0x31) @@ -104,6 +163,68 @@ class GroupCoordinatorTest { assertTrue(context.systemMessages.single().contains("removed")) } + @Test + fun `stale removal state cannot delete a newer membership`() { + val creatorKey = privateKey(0x76) + val localKey = privateKey(0x77) + val creatorStatic = ByteArray(32) { 0x78 } + val creatorFingerprint = fingerprint(creatorStatic) + val localFingerprint = "79".repeat(32) + val context = FakeGroupContext(localKey, localFingerprint) + val current = incomingGroup( + creatorKey, + creatorFingerprint, + localKey, + localFingerprint + ).copy(epoch = 3) + assertTrue(context.groupStore.upsert(current, ByteArray(32) { 0x7a })) + context.selected = current.peerID + + val staleRemoval = current.copy( + epoch = 2, + members = listOf(current.members.first()) + ) + val payload = signedState(staleRemoval, creatorKey, ByteArray(32)) + GroupCoordinator(context).handleKeyUpdate("creator", creatorStatic, payload) + + assertEquals(current, context.groupStore.group(current.groupID)) + assertNotNull(context.groupStore.key(current.groupID)) + assertEquals(current.peerID, context.selected) + assertTrue(context.removedConversations.isEmpty()) + assertTrue(context.systemMessages.isEmpty()) + } + + @Test + fun `creator cannot leave while other members remain`() { + val localKey = privateKey(0x14) + val memberKey = privateKey(0x15) + val context = FakeGroupContext(localKey, "14".repeat(32)) + val coordinator = GroupCoordinator(context) + assertTrue(coordinator.createGroup("trail crew").success) + val created = context.groupStore.groups.value.single() + val withMember = created.copy( + members = created.members + GroupMember( + "15".repeat(32), + memberKey.generatePublicKey().encoded, + "alice" + ) + ) + assertTrue( + context.groupStore.upsert( + withMember, + context.groupStore.key(created.groupID)!! + ) + ) + + val result = coordinator.leaveGroup() + + assertFalse(result.success) + assertTrue(result.message.contains("remove all other members")) + assertEquals(withMember, context.groupStore.group(created.groupID)) + assertEquals(withMember.peerID, context.selected) + assertTrue(context.removedConversations.isEmpty()) + } + @Test fun `group message requires a roster sender and deduplicates`() { val creatorKey = privateKey(0x21) @@ -222,6 +343,7 @@ private class FakeGroupContext( var selected: String? = null val peerIDs = mutableMapOf() val connected = mutableSetOf() + val groupCapabilities = mutableMapOf() val peerNames = mutableMapOf() val identities = mutableMapOf() val connectedFingerprints = mutableMapOf() @@ -246,6 +368,8 @@ private class FakeGroupContext( override fun peerIDForNickname(nickname: String) = peerIDs[nickname] override fun isPeerConnected(peerID: String) = peerID in connected + override fun peerGroupCapability(peerID: String) = + groupCapabilities[peerID] ?: PeerGroupCapability.SUPPORTED override fun peerNickname(peerID: String) = peerNames[peerID] override fun peerIdentity(peerID: String) = identities[peerID] override fun connectedPeerID(fingerprint: String) = connectedFingerprints[fingerprint]