From eb8c812690ebb5cc528df593415040aac7d7943e Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Sun, 9 Aug 2026 23:39:54 -0400 Subject: [PATCH] fix(concord): close ten review findings in the CORD-05 invite path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A high-effort review of the invite work found ten correctness bugs, nine confirmed and one plausible. All are fixed here; the on-device pass on a tablet proved the three that are observable through the UI. The load-bearing one: the kind-13303 Invite List is replaceable, and both clients merged a patch onto a base that silently degraded to EMPTY whenever the read failed — an unanswered relay or a bunker signer declining one decrypt was enough. Republishing that destroys every `signer_sk` it could not read, and those secrets cannot be regenerated, so every outstanding link is orphaned at a dead epoch. `decode` is now `decodeOrNull` and `decrypt` returns null, so "I could not read it" is distinguishable from "it is empty", and the write aborts rather than overwriting. The rest: - `join` is now ban-gated on both clients. A Refounding re-mints every outstanding link onto the new root, and an ex-member keeps the URL and its token forever, so the rotation meant to expel them handed them the new keys instead. Fails closed on an unreadable plane. - Android's Refounding re-read the entry from `liveCommunities` straight after adopting the new root, but that flow decrypts asynchronously, so every link was re-minted onto the epoch just left. `adoptConcordRoot` now returns the entry it wrote. - amy's refound folded the fresh bans locally and then never used them, re-draining from relays instead; a relay slow to echo them back would produce a new epoch whose roster never banned anyone. - Link refresh rebuilt the bundle from scratch, stripping expiry, channel grants, icon and label; it now moves the link's own current bundle and changes only the epoch's key material. - Refresh also re-posted over revocation tombstones, silently un-revoking a retired link. - The 13303 coordinate is (13303, me, "") — one list per account — but was read and written on per-community relays, forking it into divergent versions that newest-wins then collapsed. Now account-outbox only. - Minting returned the URL even when recording the link failed, handing out a link that could never be refreshed. It now fails closed. - amy's refound had no equivalent of Amethyst's recipient cap, leaving the attacker-writable half of the union unbounded. - amy's store dropped the banked epoch's `controlRoot` on the round-trip, losing the staff write key that rebuilds the anti-rollback floor. Co-Authored-By: Claude Opus 5 (1M context) --- .../amethyst/model/AccountConcordActions.kt | 186 +++++++++++------- .../amethyst/model/ConcordInviteResult.kt | 9 + .../concord/ConcordInviteScreen.kt | 2 + amethyst/src/main/res/values/strings.xml | 1 + .../amethyst/cli/commands/ConcordCommands.kt | 99 +++++++--- .../cli/commands/ConcordModCommands.kt | 96 ++++++--- .../amethyst/cli/stores/ConcordStore.kt | 6 + .../concord/cord05Invites/CommunityInvite.kt | 2 +- .../cord05Invites/ConcordInviteList.kt | 24 ++- .../cord05Invites/ConcordInviteListEvent.kt | 16 +- .../cord05Invites/ConcordInviteListTest.kt | 39 +++- 11 files changed, 339 insertions(+), 141 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt index 543206fc94..daad3aa9c7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt @@ -171,74 +171,97 @@ class AccountConcordActions( // ---- CORD-05 Invite List (kind 13303) ------------------------------------- /** - * This account's Invite List: the creator's private, self-encrypted record of every link they - * minted (`token` + `signer_sk` per entry). Empty when none was ever published. + * This account's Invite List (kind 13303): the creator's private, self-encrypted record of every + * link they minted (`token` + `signer_sk` per entry). * - * Fetched rather than read from [LocalCache] because nothing subscribes to 13303 — it is + * Returns **null** when the list could not be read — no relay answered, or the signer refused + * the decrypt — and an empty document only when the account genuinely has no list yet. Callers + * must not conflate the two: republishing an "empty" list over this replaceable coordinate + * destroys every `signer_sk` it failed to read, and those secrets cannot be regenerated. + * + * Read on the account's OUTBOX relays, never a community's: the coordinate is + * (13303, me, "") — one list for the whole account — so scoping it per community would fork it + * into divergent versions that the newest-wins rule then silently collapses. + * + * Fetched rather than read from [LocalCache] because nothing subscribes to 13303: it is * bookkeeping the user never sees, needed only at mint and at rotation. */ - private suspend fun readConcordInviteList(relays: Set): ConcordInviteListDocument { - if (relays.isEmpty()) return ConcordInviteListDocument.EMPTY + private suspend fun readConcordInviteList(): ConcordInviteListDocument? { + val relays = account.outboxRelays.flow.value + if (relays.isEmpty()) return null val filter = Filter(kinds = listOf(ConcordInviteListEvent.KIND), authors = listOf(account.signer.pubKey)) val newest = account.client .fetchAll(filters = relays.associateWith { listOf(filter) }) .maxByOrNull { it.createdAt } - return (newest as? ConcordInviteListEvent)?.decrypt(account.signer) ?: ConcordInviteListDocument.EMPTY + ?: return ConcordInviteListDocument.EMPTY // nothing published yet — safe to start one + return (newest as? ConcordInviteListEvent)?.decrypt(account.signer) } /** - * Merges [patch] into the published Invite List and republishes it. Read-merge-write, never - * overwrite: the list is replaceable and per-creator, so two of the user's devices minting - * concurrently would otherwise delete each other's `signer_sk` — and that secret is - * unrecoverable, orphaning the link at whatever epoch it was last refreshed to. + * Merges [patch] into the published Invite List and republishes it, returning whether it landed. + * + * Read-merge-write, and **aborts rather than overwriting** when the read fails: the list is + * replaceable, so publishing a patch-only document over an unread list deletes every other + * link's `signer_sk` — unrecoverable, and it strands every holder of those links at the next + * rotation. A momentarily unreachable relay or a bunker signer that declines one decrypt is + * enough to trigger that, which is exactly how the kind-13302 community list was once emptied. */ - private suspend fun publishConcordInviteList( - patch: ConcordInviteListDocument, - relays: Set, - ) { - val publishTo = relays.ifEmpty { account.outboxRelays.flow.value } - if (publishTo.isEmpty()) return - val merged = ConcordInviteList.merge(readConcordInviteList(publishTo), patch) - account.client.publish(ConcordInviteListEvent.create(account.signer, merged, TimeUtils.now()), publishTo) + private suspend fun publishConcordInviteList(patch: ConcordInviteListDocument): Boolean { + val publishTo = account.outboxRelays.flow.value + if (publishTo.isEmpty()) return false + val base = + readConcordInviteList() ?: run { + Log.w("Concord") { "Refusing to write the invite list: could not read the current one (would drop other links' signer_sk)" } + return false + } + return runCatching { + account.client.publish(ConcordInviteListEvent.create(account.signer, ConcordInviteList.merge(base, patch), TimeUtils.now()), publishTo) + true + }.onFailure { Log.w("Concord", "invite list publish failed", it) }.getOrDefault(false) } /** * Re-posts every live link this account minted for [entry]'s community at its own coordinate, - * carrying the CURRENT epoch (CORD-05). The kind-33301 bundle is addressable and authored by the + * carrying [entry]'s epoch (CORD-05). The kind-33301 bundle is addressable and authored by the * link signer, so this moves the link behind the same URL instead of orphaning it at a dead * epoch — which is the whole premise stranded recovery rests on. * - * Safe to call for every live link: recovery is ban-gated at the epoch being left, and a - * Refounding bans the members it removes on the way out, so a removed member's own recovery is - * refused even though their link now resolves. + * [entry] MUST be the post-rotation entry, passed in rather than re-read: the joined-list flow + * decrypts asynchronously, so reading it straight after adopting a new root yields the OLD + * epoch and would re-mint every link onto the epoch we just left. + * + * Each link is refreshed from its own CURRENT bundle, not rebuilt from scratch, so per-link + * fields the bundle carries — expiry, channel grants, icon, label — survive the rotation. A + * coordinate whose newest event is a revocation tombstone is left alone: re-posting a live + * bundle over it would silently un-revoke the link. */ private suspend fun refreshConcordInviteLinks(entry: ConcordCommunityListEntry): Int { val relays = entry.relays.mapNotNullTo(mutableSetOf()) { RelayUrlNormalizer.normalizeOrNull(it) }.ifEmpty { account.outboxRelays.flow.value } if (relays.isEmpty()) return 0 + val list = readConcordInviteList() ?: return 0 + val tombstoned = list.tombstones.mapTo(HashSet()) { it.token } val now = TimeUtils.now() - val refreshed = - ConcordActions.inviteFor( - communityIdHex = entry.id, - ownerPubKey = entry.owner, - ownerSaltHex = entry.ownerSalt, - communityRootHex = entry.root, - rootEpoch = entry.rootEpoch, - name = entry.name, - relays = entry.relays, - controlPk = entry.controlPk, - ) var count = 0 - for (link in readConcordInviteList(relays).entries) { + for (link in list.entries) { if (link.communityId != entry.id) continue - // An elapsed link can no longer be joined, so re-posting it would only resurrect a dead - // URL at a live epoch. - if (link.isExpired(now)) continue + // An elapsed or retired link can no longer be joined; re-posting it would only resurrect + // a dead URL at a live epoch. + if (link.isExpired(now) || link.token in tombstoned) continue runCatching { - account.client.publish( - ConcordActions.remintBundleAt(link.signerSk.hexToByteArray(), link.token.hexToByteArray(), refreshed, now), - relays, - ) + val token = link.token.hexToByteArray() + val wraps = account.client.fetchAll(filters = relays.associateWith { listOf(ConcordActions.bundleFilter(link.signerPubKeyHex())) }) + // Honour a revocation published at this coordinate, and carry the live bundle's own + // fields forward — only the epoch's key material changes. + val current = ConcordActions.classifyInvite(wraps, token) as? InviteBundleStatus.Live ?: return@runCatching + val moved = + current.invite.copy( + communityRoot = entry.root, + rootEpoch = entry.rootEpoch, + controlPk = entry.controlPk, + relays = entry.relays, + ) + account.client.publish(ConcordActions.remintBundleAt(link.signerSk.hexToByteArray(), token, moved, now), relays) count++ }.onFailure { Log.w("Concord", "invite refresh failed for ${entry.id}", it) } } @@ -289,25 +312,30 @@ class AccountConcordActions( val minted = ConcordActions.mintInviteLink(base, invite, TimeUtils.now(), entry.relays) val publishTo = entry.relays.mapNotNullTo(mutableSetOf()) { RelayUrlNormalizer.normalizeOrNull(it) }.ifEmpty { account.outboxRelays.flow.value } - if (publishTo.isNotEmpty()) account.client.publish(minted.bundleEvent, publishTo) - - // Record the link so a later Refounding can refresh THIS coordinate rather than orphaning it - // (CORD-05, kind 13303). Shared with amy and Armada, so any of the creator's clients can. - publishConcordInviteList( - ConcordInviteListDocument( - entries = - listOf( - ConcordInviteListEntry( - token = minted.token.toHexKey(), - signerSk = minted.linkSignerPrivKey.toHexKey(), - communityId = entry.id, - url = minted.url, - createdAt = TimeUtils.now(), + // Record the link BEFORE handing the URL out (CORD-05, kind 13303). A link whose `signer_sk` + // was never stored can never be refreshed, so the next Refounding orphans it and everyone + // holding it is stranded — with nothing to have warned them. Failing the mint is the honest + // outcome; a stored entry for a link nobody received is harmless by comparison. + if (!publishConcordInviteList( + ConcordInviteListDocument( + entries = + listOf( + ConcordInviteListEntry( + token = minted.token.toHexKey(), + signerSk = minted.linkSignerPrivKey.toHexKey(), + communityId = entry.id, + url = minted.url, + createdAt = TimeUtils.now(), + ), ), - ), - ), - publishTo, - ) + ), + ) + ) { + Log.w("Concord") { "Invite not minted for ${entry.id}: its link signer could not be recorded, so the link could never be refreshed" } + return null + } + + if (publishTo.isNotEmpty()) account.client.publish(minted.bundleEvent, publishTo) return minted.url } @@ -373,6 +401,32 @@ class AccountConcordActions( return ConcordInviteResult.Joined(bundle.communityId) } + // Refuse a link that readmits us after we were removed. A Refounding re-mints every + // outstanding link onto the new root (CORD-05), and an ex-member keeps the URL and its + // unlock token forever — so without this the rotation meant to expel them hands them the new + // keys instead. `recoverStrandedConcordCommunities` has always been ban-gated; this is the + // other door into the same room. + // + // Fails CLOSED on an unreadable plane: the banlist is only knowable once the bundle yields + // the root, and no verdict means no join. + val joinKeys = + ConcordActions.controlPlaneKeys( + communityRoot = bundle.communityRoot.hexToByteArray(), + communityId = bundle.communityId.hexToByteArray(), + rootEpoch = bundle.rootEpoch, + controlPk = bundle.controlPk, + ) + val joinRelays = bundle.relays.mapNotNullTo(mutableSetOf()) { RelayUrlNormalizer.normalizeOrNull(it) }.ifEmpty { relays } + val joinEditions = + ConcordActions.controlEditions( + account.client.fetchAll(filters = joinRelays.associateWith { listOf(ConcordActions.planeFilter(joinKeys.address)) }), + joinKeys, + ) + if (joinEditions.isEmpty()) return ConcordInviteResult.NotReachable + if (AuthorityResolver.resolve(joinEditions, bundle.owner).isBanned(account.signer.pubKey)) { + return ConcordInviteResult.Banned + } + val entry = ConcordCommunityListEntry( id = bundle.communityId, @@ -960,14 +1014,13 @@ class AccountConcordActions( // 5. Adopt the new epoch ourselves. This rebuilds our session under the new root and // re-folds the compacted Control Plane (with the ban), dropping the removed members. - adoptConcordRoot(entry, newRoot, build.newEpoch, build.newControlKeys.address.hexToByteArray(), newControlRoot) + val adopted = adoptConcordRoot(entry, newRoot, build.newEpoch, build.newControlKeys.address.hexToByteArray(), newControlRoot) // 6. Move every link we minted to the new epoch. Without this the Refounding orphans them, // and a member it left out — no rekey blob, no message to miss — has no way back at all. - val moved = - account.concordChannelList.liveCommunities.value - .firstOrNull { it.id == communityId } - ?.let { refreshConcordInviteLinks(it) } ?: 0 + // Uses the entry adoption just wrote: `liveCommunities` decrypts asynchronously, so + // reading it here would hand us the epoch we just left and re-mint every link onto it. + val moved = adopted?.let { refreshConcordInviteLinks(it) } ?: 0 Log.i("Concord") { "Refounding ${entry.id}: refreshed $moved invite link(s) to epoch ${build.newEpoch}" } return true } @@ -1033,8 +1086,8 @@ class AccountConcordActions( newEpoch: Long, newControlPk: ByteArray? = null, newControlRoot: ByteArray? = null, - ) { - if (!adoptedConcordRotations.add("${entry.id}:$newEpoch")) return + ): ConcordCommunityListEntry? { + if (!adoptedConcordRotations.add("${entry.id}:$newEpoch")) return null // The rewrite itself — banking the leaving epoch's address for the anti-rollback floor, // dropping stale control material on a legacy rotation, preserving invite_ref and residue — // is shared with `amy` in [ConcordReceive.withAdoptedRoot]. Only the persist + publish and @@ -1042,6 +1095,7 @@ class AccountConcordActions( val next = ConcordReceive.withAdoptedRoot(entry, newRoot, newEpoch, newControlPk, newControlRoot) account.sendMyPublicAndPrivateOutbox(account.concordChannelList.follow(next)) announceConcordGuestbookJoin(next, inviteCreator = null, inviteLabel = null) + return next } /** diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/ConcordInviteResult.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/ConcordInviteResult.kt index 6b4d2599cb..8f502e358b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/ConcordInviteResult.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/ConcordInviteResult.kt @@ -54,6 +54,15 @@ sealed interface ConcordInviteResult { */ data object Expired : ConcordInviteResult + /** + * The link opens, but this community's roster has banned us (CORD-04). + * + * A Refounding re-mints every outstanding link onto the new root, and a removed member keeps the + * URL and its unlock token forever — so honouring the link alone would hand the new keys to the + * very account the rotation expelled. + */ + data object Banned : ConcordInviteResult + /** * The bundle event was found but could not be opened with the link's token — * typically because it was minted by a newer/incompatible Concord client whose diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordInviteScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordInviteScreen.kt index 2788dee7ee..d56c800791 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordInviteScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordInviteScreen.kt @@ -124,6 +124,8 @@ fun ConcordInviteScreen( RedeemState.Failed(R.string.concord_invite_failed_incompatible, canRetry = false) is ConcordInviteResult.Revoked -> RedeemState.Failed(R.string.concord_invite_failed_revoked, canRetry = false) + is ConcordInviteResult.Banned -> + RedeemState.Failed(R.string.concord_invite_failed_banned, canRetry = false) is ConcordInviteResult.Expired -> RedeemState.Failed(R.string.concord_invite_failed_expired, canRetry = false) is ConcordInviteResult.NotReachable -> diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index f3e96dffdf..20e32ea0b7 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -321,6 +321,7 @@ This invite link is invalid or can\'t be opened with this account. This invite link can\'t be opened. It may be outdated or already replaced by a newer one, or created with a newer version of the app. Ask for a fresh invite link. This invite link has been revoked and can no longer be used. Ask for a new one. + This community has removed you. The link still works, but its member list does not admit you. This invite link has expired and can no longer be used. Ask for a fresh link. Community name is only revealed after you join Joining connects to this invite\'s relays, publishes a join announcement signed by your account, and adds the community to your list. Nothing is sent until you tap Join. diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordCommands.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordCommands.kt index 76e7a8d59b..f2890e7775 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordCommands.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordCommands.kt @@ -230,7 +230,7 @@ object ConcordCommands { controlRoot = e.controlRoot ?: priorSameEpoch?.controlRoot ?: "", generalChannelId = prior?.generalChannelId ?: "", relays = e.relays, - heldRoots = e.heldRoots.map { StoredHeldRoot(it.epoch, it.key, it.controlPk ?: "") }, + heldRoots = e.heldRoots.map { StoredHeldRoot(it.epoch, it.key, it.controlPk ?: "", it.controlRoot ?: "") }, // Survives every merge: losing the anchor makes the NEXT exclusion // unrecoverable, so a list entry without one must not clear ours. inviteRef = e.inviteRef ?: prior?.inviteRef ?: "", @@ -266,16 +266,13 @@ object ConcordCommands { // (CORD-05 §1); omitted for a legacy community, which has none to carry. val invite = ConcordActions.inviteFor(sc.communityId, sc.owner, sc.ownerSalt, sc.root, sc.rootEpoch, sc.name, sc.relays, sc.controlPk.ifBlank { null }) val minted = ConcordActions.mintInviteLink(base, invite, TimeUtils.now(), sc.relays) - val ack = ctx.publish(minted.bundleEvent, relaysFor(ctx, sc)) - RawEventSupport.publishGuard(ack, minted.bundleEvent.id)?.let { return it } - - // Record the link in the CORD-05 Invite List (kind 13303) so any of this creator's - // clients — Amethyst, Armada — can later refresh THIS coordinate instead of orphaning - // the link at a dead epoch. That list is the liveness half of stranded recovery (A2). - publishInviteList( - ctx, - extraRelays = relaysFor(ctx, sc), - patch = + // Record the link BEFORE publishing the bundle (CORD-05, kind 13303): a link whose + // `signer_sk` was never stored can never be refreshed, so the next Refounding orphans + // it and every holder is stranded. Better to mint nothing than to hand out a link that + // is already doomed. + val recorded = + publishInviteList( + ctx, ConcordInviteListDocument( entries = listOf( @@ -288,7 +285,16 @@ object ConcordCommands { ), ), ), - ) + ) + if (!recorded) { + return Output.error( + "invite_unrecordable", + "could not record the link signer in your invite list (kind 13303), so this link could never be refreshed after a Refounding — not minting it", + ) + } + + val ack = ctx.publish(minted.bundleEvent, relaysFor(ctx, sc)) + RawEventSupport.publishGuard(ack, minted.bundleEvent.id)?.let { return it } Output.emit( mapOf( @@ -318,6 +324,34 @@ object ConcordCommands { wraps.firstNotNullOfOrNull { ConcordActions.openBundle(it, parsed.fragment.token) } ?: return Output.error("not_found", "no valid bundle for this link").let { 1 } + // Refuse a link that readmits us after we were removed. A Refounding re-mints every + // outstanding link onto the new root (CORD-05), and an ex-member keeps the URL and its + // unlock token forever — so without this check the rotation that was supposed to expel + // them hands them the new keys instead. `recover` has always been ban-gated; `join` is + // the other door into the same room. + // + // Fails CLOSED on an unreadable plane: no verdict, no join. The banlist is only knowable + // after the bundle yields the root, which is why the check lives here rather than before. + val joinKeys = + ConcordActions.controlPlaneKeys( + communityRoot = bundle.communityRoot.hexToByteArray(), + communityId = bundle.communityId.hexToByteArray(), + rootEpoch = bundle.rootEpoch, + controlPk = bundle.controlPk, + ) + val joinRelays = normalize(bundle.relays).ifEmpty { relays } + val joinEditions = + ConcordActions.controlEditions( + ctx.drain(joinRelays.associateWith { listOf(ConcordActions.planeFilter(joinKeys.address)) }, pendingOnAuthRequired = true).map { it.second }, + joinKeys, + ) + if (joinEditions.isEmpty()) { + return Output.error("control_plane_unreadable", "could not fold this community's Control Plane, so whether it has banned you is unknown — refusing to join") + } + if (AuthorityResolver.resolve(joinEditions, bundle.owner).isBanned(ctx.signer.pubKey)) { + return Output.error("banned", "this community has banned this account; the link works but the roster does not admit you (CORD-04)") + } + ConcordStore(dataDir.concordFile).upsert( StoredCommunity( name = bundle.name, @@ -401,7 +435,7 @@ object ConcordCommands { rootEpoch = sc.rootEpoch, controlPk = sc.controlPk.ifBlank { null }, controlRoot = sc.controlRoot.ifBlank { null }, - heldRoots = sc.heldRoots.map { HeldRoot(it.epoch, it.root, it.controlPk.ifBlank { null }) }, + heldRoots = sc.heldRoots.map { HeldRoot(it.epoch, it.root, it.controlPk.ifBlank { null }, it.controlRoot.ifBlank { null }) }, relays = sc.relays, name = sc.name, inviteRef = sc.inviteRef.ifBlank { null }, @@ -416,7 +450,7 @@ object ConcordCommands { rootEpoch = entry.rootEpoch, controlPk = entry.controlPk ?: "", controlRoot = entry.controlRoot ?: "", - heldRoots = entry.heldRoots.map { StoredHeldRoot(it.epoch, it.key, it.controlPk ?: "") }, + heldRoots = entry.heldRoots.map { StoredHeldRoot(it.epoch, it.key, it.controlPk ?: "", it.controlRoot ?: "") }, relays = entry.relays, name = entry.name.ifBlank { sc.name }, inviteRef = entry.inviteRef ?: sc.inviteRef, @@ -585,36 +619,39 @@ object ConcordCommands { * of every link they minted, so a rotation can refresh those links instead of orphaning them. * Empty when none was ever published. */ - suspend fun readInviteList( - ctx: Context, - extraRelays: Set = emptySet(), - ): ConcordInviteListDocument { - val relays = ctx.outboxRelays() + extraRelays - if (relays.isEmpty()) return ConcordInviteListDocument.EMPTY + suspend fun readInviteList(ctx: Context): ConcordInviteListDocument? { + val relays = ctx.outboxRelays() + if (relays.isEmpty()) return null val filter = Filter(kinds = listOf(ConcordInviteListEvent.KIND), authors = listOf(ctx.signer.pubKey)) val newest = ctx .drain(relays.associateWith { listOf(filter) }) .map { it.second } .maxByOrNull { it.createdAt } - return (newest as? ConcordInviteListEvent)?.decrypt(ctx.signer) ?: ConcordInviteListDocument.EMPTY + ?: return ConcordInviteListDocument.EMPTY // nothing published yet — safe to start one + return (newest as? ConcordInviteListEvent)?.decrypt(ctx.signer) } /** - * Merges [patch] into the published list and republishes it. Read-merge-write rather than - * overwrite: the list is replaceable and per-creator, so two devices minting concurrently would - * otherwise delete each other's links (and their `signer_sk`, which is unrecoverable). + * Merges [patch] into the published list and republishes it, returning whether it landed. + * + * Read-merge-write, and **aborts rather than overwriting** when the read fails: kind 13303 is + * replaceable, so writing a patch-only document over a list we could not read deletes every + * other link's `signer_sk`. Those secrets cannot be regenerated, and losing one orphans its + * link at the next rotation, stranding everyone holding that URL. + * + * Account-scoped, like the coordinate itself — (13303, me, "") is one list for every community, + * so reading or writing it on a single community's relays would fork it. */ suspend fun publishInviteList( ctx: Context, patch: ConcordInviteListDocument, - extraRelays: Set = emptySet(), - ) { - val relays = ctx.outboxRelays() + extraRelays - if (relays.isEmpty()) return - val merged = ConcordInviteList.merge(readInviteList(ctx, extraRelays), patch) - val event = ConcordInviteListEvent.create(ctx.signer, merged, TimeUtils.now()) - ctx.publish(event, relays) + ): Boolean { + val relays = ctx.outboxRelays() + if (relays.isEmpty()) return false + val base = readInviteList(ctx) ?: return false + val event = ConcordInviteListEvent.create(ctx.signer, ConcordInviteList.merge(base, patch), TimeUtils.now()) + return ctx.publish(event, relays).values.any { it.accepted } } fun notFound(handle: String): Int { diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordModCommands.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordModCommands.kt index 23ec2a47ef..9b52786df6 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordModCommands.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/ConcordModCommands.kt @@ -33,7 +33,9 @@ import com.vitorpamplona.quartz.concord.cord02Community.ConcordCommunityState import com.vitorpamplona.quartz.concord.cord04Roles.ConcordPermissions import com.vitorpamplona.quartz.concord.cord04Roles.ControlEdition import com.vitorpamplona.quartz.concord.cord04Roles.RoleEntity +import com.vitorpamplona.quartz.concord.cord05Invites.InviteBundleStatus import com.vitorpamplona.quartz.concord.crypto.ControlPlaneKeys +import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.utils.RandomInstance @@ -272,26 +274,37 @@ object ConcordModCommands { // 1. Ban the removed on the CURRENT plane, so the compacted snapshot — and therefore the // new epoch — carries the ban. Each edition chains onto the updated banlist head. var chain = editions + val banWraps = mutableListOf() for (target in removed) { val banWrap = ConcordModeration.ban(ctx.signer, cp, sc.communityId.hexToByteArray(), target, chain, TimeUtils.now(), owner = sc.owner) - ctx.publish(banWrap, relays) + val ack = ctx.publish(banWrap, relays) + if (ack.values.none { it.accepted }) { + return Output.error("ban_not_published", "the pre-rotation ban for $target was not accepted by any relay; refusing to refound with a banlist that would not survive") + } + banWraps += banWrap chain = chain + (ConcordActions.controlEditions(listOf(banWrap), cp)) } // 2. Everyone we are keeping. See the note above on why this reaches past the roster. - val recipients = + val candidates = (rosterOf(authority) + guestbookMembersOf(ctx, sc) + channelAuthorsOf(ctx, sc, state) + me) .mapTo(HashSet()) { it.lowercase() } .apply { removeAll(removed) removeAll(authority.bannedMembers().map { it.lowercase() }.toSet()) - }.toList() + } + val recipients = boundRecipients(candidates, authority) // 3. Build: new root + fresh control_root, compacted plane, per-recipient blobs (staff // get the 136-byte form carrying the secret, everyone else the 104-byte pubkey one). val newRoot = RandomInstance.bytes(32) val newControlRoot = RandomInstance.bytes(32) - val controlWraps = ctx.drain(relays.associateWith { listOf(ConcordActions.planeFilter(cp.address)) }, pendingOnAuthRequired = true).map { it.second } + // Compact from what we KNOW the plane holds: the wraps we drained plus the bans we just + // published. Re-draining alone would race the relay's indexing, and a relay that has not + // yet echoed the ban back (or that ACKed and stored nothing) would produce a new epoch + // whose roster never banned the member we are removing. + val drained = ctx.drain(relays.associateWith { listOf(ConcordActions.planeFilter(cp.address)) }, pendingOnAuthRequired = true).map { it.second } + val controlWraps = (drained + banWraps).distinctBy { it.id } val build = ConcordActions.buildRefounding( rotatorSigner = ctx.signer, @@ -335,33 +348,30 @@ object ConcordModCommands { // Safe for every link because recovery is ban-gated at the epoch being left, and step 1 // banned everyone being removed — so a removed member's own `recover` is refused even // though their link now resolves. - val refreshedInvite = - ConcordActions.inviteFor( - stored.communityId, - stored.owner, - stored.ownerSalt, - stored.root, - stored.rootEpoch, - stored.name, - stored.relays, - stored.controlPk.ifBlank { null }, - ) val now = TimeUtils.now() var refreshed = 0 - for (link in ConcordCommands.readInviteList(ctx, relays).entries) { + val list = ConcordCommands.readInviteList(ctx) + val tombstoned = list?.tombstones?.mapTo(HashSet()) { it.token } ?: emptySet() + for (link in list?.entries.orEmpty()) { if (link.communityId != stored.communityId) continue - // An elapsed link can no longer be joined, so re-posting it would only resurrect a - // dead URL at a live epoch (CORD-05). - if (link.isExpired(now)) continue + // An elapsed or retired link can no longer be joined, so re-posting it would only + // resurrect a dead URL at a live epoch (CORD-05). + if (link.isExpired(now) || link.token in tombstoned) continue runCatching { - val event = - ConcordActions.remintBundleAt( - linkSignerPrivKey = link.signerSk.hexToByteArray(), - token = link.token.hexToByteArray(), - invite = refreshedInvite, - createdAt = now, + val token = link.token.hexToByteArray() + // Refresh from the link's CURRENT bundle so its own fields — expiry, channel + // grants, icon, label — survive the rotation, and so a coordinate whose newest + // event is a revocation tombstone is left revoked instead of being re-opened. + val wraps = ctx.drain(relays.associateWith { listOf(ConcordActions.bundleFilter(link.signerPubKeyHex())) }).map { it.second } + val live = ConcordActions.classifyInvite(wraps, token) as? InviteBundleStatus.Live ?: return@runCatching + val moved = + live.invite.copy( + communityRoot = stored.root, + rootEpoch = stored.rootEpoch, + controlPk = stored.controlPk.ifBlank { null }, + relays = stored.relays, ) - ctx.publish(event, relays) + ctx.publish(ConcordActions.remintBundleAt(link.signerSk.hexToByteArray(), token, moved, now), relays) refreshed++ } } @@ -382,6 +392,40 @@ object ConcordModCommands { } } + /** + * How many recipients one Refounding will re-key, mirroring Amethyst's own cap. + * + * Two thirds of the recipient union — Guestbook joins and observed channel authors — are + * attacker-writable: any key can announce a join or post once. Without a bound, padding those + * sets inflates the cost of the only hard removal Concord has until rotating becomes + * impractical, so the attack raises the price of its own remedy (B4 in the soft-ban audit). + */ + private const val MAX_REFOUNDING_RECIPIENTS = 5_000 + + /** + * Caps [candidates], keeping the members whose standing is owner-rooted and therefore cannot be + * padded from outside. Anything dropped is reported rather than silently truncated — a dropped + * member is stranded on the dead epoch and their only way back is `concord recover`. + */ + private fun boundRecipients( + candidates: Set, + authority: com.vitorpamplona.quartz.concord.cord04Roles.AuthorityResolver, + ): List { + if (candidates.size <= MAX_REFOUNDING_RECIPIENTS) return candidates.toList() + val vouched = (authority.roleHolders() + authority.staffMembers()).mapTo(HashSet()) { it.lowercase() } + val kept = LinkedHashSet() + candidates.filterTo(kept) { it in vouched } + for (candidate in candidates) { + if (kept.size >= MAX_REFOUNDING_RECIPIENTS) break + kept.add(candidate) + } + val dropped = candidates.size - kept.size + if (dropped > 0) { + System.err.println("[concord] refounding recipient set trimmed to ${kept.size} of ${candidates.size}: $dropped member(s) will be stranded on the prior epoch") + } + return kept.toList() + } + /** Owner + everyone holding a role — owner-rooted, so it cannot be padded from outside. */ private fun rosterOf(authority: com.vitorpamplona.quartz.concord.cord04Roles.AuthorityResolver): Set = (authority.roleHolders() + authority.staffMembers()).mapTo(HashSet()) { it.lowercase() } diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/ConcordStore.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/ConcordStore.kt index d2de85a258..c36b1ac915 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/ConcordStore.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/stores/ConcordStore.kt @@ -61,6 +61,12 @@ data class StoredHeldRoot( val root: String = "", /** That epoch's Control Plane address; blank for a legacy, pre-split epoch (CORD-02 §5). */ val controlPk: String = "", + /** + * That epoch's staff write key, banked only if we held it. A relay that gates the prior epoch's + * Control Plane on NIP-42 AUTH as the stream key will not serve those wraps without it — and + * those wraps are what rebuild the anti-rollback floor. + */ + val controlRoot: String = "", ) /** diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/CommunityInvite.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/CommunityInvite.kt index d652c70143..632a7f9e03 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/CommunityInvite.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/CommunityInvite.kt @@ -52,7 +52,7 @@ class InviteChannel( * into a kind-33301 bundle (link invites) or a NIP-59 giftwrap (direct invites). */ @Serializable -class CommunityInvite( +data class CommunityInvite( @SerialName("community_id") val communityId: String, val owner: String, @SerialName("owner_salt") val ownerSalt: String, diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteList.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteList.kt index 3eea2095c3..5b930eb261 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteList.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteList.kt @@ -21,6 +21,10 @@ package com.vitorpamplona.quartz.concord.cord05Invites import com.vitorpamplona.quartz.concord.cord04Roles.ConcordJson +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.KSerializer import kotlinx.serialization.SerialName @@ -58,6 +62,12 @@ class ConcordInviteListEntry( ) { /** True when this link can no longer be joined, so it must not be refreshed (CORD-05). */ fun isExpired(nowSecs: Long): Boolean = expiresAt != null && expiresAt <= nowSecs + + /** + * The link signer's pubkey — the addressable coordinate the bundle lives at, derived from the + * secret we kept. Refreshing or retiring a link means writing at exactly this author. + */ + fun signerPubKeyHex(): HexKey = KeyPair(privKey = signerSk.hexToByteArray()).pubKey.toHexKey() } /** A retired link: the creator's record that [token] is gone, kept so a merge cannot resurrect it. */ @@ -150,11 +160,15 @@ object ConcordInviteList { private object WireDocumentSerializer : ExtrasPreserving(WireDocument.serializer()) /** - * Decodes the plaintext document. A malformed document yields [ConcordInviteListDocument.EMPTY] - * rather than throwing — but note the sharp edge this shape shares with the community list: one - * unparseable entry aborts the whole array, so every field defaults instead of being required. + * Decodes the plaintext document, or **null** when it cannot be parsed. + * + * Null rather than an empty document on purpose: this list is replaceable, so a caller that + * treats "I could not read it" as "it is empty" and republishes destroys every `signer_sk` it + * did not manage to read — secrets that cannot be regenerated, orphaning every outstanding + * invite at a dead epoch. Callers MUST distinguish the two (see [ConcordInviteList.merge]'s + * callers). Each field still defaults, so one odd entry does not abort the whole array. */ - fun decode(json: String): ConcordInviteListDocument = + fun decodeOrNull(json: String): ConcordInviteListDocument? = try { val doc = ConcordJson.instance.decodeFromString(WireDocumentSerializer, json) ConcordInviteListDocument( @@ -166,7 +180,7 @@ object ConcordInviteList { residue = doc.extras, ) } catch (_: Exception) { - ConcordInviteListDocument.EMPTY + null } fun encode(doc: ConcordInviteListDocument): String = diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListEvent.kt index f4bbd80f1c..3816f60a6f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListEvent.kt @@ -52,15 +52,19 @@ class ConcordInviteListEvent( sig: HexKey, ) : BaseReplaceableEvent(id, pubKey, createdAt, KIND, tags, content, sig) { /** - * Decrypts the whole document with [signer] — entries, tombstones and the document residue. - * Use this (never a partial read) whenever the result will be re-encoded, or another client's - * unknown keys are dropped on the next publish. + * Decrypts the whole document with [signer] — entries, tombstones and the document residue — or + * **null** if it cannot be decrypted or parsed. + * + * Null, never empty: a caller that reads a decrypt failure as "no links yet" and republishes + * wipes every `signer_sk` on this replaceable coordinate. A bunker signer that momentarily + * refuses is enough to trigger it. Use this (never a partial read) whenever the result will be + * re-encoded, or another client's unknown keys are dropped on the next publish. */ - suspend fun decrypt(signer: NostrSigner): ConcordInviteListDocument = + suspend fun decrypt(signer: NostrSigner): ConcordInviteListDocument? = try { - ConcordInviteList.decode(signer.nip44Decrypt(content, signer.pubKey)) + ConcordInviteList.decodeOrNull(signer.nip44Decrypt(content, signer.pubKey)) } catch (_: Exception) { - ConcordInviteListDocument.EMPTY + null } companion object { diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListTest.kt index e488aeccb5..7a3904bdbc 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordInviteListTest.kt @@ -46,7 +46,7 @@ class ConcordInviteListTest { @Test fun readsTheSpecDocumentIntoTypedEntries() { - val doc = ConcordInviteList.decode(specJson) + val doc = ConcordInviteList.decodeOrNull(specJson)!! assertEquals(1, doc.entries.size) val e = doc.entries.first() @@ -65,7 +65,7 @@ class ConcordInviteListTest { @Test fun emitsTheSnakeCaseKeysAnotherClientReads() { - val json = ConcordInviteList.encode(ConcordInviteList.decode(specJson)) + val json = ConcordInviteList.encode(ConcordInviteList.decodeOrNull(specJson)!!) // Field names are the interop contract — a camelCase slip silently orphans every link. for (key in listOf("\"token\"", "\"signer_sk\"", "\"community_id\"", "\"url\"", "\"created_at\"", "\"expires_at\"", "\"entries\"", "\"tombstones\"")) { assertTrue(json.contains(key), "missing wire key $key") @@ -84,7 +84,7 @@ class ConcordInviteListTest { "doc_level_unknown": 7 } """.trimIndent() - val round = ConcordInviteList.encode(ConcordInviteList.decode(withExtras)) + val round = ConcordInviteList.encode(ConcordInviteList.decodeOrNull(withExtras)!!) assertTrue(round.contains("future_field"), "entry-level unknown key dropped") assertTrue(round.contains("doc_level_unknown"), "document-level unknown key dropped") @@ -116,9 +116,36 @@ class ConcordInviteListTest { } @Test - fun aMalformedDocumentYieldsEmptyRatherThanThrowing() { - assertEquals(0, ConcordInviteList.decode("not json").entries.size) - assertEquals(0, ConcordInviteList.decode("{\"entries\":\"wrong type\"}").entries.size) + fun aMalformedDocumentYieldsNullSoCallersCannotOverwriteWithIt() { + // Null, not empty: a caller that republishes an "empty" list over this replaceable + // coordinate destroys every signer_sk it failed to read. + assertEquals(null, ConcordInviteList.decodeOrNull("not json")) + assertEquals(null, ConcordInviteList.decodeOrNull("{\"entries\":\"wrong type\"}")) + } + + @Test + fun anUnreadableListIsDistinguishableFromAnEmptyOne() { + // The whole point of the null: a caller must be able to tell "I could not read it" from + // "there is nothing in it". Publishing a merge onto the latter is fine; onto the former it + // destroys every signer_sk on this replaceable coordinate. + assertEquals(null, ConcordInviteList.decodeOrNull("")) + + val empty = ConcordInviteList.decodeOrNull("""{"entries":[],"tombstones":[]}""") + assertEquals(0, empty!!.entries.size, "a genuinely empty list decodes, it does not fail") + + // And a merge onto an empty base keeps the patch, so starting a first list still works. + val patch = ConcordInviteListDocument(entries = listOf(ConcordInviteListEntry("t", "sk", "c", "u"))) + assertEquals(listOf("t"), ConcordInviteList.merge(empty, patch).entries.map { it.token }) + } + + @Test + fun theSignerPubKeyIsTheCoordinateTheBundleLivesAt() { + // Refreshing or revoking a link means writing at exactly this author, so it must derive from + // the secret we kept rather than being stored (and drifting) separately. + val sk = "11".repeat(32) + val entry = ConcordInviteListEntry("t", sk, "c", "u") + assertEquals(64, entry.signerPubKeyHex().length) + assertEquals(entry.signerPubKeyHex(), ConcordInviteListEntry("t2", sk, "c", "u2").signerPubKeyHex()) } @Test