diff --git a/bitchat/App/ConversationUIModel.swift b/bitchat/App/ConversationUIModel.swift index 461449cc..f8633e0d 100644 --- a/bitchat/App/ConversationUIModel.swift +++ b/bitchat/App/ConversationUIModel.swift @@ -193,7 +193,9 @@ final class ConversationUIModel: ObservableObject { private func refreshComputedState() { if let selectedPeerID = privateConversationModel.selectedPeerID { - canSendMediaInCurrentContext = !(selectedPeerID.isGeoDM || selectedPeerID.isGeoChat) + // Media transfer is not wired for groups in v1; keep it off so the + // composer can't strand a media placeholder that never sends. + canSendMediaInCurrentContext = !(selectedPeerID.isGeoDM || selectedPeerID.isGeoChat || selectedPeerID.isGroup) return } diff --git a/bitchat/Services/Groups/GroupProtocol.swift b/bitchat/Services/Groups/GroupProtocol.swift index 14f3d7f3..dc4a0abf 100644 --- a/bitchat/Services/Groups/GroupProtocol.swift +++ b/bitchat/Services/Groups/GroupProtocol.swift @@ -63,13 +63,24 @@ struct BitchatGroup: Codable, Equatable { // MARK: - TLV helpers +enum GroupTLVError: Error, Equatable { + /// A TLV value exceeded the 16-bit length field. Encoding fails instead + /// of silently truncating (which would ship a value the receiver drops). + case valueTooLong +} + private enum GroupTLV { - static func put(_ type: UInt8, _ value: Data, into out: inout Data) { + /// Appends a (type, 16-bit length, value) triple. Throws rather than + /// truncating when `value` does not fit the 16-bit length field, so an + /// oversize field surfaces a send failure instead of a silently truncated + /// blob the recipient rejects during decrypt/verify. + static func put(_ type: UInt8, _ value: Data, into out: inout Data) throws { + guard value.count <= Int(UInt16.max) else { throw GroupTLVError.valueTooLong } out.append(type) - let length = UInt16(clamping: value.count) + let length = UInt16(value.count) out.append(UInt8((length >> 8) & 0xFF)) out.append(UInt8(length & 0xFF)) - out.append(value.prefix(Int(length))) + out.append(value) } /// Iterates (type, value) pairs; returns nil on malformed framing. @@ -131,7 +142,10 @@ enum GroupRosterCoding { member.signingKey.count == signingKeyLength else { return nil } out.append(fingerprintData) out.append(member.signingKey) - let nickname = Data(member.nickname.utf8).prefix(maxNicknameBytes) + // Truncate on a Character boundary so the byte prefix is always + // valid UTF-8; a raw byte-prefix could split a multi-byte scalar + // and make the whole signed roster undecodable on the recipient. + let nickname = truncatedNicknameBytes(member.nickname) out.append(UInt8(nickname.count)) out.append(nickname) } @@ -160,6 +174,16 @@ enum GroupRosterCoding { guard offset == data.endIndex else { return nil } return members } + + /// UTF-8 bytes of `nickname` trimmed to at most `maxNicknameBytes`, + /// dropping whole Characters so the result is never split mid-scalar. + private static func truncatedNicknameBytes(_ nickname: String) -> Data { + var candidate = nickname + while Data(candidate.utf8).count > maxNicknameBytes { + candidate.removeLast() + } + return Data(candidate.utf8) + } } // MARK: - Group state payload (groupInvite / groupKeyUpdate over Noise) @@ -192,14 +216,17 @@ struct GroupStatePayload: Equatable { static let signingDomain = Data("bitchat-group-v1".utf8) - /// The bytes the creator signs. Binding the key and roster by hash keeps - /// the signed content fixed-size. - static func signingContent(groupID: Data, epoch: UInt32, key: Data, rosterBlob: Data) -> Data { + /// The bytes the creator signs. Binding the key, roster, and name by hash + /// keeps the signed content fixed-size. The name is covered so a relay + /// that caches/replays a signed state (e.g. store-and-forward) cannot swap + /// the display name while keeping a valid creator signature. + static func signingContent(groupID: Data, epoch: UInt32, key: Data, rosterBlob: Data, name: String) -> Data { var content = signingDomain content.append(groupID) content.append(GroupTLV.epochData(epoch)) content.append(key.sha256Hash()) content.append(rosterBlob.sha256Hash()) + content.append(Data(name.utf8).sha256Hash()) return content } @@ -211,7 +238,7 @@ struct GroupStatePayload: Equatable { sign: (Data) -> Data? ) -> GroupStatePayload? { guard let rosterBlob = GroupRosterCoding.encode(group.members) else { return nil } - let content = signingContent(groupID: group.groupID, epoch: group.epoch, key: key, rosterBlob: rosterBlob) + let content = signingContent(groupID: group.groupID, epoch: group.epoch, key: key, rosterBlob: rosterBlob, name: group.name) guard let signature = sign(content) else { return nil } return GroupStatePayload( groupID: group.groupID, @@ -229,13 +256,17 @@ struct GroupStatePayload: Equatable { let fingerprintData = Data(hexString: creatorFingerprint), fingerprintData.count == 32 else { return nil } var out = Data() - GroupTLV.put(FieldType.groupID.rawValue, groupID, into: &out) - GroupTLV.put(FieldType.name.rawValue, Data(name.utf8), into: &out) - GroupTLV.put(FieldType.key.rawValue, key, into: &out) - GroupTLV.put(FieldType.epoch.rawValue, GroupTLV.epochData(epoch), into: &out) - GroupTLV.put(FieldType.roster.rawValue, rosterBlob, into: &out) - GroupTLV.put(FieldType.creatorFingerprint.rawValue, fingerprintData, into: &out) - GroupTLV.put(FieldType.signature.rawValue, signature, into: &out) + do { + try GroupTLV.put(FieldType.groupID.rawValue, groupID, into: &out) + try GroupTLV.put(FieldType.name.rawValue, Data(name.utf8), into: &out) + try GroupTLV.put(FieldType.key.rawValue, key, into: &out) + try GroupTLV.put(FieldType.epoch.rawValue, GroupTLV.epochData(epoch), into: &out) + try GroupTLV.put(FieldType.roster.rawValue, rosterBlob, into: &out) + try GroupTLV.put(FieldType.creatorFingerprint.rawValue, fingerprintData, into: &out) + try GroupTLV.put(FieldType.signature.rawValue, signature, into: &out) + } catch { + return nil + } return out } @@ -292,7 +323,7 @@ struct GroupStatePayload: Equatable { guard members.count <= BitchatGroup.maxMembers, let creator = members.first(where: { $0.fingerprint == creatorFingerprint }), let rosterBlob = GroupRosterCoding.encode(members) else { return false } - let content = GroupStatePayload.signingContent(groupID: groupID, epoch: epoch, key: key, rosterBlob: rosterBlob) + let content = GroupStatePayload.signingContent(groupID: groupID, epoch: epoch, key: key, rosterBlob: rosterBlob, name: name) return GroupCrypto.verify(signature: signature, for: content, publicKey: creator.signingKey) } @@ -326,12 +357,12 @@ struct GroupMessageEnvelope: Equatable { case ciphertext = 0x04 } - func encode() -> Data { + func encode() throws -> Data { var out = Data() - GroupTLV.put(FieldType.groupID.rawValue, groupID, into: &out) - GroupTLV.put(FieldType.epoch.rawValue, GroupTLV.epochData(epoch), into: &out) - GroupTLV.put(FieldType.nonce.rawValue, nonce, into: &out) - GroupTLV.put(FieldType.ciphertext.rawValue, ciphertext, into: &out) + try GroupTLV.put(FieldType.groupID.rawValue, groupID, into: &out) + try GroupTLV.put(FieldType.epoch.rawValue, GroupTLV.epochData(epoch), into: &out) + try GroupTLV.put(FieldType.nonce.rawValue, nonce, into: &out) + try GroupTLV.put(FieldType.ciphertext.rawValue, ciphertext, into: &out) return out } @@ -392,10 +423,14 @@ enum GroupCrypto { case signature = 0x06 } - /// Bytes the sender signs: domain | groupID | messageID | timestamp | content. - static func messageSigningContent(groupID: Data, messageID: String, timestampMs: UInt64, content: String) -> Data { + /// Bytes the sender signs: domain | groupID | epoch | messageID | timestamp | content. + /// Covering the epoch stops a current member from re-sealing another + /// member's decrypted inner bytes under a later epoch key (the signature + /// would no longer verify at the new epoch). + static func messageSigningContent(groupID: Data, epoch: UInt32, messageID: String, timestampMs: UInt64, content: String) -> Data { var data = messageSigningDomain data.append(groupID) + data.append(GroupTLV.epochData(epoch)) data.append(Data(messageID.utf8)) data.append(GroupTLV.timestampData(timestampMs)) data.append(Data(content.utf8)) @@ -424,6 +459,7 @@ enum GroupCrypto { ) throws -> Data { let signingContent = messageSigningContent( groupID: groupID, + epoch: epoch, messageID: messageID, timestampMs: timestampMs, content: content @@ -433,12 +469,12 @@ enum GroupCrypto { } var inner = Data() - GroupTLV.put(InnerField.messageID.rawValue, Data(messageID.utf8), into: &inner) - GroupTLV.put(InnerField.senderSigningKey.rawValue, senderSigningKey, into: &inner) - GroupTLV.put(InnerField.senderNickname.rawValue, Data(senderNickname.utf8), into: &inner) - GroupTLV.put(InnerField.timestamp.rawValue, GroupTLV.timestampData(timestampMs), into: &inner) - GroupTLV.put(InnerField.content.rawValue, Data(content.utf8), into: &inner) - GroupTLV.put(InnerField.signature.rawValue, signature, into: &inner) + try GroupTLV.put(InnerField.messageID.rawValue, Data(messageID.utf8), into: &inner) + try GroupTLV.put(InnerField.senderSigningKey.rawValue, senderSigningKey, into: &inner) + try GroupTLV.put(InnerField.senderNickname.rawValue, Data(senderNickname.utf8), into: &inner) + try GroupTLV.put(InnerField.timestamp.rawValue, GroupTLV.timestampData(timestampMs), into: &inner) + try GroupTLV.put(InnerField.content.rawValue, Data(content.utf8), into: &inner) + try GroupTLV.put(InnerField.signature.rawValue, signature, into: &inner) do { let symmetricKey = SymmetricKey(data: key) @@ -453,7 +489,7 @@ enum GroupCrypto { nonce: Data(sealed.nonce), ciphertext: ciphertext ) - return envelope.encode() + return try envelope.encode() } catch { throw GroupCryptoError.sealFailed } @@ -513,6 +549,7 @@ enum GroupCrypto { let signingContent = messageSigningContent( groupID: envelope.groupID, + epoch: envelope.epoch, messageID: messageID, timestampMs: timestampMs, content: content diff --git a/bitchat/ViewModels/ChatGroupCoordinator.swift b/bitchat/ViewModels/ChatGroupCoordinator.swift index fe576132..a83cb024 100644 --- a/bitchat/ViewModels/ChatGroupCoordinator.swift +++ b/bitchat/ViewModels/ChatGroupCoordinator.swift @@ -35,6 +35,8 @@ protocol ChatGroupContext: AnyObject { func cryptoIdentity(for peerID: PeerID) -> (fingerprint: String, signingKey: Data)? /// The connected short peer ID whose fingerprint matches, if any. func connectedPeerID(forFingerprint fingerprint: String) -> PeerID? + /// Whether the user has blocked the identity with this Noise fingerprint. + func isFingerprintBlocked(_ fingerprint: String) -> Bool // MARK: Transport func sendGroupInvitePayload(_ payload: Data, to peerID: PeerID) @@ -99,6 +101,10 @@ extension ChatViewModel: ChatGroupContext { return meshService.isPeerConnected(shortID) ? shortID : nil } + func isFingerprintBlocked(_ fingerprint: String) -> Bool { + identityManager.isBlocked(fingerprint: fingerprint) + } + func sendGroupInvitePayload(_ payload: Data, to peerID: PeerID) { meshService.sendGroupInvite(payload, to: peerID) } @@ -230,9 +236,12 @@ final class ChatGroupCoordinator { signingKey: identity.signingKey, nickname: context.peerNickname(for: peerID) ?? nickname ) + // Rotate the key (epoch + 1) on every roster change, not just removals. + // A monotonically increasing epoch per roster gives the receiver a + // strict ordering: two out-of-order invite states can no longer share + // an epoch and last-writer-wins a just-added member back out. let members = group.members + [newMember] - guard let updated = context.groupStore.updateRoster(groupID: group.groupID, members: members), - let key = context.groupStore.key(forGroupID: group.groupID), + guard let (updated, key) = context.groupStore.rotateKey(groupID: group.groupID, members: members), let payload = signedStatePayload(for: updated, key: key) else { return .error(message: String(localized: "system.group.invite_failed", comment: "Error when building or signing a group invite fails")) } @@ -280,6 +289,7 @@ final class ChatGroupCoordinator { } distributeState(payload, group: rotated, excluding: [], type: .keyUpdate) + notifyRemovedMember(member, rotated: rotated) return .success(message: String( format: String(localized: "system.group.removed_member", comment: "System message after removing a member and rotating the key; placeholder is the nickname"), @@ -402,6 +412,12 @@ final class ChatGroupCoordinator { } // Our own broadcast echoed back via relay or sync replay. guard plaintext.senderSigningKey != context.mySigningPublicKey() else { return } + // Honor /block inside groups too: drop display + notification for a + // blocked member, consistent with every other inbound path. + guard !context.isFingerprintBlocked(member.fingerprint) else { + SecureLogger.debug("Dropping group message from blocked member", category: .security) + return + } let groupPeerID = group.peerID // Trust the authenticated inner timestamp (clamped so a future-dated @@ -493,6 +509,26 @@ private extension ChatGroupCoordinator { } } + /// Tells a just-removed member they're out so their client can deactivate + /// the group instead of silently going dark (dropping every message under + /// the epoch it no longer has the key for). The notice is a creator-signed + /// state whose roster excludes the removee — their `applyGroupState` + /// removal branch fires on the missing-self roster and surfaces the + /// "removed from group" system message. + /// + /// It carries a throwaway all-zero key, never the rotated key, so the + /// removee cannot decrypt post-removal traffic. State is sent 1:1 over + /// authenticated Noise, so no remaining member ever receives this blob + /// (and even if one did, its own missing-self check would not match). + /// If the removee is offline the notice can't be delivered — same v1 + /// limitation as any other missed key update, documented in the PR. + func notifyRemovedMember(_ removed: GroupMember, rotated: BitchatGroup) { + guard let peerID = context.connectedPeerID(forFingerprint: removed.fingerprint) else { return } + let throwawayKey = Data(count: BitchatGroup.keyLength) + guard let payload = signedStatePayload(for: rotated, key: throwawayKey) else { return } + context.sendGroupKeyUpdatePayload(payload, to: peerID) + } + func applyGroupState(from peerID: PeerID, payload: Data, isInvite: Bool) { guard let state = GroupStatePayload.decode(payload) else { SecureLogger.warning("Malformed group state payload from \(peerID.id.prefix(8))…", category: .security) diff --git a/bitchat/ViewModels/ChatLifecycleCoordinator.swift b/bitchat/ViewModels/ChatLifecycleCoordinator.swift index bea1b5b2..0ccdd39c 100644 --- a/bitchat/ViewModels/ChatLifecycleCoordinator.swift +++ b/bitchat/ViewModels/ChatLifecycleCoordinator.swift @@ -185,6 +185,13 @@ final class ChatLifecycleCoordinator { func markPrivateMessagesAsRead(from peerID: PeerID) { context.markChatAsRead(from: peerID) + // Group chats are keyed under a virtual group_ peerID; no member IS the + // conversation peer, so the receipt loops below (which gate on + // senderPeerID == peerID) must never emit a read/delivered receipt for + // one. This guard makes that explicit so a future refactor of the + // receipt matching can't silently start leaking receipts into groups. + guard !peerID.isGroup else { return } + if peerID.isGeoDM, let recipientHex = context.nostrKeyMapping[peerID], case .location(let channel) = context.activeChannel, diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 646e04bc..0475ce17 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -102,7 +102,9 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, TransportEventDele @MainActor var canSendMediaInCurrentContext: Bool { if let peer = selectedPrivateChatPeer { - return !(peer.isGeoDM || peer.isGeoChat) + // Media transfer is not wired for groups in v1 (sendFilePrivate + // rejects the virtual group_ recipient), so keep the affordance off. + return !(peer.isGeoDM || peer.isGeoChat || peer.isGroup) } switch activeChannel { case .mesh: return true diff --git a/bitchatTests/Services/GroupProtocolTests.swift b/bitchatTests/Services/GroupProtocolTests.swift index a99c48a9..f9f19e7f 100644 --- a/bitchatTests/Services/GroupProtocolTests.swift +++ b/bitchatTests/Services/GroupProtocolTests.swift @@ -142,7 +142,7 @@ struct GroupProtocolTests { Issue.record("roster should encode") return } - let content = GroupStatePayload.signingContent(groupID: groupID, epoch: 1, key: key, rosterBlob: rosterBlob) + let content = GroupStatePayload.signingContent(groupID: groupID, epoch: 1, key: key, rosterBlob: rosterBlob, name: group.name) let payload = GroupStatePayload( groupID: groupID, name: group.name, @@ -299,4 +299,76 @@ struct GroupProtocolTests { #expect(GroupMessageEnvelope.decode(Data([0x01, 0x00])) == nil) #expect(GroupStatePayload.decode(Data([0xFF, 0x00, 0x01])) == nil) } + + // MARK: - Oversize / UTF-8 safety (Codex findings) + + @Test func oversizeMessageContentFailsToSealInsteadOfTruncating() { + // A content whose UTF-8 exceeds the 16-bit TLV length must fail to + // seal (surfacing send_failed) rather than silently truncate into a + // ciphertext recipients would drop. + let oversize = String(repeating: "a", count: 70_000) + #expect(throws: (any Error).self) { + _ = try GroupCrypto.sealMessage( + content: oversize, + messageID: "big", + senderNickname: "alice", + senderSigningKey: member.member.signingKey, + timestampMs: 1, + groupID: groupID, + epoch: 1, + key: key, + sign: member.sign + ) + } + } + + @Test func multiByteNicknameTruncatesOnScalarBoundary() throws { + // 40 euro signs = 120 UTF-8 bytes; a raw 64-byte prefix would split + // the 21st scalar and make the roster undecodable. Truncation must + // land on a Character boundary so the blob round-trips. + let euros = String(repeating: "€", count: 40) + let wide = GroupMember( + fingerprint: creator.fingerprint, + signingKey: creator.member.signingKey, + nickname: euros + ) + let blob = try #require(GroupRosterCoding.encode([wide])) + let decoded = try #require(GroupRosterCoding.decode(blob)) + #expect(decoded.count == 1) + #expect(Data(decoded[0].nickname.utf8).count <= 64) + #expect(decoded[0].nickname.allSatisfy { $0 == "€" }) + #expect(!decoded[0].nickname.isEmpty) + } + + // MARK: - Signable-bytes forward-proofing + + @Test func creatorSignatureCoversName() throws { + let group = makeGroup() + let payload = try #require(GroupStatePayload.makeSigned(group: group, key: key, sign: creator.sign)) + #expect(payload.verifyCreatorSignature()) + + // Swapping only the display name must invalidate the creator signature. + let renamed = GroupStatePayload( + groupID: payload.groupID, + name: "totally different name", + key: payload.key, + epoch: payload.epoch, + members: payload.members, + creatorFingerprint: payload.creatorFingerprint, + signature: payload.signature + ) + #expect(!renamed.verifyCreatorSignature()) + } + + @Test func messageSignatureCoversEpoch() { + // The signed bytes differ by epoch, so a signature captured at one + // epoch cannot verify when re-sealed under a later epoch key. + let atEpoch1 = GroupCrypto.messageSigningContent( + groupID: groupID, epoch: 1, messageID: "m", timestampMs: 1, content: "x" + ) + let atEpoch2 = GroupCrypto.messageSigningContent( + groupID: groupID, epoch: 2, messageID: "m", timestampMs: 1, content: "x" + ) + #expect(atEpoch1 != atEpoch2) + } }