diff --git a/bitchat/App/ConversationUIModel.swift b/bitchat/App/ConversationUIModel.swift index 4883a04c..920cbfff 100644 --- a/bitchat/App/ConversationUIModel.swift +++ b/bitchat/App/ConversationUIModel.swift @@ -164,7 +164,13 @@ final class ConversationUIModel: ObservableObject { !isSentByCurrentUser(message), let peerID = message.senderPeerID else { return false } guard let fingerprint = chatViewModel.getFingerprint(for: peerID) else { return false } - return chatViewModel.peerIdentityStore.isVerified(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, + senderPeerID: peerID) } func senderDisplayName(for peerID: PeerID, fallbackMessages: [BitchatMessage]) -> String? { diff --git a/bitchat/App/PeerListModel.swift b/bitchat/App/PeerListModel.swift index 4f1fc633..4cf8bb51 100644 --- a/bitchat/App/PeerListModel.swift +++ b/bitchat/App/PeerListModel.swift @@ -232,9 +232,20 @@ final class PeerListModel: ObservableObject { let isMe = peer.peerID == myPeerID let fingerprint = isMe ? nil : chatViewModel.getFingerprint(for: peer.peerID) let isVerifiedFingerprint = fingerprint.map { peerIdentityStore.isVerified($0) } ?? false - let verifiedBadge = !peer.isConnected && isVerifiedFingerprint + // A seal is bound to the name it was earned under. Without that, + // a key that earned one vouch as "ravi" can rename itself to + // "medic" and keep rendering the seal beside the new name. The + // check compares announced names, NOT `peer.nickname`, which is + // already collision-resolved: two connected peers both claiming + // "medic" are rendered "medic#a1b2" and "medic#c3d4", so comparing + // the displayed string would drop the real medic's seal during the + // very attack this defends against. Petnames are handled there too. + let renamedSinceTrust = fingerprint.map { + chatViewModel.trustedNicknameMismatch($0) + } ?? false + let verifiedBadge = !peer.isConnected && isVerifiedFingerprint && !renamedSinceTrust // Vouched is subordinate to verified: never show both seals. - let vouchedBadge = !isVerifiedFingerprint + let vouchedBadge = !isVerifiedFingerprint && !renamedSinceTrust && (fingerprint.map { chatViewModel.isVouchedFingerprint($0) } ?? false) return MeshPeerRow( diff --git a/bitchat/App/VerificationModel.swift b/bitchat/App/VerificationModel.swift index 4e1de457..d7ea4f63 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 @@ -146,11 +162,30 @@ final class VerificationModel: ObservableObject { } else { vouchers = [] } + // Each voucher is named by the name YOU VERIFIED IT UNDER, not by + // whatever it announces now. + // + // This list is a trust attribution — "people you verified vouch for + // this peer" — so it has the same problem as a seal, one level out. Use + // the live name and a voucher that renamed is attributed under its new + // one: Eve gets verified as "ravi", vouches for Mallory, renames to + // "medic", and Mallory's sheet reads "vouched by medic". The vouch is + // real and the key is trusted; the NAME beside it was never verified, + // which is the whole subject of this change. + // + // Order: a petname first, because it is your own label and nothing on + // the network can influence it; then the pinned baseline; then the + // claimed nickname, for vouchers recorded before baselines existed — + // the same fail-open as everywhere else here. let voucherNames = vouchers.compactMap { record -> String? in guard let social = chatViewModel.identityManager.getSocialIdentity(for: record.voucherFingerprint) else { return nil } if let petname = social.localPetname, !petname.isEmpty { return petname } + if let pinned = chatViewModel.identityManager.trustedNickname( + fingerprint: record.voucherFingerprint), !pinned.isEmpty { + return pinned + } return social.claimedNickname.isEmpty ? nil : social.claimedNickname } @@ -161,8 +196,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/IdentityModels.swift b/bitchat/Identity/IdentityModels.swift index 8a72fcba..f82169f6 100644 --- a/bitchat/Identity/IdentityModels.swift +++ b/bitchat/Identity/IdentityModels.swift @@ -202,6 +202,15 @@ struct IdentityCache: Codable { // entries verified before this field exists sort as oldest) var verifiedAt: [String: Date]? = nil + /// The self-claimed nickname each fingerprint was presenting at the moment + /// trust in it was established here — set on verification and on a first + /// accepted vouch. A trust badge means "this key was the Medic I checked", + /// so it has to be pinned to the name it was earned under; without this + /// baseline a vouched key can rename itself onto a name the user trusts and + /// keep rendering the seal beside it. Absent for peers trusted by older + /// builds, which is why a missing entry never suppresses a badge. + var trustedNicknames: [String: String]? = nil + // Stable Noise fingerprints that proved encrypted private-media support // inside an authenticated Noise session. Optional for decoding caches // written before this migration. Entries are monotonic until a panic wipe @@ -240,6 +249,7 @@ struct IdentityCache: Codable { vouchesByVouchee = try container.decodeIfPresent([String: [VouchRecord]].self, forKey: .vouchesByVouchee) vouchBatchSentAt = try container.decodeIfPresent([String: Date].self, forKey: .vouchBatchSentAt) verifiedAt = try container.decodeIfPresent([String: Date].self, forKey: .verifiedAt) + trustedNicknames = try container.decodeIfPresent([String: String].self, forKey: .trustedNicknames) privateMediaCapableFingerprints = try container.decodeIfPresent(Set.self, forKey: .privateMediaCapableFingerprints) authenticatedSigningKeysByFingerprint = try container.decodeIfPresent([String: Data].self, forKey: .authenticatedSigningKeysByFingerprint) cryptographicIdentities = try container.decodeIfPresent([String: CryptographicIdentity].self, forKey: .cryptographicIdentities) ?? [:] diff --git a/bitchat/Identity/SecureIdentityStateManager.swift b/bitchat/Identity/SecureIdentityStateManager.swift index 4204e940..2f0f7b54 100644 --- a/bitchat/Identity/SecureIdentityStateManager.swift +++ b/bitchat/Identity/SecureIdentityStateManager.swift @@ -130,6 +130,16 @@ 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 `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, senderPeerID: PeerID?) -> Bool // MARK: Vouching (transitive verification) @discardableResult @@ -150,6 +160,20 @@ protocol SecureIdentityStateManagerProtocol { func hasObservedPrivateMediaCapability(fingerprint: String) -> Bool } +extension SecureIdentityStateManagerProtocol { + // A default implementation so a conformance only has to provide the two + // primitives. `SecureIdentityStateManager` overrides it to answer in a + // single lock acquisition — this runs per row, per render, and the format + // cache does not shield the call in ChatMessageFormatter because the + // answer is part of its cache key. + func sealAppliesToRow(fingerprint: String, renderedSender: String, senderPeerID: PeerID?) -> Bool { + guard isVerified(fingerprint: fingerprint) else { return false } + guard let pinned = trustedNickname(fingerprint: fingerprint), !pinned.isEmpty + else { return true } + return pinned.matchesRenderedName(renderedSender, decoratedFor: senderPeerID) + } +} + /// 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. @@ -428,6 +452,15 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { } else if self.cache.socialIdentities[fingerprint] == nil { self.cache.socialIdentities[fingerprint] = identity } + // A vouch usually arrives for a peer we have not seen announce + // — it comes over Noise from someone else, and the vouchee may + // be several hops away. There was no name to bind to then, so + // bind on the first announce we see while the trust stands. + self.pinTrustedNicknameLocked( + fingerprint: fingerprint, + overwrite: false, + requireExistingTrust: true + ) } self.saveIdentityCache() @@ -678,9 +711,15 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { var verifiedAt = self.cache.verifiedAt ?? [:] verifiedAt[fingerprint] = Date() self.cache.verifiedAt = verifiedAt + // Re-verifying overwrites the baseline: the user just checked + // this key again, under whatever name it presents now. + self.pinTrustedNicknameLocked(fingerprint: fingerprint, overwrite: true) } else { self.cache.verifiedFingerprints.remove(fingerprint) self.cache.verifiedAt?.removeValue(forKey: fingerprint) + if self.cache.vouchesByVouchee?[fingerprint]?.isEmpty ?? true { + self.cache.trustedNicknames?.removeValue(forKey: fingerprint) + } } // Update trust level if social identity exists @@ -698,6 +737,94 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { return cache.verifiedFingerprints.contains(fingerprint) } } + + // MARK: - Nickname binding + // + // A vouch signs `voucheeFingerprint | voucheeSigningKey | timestampMs` and + // deliberately says nothing about a name — the attestation is right to stay + // name-free. But the badge is *rendered* next to a self-claimed nickname, + // so the binding has to exist somewhere, and the receiver is the only party + // that can hold it: it is the one that decided to trust this key while it + // was presenting a particular name. + + /// Requires `queue`. Records the peer's currently claimed nickname as the + /// name its trust is bound to. Empty nicknames are not pinned — a key + /// verified before its first announce has no name to bind to, and pinning + /// "" would then read as a mismatch against every later announce. + /// + /// `requireExistingTrust` is for the announce path, which runs for every + /// peer: pinning a name before there is any trust would bind the badge to + /// whatever name we happened to see first, so a peer who renamed *before* + /// being vouched would have its legitimate badge suppressed. + private func pinTrustedNicknameLocked(fingerprint: String, + overwrite: Bool, + requireExistingTrust: Bool = false) { + if requireExistingTrust { + let trusted = cache.verifiedFingerprints.contains(fingerprint) + || !(cache.vouchesByVouchee?[fingerprint] ?? []).isEmpty + guard trusted else { return } + } + guard let claimed = cache.socialIdentities[fingerprint]?.claimedNickname, + !claimed.isEmpty else { return } + var pinned = cache.trustedNicknames ?? [:] + if !overwrite, pinned[fingerprint] != nil { return } + pinned[fingerprint] = claimed + cache.trustedNicknames = pinned + } + + /// True only when a baseline exists AND the peer now announces something + /// else. + /// + /// Deliberately compares announced name to announced name rather than to + /// anything on screen. Display strings are decorated: `PeerDisplayNameResolver` + /// appends `#abcd` to CONNECTED peers whose nicknames collide — which is + /// precisely what happens during this attack — so comparing a rendered name + /// would suppress the seal of the peer being impersonated, exactly when it + /// matters most. + /// + /// Fails OPEN on a missing baseline on purpose: peers trusted by builds + /// 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] } + } + + /// 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, senderPeerID: PeerID?) -> 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. + return pinned.matchesRenderedName(renderedSender, decoratedFor: senderPeerID) + } + } + + /// 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 } + // See `nicknameBindingKey`: NFC plus case, so a decomposed and a + // precomposed "café" are one name and recasing is not a rename. + return pinned.nicknameBindingKey != claimed.nicknameBindingKey + } func getVerifiedFingerprints() -> Set { queue.sync { @@ -760,6 +887,10 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { var vouches = self.cache.vouchesByVouchee ?? [:] vouches[voucheeFingerprint] = capped self.cache.vouchesByVouchee = vouches + // Pin on the FIRST vouch only. A later vouch for the same key must + // not move the baseline, or an attacker could rename and then have + // a second voucher silently re-anchor the badge to the new name. + self.pinTrustedNicknameLocked(fingerprint: voucheeFingerprint, overwrite: false) self.saveIdentityCache() return true } diff --git a/bitchat/Services/BLE/BLEPeerSenderDisplayName.swift b/bitchat/Services/BLE/BLEPeerSenderDisplayName.swift index c17d8d7e..4f4868f4 100644 --- a/bitchat/Services/BLE/BLEPeerSenderDisplayName.swift +++ b/bitchat/Services/BLE/BLEPeerSenderDisplayName.swift @@ -50,9 +50,16 @@ enum BLEPeerSenderDisplayName { localNickname: String, peers: [PeerID: BLEPeerInfo] ) -> String { + // Folded, for the same reason as `PeerDisplayNameResolver`: the suffix + // exists to make namesakes distinguishable, so a case variant or a + // fullwidth M has to count as a collision. This is the second place + // collisions are counted — it feeds sender names on file transfers — + // and it had the same exact-match blind spot. + let target = PeerDisplayNameResolver.collisionKey(collisionNickname) let hasCollision = peers.values.contains { - $0.isConnected && $0.nickname == collisionNickname && $0.peerID != peerID - } || localNickname == collisionNickname + $0.isConnected && PeerDisplayNameResolver.collisionKey($0.nickname) == target + && $0.peerID != peerID + } || PeerDisplayNameResolver.collisionKey(localNickname) == target guard hasCollision else { return displayName } return displayName + "#" + String(peerID.id.prefix(4)) 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 Medic, 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..46ef3c70 100644 --- a/bitchat/Utils/String+Nickname.swift +++ b/bitchat/Utils/String+Nickname.swift @@ -7,6 +7,7 @@ // import Foundation +import BitFoundation // PeerID, for the row decoration check extension String { /// Canonical form for nickname storage and comparison (Unicode NFC). @@ -17,6 +18,76 @@ extension String { precomposedStringWithCanonicalMapping } + /// The form two nicknames are compared in to decide whether they are the + /// SAME NAME — used for the verified-name binding. + /// + /// NFC plus a locale-independent case fold, twice normalised because case + /// folding can itself emit decomposed sequences (Turkish İ lowercases to + /// i + U+0307). Recasing your own nickname is not a rename, so it must not + /// break the binding. + /// + /// Deliberately NFC and not NFKC: a fullwidth `Medic` merely *looks* like + /// `Medic`, so it is a different name and must break the binding. Folding + /// look-alikes is `PeerDisplayNameResolver.collisionKey`'s job, which is + /// asking a different question — whether two peers need telling apart. + var nicknameBindingKey: String { + normalizedNickname.lowercased().normalizedNickname + } + + /// 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) + // ASCII hex only, matching how `splitSuffix()` recognises the suffix + // this device generates. `Character.isHexDigit` would also accept + // fullwidth digits, so a nickname literally ending in "#ABCD" + // would be truncated and could then match a baseline it is not. + let isAsciiHex: (Character) -> Bool = { c in + ("0"..."9").contains(c) || ("a"..."f").contains(c) || ("A"..."F").contains(c) + } + guard tail.first == "#", tail.dropFirst().allSatisfy(isAsciiHex) else { return self } + return String(dropLast(5)) + } + + /// Does a seal pinned to `self` still apply beside `rendered` — a name as + /// it appears on a message row, which the list may have decorated? + /// + /// `decoration` is the peer's own `#abcd`, built from its peerID exactly as + /// `PeerDisplayNameResolver` builds it. That is what makes this answerable + /// at all: a row string on its own cannot distinguish the list decorating + /// `ravi` with `#a1b2` from the peer announcing the name `ravi#a1b2`. + /// + /// This used to strip any trailing `#` plus four hex, unconditionally, and + /// was wrong in both directions because a peer announces whatever string it + /// likes and `#` plus four hex is a legal thing to announce: + /// + /// pinned `ravi#cafe`, row shows `ravi#cafe` → seal LOST with no rename + /// pinned `ravi`, row shows `ravi#cafe` → seal KEPT after a rename + /// + /// The second is the one that matters, and it survives a "try the raw name + /// first" fix — which is why this takes the peerID instead. `#cafe` is + /// indistinguishable from the decoration this app generates itself, so the + /// row reads as "ravi, disambiguated": a better disguise than an unrelated + /// nickname, not a worse one. Only the suffix this peer would actually be + /// given comes off. + /// + /// The live check must NOT use this. What a peer announces has no + /// decoration to remove. + func matchesRenderedName(_ rendered: String, decoratedFor peerID: PeerID?) -> Bool { + if nicknameBindingKey == rendered.nicknameBindingKey { return true } + guard let peerID else { return false } + let mine = "#" + String(peerID.id.prefix(4)) + guard rendered.count > mine.count, rendered.hasSuffix(mine) else { return false } + let undecorated = String(rendered.dropLast(mine.count)) + return nicknameBindingKey == undecorated.nicknameBindingKey + } + /// 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 1a00315b..1a31d591 100644 --- a/bitchat/ViewModels/ChatMessageFormatter.swift +++ b/bitchat/ViewModels/ChatMessageFormatter.swift @@ -448,7 +448,11 @@ private extension ChatMessageFormatter { let fingerprint = viewModel.getFingerprint(for: peerID) else { return false } - return viewModel.peerIdentityStore.isVerified(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, + senderPeerID: peerID) } func appendVerifiedSeal( diff --git a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift index e2d0fae8..0d6d92b8 100644 --- a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift +++ b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift @@ -76,6 +76,8 @@ protocol ChatPeerIdentityContext: AnyObject { /// falling back to `fallback` when none was stored. Returns the migrated fingerprint. func migrateFingerprintMapping(from oldPeerID: PeerID, to newPeerID: PeerID, fallback: String?) -> String? func isVerifiedFingerprint(_ fingerprint: String) -> Bool + /// Whether the peer renamed itself since its trust was established. + func trustedNicknameMismatch(_ fingerprint: String) -> Bool func setEncryptionStatus(_ status: EncryptionStatus?, for peerID: PeerID) func cachedEncryptionStatus(for peerID: PeerID) -> EncryptionStatus? func setCachedEncryptionStatus(_ status: EncryptionStatus, for peerID: PeerID) @@ -437,7 +439,9 @@ final class ChatPeerIdentityCoordinator { @MainActor func getEncryptionStatus(for peerID: PeerID) -> EncryptionStatus { if let cachedStatus = context.cachedEncryptionStatus(for: peerID) { - return cachedStatus + // Demote on the cache-hit path too — this is the common one, and + // nothing invalidates the cache on a rename. + return demotedIfRenamed(cachedStatus, for: peerID) } // The status must reflect the LIVE session, never history. The old @@ -462,7 +466,33 @@ final class ChatPeerIdentityCoordinator { } context.setCachedEncryptionStatus(status, for: peerID) - return status + return demotedIfRenamed(status, for: peerID) + } + + /// `.noiseVerified` renders the same filled seal as the verified badge, and + /// for a CONNECTED peer it is the only thing that draws one — so the name + /// binding has to apply here too, or the live impersonation keeps its seal. + /// + /// Applied on the way out rather than stored: the cached status is only + /// invalidated by verification and session changes, never by a rename, so a + /// demotion written into the cache would be both stale and sticky. The + /// session really is Noise-secured; only the claim about *who* is weakened, + /// which is exactly the difference between the two cases. + /// + /// No BEHAVIOUR branches on the distinction — `VerificationModel:33`, + /// `ContentSheetViews:659` and `:760` all treat `.noiseVerified` and + /// `.noiseSecured` alike. Four things the user perceives do, and all four + /// are meant to move: the icon (`checkmark.seal.fill` → `lock.fill`), the + /// `description` caption, the `accessibilityDescription` — so VoiceOver + /// stops saying "verified" on a demoted DM header — and the green tint on + /// the nickname at `FingerprintView:87`, which is the one site that does + /// NOT treat the two statuses identically. + @MainActor + private func demotedIfRenamed(_ status: EncryptionStatus, for peerID: PeerID) -> EncryptionStatus { + guard status == .noiseVerified, + let fingerprint = getFingerprint(for: peerID), + context.trustedNicknameMismatch(fingerprint) else { return status } + return .noiseSecured } @MainActor diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 63fba0f8..3753cb0f 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1973,6 +1973,31 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage identityManager.isVouched(fingerprint: fingerprint) } + /// Whether this peer now announces a different nickname than the one its + /// trust was established under. Every seal is suppressed in that case: the + /// seal attests to a key, but it is read as a name, and a key that renamed + /// itself onto a name the user trusts is the impersonation this catches. + @MainActor + func trustedNicknameMismatch(_ fingerprint: String) -> Bool { + 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, + senderPeerID: PeerID?) -> Bool { + identityManager.sealAppliesToRow(fingerprint: fingerprint, renderedSender: renderedSender, + senderPeerID: senderPeerID) + } + // MARK: - BitchatDelegate Methods // MARK: - Command Handling diff --git a/bitchat/Views/FingerprintView.swift b/bitchat/Views/FingerprintView.swift index bce7e89c..68b03d08 100644 --- a/bitchat/Views/FingerprintView.swift +++ b/bitchat/Views/FingerprintView.swift @@ -223,14 +223,33 @@ struct FingerprintView: View { // Verification status if fingerprintState.canToggleVerification { + // The seal is withheld when this key now presents a name it + // was not verified under — and the WORD goes with the glyph. + // + // An earlier revision kept "✓ VERIFIED" here on the grounds + // that the key genuinely is verified. Review put the + // objection better than I had: this badge sits a few lines + // under `peerNickname`, so it is not a claim about a bare + // key, it is a claim about THIS NAME — and that claim is + // false. Worse, this sheet is the screen someone opens + // *because* a seal vanished from a row, so the one surface + // they consult to resolve the doubt was the one endorsing + // the rename. + // + // No new string: the existing not-verified copy names the + // current nickname and offers to verify it, which is the + // recovery — re-verifying re-pins the baseline to the name + // on screen. The key's stored verified state is untouched. + let sealApplies = fingerprintState.isVerified + && !fingerprintState.nameChangedSinceVerification VStack(spacing: 12) { - Text(fingerprintState.isVerified ? Strings.verifiedBadge : Strings.notVerifiedBadge) + Text(sealApplies ? Strings.verifiedBadge : Strings.notVerifiedBadge) .bitchatFont(size: 14, weight: .bold) - .foregroundColor(fingerprintState.isVerified ? Color.green : Color.orange) + .foregroundColor(sealApplies ? Color.green : Color.orange) .frame(maxWidth: .infinity) Group { - if fingerprintState.isVerified { + if sealApplies { Text(Strings.verifiedMessage) } else { Text(Strings.verifyHint(fingerprintState.peerNickname)) @@ -243,7 +262,7 @@ struct FingerprintView: View { .fixedSize(horizontal: false, vertical: true) .frame(maxWidth: .infinity) - if !fingerprintState.isVerified { + if !sealApplies { Button(action: { verificationModel.verifyFingerprint(for: peerID) dismiss() diff --git a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift index 0bfcd306..d6e69821 100644 --- a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift +++ b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift @@ -144,6 +144,12 @@ private final class MockChatPeerIdentityContext: ChatPeerIdentityContext { verifiedFingerprintSet.contains(fingerprint) } + var renamedSinceTrust: Set = [] + + func trustedNicknameMismatch(_ fingerprint: String) -> Bool { + renamedSinceTrust.contains(fingerprint) + } + func setEncryptionStatus(_ status: EncryptionStatus?, for peerID: PeerID) { encryptionStatuses[peerID] = status } @@ -331,6 +337,52 @@ struct ChatPeerIdentityCoordinatorContextTests { #expect(context.unreadPrivateMessages.isEmpty) } + @Test @MainActor + func getEncryptionStatus_demotesToSecuredWhenTheKeyRenamedSinceTrust() async { + // `.noiseVerified` renders the same filled seal as the verified badge, + // and for a CONNECTED peer it is the only thing that draws one. A + // verified key that renames itself onto a trusted name must therefore + // lose it here too, or the live impersonation keeps its seal. + let context = MockChatPeerIdentityContext() + let coordinator = ChatPeerIdentityCoordinator(context: context) + let peerID = PeerID(str: "1122334455667788") + + context.noiseSessionStates[peerID] = .established + context.fingerprintsByPeerID[peerID] = "fp" + context.verifiedFingerprintSet = ["fp"] + #expect(coordinator.getEncryptionStatus(for: peerID) == .noiseVerified) + + // The rename lands. Nothing invalidates the encryption cache on a + // nickname change, so the demotion has to survive the cached value. + context.renamedSinceTrust = ["fp"] + #expect(coordinator.getEncryptionStatus(for: peerID) == .noiseSecured, + "still encrypted, no longer a claim about who") + #expect(context.cachedEncryptionStatuses[peerID] == .noiseVerified, + "the demotion is computed on read, never written into the cache") + + // Re-verifying under the new name restores it. + context.renamedSinceTrust = [] + #expect(coordinator.getEncryptionStatus(for: peerID) == .noiseVerified) + } + + @Test @MainActor + func getEncryptionStatus_leavesUnverifiedStatusesAlone() async { + let context = MockChatPeerIdentityContext() + let coordinator = ChatPeerIdentityCoordinator(context: context) + let peerID = PeerID(str: "1122334455667788") + + // A secured-but-unverified session has no seal to lose, and a rename + // must not disturb the lock it does show. + context.noiseSessionStates[peerID] = .established + context.fingerprintsByPeerID[peerID] = "fp" + context.renamedSinceTrust = ["fp"] + #expect(coordinator.getEncryptionStatus(for: peerID) == .noiseSecured) + + coordinator.invalidateEncryptionCache(for: peerID) + context.noiseSessionStates[peerID] = .handshaking + #expect(coordinator.getEncryptionStatus(for: peerID) == .noiseHandshaking) + } + @Test @MainActor func getEncryptionStatus_computesVerifiedStatusAndCachesIt() async { let context = MockChatPeerIdentityContext() diff --git a/bitchatTests/Mocks/MockIdentityManager.swift b/bitchatTests/Mocks/MockIdentityManager.swift index 930d57ff..8d00187b 100644 --- a/bitchatTests/Mocks/MockIdentityManager.swift +++ b/bitchatTests/Mocks/MockIdentityManager.swift @@ -98,6 +98,22 @@ final class MockIdentityManager: SecureIdentityStateManagerProtocol { func setVerified(fingerprint: String, verified: Bool) {} + var trustedNicknames: [String: String] = [:] + + 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 } + // Folded like the real one. A mock that compared raw strings would + // call a recase a rename, so a view test could pass here and fail in + // production — and which fold applies is the whole subject of this + // change, so it is the last thing a double should get wrong. + return pinned.nicknameBindingKey != claimed.nicknameBindingKey + } + func isVerified(fingerprint: String) -> Bool { true } diff --git a/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift b/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift new file mode 100644 index 00000000..9a455438 --- /dev/null +++ b/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift @@ -0,0 +1,512 @@ +import Foundation +import BitFoundation +import Testing + +@testable import bitchat + +/// Binds a trust seal to the nickname it was earned under. +/// +/// `VouchAttestation` signs `voucheeFingerprint | voucheeSigningKey | +/// timestampMs` and deliberately says nothing about a name — a name-free +/// attestation is the right wire format. But the seal is *rendered* beside a +/// self-claimed nickname, so the binding has to live on the receiver, which is +/// the only party that knows what name the key was presenting when it decided +/// to trust it. These tests pin that rule. +/// +/// `@MainActor` for the same reason as `SecureIdentityStateManagerVouchTests`: +/// the manager's blocking `queue.sync` reads must stay off the Swift +/// Concurrency cooperative pool, or CI's few-core runners deadlock. +@MainActor +struct SecureIdentityStateManagerNicknameBindingTests { + private let voucher = String(repeating: "0a", count: 32) + private let secondVoucher = String(repeating: "0c", count: 32) + private let vouchee = String(repeating: "0b", count: 32) + + private func makeManager() -> SecureIdentityStateManager { + SecureIdentityStateManager(MockKeychain()) + } + + /// Mirrors the announce path: the peer tells us what it calls itself. + private func announce(_ manager: SecureIdentityStateManager, + _ fingerprint: String, + as nickname: String) { + manager.upsertCryptographicIdentity( + fingerprint: fingerprint, + noisePublicKey: Data(repeating: 0x01, count: 32), + signingPublicKey: nil, + claimedNickname: nickname + ) + } + + private func setPetname(_ manager: SecureIdentityStateManager, + _ fingerprint: String, + _ petname: String?) { + guard var identity = manager.getSocialIdentity(for: fingerprint) else { return } + identity.localPetname = petname + manager.updateSocialIdentity(identity) + } + + // MARK: - The attack this closes + + @Test + func vouchPinsTheNicknameItWasEarnedUnder() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: voucher, verified: true) + + #expect(manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date())) + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + } + + @Test + func renamingOntoATrustedNameAfterEarningAVouchIsAMismatch() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: voucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date()) + + announce(manager, vouchee, as: "medic") + + #expect(manager.isVouched(fingerprint: vouchee), + "the vouch itself is still valid; only its binding to a name broke") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + } + + @Test + func aLaterVoucherCannotReanchorTheBaselineToTheNewName() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: voucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date()) + announce(manager, vouchee, as: "medic") + + manager.setVerified(fingerprint: secondVoucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: secondVoucher, timestamp: Date()) + + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee), + "the second vouch did not launder the rename") + } + + // MARK: - Vouches for peers we have not seen yet + + @Test + func aVouchForAnUnseenPeerBindsOnTheirFirstAnnounce() { + // The usual case: the vouch arrives over Noise from someone else and + // the vouchee is not in our announce set yet, so there is no name to + // bind to at record time. + let manager = makeManager() + manager.setVerified(fingerprint: voucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date()) + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "nothing is bound yet, so nothing can mismatch") + + announce(manager, vouchee, as: "ravi") + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + } + + @Test + func theAnnouncePathDoesNotBindBeforeTrustExists() { + // Otherwise a peer who renamed BEFORE being vouched would be bound to + // the name we happened to see first, and lose a legitimate seal. + let manager = makeManager() + announce(manager, vouchee, as: "rav") + announce(manager, vouchee, as: "ravi") + + manager.setVerified(fingerprint: voucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date()) + + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "bound to the name it was vouched under, not the name seen first") + } + + @Test + func aFirstAnnounceThatIsAlreadyTheImpersonatingNameIsNotCaught() { + // The documented limit of receiver-side binding: if we never saw this + // key under its real name, the impersonating name IS the baseline. + // Only signing the nickname into the attestation closes this, which is + // a wire change and deliberately not attempted here. + let manager = makeManager() + manager.setVerified(fingerprint: voucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date()) + + announce(manager, vouchee, as: "medic") + + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "known gap, pinned here so it is explicit rather than a surprise") + } + + // MARK: - Not fooled by display decoration + + @Test + func theCheckIsUnaffectedByCollisionSuffixes() { + // `PeerDisplayNameResolver` renders two CONNECTED peers who both claim + // "medic" as "medic#a1b2" and "medic#c3d4" — which is exactly what + // happens during this attack. Comparing a rendered name would drop the + // real medic's seal at the worst possible moment, so the check reads + // announced names only. This pins that the resolver really does + // decorate, and that the binding ignores it. + let real = PeerDisplayNameResolver.resolve( + [(peerID: PeerID(str: "a1b2c3d4"), nickname: "medic", isConnected: true), + (peerID: PeerID(str: "c3d4e5f6"), nickname: "medic", isConnected: true)], + selfNickname: "me") + #expect(real[PeerID(str: "a1b2c3d4")] == "medic#a1b2", "the resolver does decorate") + + let manager = makeManager() + announce(manager, vouchee, as: "medic") + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "medic") // still "medic" on the wire + + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "the impersonated peer keeps its seal while a namesake is connected") + } + + @Test + func aLocalPetnameKeepsTheSeal() { + // A petname outranks the claimed nickname everywhere it is displayed, + // so a rename cannot spoof anything and the seal stands. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + + setPetname(manager, vouchee, "my neighbour") + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + + setPetname(manager, vouchee, nil) + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + } + + @Test + func namesAreComparedInCanonicalForm() { + // Same rule as `normalizedNickname` everywhere else: a decomposed and a + // precomposed "café" are one name, not a rename. + let manager = makeManager() + announce(manager, vouchee, as: "cafe\u{0301}") // e + combining acute + manager.setVerified(fingerprint: vouchee, verified: true) + + announce(manager, vouchee, as: "caf\u{00E9}") // precomposed é + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + + announce(manager, vouchee, as: "cafe") + #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", senderPeerID: nil), + "the row posted as medic must not be sealed") + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi", senderPeerID: nil), + "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", senderPeerID: nil), + "but the archived ravi row is still accurate") + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic", senderPeerID: nil)) + } + + @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", senderPeerID: nil)) + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi@hq#a1b2", senderPeerID: decorator)) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi", senderPeerID: nil)) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic#a1b2", senderPeerID: decorator)) + } + + @Test + func aRowIsNotSealedForAnUnverifiedKeyAndFailsOpenWithNoBaseline() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi", senderPeerID: nil), + "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", senderPeerID: nil)) + } + + @Test + func theBaselineIsKeptSoASheetCanNameIt() { + // Suppressing the seal is the security fix; saying WHICH name was + // verified is what lets a reader recognise the peer instead of + // guessing. The baseline therefore survives the rename it detects. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi") + + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi", + "the baseline is what the sheet needs to name") + } + + @Test + func anAnnouncedSuffixIsNotADecoration() { + // Found by generalising a review finding on the Android mirror of this + // patch. `sealAppliesToRow` stripped a trailing "#abcd" from the + // rendered sender unconditionally — but a peer announces whatever + // string it likes, and "#" plus four hex is a legal thing to announce, + // which is why `splitSuffix` exists at all. So the one rule was wrong + // in both directions. + // + // The half that matters: a key pinned as "ravi" renames to "ravi#cafe" + // and its rows keep the seal. "#cafe" is indistinguishable from the + // decoration this app generates itself, so the row reads as "ravi, + // disambiguated" — a better disguise than an unrelated name, not worse. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi#cafe", senderPeerID: nil), + "a rename onto a hex-looking suffix must not keep the seal") + + // ...and the other half: a key honestly verified under a name that + // simply ends that way must not lose its seal for standing still. + let ends = makeManager() + announce(ends, vouchee, as: "ravi#cafe") + ends.setVerified(fingerprint: vouchee, verified: true) + #expect(ends.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi#cafe", senderPeerID: nil), + "an announced name that ends in a suffix still matches itself") + #expect(ends.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi#cafe#a1b2", senderPeerID: decorator), + "and still matches once the list decorates it") + #expect(!ends.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi", senderPeerID: nil), + "while the undecorated base is a different name") + } + + @Test + func theLiveCheckNeverUndecorates() { + // The live question has no decoration to remove: `peer.nickname` is + // what the peer claims. Stripping there would let exactly the rename + // above through on the peer list as well as on a row. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "ravi#cafe") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee), + "announcing ravi#cafe after being verified as ravi is a rename") + } + + /// A peerID whose first four hex are "a1b2", so `#a1b2` is the decoration + /// `PeerDisplayNameResolver` would give this peer and nothing else is. + private var decorator: PeerID { PeerID(str: "a1b2c3d4") } + + @Test + func theSheetStopsEndorsingAfterARename() { + // The fingerprint sheet drives its badge, its message and its verify + // button off `nameChangedSinceVerification`, so the word demotes with + // the glyph. Pinned here at the state level, which is what the view + // reads: the view itself needs an app bundle these tests cannot build. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "nothing to explain while the name holds") + + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee), + "the sheet is told the name changed, so its copy can demote") + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi", + "and the old name survives, so a future revision can name it") + #expect(manager.isVerified(fingerprint: vouchee), + "while the stored verification is untouched — re-verifying is the recovery") + } + + @Test + func aVoucherIsNamedByTheNameItWasVerifiedUnder() { + // The voucher list in the fingerprint sheet is a trust attribution, so + // it has the seal's problem one level out: name a voucher by what it + // announces NOW and a renamed voucher is credited under the new name. + // Eve verified as "ravi", vouches for Mallory, renames to "medic", and + // Mallory's sheet reads "vouched by medic". + // + // The baseline outlives the rename precisely so the sheet can say + // "ravi" — the person you actually checked. Pinned at the manager, + // which is where the view reads it from. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi", + "a voucher that renamed is still named by what it was verified as") + } + + // MARK: - What counts as the same name + + @Test + func recasingYourOwnNicknameIsNotARename() { + // Found auditing my own revision: the binding compared NFC only, so + // changing "Ravi" to "ravi" broke it and silently dropped the seal. + let manager = makeManager() + announce(manager, vouchee, as: "Ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + + announce(manager, vouchee, as: "ravi") + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "RAVI", senderPeerID: nil)) + } + + @Test + func aLookAlikeNameDoesBreakTheBinding() { + // The other half of that: the binding key is NFC, deliberately not + // NFKC. A fullwidth M merely LOOKS like M, so it is a different name + // and must break the binding — folding look-alikes is the collision + // resolver's job, which answers a different question. + let manager = makeManager() + announce(manager, vouchee, as: "Medic") + manager.setVerified(fingerprint: vouchee, verified: true) + + announce(manager, vouchee, as: "\u{FF2D}edic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "\u{FF2D}edic", senderPeerID: nil)) + } + + @Test + func onlyAnAsciiCollisionSuffixIsStripped() { + // `Character.isHexDigit` also accepts fullwidth digits, so a nickname + // literally ending in "#ABCD" was being truncated and could then + // match a baseline it is not. ASCII only, like `splitSuffix()`. + let manager = makeManager() + announce(manager, vouchee, as: "medic") + manager.setVerified(fingerprint: vouchee, verified: true) + + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic#a1b2", senderPeerID: decorator)) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, + renderedSender: "medic#\u{FF21}\u{FF22}\u{FF23}\u{FF24}", + senderPeerID: decorator), + "a fullwidth tail is part of the name, not a suffix this device generates") + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic#zzzz", senderPeerID: nil), + "non-hex is part of the name too") + } + + // MARK: - Rebinding and clearing + + @Test + func verifyingInPersonRebindsToTheCurrentName() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: voucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date()) + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + + // The user scanned this key themselves, under the name it shows now. + manager.setVerified(fingerprint: vouchee, verified: true) + + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + } + + @Test + func unverifyingClearsTheBaselineOnlyWhenNoVouchRemains() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + + manager.setVerified(fingerprint: vouchee, verified: false) + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "with no trust left there is no baseline, so nothing to mismatch") + + // With a vouch still standing, the baseline has to survive: it is what + // that vouch's seal is bound to. + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: voucher, verified: true) + manager.recordVouch(voucheeFingerprint: vouchee, voucherFingerprint: voucher, timestamp: Date()) + manager.setVerified(fingerprint: vouchee, verified: true) + manager.setVerified(fingerprint: vouchee, verified: false) + announce(manager, vouchee, as: "medic") + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee), + "the vouch's seal still needs the name it was bound to") + } + + // MARK: - Failing open + + @Test + func aMissingBaselineNeverSuppressesASeal() { + // Verified before the peer ever announced a name — there is nothing to + // bind to, and pinning "" would read as a mismatch against every later + // announce. Peers trusted by builds before this shipped land here too, + // so an upgrade must not silently drop their seals. + let manager = makeManager() + manager.setVerified(fingerprint: vouchee, verified: true) + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + + announce(manager, vouchee, as: "medic") + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + } + + @Test + func anEmptyClaimedNicknameIsNotAMismatch() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "") + + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee)) + } + + // MARK: - Persistence compatibility + + @Test + func aCacheWrittenBeforeThisFieldExistedStillLoads() throws { + let legacyJSON = Data(""" + {"socialIdentities":{},"nicknameIndex":{},"verifiedFingerprints":["\(vouchee)"],\ + "lastInteractions":{},"blockedNostrPubkeys":[],"cryptographicIdentities":{},"version":1} + """.utf8) + + let decoded = try JSONDecoder().decode(IdentityCache.self, from: legacyJSON) + + #expect(decoded.trustedNicknames == nil) + #expect(decoded.verifiedFingerprints.contains(vouchee)) + } + + @Test + func theBaselineSurvivesAnEncodeDecodeRoundTrip() throws { + var cache = IdentityCache() + cache.trustedNicknames = [vouchee: "ravi"] + + let decoded = try JSONDecoder().decode(IdentityCache.self, from: JSONEncoder().encode(cache)) + + #expect(decoded.trustedNicknames?[vouchee] == "ravi") + } +} diff --git a/bitchatTests/Services/UnifiedPeerServiceTests.swift b/bitchatTests/Services/UnifiedPeerServiceTests.swift index 4bc6f775..ec604c23 100644 --- a/bitchatTests/Services/UnifiedPeerServiceTests.swift +++ b/bitchatTests/Services/UnifiedPeerServiceTests.swift @@ -289,6 +289,11 @@ private final class TestIdentityManager: SecureIdentityStateManagerProtocol { verified } + // 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) @discardableResult diff --git a/bitchatTests/Utils/PeerDisplayNameResolverTests.swift b/bitchatTests/Utils/PeerDisplayNameResolverTests.swift new file mode 100644 index 00000000..ad1c76b1 --- /dev/null +++ b/bitchatTests/Utils/PeerDisplayNameResolverTests.swift @@ -0,0 +1,104 @@ +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 M") + #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 theSenderDisplayNameResolverFoldsTheSameWay() { + // Collisions are counted in TWO places. This one feeds sender names on + // file transfers and had the same exact-match blind spot; folding only + // the peer-list resolver would have left it behind. + let mine = PeerID(str: "cccc3333") + func info(_ id: PeerID, _ nickname: String) -> BLEPeerInfo { + BLEPeerInfo(peerID: id, nickname: nickname, isConnected: true, + noisePublicKey: nil, signingPublicKey: nil, + isVerifiedNickname: true, lastSeen: Date()) + } + let peers: [PeerID: BLEPeerInfo] = [ + real: info(real, "Medic"), + other: info(other, "\u{FF2D}edic"), + ] + let name = BLEPeerSenderDisplayName.resolveKnownPeer( + peerID: other, localPeerID: mine, localNickname: "me", + peers: peers, allowConnectedUnverified: true) + #expect(name?.contains("#") == true, + "a fullwidth namesake must be suffixed here too") + } + + @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 M is preserved on screen") + } +}