diff --git a/bitchat/Services/Groups/GroupStore.swift b/bitchat/Services/Groups/GroupStore.swift index 3a45dcb7..0fd59a1c 100644 --- a/bitchat/Services/Groups/GroupStore.swift +++ b/bitchat/Services/Groups/GroupStore.swift @@ -68,7 +68,8 @@ final class GroupStore: ObservableObject { } /// Inserts or replaces a group and its current key. Rejects rosters over - /// the hard cap or groups whose creator is missing from the roster. + /// the hard cap, groups whose creator is missing from the roster, and any + /// attempt to change the creator of a group we already hold. @discardableResult func upsert(_ group: BitchatGroup, key: Data) -> Bool { guard group.groupID.count == BitchatGroup.groupIDLength, @@ -76,6 +77,19 @@ final class GroupStore: ObservableObject { !group.members.isEmpty, group.members.count <= BitchatGroup.maxMembers, group.creator != nil else { return false } + // A group keeps the creator it was created with. A peer naming + // themselves creator of a known groupID at a higher epoch would + // otherwise replace the roster and key wholesale. `applyGroupState` + // pins this before its removal branch; here is the one point every + // write goes through. + if let existing = groups.first(where: { $0.groupID == group.groupID }), + existing.creatorFingerprint != group.creatorFingerprint { + SecureLogger.warning( + "Refusing group state: creator \(group.creatorFingerprint.prefix(8))… does not match stored creator \(existing.creatorFingerprint.prefix(8))…", + category: .security + ) + return false + } guard keychain.saveIdentityKey(key, forKey: Self.keychainKey(for: group.groupID)) else { SecureLogger.error("Failed to store group key in keychain", category: .security) return false @@ -176,8 +190,11 @@ final class GroupStore: ObservableObject { let stored = try? JSONDecoder().decode([BitchatGroup].self, from: data) else { return } - // Only groups whose key survived in the keychain are usable. - groups = stored.filter { key(forGroupID: $0.groupID) != nil } + // Only groups whose key survived in the keychain are usable. Disk bypasses + // `upsert`, so re-check the creator invariant to avoid unmatchable creators. + groups = stored.filter { + !$0.creatorFingerprint.isEmpty && $0.creator != nil && key(forGroupID: $0.groupID) != nil + } } private static func defaultFileURL() -> URL? { diff --git a/bitchat/ViewModels/ChatGroupCoordinator.swift b/bitchat/ViewModels/ChatGroupCoordinator.swift index b8a2b2a6..131cbe94 100644 --- a/bitchat/ViewModels/ChatGroupCoordinator.swift +++ b/bitchat/ViewModels/ChatGroupCoordinator.swift @@ -558,6 +558,18 @@ private extension ChatGroupCoordinator { let myFingerprint = context.myNoiseFingerprint() let existing = context.groupStore.group(withID: state.groupID) + // A group keeps the creator it was created with. The checks above only + // prove the sender is the creator the state names, which an attacker + // satisfies by naming themselves. Checked before the removal branch, + // which drops the group without ever reaching `upsert`. + if let existing, existing.creatorFingerprint != state.creatorFingerprint { + SecureLogger.warning( + "Dropping group state claiming creator \(state.creatorFingerprint.prefix(8))… for a group created by \(existing.creatorFingerprint.prefix(8))…", + category: .security + ) + return + } + // A creator-signed roster that no longer includes us is a removal. guard state.members.contains(where: { $0.fingerprint == myFingerprint }) else { if let existing { diff --git a/bitchatTests/Services/GroupStoreTests.swift b/bitchatTests/Services/GroupStoreTests.swift index adf04806..2f426c49 100644 --- a/bitchatTests/Services/GroupStoreTests.swift +++ b/bitchatTests/Services/GroupStoreTests.swift @@ -81,6 +81,53 @@ struct GroupStoreTests { #expect(store.group(withID: group.groupID)?.members == [creator]) } + // MARK: - Creator pinning + + @Test func upsertRefusesToChangeTheCreatorOfAnExistingGroup() throws { + let store = GroupStore(keychain: MockKeychain(), persistsToDisk: false) + let creator = makeMember(seed: 0xC1, nickname: "creator") + let me = makeMember(seed: 0x0E, nickname: "me") + let attacker = makeMember(seed: 0xEE, nickname: "attacker") + let group = try #require(store.createGroup(named: "trip", creator: creator)) + #expect(store.updateRoster(groupID: group.groupID, members: [creator, me]) != nil) + let realKey = try #require(store.key(forGroupID: group.groupID)) + + // Same groupID, same name, higher epoch, attacker as creator. Every + // other check in applyGroupState passes for this. + let hijack = BitchatGroup( + groupID: group.groupID, + name: group.name, + epoch: group.epoch + 1, + members: [attacker, me], + creatorFingerprint: attacker.fingerprint + ) + + #expect(!store.upsert(hijack, key: Data(repeating: 0xAB, count: 32))) + + let stored = try #require(store.group(withID: group.groupID)) + #expect(stored.creatorFingerprint == creator.fingerprint) + #expect(stored.epoch == group.epoch) + #expect(store.key(forGroupID: group.groupID) == realKey) + } + + @Test func upsertStillAcceptsANewEpochFromTheSameCreator() throws { + // The other side of the boundary: pinning the creator must not stop the + // real creator rotating the key, or removing a member breaks. + let store = GroupStore(keychain: MockKeychain(), persistsToDisk: false) + let creator = makeMember(seed: 0xC1, nickname: "creator") + let me = makeMember(seed: 0x0E, nickname: "me") + let group = try #require(store.createGroup(named: "trip", creator: creator)) + + var rotated = group + rotated.epoch = group.epoch + 1 + rotated.members = [creator, me] + let newKey = Data(repeating: 0x5A, count: 32) + + #expect(store.upsert(rotated, key: newKey)) + #expect(store.group(withID: group.groupID)?.epoch == group.epoch + 1) + #expect(store.key(forGroupID: group.groupID) == newKey) + } + // MARK: - Rotation @Test func rotateKeyBumpsEpochAndReplacesKey() throws {