nik/bitchat-android
1
0
Fork 0
mirror of https://github.com/permissionlesstech/bitchat-android.git synced 2026-09-19 04:59:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

Demote the sheet's copy with its glyph, and drop a fail-open default

Browse Source 
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>
This commit is contained in:
Shubham Bhandari 2026-09-15 11:32:47 +08:00
parent 8d656242cc
commit d637b5bc1b
3 changed files with 24 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
app/src/main/java/com/bitchat/android/ui/ChatViewModel.kt
View File

@ -1276,11 +1276,16 @@ class ChatViewModel(
* rows display a name held in the favourites record rather than a live
* announce, so they must be checked against their own name — asking about
* a "current" name a peer is not announcing would answer nothing.
*
* Deliberately has no default. A missing name fails OPEN, by design, so a
* defaulted parameter would let any future caller reinstate the rename
* attack by simply not passing one — silently, and without touching this
* file. Make it a decision at the call site.
*/
fun isNoisePublicKeyVerified(
noisePublicKey: ByteArray,
verifiedFingerprints: Set<String>,
renderedName: String? = null,
renderedName: String?,
): Boolean {
val fingerprint = verificationHandler.fingerprintFromNoiseBytes(noisePublicKey)
if (!verifiedFingerprints.contains(fingerprint)) return false

26
app/src/main/java/com/bitchat/android/ui/SecurityVerificationSheet.kt
View File

@ -146,7 +146,7 @@ fun SecurityVerificationSheet(
)
SecurityVerificationActions(
isVerified = isVerified,
isVerified = nameBound && isVerified,
fingerprint = fingerprint,
displayName = displayName,
accent = accent,
@ -190,16 +190,24 @@ private fun buildStatusInfo(
sessionState: String?,
accent: Color
): SecurityStatusInfo {
// The glyph is the seal; the text is the explanation. They part company for
// exactly one state: verified key, different name. The icon and tint fall
// back to the session's own state (a padlock on an encrypted session), so
// the sheet stops asserting the name while the word "verified" still tells
// the truth about the key. Adding a sentence that says both would need a
// new string in all 34 locales, and machine-translating a security warning
// is not something to do in passing.
// Everything here demotes together: glyph, tint and text.
//
// The first revision kept the word "verified" on the grounds that the key
// genuinely is verified, and that was wrong for a reason review put better
// than I had: this 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. Worse, it 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.
//
// It needs no new string. An established session falls through to
// "encrypted" with a padlock, which is true and asserts nothing about
// identity, and the actions block below already names the current nickname
// in its not-verified copy and offers to verify it — which is exactly the
// recovery: re-verifying re-pins the baseline to the name on screen.
val sealed = isVerified && nameBound
val text = when {
isVerified -> stringResource(R.string.fingerprint_status_verified)
sealed -> stringResource(R.string.fingerprint_status_verified)
sessionState == "established" -> stringResource(R.string.fingerprint_status_encrypted)
sessionState == "handshaking" -> stringResource(R.string.fingerprint_status_handshaking)
sessionState == "failed" -> stringResource(R.string.fingerprint_status_failed)

2
app/src/main/java/com/bitchat/android/ui/VerificationHandler.kt
View File

@ -60,7 +60,7 @@ class VerificationHandler(
return !verifiedNicknameMismatch(peerID)
}
fun isNoisePublicKeyVerified(noisePublicKey: ByteArray, renderedName: String? = null): Boolean {
fun isNoisePublicKeyVerified(noisePublicKey: ByteArray, renderedName: String?): Boolean {
val fingerprint = fingerprintFromNoiseBytes(noisePublicKey)
if (!_verifiedFingerprints.value.contains(fingerprint)) return false
return sealAppliesToName(fingerprint, renderedName)