diff --git a/bitchat/Nostr/NostrEmbeddedBitChat.swift b/bitchat/Nostr/NostrEmbeddedBitChat.swift index 5ee5644c..d5e4c73b 100644 --- a/bitchat/Nostr/NostrEmbeddedBitChat.swift +++ b/bitchat/Nostr/NostrEmbeddedBitChat.swift @@ -55,7 +55,8 @@ struct NostrEmbeddedBitChat { } /// Build a `bitchat1:` ACK (delivered/read) without an embedded recipient peer ID (geohash DMs). - static func encodeAckForNostrNoRecipient(type: NoisePayloadType, messageID: String, senderPeerID: PeerID) -> String? { + /// The sender ID is random per envelope; see `unlinkableSenderID()`. + static func encodeAckForNostrNoRecipient(type: NoisePayloadType, messageID: String) -> String? { guard type == .delivered || type == .readReceipt else { return nil } var payload = Data([type.rawValue]) @@ -63,7 +64,7 @@ struct NostrEmbeddedBitChat { let packet = BitchatPacket( type: MessageType.noiseEncrypted.rawValue, - senderID: Data(hexString: senderPeerID.id) ?? Data(), + senderID: unlinkableSenderID(), recipientID: nil, timestamp: UInt64(Date().timeIntervalSince1970 * 1000), payload: payload, @@ -76,7 +77,8 @@ struct NostrEmbeddedBitChat { } /// Build a `bitchat1:` payload without an embedded recipient peer ID (used for geohash DMs). - static func encodePMForNostrNoRecipient(content: String, messageID: String, senderPeerID: PeerID) -> String? { + /// The sender ID is random per envelope; see `unlinkableSenderID()`. + static func encodePMForNostrNoRecipient(content: String, messageID: String) -> String? { let pm = PrivateMessagePacket(messageID: messageID, content: content) guard let tlv = pm.encode() else { return nil } @@ -85,7 +87,7 @@ struct NostrEmbeddedBitChat { let packet = BitchatPacket( type: MessageType.noiseEncrypted.rawValue, - senderID: Data(hexString: senderPeerID.id) ?? Data(), + senderID: unlinkableSenderID(), recipientID: nil, timestamp: UInt64(Date().timeIntervalSince1970 * 1000), payload: payload, @@ -97,6 +99,18 @@ struct NostrEmbeddedBitChat { return "bitchat1:" + base64URLEncode(data) } + /// Sender ID for the recipient-less envelopes above. They go out under a + /// per-geohash identity (and as the acks for inbound account DMs), where + /// no receiver reads this field: iOS and Android both key these + /// conversations on the Nostr sender pubkey. The mesh peer ID must never + /// go here: it is the stable Noise fingerprint prefix, so anyone who DMs + /// a geohash identity (and gets the automatic DELIVERED ack back) could + /// link that persona to the device and to its other geohash personas. + /// Fresh random bytes per envelope keep even our own envelopes unlinkable. + private static func unlinkableSenderID() -> Data { + Data((0.. PeerID { if let maybeData = Data(hexString: recipientPeerID.id) { if maybeData.count == 32 { diff --git a/bitchat/Services/NostrTransport.swift b/bitchat/Services/NostrTransport.swift index 2d1c380b..8c62d711 100644 --- a/bitchat/Services/NostrTransport.swift +++ b/bitchat/Services/NostrTransport.swift @@ -67,7 +67,8 @@ final class NostrTransport: Transport, @unchecked Sendable { } } - // Provide BLE short peer ID for BitChat embedding + // BLE short peer ID embedded in favorites (recipient-addressed) envelopes. + // Geohash envelopes never carry it (see NostrEmbeddedBitChat). var senderPeerID = PeerID(str: "") // Throttle outbound acks — READ receipts and DELIVERED acks, direct and @@ -315,7 +316,7 @@ extension NostrTransport { Task { @MainActor in guard !recipientHex.isEmpty else { return } SecureLogger.debug("GeoDM: send PM mid=\(messageID.prefix(8))…", category: .session) - guard let embedded = NostrEmbeddedBitChat.encodePMForNostrNoRecipient(content: content, messageID: messageID, senderPeerID: senderPeerID) else { + guard let embedded = NostrEmbeddedBitChat.encodePMForNostrNoRecipient(content: content, messageID: messageID) else { SecureLogger.error("NostrTransport: failed to embed geohash PM packet", category: .session) return } @@ -382,12 +383,12 @@ extension NostrTransport { case .deliveredGeohash(let messageID, let recipientHex, let identity): SecureLogger.debug("GeoDM: send DELIVERED mid=\(messageID.prefix(8))…", category: .session) - guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .delivered, messageID: messageID, senderPeerID: senderPeerID) else { return } + guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .delivered, messageID: messageID) else { return } sendWrappedMessage(content: embedded, recipientHex: recipientHex, senderIdentity: identity, registerPending: true) case .readGeohash(let messageID, let recipientHex, let identity): SecureLogger.debug("GeoDM: send READ mid=\(messageID.prefix(8))…", category: .session) - guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .readReceipt, messageID: messageID, senderPeerID: senderPeerID) else { return } + guard let embedded = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .readReceipt, messageID: messageID) else { return } sendWrappedMessage(content: embedded, recipientHex: recipientHex, senderIdentity: identity, registerPending: true) } } diff --git a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift index e011c763..602ce144 100644 --- a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift +++ b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift @@ -226,10 +226,11 @@ extension ChatViewModel: ChatPrivateConversationContext { NotificationService.shared.sendPrivateMessageNotification(from: senderName, message: message, peerID: peerID) } + /// Deliberately no `senderPeerID`: geohash envelopes (and the acks for + /// inbound account DMs sent through here) must never carry the mesh + /// peer ID. The recipient-less encoders fill that field with random bytes. private func makeGeohashNostrTransport() -> NostrTransport { - let transport = NostrTransport(keychain: keychain, idBridge: idBridge) - transport.senderPeerID = meshService.myPeerID - return transport + NostrTransport(keychain: keychain, idBridge: idBridge) } } @@ -491,18 +492,21 @@ final class ChatPrivateConversationCoordinator { guard let pm = PrivateMessagePacket.decode(from: payload.data) else { return } let messageId = pm.messageID - // Ack before the dedup guard: a re-sent copy means the sender may not - // have our DELIVERED yet, and markGeoDeliveryAckSent dedups the - // actual sends. - sendDeliveryAckIfNeeded(to: messageId, senderPubKey: senderPubkey, from: id) + // Block check before anything that answers the sender: a blocked + // sender gets no DELIVERED or READ ack back (not even a liveness + // signal) and doesn't consume a dedup slot. + if context.isNostrBlocked(pubkeyHexLowercased: senderPubkey) { + return + } guard markInboundGeoDMSeen(messageId) else { return } SecureLogger.info("GeoDM: recv PM <- sender=\(senderPubkey.prefix(8))… mid=\(messageId.prefix(8))…", category: .session) - if context.isNostrBlocked(pubkeyHexLowercased: senderPubkey) { - return - } + // Ack only once the block and dedup checks pass. Outbox retries of a + // message we've already acked are dropped by the dedup guard, and + // markGeoDeliveryAckSent dedups the actual sends by message ID. + sendDeliveryAckIfNeeded(to: messageId, senderPubKey: senderPubkey, from: id) // Prefer the favorite's stored nickname when the sender resolved to a // known noise key; the Nostr display name is a geohash-scoped diff --git a/bitchatTests/ChatNostrCoordinatorContextTests.swift b/bitchatTests/ChatNostrCoordinatorContextTests.swift index 6200d4bf..74b0d0e2 100644 --- a/bitchatTests/ChatNostrCoordinatorContextTests.swift +++ b/bitchatTests/ChatNostrCoordinatorContextTests.swift @@ -343,8 +343,7 @@ struct ChatNostrCoordinatorContextTests { let sender = try NostrIdentity.generate() let embedded = try #require(NostrEmbeddedBitChat.encodePMForNostrNoRecipient( content: "psst", - messageID: "gm-1", - senderPeerID: PeerID(str: "aabbccddeeff0011") + messageID: "gm-1" )) let giftWrap = try NostrProtocol.createPrivateMessage( content: embedded, @@ -386,8 +385,7 @@ struct ChatNostrCoordinatorContextTests { let sender = try NostrIdentity.generate() let embedded = try #require(NostrEmbeddedBitChat.encodePMForNostrNoRecipient( content: "pre-wipe secret", - messageID: "gm-wipe-1", - senderPeerID: PeerID(str: "aabbccddeeff0011") + messageID: "gm-wipe-1" )) let giftWrap = try NostrProtocol.createPrivateMessage( content: embedded, diff --git a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift index 54cb341e..aa4e8b1b 100644 --- a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift +++ b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift @@ -491,6 +491,49 @@ struct ChatPrivateConversationCoordinatorContextTests { #expect(context.privateChats[convKey]?.count == 1) } + @Test @MainActor + func geoPrivateMessage_blockedSenderGetsNoAckOrReadReceipt() async { + let context = MockChatPrivateConversationContext() + let coordinator = ChatPrivateConversationCoordinator(context: context) + let convKey = PeerID(str: "nostr_abcdef12") + let senderPubkey = "feedface00112233" + context.blockedNostrPubkeys = [senderPubkey] + // Viewing the chat: an admitted message would also get a READ ack. + context.selectedPrivateChatPeer = convKey + let payloadData = PrivateMessagePacket(messageID: "geo-blocked", content: "you there?").encode()! + let payload = NoisePayload(type: .privateMessage, data: payloadData) + + coordinator.handlePrivateMessage( + payload, + senderPubkey: senderPubkey, + convKey: convKey, + id: MockChatPrivateConversationContext.dummyIdentity, + messageTimestamp: Date() + ) + + // Nothing goes back to a blocked sender, not even a DELIVERED ack + // that would confirm this identity is online. + #expect(context.geoDeliveryAcks.isEmpty) + #expect(context.geoReadReceipts.isEmpty) + #expect(context.sentGeoDeliveryAcks.isEmpty) + #expect(context.privateChats[convKey] == nil) + #expect(context.privateMessageNotifications.isEmpty) + + // The dropped copy didn't consume the dedup slot: after unblocking, a + // retry of the same message is admitted and acked exactly once. + context.blockedNostrPubkeys = [] + coordinator.handlePrivateMessage( + payload, + senderPubkey: senderPubkey, + convKey: convKey, + id: MockChatPrivateConversationContext.dummyIdentity, + messageTimestamp: Date() + ) + #expect(context.geoDeliveryAcks.map(\.messageID) == ["geo-blocked"]) + #expect(context.geoReadReceipts.map(\.messageID) == ["geo-blocked"]) + #expect(context.privateChats[convKey]?.map(\.id) == ["geo-blocked"]) + } + @Test @MainActor func accountDM_handsOpenShortIDConversationToStableWhenOffline() async { let context = MockChatPrivateConversationContext() diff --git a/bitchatTests/ChatViewModelExtensionsTests.swift b/bitchatTests/ChatViewModelExtensionsTests.swift index 6f9609ee..51e5f7fd 100644 --- a/bitchatTests/ChatViewModelExtensionsTests.swift +++ b/bitchatTests/ChatViewModelExtensionsTests.swift @@ -452,7 +452,7 @@ struct ChatViewModelNostrExtensionTests { ) ], for: convKey) - let content = try ackContent(type: .delivered, messageID: messageID, senderPeerID: PeerID(str: "0123456789abcdef")) + let content = try ackContent(type: .delivered, messageID: messageID) let giftWrap = try NostrProtocol.createPrivateMessage( content: content, recipientPubkey: recipient.publicKeyHex, @@ -490,7 +490,7 @@ struct ChatViewModelNostrExtensionTests { ) ], for: convKey) - let content = try ackContent(type: .readReceipt, messageID: messageID, senderPeerID: PeerID(str: "0123456789abcdef")) + let content = try ackContent(type: .readReceipt, messageID: messageID) let giftWrap = try NostrProtocol.createPrivateMessage( content: content, recipientPubkey: recipient.publicKeyHex, @@ -516,8 +516,7 @@ struct ChatViewModelNostrExtensionTests { let content = try privateMessageContent( text: "Hello from gift wrap", - messageID: messageID, - senderPeerID: PeerID(str: "0123456789abcdef") + messageID: messageID ) let giftWrap = try NostrProtocol.createPrivateMessage( content: content, @@ -548,8 +547,7 @@ struct ChatViewModelNostrExtensionTests { let content = try privateMessageContent( text: "Blocked", - messageID: messageID, - senderPeerID: PeerID(str: "0123456789abcdef") + messageID: messageID ) let giftWrap = try NostrProtocol.createPrivateMessage( content: content, @@ -559,14 +557,17 @@ struct ChatViewModelNostrExtensionTests { viewModel.handleGiftWrap(giftWrap, id: recipient) - // Gift-wrap decryption runs off the main actor; wait for the ack - // (sent even for blocked senders) to know processing finished. - let didAck = await TestHelpers.waitUntil( - { viewModel.sentGeoDeliveryAcks.contains(messageID) }, + // Gift-wrap decryption runs off the main actor. The key mapping is + // registered in the same main-actor hop that runs the PM handler, so + // once it shows up the handler has already returned. + let didProcess = await TestHelpers.waitUntil( + { viewModel.nostrKeyMapping[convKey] == sender.publicKeyHex }, timeout: TestConstants.settleTimeout ) - #expect(didAck) + #expect(didProcess) #expect(viewModel.privateChats[convKey] == nil) + // A blocked sender gets nothing back, not even a DELIVERED ack. + #expect(!viewModel.sentGeoDeliveryAcks.contains(messageID)) } @Test @MainActor @@ -591,7 +592,7 @@ struct ChatViewModelNostrExtensionTests { ) ], for: convKey) - let content = try ackContent(type: .delivered, messageID: messageID, senderPeerID: PeerID(str: "0123456789abcdef")) + let content = try ackContent(type: .delivered, messageID: messageID) let giftWrap = try NostrProtocol.createPrivateMessage( content: content, recipientPubkey: recipient.publicKeyHex, @@ -1262,22 +1263,20 @@ private func base64URLEncode(_ data: Data) -> String { .replacingOccurrences(of: "=", with: "") } -private func ackContent(type: NoisePayloadType, messageID: String, senderPeerID: PeerID) throws -> String { +private func ackContent(type: NoisePayloadType, messageID: String) throws -> String { if let content = NostrEmbeddedBitChat.encodeAckForNostrNoRecipient( type: type, - messageID: messageID, - senderPeerID: senderPeerID + messageID: messageID ) { return content } throw ChatViewModelExtensionsTestError.invalidAckContent } -private func privateMessageContent(text: String, messageID: String, senderPeerID: PeerID) throws -> String { +private func privateMessageContent(text: String, messageID: String) throws -> String { if let content = NostrEmbeddedBitChat.encodePMForNostrNoRecipient( content: text, - messageID: messageID, - senderPeerID: senderPeerID + messageID: messageID ) { return content } diff --git a/bitchatTests/NostrProtocolTests.swift b/bitchatTests/NostrProtocolTests.swift index 4a8d33e7..f63928c9 100644 --- a/bitchatTests/NostrProtocolTests.swift +++ b/bitchatTests/NostrProtocolTests.swift @@ -341,10 +341,9 @@ struct NostrProtocolTests { // Build a DELIVERED ack embedded payload (geohash-style, no recipient peer ID) let messageID = "TEST-MSG-DELIVERED-1" - let senderPeerID = PeerID(str: "0123456789abcdef") // 8-byte hex peer ID let embedded = try #require( - NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .delivered, messageID: messageID, senderPeerID: senderPeerID), + NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .delivered, messageID: messageID), "Failed to embed delivered ack" ) @@ -391,9 +390,8 @@ struct NostrProtocolTests { let recipient = try NostrIdentity.generate() let messageID = "TEST-MSG-READ-1" - let senderPeerID = PeerID(str: "fedcba9876543210") // 8-byte hex peer ID let embedded = try #require( - NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .readReceipt, messageID: messageID, senderPeerID: senderPeerID), + NostrEmbeddedBitChat.encodeAckForNostrNoRecipient(type: .readReceipt, messageID: messageID), "Failed to embed read ack" ) diff --git a/bitchatTests/Services/NostrTransportTests.swift b/bitchatTests/Services/NostrTransportTests.swift index 5f657000..70807e66 100644 --- a/bitchatTests/Services/NostrTransportTests.swift +++ b/bitchatTests/Services/NostrTransportTests.swift @@ -173,6 +173,9 @@ struct NostrTransportTests { #expect(privateMessage.messageID == "pm-1") #expect(privateMessage.content == "hello over nostr") #expect(result.packet.recipientID == shortPeerID.routingData) + // Favorites DMs keep the mesh sender ID: the recipient already binds + // this npub to our Noise key. + #expect(result.packet.senderID == transport.senderPeerID.routingData) #expect(probe.pendingGiftWrapIDs.isEmpty) } @@ -257,6 +260,7 @@ struct NostrTransportTests { #expect(result.payload.type == .delivered) #expect(String(data: result.payload.data, encoding: .utf8) == "ack-1") #expect(result.packet.recipientID == fullPeerID.toShort().routingData) + #expect(result.packet.senderID == transport.senderPeerID.routingData) } @Test("Geohash private message registers pending gift wrap") @@ -300,6 +304,66 @@ struct NostrTransportTests { #expect(probe.pendingGiftWrapIDs == [event.id]) } + @Test("Geohash envelopes never carry the mesh peer ID") + @MainActor + func geohashEnvelopesNeverCarryMeshPeerID() async throws { + let keychain = MockKeychain() + let idBridge = NostrIdentityBridge(keychain: keychain) + let sender = try NostrIdentity.generate() + let recipient = try NostrIdentity.generate() + let probe = NostrTransportProbe() + let transport = NostrTransport( + keychain: keychain, + idBridge: idBridge, + dependencies: makeDependencies( + currentIdentity: { sender }, + registerPendingGiftWrap: probe.recordPendingGiftWrap(id:), + sendEvent: probe.record(event:), + scheduleAfter: { delay, action in + probe.enqueueScheduledAction(delay: delay, action: action) + } + ) + ) + // Even a transport that knows the mesh peer ID must not embed it + // under a geohash identity: whoever receives the envelope (including + // the automatic DELIVERED ack) could link that persona to the device. + let meshPeerID = PeerID(str: "0123456789abcdef") + let meshIDBytes = try #require(meshPeerID.routingData) + transport.senderPeerID = meshPeerID + + transport.sendPrivateMessageGeohash( + content: "geo hello", + toRecipientHex: recipient.publicKeyHex, + from: sender, + messageID: "geo-pm" + ) + transport.sendDeliveryAckGeohash(for: "geo-delivered", toRecipientHex: recipient.publicKeyHex, from: sender) + transport.sendReadReceiptGeohash("geo-read", toRecipientHex: recipient.publicKeyHex, from: sender) + + // The PM goes out directly; the two acks share the pacer, so the + // second leaves only once the queued throttle action runs. + let sentFirst = await TestHelpers.waitUntil( + { probe.sentEvents.count == 2 && probe.scheduledActionCount == 1 }, + timeout: TestConstants.settleTimeout + ) + try #require(sentFirst, "Expected the PM and the first paced ack") + try #require(probe.runNextScheduledAction(), "Expected queued throttle action after first ack") + let sentAll = await TestHelpers.waitUntil({ probe.sentEvents.count == 3 }, timeout: TestConstants.settleTimeout) + try #require(sentAll, "Expected the second paced ack") + + let envelopes = try probe.sentEvents.map { try decodeEmbeddedPayload(from: $0, recipient: recipient) } + #expect(Set(envelopes.map(\.payload.type)) == [.privateMessage, .delivered, .readReceipt]) + for envelope in envelopes { + #expect(envelope.packet.recipientID == nil) + #expect(envelope.packet.senderID.count == BinaryProtocol.senderIDSize) + #expect(envelope.packet.senderID != meshIDBytes) + #expect(envelope.packetData.range(of: meshIDBytes) == nil) + } + // Fresh random bytes per envelope, so the field can't tie two + // envelopes (or two geohash identities) to one sender either. + #expect(Set(envelopes.map(\.packet.senderID)).count == envelopes.count) + } + @Test("Read receipt queue sends in order and waits for scheduler") @MainActor func readReceiptQueueThrottlesSequentially() async throws { @@ -456,7 +520,7 @@ struct NostrTransportTests { private func decodeEmbeddedPayload( from event: NostrEvent, recipient: NostrIdentity - ) throws -> (packet: BitchatPacket, payload: NoisePayload, senderPubkey: String) { + ) throws -> (packet: BitchatPacket, payload: NoisePayload, senderPubkey: String, packetData: Data) { let (content, senderPubkey, _) = try NostrProtocol.decryptPrivateMessage( giftWrap: event, recipientIdentity: recipient @@ -470,7 +534,7 @@ struct NostrTransportTests { let payload = NoisePayload.decode(packet.payload) else { throw NostrTransportTestError.invalidPacket } - return (packet, payload, senderPubkey) + return (packet, payload, senderPubkey, packetData) } private func decodePrivateMessage(from payload: NoisePayload) throws -> PrivateMessagePacket {