Audit LOW: empty offer, attachment id, QR binding, skipped-key order
Same empty-offer and attachment-id rules as iOS. The QR scanner now binds a scanned code by its fnid rather than the handle even on the home relay — the fnid certifies itself, a handle is a name the relay maps, so resolving by handle handed the relay the choice of who you scanned. And a skipped message key is consumed only after its tag verifies: it was defused today by the caller discarding the session, but any future in-memory ratchet cache would have turned it into real message loss. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
cbe2292d3c
commit
130edd7019
@@ -724,12 +724,13 @@ internal fun ColumnScope.SignedIn(
|
|||||||
onFound = { raw ->
|
onFound = { raw ->
|
||||||
scanning = false
|
scanning = false
|
||||||
parseScannedAddress(raw)?.let { card ->
|
parseScannedAddress(raw)?.let { card ->
|
||||||
// A same-relay code that carries the handle can go through
|
// 🔴 **Bind by the fnid, never the handle (audit LOW).**
|
||||||
// the ordinary username lookup; anything else fills the
|
// The fnid is derived from the account key and certifies
|
||||||
// field with the address it named.
|
// itself; a handle is a name the RELAY maps to an account,
|
||||||
scannedPrefill = if (card.handle != null &&
|
// so resolving a scanned code by handle handed the relay
|
||||||
card.relay.equals(phase.relay, ignoreCase = true)
|
// the choice of who you just scanned. iOS always binds the
|
||||||
) card.handle else "${card.fnid}@${card.relay}"
|
// address; this now matches it, same relay or not.
|
||||||
|
scannedPrefill = "${'$'}{card.fnid}@${'$'}{card.relay}"
|
||||||
addingContact = true
|
addingContact = true
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -344,12 +344,25 @@ class Conversations(
|
|||||||
sendPlaintext(toUserId, Content.callSignal(signal))
|
sendPlaintext(toUserId, Content.callSignal(signal))
|
||||||
|
|
||||||
/** Download and open a blob a message referred to. */
|
/** Download and open a blob a message referred to. */
|
||||||
|
/**
|
||||||
|
* 64 lowercase hex, and nothing else — the shape the relay mints and the
|
||||||
|
* only shape it will serve (audit LOW).
|
||||||
|
*
|
||||||
|
* 🔴 The id arrives from a PEER and is interpolated into a URL path. The
|
||||||
|
* relay validates it before it reaches a filesystem; the client did not,
|
||||||
|
* so a crafted id could reshape the victim's own authenticated GET.
|
||||||
|
*/
|
||||||
|
private fun isValidAttachmentToken(id: String): Boolean =
|
||||||
|
id.length == 64 && id.all { it in '0'..'9' || it in 'a'..'f' }
|
||||||
|
|
||||||
fun fetchAttachment(ref: Attachment.Ref): ByteArray =
|
fun fetchAttachment(ref: Attachment.Ref): ByteArray =
|
||||||
// The size came through the ratchet, so the relay cannot inflate it —
|
// The size came through the ratchet, so the relay cannot inflate it —
|
||||||
// which is what makes it usable as a ceiling (audit M5).
|
// which is what makes it usable as a ceiling (audit M5).
|
||||||
Attachment.open(
|
Attachment.open(
|
||||||
relay.downloadAttachment(
|
relay.downloadAttachment(
|
||||||
ref.id,
|
ref.id.also {
|
||||||
|
require(isValidAttachmentToken(it)) { "malformed attachment id" }
|
||||||
|
},
|
||||||
maxBytes = if (ref.bytes > 0) minOf(ref.bytes + 4096L,
|
maxBytes = if (ref.bytes > 0) minOf(ref.bytes + 4096L,
|
||||||
RelayClient.HARD_ATTACHMENT_CAP)
|
RelayClient.HARD_ATTACHMENT_CAP)
|
||||||
else RelayClient.HARD_ATTACHMENT_CAP),
|
else RelayClient.HARD_ATTACHMENT_CAP),
|
||||||
|
|||||||
@@ -162,6 +162,14 @@ class Trust(
|
|||||||
)
|
)
|
||||||
return IdentityTrust.PINNED
|
return IdentityTrust.PINNED
|
||||||
}
|
}
|
||||||
|
// 🔴 **An empty offer is not agreement (audit LOW).** `all {}` on an
|
||||||
|
// empty list is vacuously true, so a directory answering with NO keys
|
||||||
|
// for someone already pinned read as "unchanged" — the strongest
|
||||||
|
// verdict — when it had said nothing at all. It is not evidence of a
|
||||||
|
// swap either, so it reports UNKNOWN: nothing to compare, nothing to
|
||||||
|
// send to. The first-contact branch above already guards this case;
|
||||||
|
// this is the same rule once keys exist.
|
||||||
|
if (encoded.isEmpty()) return IdentityTrust.UNKNOWN
|
||||||
// ⚠️ EVERY offered key must already be pinned — not "any". A relay
|
// ⚠️ EVERY offered key must already be pinned — not "any". A relay
|
||||||
// adding one key of its own alongside the real ones would otherwise
|
// adding one key of its own alongside the real ones would otherwise
|
||||||
// pass, and that is precisely the attack: the victim's real devices
|
// pass, and that is precisely the attack: the victim's real devices
|
||||||
|
|||||||
@@ -201,10 +201,23 @@ class DoubleRatchet private constructor(
|
|||||||
val aad = associatedData + message.header.canonicalJson()
|
val aad = associatedData + message.header.canonicalJson()
|
||||||
|
|
||||||
// A key held for a message that never arrived, or arrived late.
|
// A key held for a message that never arrived, or arrived late.
|
||||||
|
//
|
||||||
|
// 🔴 **Removed only AFTER the tag verifies (audit LOW).** Consuming it
|
||||||
|
// first meant a forged ciphertext for a stashed (dh, n) burned the key
|
||||||
|
// the real delayed message needed — the genuine message could then
|
||||||
|
// never be opened. Today the caller discards the whole session object
|
||||||
|
// on a failed decrypt, so the loss never reaches the vault; that is a
|
||||||
|
// property of the CALLER, and the moment anything caches a ratchet in
|
||||||
|
// memory it would become a real message-loss bug. iOS has always
|
||||||
|
// removed after the open for exactly this reason.
|
||||||
val skippedKey = SkippedKey(Bytes.b64(message.header.dh), message.header.n)
|
val skippedKey = SkippedKey(Bytes.b64(message.header.dh), message.header.n)
|
||||||
skipped.remove(skippedKey)?.let { mk ->
|
skipped[skippedKey]?.let { mk ->
|
||||||
val (aesKey, nonce) = aeadParams(mk)
|
val (aesKey, nonce) = aeadParams(mk)
|
||||||
return Aead.open(aesKey, nonce + message.ciphertext, aad)
|
val plaintext = Aead.open(aesKey, nonce + message.ciphertext, aad)
|
||||||
|
// LinkedHashMap keeps insertion order, so removing from it is the
|
||||||
|
// whole eviction bookkeeping — there is no separate order list.
|
||||||
|
skipped.remove(skippedKey)
|
||||||
|
return plaintext
|
||||||
}
|
}
|
||||||
|
|
||||||
// A new ratchet key from the far side: catch up on their old chain,
|
// A new ratchet key from the far side: catch up on their old chain,
|
||||||
|
|||||||
Reference in New Issue
Block a user