diff --git a/bitchat/App/ConversationUIModel.swift b/bitchat/App/ConversationUIModel.swift index 508ca493..a201eba6 100644 --- a/bitchat/App/ConversationUIModel.swift +++ b/bitchat/App/ConversationUIModel.swift @@ -164,10 +164,12 @@ final class ConversationUIModel: ObservableObject { !isSentByCurrentUser(message), let peerID = message.senderPeerID else { return false } guard let fingerprint = chatViewModel.getFingerprint(for: peerID) else { return false } - guard chatViewModel.peerIdentityStore.isVerified(fingerprint) else { return false } - // A verified key that renamed itself onto a name the user trusts must - // not carry the seal beside the new one. - return !chatViewModel.trustedNicknameMismatch(fingerprint) + // The row renders `message.sender`, frozen at receipt, so the question + // is whether THAT name is the one this key was verified under — not + // whether its current name is. Checking the current name would let a + // rename away, a post, and a rename back put a seal beside a name the + // key was never trusted under. One call, one lock. + return chatViewModel.sealAppliesToRow(fingerprint, renderedSender: message.sender) } func senderDisplayName(for peerID: PeerID, fallbackMessages: [BitchatMessage]) -> String? { diff --git a/bitchat/App/VerificationModel.swift b/bitchat/App/VerificationModel.swift index 4e1de457..992713c2 100644 --- a/bitchat/App/VerificationModel.swift +++ b/bitchat/App/VerificationModel.swift @@ -16,6 +16,15 @@ struct FingerprintPresentationState: Equatable { let voucherCount: Int /// Display names of the (verified) vouchers, where known. let voucherNames: [String] + /// The nickname this key was announcing when it was verified or first + /// vouched, if anything was bound. Non-nil and different from the name on + /// screen is what `nameChangedSinceVerification` reports. + let verifiedAsNickname: String? + /// The key is trusted, but it now announces a different name than when + /// that trust was established. The sheet is the one place this should be + /// explained rather than collapsed into a glyph, so it is surfaced here + /// instead of silently suppressing the badge. + let nameChangedSinceVerification: Bool /// Vouched for by ≥1 peer the user verified (and not explicitly verified). var isVouched: Bool { voucherCount > 0 } @@ -136,6 +145,13 @@ final class VerificationModel: ObservableObject { let isVerified = theirFingerprint.map { peerIdentityStore.isVerified($0) } ?? false let localPetname = theirFingerprint .flatMap { chatViewModel.identityManager.getSocialIdentity(for: $0)?.localPetname } + // A vouched seal is bound to the name it was earned under, exactly like + // the verified one. Missing baseline means nothing was bound, so this + // fails open like every other site. + let nameBound = theirFingerprint + .map { !chatViewModel.identityManager.trustedNicknameMismatch(fingerprint: $0) } ?? true + let verifiedAsNickname = theirFingerprint + .flatMap { chatViewModel.identityManager.trustedNickname(fingerprint: $0) } // Vouch state is recomputed on read: only vouchers still in the // verified set count, so removing a verification silently retires the @@ -161,8 +177,12 @@ final class VerificationModel: ObservableObject { myFingerprint: chatViewModel.getMyFingerprint(), isVerified: isVerified, localPetname: localPetname, - voucherCount: vouchers.count, - voucherNames: voucherNames + // Suppressed together: a voucher list beside a name nobody + // vouched for is the same claim as the seal itself. + voucherCount: nameBound ? vouchers.count : 0, + voucherNames: nameBound ? voucherNames : [], + verifiedAsNickname: verifiedAsNickname, + nameChangedSinceVerification: !nameBound ) } diff --git a/bitchat/Identity/SecureIdentityStateManager.swift b/bitchat/Identity/SecureIdentityStateManager.swift index 91b82624..622d9c58 100644 --- a/bitchat/Identity/SecureIdentityStateManager.swift +++ b/bitchat/Identity/SecureIdentityStateManager.swift @@ -130,9 +130,19 @@ protocol SecureIdentityStateManagerProtocol { func setVerified(fingerprint: String, verified: Bool) func isVerified(fingerprint: String) -> Bool func getVerifiedFingerprints() -> Set + /// The nickname `fingerprint` was announcing when trust in it was + /// established, or nil if nothing was bound. Shown to the user, so a sheet + /// can say *which* name was verified rather than only that one changed. + func trustedNickname(fingerprint: String) -> String? /// Whether this peer now announces a different nickname than the one its /// trust was earned under. func trustedNicknameMismatch(fingerprint: String) -> Bool + /// Verified AND still presenting the name it was verified under — what a + /// seal beside a LIVE name is actually asserting. One lock, not two. + func isVerifiedAndNameBound(fingerprint: String) -> Bool + /// Verified AND `renderedSender` is the name it was verified under — what + /// a seal beside a name frozen on a message row is asserting. + func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool // MARK: Vouching (transitive verification) @discardableResult @@ -153,6 +163,26 @@ protocol SecureIdentityStateManagerProtocol { func hasObservedPrivateMediaCapability(fingerprint: String) -> Bool } +extension SecureIdentityStateManagerProtocol { + // Default implementations so a conformance only has to provide the two + // primitives. `SecureIdentityStateManager` overrides both to answer in a + // single lock acquisition — these seal checks run per row, per render, and + // the format cache does not shield the one in ChatMessageFormatter because + // the answer is part of its cache key. + func isVerifiedAndNameBound(fingerprint: String) -> Bool { + isVerified(fingerprint: fingerprint) && !trustedNicknameMismatch(fingerprint: fingerprint) + } + + func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool { + guard isVerified(fingerprint: fingerprint) else { return false } + guard let pinned = trustedNickname(fingerprint: fingerprint), !pinned.isEmpty + else { return true } + let shown = renderedSender.withoutCollisionSuffix + guard !shown.isEmpty else { return true } + return pinned.normalizedNickname == shown.normalizedNickname + } +} + /// Singleton manager for secure identity state persistence and retrieval. /// Provides thread-safe access to identity mappings with encryption at rest. /// All identity data is stored encrypted in the device Keychain for security. @@ -765,19 +795,54 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { /// before this existed have none, and dropping their seals on upgrade would /// train users to ignore the signal. func trustedNicknameMismatch(fingerprint: String) -> Bool { + queue.sync { liveNameMismatchLocked(fingerprint) } + } + + func trustedNickname(fingerprint: String) -> String? { + queue.sync { cache.trustedNicknames?[fingerprint] } + } + + func isVerifiedAndNameBound(fingerprint: String) -> Bool { queue.sync { - guard let pinned = cache.trustedNicknames?[fingerprint], !pinned.isEmpty - else { return false } - let social = cache.socialIdentities[fingerprint] - // A local petname outranks the claimed nickname everywhere it is - // displayed, so there is nothing to spoof and the seal stands. - if let petname = social?.localPetname, !petname.isEmpty { return false } - guard let claimed = social?.claimedNickname, !claimed.isEmpty else { return false } - // NFC, matching `normalizedNickname` everywhere else nicknames are - // compared: a decomposed and a precomposed "café" are one name. - return pinned.normalizedNickname != claimed.normalizedNickname + guard cache.verifiedFingerprints.contains(fingerprint) else { return false } + return !liveNameMismatchLocked(fingerprint) } } + + /// A message row shows the sender name frozen at receipt, so asking about + /// the peer's CURRENT name is the wrong question: rename away, post, rename + /// back, and the live name matches the baseline again while the archived row + /// still reads the name it was posted under. Each row is therefore checked + /// against its own name, which also makes every row self-consistent — a + /// historical "ravi ✓" stays sealed even while that key is currently + /// renamed, because that row really was ravi. + func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool { + queue.sync { + guard cache.verifiedFingerprints.contains(fingerprint) else { return false } + guard let pinned = cache.trustedNicknames?[fingerprint], !pinned.isEmpty + else { return true } // nothing bound: fail open + // No petname escape here, unlike the live check: the formatter + // renders `message.sender`, never a petname, so a petname does not + // stand between the reader and a spoofed name on this row. + let shown = renderedSender.withoutCollisionSuffix + guard !shown.isEmpty else { return true } + return pinned.normalizedNickname == shown.normalizedNickname + } + } + + /// Requires `queue`. + private func liveNameMismatchLocked(_ fingerprint: String) -> Bool { + guard let pinned = cache.trustedNicknames?[fingerprint], !pinned.isEmpty + else { return false } + let social = cache.socialIdentities[fingerprint] + // A local petname outranks the claimed nickname everywhere a LIVE name + // is displayed, so there is nothing to spoof and the seal stands. + if let petname = social?.localPetname, !petname.isEmpty { return false } + guard let claimed = social?.claimedNickname, !claimed.isEmpty else { return false } + // NFC, matching `normalizedNickname` everywhere else nicknames are + // compared: a decomposed and a precomposed "café" are one name. + return pinned.normalizedNickname != claimed.normalizedNickname + } func getVerifiedFingerprints() -> Set { queue.sync { diff --git a/bitchat/Utils/PeerDisplayNameResolver.swift b/bitchat/Utils/PeerDisplayNameResolver.swift index a92953d3..cf6ec094 100644 --- a/bitchat/Utils/PeerDisplayNameResolver.swift +++ b/bitchat/Utils/PeerDisplayNameResolver.swift @@ -3,23 +3,40 @@ import BitFoundation /// Resolves a stable display name for peers, adding a short suffix when collisions exist. struct PeerDisplayNameResolver { + /// The form two nicknames are judged to collide in. Never displayed. + static func collisionKey(_ nickname: String) -> String { + nickname.precomposedStringWithCompatibilityMapping.lowercased() + } + /// Computes display names with a `#xxxx` suffix for connected peers when nickname collisions occur. /// - Parameters: /// - peers: Array of tuples (peerID, nickname, isConnected). /// - selfNickname: The local user's current nickname, included in collision counts to suffix remotes matching it. /// - Returns: Map of peerID -> displayName. static func resolve(_ peers: [(peerID: PeerID, nickname: String, isConnected: Bool)], selfNickname: String) -> [PeerID: String] { - // Count collisions among connected peers and include our own nickname + // Count collisions on a FOLDED key, not the displayed name. The point + // of the suffix is to make namesakes distinguishable, so anything that + // renders alike has to count as a collision. + // + // Compatibility mapping plus case folding covers more than it looks: + // fullwidth Ｍedic, mathematical 𝐌edic, script ℳedic and roman-numeral + // Ⅿedic all fold to "medic", and NFKC subsumes the canonical folding + // that Swift's String keys were already giving us for free. + // + // What it does NOT cover is cross-script homoglyphs — Cyrillic М and + // Greek Ο have their own compatibility forms — which needs a UTS #39 + // confusables table. Suffixing the displayed name is unchanged; only + // the collision key folds. var counts: [String: Int] = [:] for p in peers where p.isConnected { - counts[p.nickname, default: 0] += 1 + counts[Self.collisionKey(p.nickname), default: 0] += 1 } - counts[selfNickname, default: 0] += 1 + counts[Self.collisionKey(selfNickname), default: 0] += 1 var result: [PeerID: String] = [:] for p in peers { var name = p.nickname - if p.isConnected, (counts[p.nickname] ?? 0) > 1 { + if p.isConnected, (counts[Self.collisionKey(p.nickname)] ?? 0) > 1 { name += "#" + String(p.peerID.id.prefix(4)) } result[p.peerID] = name diff --git a/bitchat/Utils/String+Nickname.swift b/bitchat/Utils/String+Nickname.swift index 9a652667..2902613f 100644 --- a/bitchat/Utils/String+Nickname.swift +++ b/bitchat/Utils/String+Nickname.swift @@ -17,6 +17,21 @@ extension String { precomposedStringWithCanonicalMapping } + /// Strips ONLY a trailing `#abcd` collision suffix, leaving everything + /// else alone. + /// + /// Deliberately not `splitSuffix()`: that also removes every `@` in the + /// string, which is right for parsing a mention but wrong for comparing a + /// nickname — nothing in `validateNickname` forbids `@`, so `ravi@hq` + /// would compare unequal to itself. + var withoutCollisionSuffix: String { + guard count >= 5 else { return self } + let tail = suffix(5) + guard tail.first == "#", + tail.dropFirst().allSatisfy({ $0.isHexDigit }) else { return self } + return String(dropLast(5)) + } + /// Split a nickname into base and a '#abcd' suffix if present func splitSuffix() -> (String, String) { let name = self.replacingOccurrences(of: "@", with: "") diff --git a/bitchat/ViewModels/ChatMessageFormatter.swift b/bitchat/ViewModels/ChatMessageFormatter.swift index bb661a35..6b5f9899 100644 --- a/bitchat/ViewModels/ChatMessageFormatter.swift +++ b/bitchat/ViewModels/ChatMessageFormatter.swift @@ -448,10 +448,10 @@ private extension ChatMessageFormatter { let fingerprint = viewModel.getFingerprint(for: peerID) else { return false } - guard viewModel.peerIdentityStore.isVerified(fingerprint) else { return false } - // Bound to the name the key announces, for the same reason as the peer - // list and the DM seal: the badge attests to a key but is read as a name. - return !viewModel.trustedNicknameMismatch(fingerprint) + // Bound to the name ON THIS ROW, which is frozen at receipt — see + // `sealAppliesToRow`. This check feeds the format cache key, so the + // cache does not shield it; a single call keeps it to one lock. + return viewModel.sealAppliesToRow(fingerprint, renderedSender: message.sender) } func appendVerifiedSeal( diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index b2d9f48c..9e098ff9 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1982,6 +1982,20 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage identityManager.trustedNicknameMismatch(fingerprint: fingerprint) } + /// The name this key was verified under, for a sheet that needs to say + /// which one it was. + @MainActor + func trustedNickname(_ fingerprint: String) -> String? { + identityManager.trustedNickname(fingerprint: fingerprint) + } + + /// A seal beside a name frozen on a message row: is THAT name the one this + /// key was verified under? See `sealAppliesToRow`. + @MainActor + func sealAppliesToRow(_ fingerprint: String, renderedSender: String) -> Bool { + identityManager.sealAppliesToRow(fingerprint: fingerprint, renderedSender: renderedSender) + } + // MARK: - BitchatDelegate Methods // MARK: - Command Handling diff --git a/bitchatTests/Mocks/MockIdentityManager.swift b/bitchatTests/Mocks/MockIdentityManager.swift index f0e1b656..2e1fc12d 100644 --- a/bitchatTests/Mocks/MockIdentityManager.swift +++ b/bitchatTests/Mocks/MockIdentityManager.swift @@ -102,6 +102,8 @@ final class MockIdentityManager: SecureIdentityStateManagerProtocol { var claimedNicknames: [String: String] = [:] + func trustedNickname(fingerprint: String) -> String? { trustedNicknames[fingerprint] } + func trustedNicknameMismatch(fingerprint: String) -> Bool { guard let pinned = trustedNicknames[fingerprint], !pinned.isEmpty, let claimed = claimedNicknames[fingerprint], !claimed.isEmpty else { return false } diff --git a/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift b/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift index 33945919..060a2693 100644 --- a/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift +++ b/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift @@ -195,6 +195,91 @@ struct SecureIdentityStateManagerNicknameBindingTests { #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) } + // MARK: - A seal beside a name frozen on a message row + + @Test + func renamingBackDoesNotRestoreTheSealOnARowPostedUnderTheOtherName() { + // The hole in checking only the CURRENT name: a message row renders + // `message.sender` frozen at receipt, so rename away, post, rename + // back, and the live name matches the baseline again while the archived + // row still reads the name it was posted under. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + + announce(manager, vouchee, as: "medic") // rename + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + announce(manager, vouchee, as: "ravi") // ...and back + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "the live name is bound again, which is why the live check alone is not enough") + + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic"), + "the row posted as medic must not be sealed") + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi"), + "a row posted as ravi still is — that row really was ravi") + } + + @Test + func aHistoricalRowKeepsItsSealWhileTheKeyIsCurrentlyRenamed() { + // The converse, so the rule is not simply "suppress harder": a row + // posted under the verified name stays truthful even once that key has + // moved on, otherwise ordinary renames retroactively unseal history. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "medic") + + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee), "live name is unbound") + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi"), + "but the archived ravi row is still accurate") + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic")) + } + + @Test + func aRowSenderCarryingACollisionSuffixIsStillMatched() { + // Message senders render as "ravi#a1b2" when nicknames collide. Only a + // trailing #abcd is stripped — `splitSuffix()` would also remove every + // "@", and nothing forbids one in a nickname. + let manager = makeManager() + announce(manager, vouchee, as: "ravi@hq") + manager.setVerified(fingerprint: vouchee, verified: true) + + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi@hq")) + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi@hq#a1b2")) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi")) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic#a1b2")) + } + + @Test + func aRowIsNotSealedForAnUnverifiedKeyAndFailsOpenWithNoBaseline() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi"), + "no verification, no seal") + + // Verified before any announce: nothing bound, so rows stay sealed as + // they did before this existed. + let unbound = makeManager() + unbound.setVerified(fingerprint: vouchee, verified: true) + #expect(unbound.sealAppliesToRow(fingerprint: vouchee, renderedSender: "anything")) + } + + @Test + func theCombinedLiveQueryAgreesWithItsParts() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + #expect(!manager.isVerifiedAndNameBound(fingerprint: vouchee), "not verified yet") + + manager.setVerified(fingerprint: vouchee, verified: true) + #expect(manager.isVerifiedAndNameBound(fingerprint: vouchee)) + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi") + + announce(manager, vouchee, as: "medic") + #expect(!manager.isVerifiedAndNameBound(fingerprint: vouchee)) + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi", + "the baseline is what the sheet needs to name") + } + // MARK: - Rebinding and clearing @Test diff --git a/bitchatTests/Services/UnifiedPeerServiceTests.swift b/bitchatTests/Services/UnifiedPeerServiceTests.swift index 0176634a..ec604c23 100644 --- a/bitchatTests/Services/UnifiedPeerServiceTests.swift +++ b/bitchatTests/Services/UnifiedPeerServiceTests.swift @@ -291,6 +291,7 @@ private final class TestIdentityManager: SecureIdentityStateManagerProtocol { // No name binding is exercised here: these tests drive peer-list assembly, // not trust display. + func trustedNickname(fingerprint: String) -> String? { nil } func trustedNicknameMismatch(fingerprint: String) -> Bool { false } // MARK: Vouching (unused by these tests) diff --git a/bitchatTests/Utils/PeerDisplayNameResolverTests.swift b/bitchatTests/Utils/PeerDisplayNameResolverTests.swift new file mode 100644 index 00000000..10a2ee48 --- /dev/null +++ b/bitchatTests/Utils/PeerDisplayNameResolverTests.swift @@ -0,0 +1,82 @@ +import Foundation +import BitFoundation +import Testing + +@testable import bitchat + +/// The `#abcd` suffix exists so two peers claiming one nickname are +/// distinguishable. It is therefore only as good as the notion of "one +/// nickname" it counts with — anything that renders alike has to collide, or +/// the guard stays silent exactly when two rows look the same. +@MainActor +struct PeerDisplayNameResolverTests { + private let real = PeerID(str: "aaaa1111") + private let other = PeerID(str: "bbbb2222") + + /// True when the resolver decided these two need telling apart. + private func suffixed(_ a: String, _ b: String, + connected: Bool = true, selfNickname: String = "me") -> Bool { + let out = PeerDisplayNameResolver.resolve( + [(peerID: real, nickname: a, isConnected: true), + (peerID: other, nickname: b, isConnected: connected)], + selfNickname: selfNickname) + return (out[real] ?? "").contains("#") || (out[other] ?? "").contains("#") + } + + @Test + func namesThatRenderAlikeCollide() { + #expect(suffixed("Medic", "Medic"), "exact duplicate") + #expect(suffixed("Medic", "medic"), "case alone must not make two peers look distinct") + #expect(suffixed("Medic", "\u{FF2D}edic"), "fullwidth Ｍ") + #expect(suffixed("Medic", "\u{1D40C}edic"), "mathematical bold 𝐌") + #expect(suffixed("Medic", "\u{2133}edic"), "script ℳ") + #expect(suffixed("Medic", "\u{216F}edic"), "roman numeral Ⅿ") + #expect(suffixed("Jos\u{00E9}", "Jose\u{0301}"), "canonically equivalent spellings") + } + + @Test + func unrelatedNamesDoNotCollide() { + #expect(!suffixed("Medic", "Zebra")) + #expect(!suffixed("Medic", "Medic2")) + #expect(!suffixed("Medic", "")) + } + + @Test + func crossScriptHomoglyphsAreStillMissed() { + // Cyrillic М and Greek Ο have their own compatibility forms, so + // folding cannot reach them — that needs a UTS #39 confusables table. + // Pinned as a KNOWN gap so it is explicit rather than a surprise, and + // so whoever adds the table has a test to flip. + #expect(!suffixed("Medic", "\u{041C}\u{0435}dic"), "Cyrillic М + е: known gap") + #expect(!suffixed("Organizer", "\u{039F}rganizer"), "Greek Ο: known gap") + } + + @Test + func onlyConnectedPeersAreSuffixed() { + // Unchanged by the folding: an offline namesake still gets no suffix, + // which is its own gap and not one this touches. + #expect(!suffixed("Medic", "medic", connected: false)) + } + + @Test + func ourOwnNicknameCountsTowardCollisions() { + // A remote peer claiming a folded variant of MY nickname must be + // suffixed, or it renders as me. + let out = PeerDisplayNameResolver.resolve( + [(peerID: other, nickname: "\u{FF2D}edic", isConnected: true)], + selfNickname: "Medic") + #expect((out[other] ?? "").contains("#")) + } + + @Test + func theDisplayedNameIsNeverFolded() { + // Only the collision KEY folds. What is shown stays as its owner typed + // it, suffix aside. + let out = PeerDisplayNameResolver.resolve( + [(peerID: real, nickname: "Medic", isConnected: true), + (peerID: other, nickname: "\u{FF2D}edic", isConnected: true)], + selfNickname: "me") + #expect(out[real]?.hasPrefix("Medic") == true) + #expect(out[other]?.hasPrefix("\u{FF2D}edic") == true, "the fullwidth Ｍ is preserved on screen") + } +}