From 153191e722d7f7a85983f2cd9841109501ddd517 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Sun, 19 Jul 2026 18:59:33 -0400 Subject: [PATCH] fix(nip46): stop auto-signing relay AUTH under the default policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `REASONABLE` — the default policy on connect — auto-approved kind 22242 (NIP-42 relay auth). The in-code justification was that the event is ephemeral and bound to one relay and challenge, so it cannot be replayed elsewhere. That is true and beside the point: the requesting app supplies the `relay` and `challenge` tags verbatim, so it never needs to replay — it just asks for a FRESH signature naming any relay it likes. A paired app could therefore, with no prompt, open its own socket to any NIP-42 relay, take the challenge, get 22242 signed, and authenticate to that relay AS THE USER. That yields read access to whatever the relay gates behind AUTH — notably the kind-1059 giftwrap inbox and its full DM metadata (who, when, how many) — and burns quota on paid relays, which bill whoever authenticates. Amethyst auto-signing AUTH for relays the USER configured is not the same as letting a third party name the relay; the comment conflated them. 22242 now falls through to ASK. The existing test asserted the vulnerable behaviour with the same flawed reasoning, so it is inverted here rather than merely extended. Co-Authored-By: Claude Opus 4.8 --- .../signers/NostrSignerPermissionLedger.kt | 14 +++++++---- .../NostrSignerPermissionLedgerTest.kt | 24 ++++++++++--------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt index ae8d1cd9d4..9688631c8b 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedger.kt @@ -30,7 +30,6 @@ import com.vitorpamplona.quartz.nip28PublicChat.message.ChannelMessageEvent import com.vitorpamplona.quartz.nip35Torrents.TorrentCommentEvent import com.vitorpamplona.quartz.nip35Torrents.TorrentEvent import com.vitorpamplona.quartz.nip38UserStatus.StatusEvent -import com.vitorpamplona.quartz.nip42RelayAuth.RelayAuthEvent import com.vitorpamplona.quartz.nip53LiveActivities.chat.LiveActivitiesChatMessageEvent import com.vitorpamplona.quartz.nip54Wiki.WikiNoteEvent import com.vitorpamplona.quartz.nip56Reports.ReportEvent @@ -187,9 +186,15 @@ class NostrSignerPermissionLedger( * - **zap request** (9734) — moves nothing; it only fetches a Lightning invoice. The payment * itself is the separately-gated `value.payInvoice` capability that prompts on *every* use * regardless of policy. - * - **relay auth** (22242, NIP-42) — an ephemeral proof-of-key bound to a single relay and - * challenge (it cannot be replayed to another relay). Amethyst's own client auto-signs it - * for every logged-in account, so treating it as background noise matches existing behavior. + * + * **Relay auth (22242) is deliberately NOT here.** It looks harmless — the event is ephemeral + * and bound to one relay+challenge, so it cannot be replayed elsewhere. But replay is not the + * threat: the requesting app supplies the `relay` and `challenge` tags verbatim, so it can ask + * for a *fresh* signature naming any relay it likes, then AUTH to that relay as the user. That + * yields read access to whatever the relay gates behind AUTH — notably the user's kind-1059 + * giftwrap inbox and its full DM metadata — and burns the quota on paid relays, which bill + * whoever authenticates. Amethyst auto-signing AUTH for relays *the user configured* is not + * the same as letting a third party name the relay. * * None of the members can silently: spend money, overwrite account configuration (profile 0, * contacts 3, relay/mute/bookmark lists are replaceable — a bad write can wipe settings), @@ -232,7 +237,6 @@ class NostrSignerPermissionLedger( TorrentCommentEvent.KIND, // 2004 — NIP-35 torrent comments HighlightEvent.KIND, // 9802 — highlighted snippets shared publicly LnZapRequestEvent.KIND, // 9734 — Lightning zap request; the payment itself still prompts - RelayAuthEvent.KIND, // 22242 — NIP-42 relay auth; ephemeral, bound to one relay+challenge LongTextNoteEvent.KIND, // 30023 — NIP-23 long-form articles (addressable content) StatusEvent.KIND, // 30315 — ephemeral user status / presence WikiNoteEvent.KIND, // 30818 — NIP-54 wiki articles (addressable content) diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt index f07eb5e833..c6a0ed1fc0 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/connectedApps/signers/NostrSignerPermissionLedgerTest.kt @@ -51,14 +51,13 @@ class NostrSignerPermissionLedgerTest { ledger.setPolicy(coordinate, AppSignerPolicy.REASONABLE) // Profile (0), contacts (3), deletion (5), relay list (10002), nutzap (9321), NIP-98 HTTP - // auth (27235), and gift-wrapped DM (1059) must never auto-sign — they change config, - // delete, spend ecash, authorize arbitrary HTTP calls, or leak. These are replaceable - // *configuration* (0/3/10002), unlike addressable *content* such as long-form (30023) which - // is allowed. A nutzap in particular *is* the payment (it carries the ecash proofs), unlike a - // zap request (9734) which only fetches an invoice; and NIP-98 authorizes destructive/admin - // HTTP requests, unlike NIP-42 relay auth (22242) which is a replay-bound read proof. - // Decryption reveals private content, so it also asks. - for (kind in listOf(0, 3, 5, 10002, 9321, 27235, 1059)) { + // auth (27235), relay auth (22242), and gift-wrapped DM (1059) must never auto-sign — they + // change config, delete, spend ecash, authorize arbitrary HTTP calls, authenticate as the + // user, or leak. These are replaceable *configuration* (0/3/10002), unlike addressable + // *content* such as long-form (30023) which is allowed. A nutzap in particular *is* the + // payment (it carries the ecash proofs), unlike a zap request (9734) which only fetches an + // invoice. Decryption reveals private content, so it also asks. + for (kind in listOf(0, 3, 5, 10002, 9321, 27235, 22242, 1059)) { assertEquals( NostrOpDecision.ASK, ledger.decide(coordinate, NostrSignerOp.SignKind(kind)), @@ -72,9 +71,12 @@ class NostrSignerPermissionLedgerTest { assertEquals(NostrOpDecision.ALLOW, ledger.decide(coordinate, NostrSignerOp.SignKind(9734))) assertEquals(NostrOpDecision.ASK, ledger.decide(coordinate, NostrSignerOp.SignKind(9321))) - // Contrast: NIP-42 relay auth (22242) auto-signs — it is a replay-bound read proof — but - // NIP-98 HTTP auth (27235) does not, since it can authorize destructive/admin HTTP calls. - assertEquals(NostrOpDecision.ALLOW, ledger.decide(coordinate, NostrSignerOp.SignKind(22242))) + // NIP-42 relay auth (22242) must ASK. Being replay-bound is not the protection it looks + // like: the requesting app supplies the relay and challenge tags, so it can obtain a FRESH + // signature naming any relay and then AUTH there as the user — reading whatever that relay + // gates behind AUTH (the kind-1059 giftwrap inbox and its DM metadata) and spending the + // quota on paid relays, which bill whoever authenticates. + assertEquals(NostrOpDecision.ASK, ledger.decide(coordinate, NostrSignerOp.SignKind(22242))) assertEquals(NostrOpDecision.ASK, ledger.decide(coordinate, NostrSignerOp.SignKind(27235))) }