From 5a18dc41c8eca9d7415a02eb35eafcb86588b443 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 20:14:07 +0000 Subject: [PATCH] fix(marmot): inner events are unsigned rumors per MIP-03 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MIP-03 ("Application Messages" → Security Requirements) is explicit: > "Inner events MUST remain unsigned (no `sig` field) > This ensures leaked events cannot be published to public relays." Authentication comes from (a) MLS framing — the sender's LeafNode credential signs the outer MLS Application message — and (b) the mandatory check in MarmotInboundProcessor.processPrivateMessage that the inner event's `pubkey` equals the MLS sender's credential identity. The Nostr Schnorr signature is redundant, and leaving it populated means a leaked plaintext can be replayed as a valid public kind:9. whitenoise-rs is spec-compliant (builds via `UnsignedEvent::new()` + `ensure_id()`, never calls sign). Amethyst was the non-conforming side on both directions: 1. Send path (`MarmotManager.buildTextMessage`, `AccountViewModel.sendMarmotGroupMediaMessage`) called `signer.sign(template)`, producing a signed rumor that shipped a valid Schnorr signature through the encrypted channel. Switch both to `RumorAssembler.assembleRumor` — same id derivation (SHA-256 over canonicalized [0, pubkey, createdAt, kind, tags, content]), but `sig = ""`. 2. Restore path (`Account.kt` on startup reload of persisted inner events) called `cache.justConsume(innerEvent, null, false)`, which routed through `consumeRegularEvent` → `justVerify` → `event.verify()` → FAIL on empty sig → Note registered with `event = null`, message never rendered. Pass `wasVerified = true`, matching what the live receive path already does after the previous commit (573c5c2b). Existing on-disk persisted messages from older signed-rumor builds still load — `wasVerified=true` skips sig verify entirely, so both legacy signed and spec-correct unsigned rumors deserialize cleanly. --- .../vitorpamplona/amethyst/model/Account.kt | 6 +++- .../ui/screen/loggedIn/AccountViewModel.kt | 11 +++++++- .../amethyst/commons/marmot/MarmotManager.kt | 28 +++++++++++++------ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 38680c8c2..7fd284b0c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -3036,7 +3036,11 @@ class Account( val innerEvent = com.vitorpamplona.quartz.nip01Core.core.Event .fromJson(json) - val isNew = cache.justConsume(innerEvent, null, false) + // MIP-03 inner events are unsigned rumors + // (empty sig); pass wasVerified=true so + // LocalCache skips the Nostr secp256k1 check + // that would silently reject them. + val isNew = cache.justConsume(innerEvent, null, true) val innerNote = cache.getOrCreateNote(innerEvent.id) if (isNew) { innerNote.event = innerEvent diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index b9e9220e6..aa14e675e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1477,7 +1477,16 @@ class AccountViewModel( alt(caption) } } - val innerEvent = account.signer.sign(template) + // MIP-03: inner events MUST remain unsigned (no `sig`) so a leaked + // plaintext can't be replayed as a valid public kind:9. Authorship + // is authenticated by the MLS sender's LeafNode + the pubkey↔ + // credential-identity equality check on the receive side. + val innerEvent = + com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler + .assembleRumor( + account.signer.pubKey, + template, + ) val relays = marmotGroupRelays(nostrGroupId) account.sendMarmotGroupMessage(nostrGroupId, innerEvent, relays) } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 06a132c53..c1fc14d85 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -155,17 +155,25 @@ class MarmotManager( ): OutboundGroupEvent = outboundProcessor.buildGroupEvent(nostrGroupId, innerEvent) /** - * Build a kind:9 chat-message GroupEvent from plain text. The inner event is - * signed with this manager's signer and optionally persisted to the local - * decrypted-message log so `loadStoredMessages` reflects our own outbound - * immediately (without waiting for relay loopback). + * Build a kind:9 chat-message GroupEvent from plain text. The inner + * event is built as an UNSIGNED rumor per MIP-03 ("Inner events MUST + * remain unsigned — this ensures leaked events cannot be published to + * public relays"): the MLS sender authenticates via the LeafNode + * credential + the `pubkey` ↔ sender-identity equality check, so + * the inner Nostr signature is redundant, and leaving it in would + * let a leaked plaintext be replayed as a valid public kind:9. * - * Platform callers that already maintain their own "own event" cache (i.e. - * Amethyst's `LocalCache.justConsumeMyOwnEvent`) should pass `persistOwn = false`. - * Headless callers (CLI) should leave it at the default. + * Optionally persisted to the local decrypted-message log so + * `loadStoredMessages` reflects our own outbound immediately + * (without waiting for relay loopback). + * + * Platform callers that already maintain their own "own event" cache + * (i.e. Amethyst's `LocalCache.justConsumeMyOwnEvent`) should pass + * `persistOwn = false`. Headless callers (CLI) should leave it at + * the default. * * @return the signed kind:445 outer event together with the inner kind:9 - * event id, so the caller can reference it for replies/reactions. + * rumor id, so the caller can reference it for replies/reactions. */ suspend fun buildTextMessage( nostrGroupId: HexKey, @@ -175,7 +183,9 @@ class MarmotManager( val template = com.vitorpamplona.quartz.nip01Core.signers .eventTemplate(kind = 9, description = text) - val innerEvent = signer.sign(template) + val innerEvent = + com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler + .assembleRumor(signer.pubKey, template) val outbound = buildGroupMessage(nostrGroupId, innerEvent) if (persistOwn) persistDecryptedMessage(nostrGroupId, innerEvent.toJson()) return TextMessageBundle(outbound = outbound, innerEvent = innerEvent)