mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
fix(nip46): gate identity reads on pairing; make decrypt consent informed
Two problems in the remote signer, both about a client getting something
without the user meaningfully agreeing to it.
**`get_public_key` and `get_relays` answered anyone.** Every other method
runs through `ifAuthorized`; these did not, and nothing required a prior
successful `connect`. The service decrypts and dispatches any well-formed
kind-24133 envelope, so anyone holding the `bunker://` URI — pasted into a
malicious app, posted for support, leaked in a screenshot — could ask it
which account it belongs to, without the secret and without connecting.
`get_relays` additionally handed over the inbox relay set. That defeated
the transport/identity split, which otherwise works: the relay-visible
traffic really is anonymous, since the p-tag and author are a transport
key and the payload is NIP-44.
Both now require the client to be paired. The authorizer interface gains
`isPaired` with NO default, so a future authorizer has to state its own
rule rather than silently inheriting "everyone is paired".
`ping` is deliberately left open. It reveals nothing the caller does not
already have — a signer is alive at a pubkey they hold — and first-party
behaviour could be confirmed but third-party clients that ping before
connecting could not be ruled out. Breaking a legitimate handshake to
close a minor oracle is a bad trade. The choice is pinned by a test that
also asserts the pairing check is never consulted, so it stays deliberate
rather than drifting back by accident.
**Decrypt consent showed nothing at all.** The bridge populated the
content preview and raw data only for signing requests, so a decrypt
request produced an empty preview block — no ciphertext, no counterparty,
not even the "Show event" toggle — leaving "AppName wants to read your
private messages" with *Allow always* as the primary button. Meanwhile
the coordinator documented the opposite: "Amethyst decrypts first, then
asks permission to expose." That was never implemented.
Now:
- The counterparty is resolved and shown, so the prompt reads "…read your
private messages **with Alice**". It never degrades to nothing —
cached name, else a shortened npub. Knowing *whose* messages is a
categorically different decision.
- The message is decrypted BEFORE prompting and the plaintext is the
preview, as documented. It is a local operation and nothing is exposed
until approval. Failure, blank and hang all collapse to an explanatory
string under a timeout, so the dialog is never empty and cannot stall.
- A narrower grant is offered ALONGSIDE the broad one, not instead of it:
`DecryptFrom(counterparty)` keyed `decrypt:<hex>` next to `Decrypt`.
The dialog's primary button becomes "Always allow for Alice" with the
broad option demoted. Because the ledger stores an opaque op key, no
persisted decision migrates and the storage format is untouched.
Scoping decrypt per counterparty *instead* would have been worse than
the bug: a DM client would prompt once per conversation, training users
to approve everything. A narrow option beside the broad one gives
granularity without the prompt explosion.
Also fixes a latent bug found on the way: `AllowForSession` recorded the
*requested* op rather than the *granted* one, which would have widened a
narrow session grant back to broad.
Verified by three sabotage passes; the tests that stayed green under them
are the intended negative guards. One existing test asserted the buggy
behaviour outright ("public reads are never gated") and was rewritten.
Not done: the batched consent sheet still records the broad op for
"remember" — offering the narrow choice per row there is a UX design
question, not a mechanical change.
Needs a device check before release: the decrypt preview runs the account
signer before consent. That is free for a local key, but an account backed
by an external NIP-55 signer (Amber) may show Amber's own prompt ahead of
Amethyst's.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
7792c42f1d
commit
73d59a29bf
@@ -47,6 +47,7 @@ import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
import kotlinx.coroutines.withContext
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
|
||||
/**
|
||||
* `amy bunker [--relay URL[,URL…]] [--secret S] [--perms P] [--interactive] [--timeout SECS]`
|
||||
@@ -94,11 +95,25 @@ object BunkerCommand {
|
||||
) : Nip46RequestAuthorizer {
|
||||
private val promptLock = Mutex()
|
||||
|
||||
/**
|
||||
* Clients that presented the right secret in this process's lifetime. The CLI bunker keeps no
|
||||
* state on disk, so pairing is in-memory: a client must `connect` once per `amy bunker` run
|
||||
* before it can read the hosted identity.
|
||||
*/
|
||||
private val paired = ConcurrentHashMap.newKeySet<HexKey>()
|
||||
|
||||
/**
|
||||
* Ungated (the headless default hosting the operator's own key) everything is open, matching
|
||||
* [authorize]. Gated, the identity reads require a `connect` first.
|
||||
*/
|
||||
override suspend fun isPaired(clientPubKey: HexKey): Boolean = !gated || clientPubKey in paired
|
||||
|
||||
override suspend fun onConnect(
|
||||
clientPubKey: HexKey,
|
||||
request: BunkerRequestConnect,
|
||||
): Nip46ConnectDecision =
|
||||
if (request.secret == secret) {
|
||||
paired.add(clientPubKey)
|
||||
Nip46ConnectDecision.Accept(BunkerRequestProcessor.ACK)
|
||||
} else {
|
||||
Nip46ConnectDecision.Reject("invalid secret")
|
||||
|
||||
Reference in New Issue
Block a user