Checked the wear module for the same mistake review had just caught twice on
the phone, and it was there.
`WearPeerIdentityState.snapshot` now reads the announced nickname to decide
whether the seal still applies, but all four screens cache it with
`remember(peerID, revision)` — and `revision` is bumped by favourites and
verification changes, never by a peer renaming. So the name repainted while the
cached `isVerified`, and its seal, survived. Same defect Codex found in
`PeopleSection` and `PrivateChatSheet`, in a module neither reviewer looked at.
All four now key on the announced nickname: DmScreen, PeerDebugScreen,
VerificationCodeScreen and UserDetailScreen. The last of those I had missed
even after deciding to audit this module — grepping for the snapshot call found
three, and the fourth only turned up when I grepped for the remember key
instead. It draws a seal like the others.
Nothing else here needs gating: every wear screen reads `identity.isVerified`
straight from the snapshot, so there is no second independent source the way
the phone's SecurityVerificationSheet had one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both of @Chessing234's follow-ups, and both were right.
The sheet kept saying "verified". I gated the glyph and the tint and argued the
word should stay because the key genuinely is verified. That status sits
directly beside `displayName`, so it is not a claim about a bare key — it is a
claim about THIS NAME, and that claim is false. And it is the screen someone
opens *because* a seal vanished from a row, so the one surface they consult to
resolve the doubt was reassuring them.
`buildStatusInfo`'s text now demotes with the glyph, and so does
`SecurityVerificationActions` — which was a SECOND seal in the same sheet,
green `Icons.Filled.Verified` with `fingerprint_verified_label`, that the first
revision never gated at all.
No new string is needed, which was the objection I had to fixing this: an
established session falls through to "encrypted" with a padlock, true and
silent about identity, and the not-verified block below already names the
current nickname in `fingerprint_not_verified_message_fmt` and offers to verify
it. That offer is the recovery — re-verifying re-pins the baseline to the name
on screen — and the stored verification is untouched underneath.
The consequence, said out loud: a renamed peer's sheet now offers "verify"
rather than "remove verification", so unverify is one re-verification away
instead of immediately to hand. Better than a green checkmark beside a name
nobody verified, but a real change to the affordance.
`isNoisePublicKeyVerified(..., renderedName = null)` fail-opened, so any future
caller that simply omitted the name would have reinstated the rename attack,
silently and without touching that file. The default is gone on both the
ChatViewModel and the handler; the single caller already passes the name.
15 binding cases still green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the review finding about suffix stripping. I said I would bring
the iOS fix across; checking how this app actually builds a row name says the
answer here is different, and simpler.
iOS appends "#" plus four hex of the peerID in PeerDisplayNameResolver, so a
row there genuinely carries a synthetic decoration and the fix is to remove
exactly that suffix and no other — which needs the peerID, since the string
alone cannot distinguish a decoration from an announced name.
Nothing decorates a mesh nickname here. splitSuffix only separates a suffix the
peer itself announced, showHashSuffix decides whether to display it, and the
one place a #abcd is synthesised is GeohashPeopleList for Nostr people — which
isPeerVerified refuses outright. So a mesh row shows what was announced, and
the undecorate step I added was a fail-OPEN: pinned "medic", row shows
"medic#cafe", suffix removed, seal kept. Exactly the case the review raised,
still live on the rendered path after I had "fixed" it.
sealAppliesToRendered now defers to sealAppliesToAnnounced. It stays a separate
named function so there is one place to change if a row here ever does start
carrying a decoration, and so nobody reads one comparison as the two questions
having merged — collapsing them is what caused this in the first place.
Mutation-checked: put the stripping back and two cases fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex raised three P1s on the first revision. I checked each against the code
before touching anything; all three are real, and the first two are the same
mistake in two costumes — one predicate asked two different questions.
1. An announced suffix is not a UI decoration.
`sealApplies` stripped a trailing `#abcd` from whatever it was given, including
the LIVE announced nickname. A peer announces whatever string it likes, and `#`
plus four hex is a legal thing to announce — this app's own `splitSuffix`
exists because announced names carry them. So the single predicate was wrong in
both directions at once: a key pinned as `medic` could rename to `medic#cafe`
and keep its seal, and a key honestly verified as `medic#cafe` lost its seal
without renaming at all.
There are now two functions, `sealAppliesToAnnounced` and
`sealAppliesToRendered`, because there are two questions: what a peer claims,
and what a row shows after the list may have decorated it. The rendered one
tries the raw name before undecorating, so a name that genuinely ends in a
suffix still matches itself. The iOS patch has exactly this split and I
collapsed it here.
2. The check was not reactive.
`PeopleSection` caches `isPeerVerified` with
`remember(verifiedFingerprints, peerFingerprints, connectedPeers)`, and the
per-peer sheet with `remember(peerID, verifiedFingerprints)`. My gate reads the
announced nickname inside those lambdas, and a rename changes none of those
keys — so the NAME repainted while the cached `true`, and the seal, survived
until something unrelated invalidated the cache. That is the connected peer
still showing a seal after renaming: the live impersonation, and the single
case this patch exists to stop. `peerNicknames` is now a key at both sites.
3. The watch was never patched at all.
There is a `:wear` module in settings.gradle.kts with its own
`WearPeerIdentityState`, its own verify button and its own seal, and it calls
`setVerifiedFingerprint` without a nickname while deriving `isVerified` from
the fingerprint set alone. Every peer verified on a watch failed open for ever.
I missed it because I audited `app/` and assumed that was the app — I never
read settings.gradle.kts. It now pins the announced name on verify and applies
the binding in `snapshot`.
15 binding cases, up from 13. Mutation-checked: put the stripping back into the
announced check and the new case fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A verified key can rename itself onto a nickname the user trusts and keep
drawing the seal beside the new one.
Verification binds a FINGERPRINT, which is right. But the seal is rendered
beside a self-claimed nickname, and nothing binds those two together:
1. Eve announces "ravi" and gets verified in person by someone.
2. Eve announces "medic". The rename is free and silent — PeerManager
computes `nicknameChanged` only to decide whether to refresh the list.
3. Every device that verified Eve now shows a second trusted-looking medic.
This is the Android mirror of permissionlesstech/bitchat#1708. It is smaller,
because this app has no vouching: there is no transitive trust to launder
onward, only the seal itself to withdraw.
The binding
`SecureIdentityStateManager` records the nickname a key was announcing when it
was verified, in its own store. Deliberately NOT the existing
`cached_fingerprint_nicknames`, which is a LAST SEEN cache overwritten on
every peer-list refresh — comparing against that would always match and catch
nothing.
Pinned on verification and on re-verification (the user just checked this key
again, under whatever name it shows now), cleared on unverify. Never pinned
from `resolvePeerDisplayName`, which falls back to a truncated peerID: that is
an identifier, not a claimed name, and pinning it would drop a seal on the
peer's first real announce.
A missing baseline never suppresses. Peers verified by earlier builds have
none, and dropping their seals on upgrade would teach people to ignore the
signal.
Where the seal comes from
Four surfaces, all gated: the connected peer row and the peer sheet (via
`ChatViewModel.isPeerVerified`), the conversation row, and offline favourite
rows. The last two are checked against the name THEY render rather than a live
announce — an offline favourite shows a name held in the favourites record, so
asking about a "current" name it is not announcing would answer nothing.
`VerificationHandler.isPeerVerified` / `isNoisePublicKeyVerified` have no
callers today and are gated anyway: leaving them ungated would hand the next
caller the answer this change exists to stop giving.
One surface is deliberately half covered
In the fingerprint sheet the seal GLYPH and its green tint are withheld, and
the word "verified" is not. The key genuinely is verified, so saying otherwise
would be false — and leaving the sheet untouched would let someone tap through
from a row whose seal just vanished and be reassured by a green checkmark. The
right fix is a sentence that says both, and a new string has to ship in all 34
locales; machine-translating a security warning is not something to do in
passing. Happy to wire it if someone supplies the wording.
Comparison rules
NFC, then a locale-independent case fold, then NFC again — folding can itself
emit decomposed sequences, and `Locale.ROOT` is not optional or a Turkish
phone would disagree with every other device about whether a peer had renamed.
Recasing your own nickname is not a rename; a fullwidth or Cyrillic look-alike
IS one. A trailing `#abcd` is stripped before comparing, ASCII hex only, since
`Char.isDigit()` accepts fullwidth digits and would truncate a nickname
literally ending in "#ＡＢＣＤ" into something that could match a baseline it
is not.
Tests
`NicknameBindingTest`, 13 cases: the rename attack, the rename-onto-a-
look-alike cases, recasing, combining accents, Turkish dotted I, the hash
suffix and the fullwidth-hex trap, "@" in a nickname, and both fail-open
paths. The logic lives in a pure-Kotlin `NicknameBinding` object with no
Android imports precisely so the security decision is testable without a view.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
If SQLite clearAll fails mid-panic, in-memory state was already cleared
but a process restart could reload encrypted history from disk (#699).
Fall back to deleting the database files before reporting failure.
A relay that fails is retried on an exponential backoff, then abandoned
for the lifetime of the process. Nothing brings it back: the relay layer
registers no connectivity callback, the periodic subscription validator
only repairs subscriptions on sockets that are already open (and returns
immediately when connectedRelayCount is 0, which is exactly the state
after an outage), and connect() runs once from NostrClient.initialize().
The remaining paths that reset reconnectAttempts are a manual retry, a
Tor state change, and a successful open.
Two ways a relay died permanently:
- Any error whose message mentioned DNS returned before scheduling
anything at all. "Unable to resolve host" is what this device reports
when it simply has no network, so a moment in a tunnel killed every
relay at once, with no retry ever.
- Otherwise the schedule stopped at MAX_RECONNECT_ATTEMPTS. With
INITIAL=1s and MULTIPLIER=2 that is nine waits totalling about eight
and a half minutes, after which the relay was dead. MAX_BACKOFF_INTERVAL
was unreachable: attempt 9 asks for 256s and attempt 10 gave up, so the
five-minute ceiling the constant defines never applied to anything.
Let the backoff saturate at MAX_BACKOFF_INTERVAL and keep retrying there.
A name-resolution failure now backs off like any other error. Steady state
costs one connection attempt per relay per five minutes; the previous
behaviour cost the user every internet DM, delivery receipt and geohash
channel until they noticed and restarted the app.
The schedule moves into RelayReconnectPolicy so it is unit-testable
without OkHttp or a Context.
