diff --git a/docs/COMPARISON-lnbits-nostr_bunker.md b/docs/COMPARISON-lnbits-nostr_bunker.md deleted file mode 100644 index 5aa6d77..0000000 --- a/docs/COMPARISON-lnbits-nostr_bunker.md +++ /dev/null @@ -1,108 +0,0 @@ -# nsecbunkerd vs. `lnbits/nostr_bunker` - -A comparison of this daemon (the aiolabs fork of `kind-0/nsecbunkerd`) against the -upstream LNbits extension [`lnbits/nostr_bunker`](https://github.com/lnbits/nostr_bunker). - -> Source verified 2026-06-19 against `lnbits/nostr_bunker@main` (`services.py`, -> `models.py`, `crud.py`). The two projects share a name and a NIP (NIP-46 remote -> signing) but are architecturally **inverted**: this daemon *uses* LNbits as a -> downstream wallet provider; the upstream extension *is* an LNbits extension that -> turns a wallet account into the bunker. - -## The one thing that matters: where the nsec lives - -| | nsecbunkerd (this fork) | `lnbits/nostr_bunker` | -|---|---|---| -| **Signing key location** | On the **daemon** host, separate process from LNbits | On the **LNbits** host, inside the extension DB | -| **At-rest protection** | Passphrase-encrypted (LND-style unlock) for manually-added keys | **Plaintext** in `nostr_bunker.bunkers_data.nsec` — no encryption | -| **Integration direction** | LNbits is a *downstream dependency* (wallet factory) | LNbits is the *host* (wallet account = signer identity) | - -`crud.py:create_bunkers_data()` writes the nsec straight through -`db.insert("nostr_bunker.bunkers_data", ...)` with no encryption step; `models.py` -`BunkersData.nsec` is "the normalized private key stored directly." This is the exact -posture the aiolabs roadmap (`aiolabs/lnbits#18`, "no nsec at rest on LNbits") exists -to eliminate: the LNbits host runs extension code, payment plumbing, and a public API, -so disk/root compromise there must NOT equal Nostr-identity compromise. The -standalone-daemon model keeps signing off that host; the upstream extension puts the -key right back on it, unencrypted. - -## Full side-by-side - -| Dimension | nsecbunkerd (this fork) | `lnbits/nostr_bunker` | -|---|---|---| -| **Form factor** | Standalone Node daemon (own process/container) | LNbits extension, runs inside the LNbits process | -| **Stack** | TypeScript + NDK 3.0.3 + nostr-tools 2.20 + Prisma/SQLite | Python + Vue/Quasar UMD frontend | -| **Relay transport** | Daemon opens its own relay connections (NDK); per-key kind:24133 subs pinned to explicit relays (#21) | Piggybacks the `nostrclient` extension's shared relay layer (`nostr_client.relay_manager.publish_message()`) | -| **Tenancy** | Multi-key, multi-domain, multi-user from one daemon | One bunker per wallet account; multiplexes clients via multiple `bunker://` URLs | -| **Admin / control plane** | Whitelisted admin npubs over E2E-encrypted Nostr events; separate bunker key holds no user key material; optional remote `app.nsecbunker.com` UI | LNbits admin UI; wallet owner is implicitly the operator | -| **Account provisioning** | OAuth-like flow: remote `create_account` → NIP-05 file write → NIP-89 (`kind:31990`) announce → mints LNbits wallet via `usermanager` API + nostdress `lud16` | None — the LNbits account already exists; the wallet *is* the identity | - -## NIP-46 surface - -Both implement NIP-46 over kind:24133 and accept **both** NIP-04 and NIP-44 v2 -(upstream `services.py` tries `nip44_decrypt` first, falls back to `nip04_decrypt`). - -| Method | nsecbunkerd | `lnbits/nostr_bunker` | -|---|---|---| -| `connect` | ✓ | ✓ (returns secret/ack after permission check) | -| `get_public_key` | ✓ | ✓ | -| `sign_event` | ✓ (ACL-gated, wire-name vocab #14) | ✓ (`_assert_method_allowed` + auto/confirm flow) | -| `nip04_encrypt` / `decrypt` | ✓ | ✓ | -| `nip44_encrypt` / `decrypt` | ✓ | ✓ | -| `ping` | ✓ | ✓ (`pong`) | -| `switch_relays` | — | ✓ (returns relay list as JSON) | - -## Policy / permission model - -This is where the designs genuinely diverge, and where upstream has something worth -borrowing. - -**nsecbunkerd** — relational ACL across several tables: -- `KeyUser` — a (keyName, userPubkey) grant -- `SigningCondition` — per-method/kind/content allow rules -- `Policy` / `PolicyRule` — reusable rule sets with per-rule `maxUsageCount` + expiry -- `Token` — redeemable connection grant bound to a policy, with `redeemedAt` / `revokedAt` -- Live-policy auth re-evaluated at request time (#11) - -**`lnbits/nostr_bunker`** — policy is **the `bunker://` URL itself**. Each `UrlData` -row carries its own: -- `relays`, `secret`, `client_pubkey` -- `permissions` (e.g. `sign_event:{kind}`), `can_read`, `can_write` -- `auto_sign` (default `False`) vs `confirm_sign` (default `True`) -- `expires_at` -- `post_rate_limit_per_day` — daily cap on kind:1, enforced by counting - `get_signing_requests_since()` over 24h (`_assert_post_rate_limit`) - -Pending approvals live in `SigningRequest` (status: pending/approved/signed/rejected/error), -mirroring this fork's `Request` + manual-approval flow. - -**Takeaway:** upstream's "one bunker, many scoped URLs, each URL is a self-contained -grant" is arguably cleaner than this fork's `Token`+`Policy`+`SigningCondition` triad -for the common case of "issue a narrowly-scoped grant to one client." If the ACL surface -here is ever simplified, that URL-as-grant model is the reference design — note in -particular the built-in `post_rate_limit_per_day`, which this fork has no direct -equivalent for. - -## Where each fits the aiolabs stack - -- **nsecbunkerd is the signer; LNbits is a client of it.** This is the `#18` endgame: - LNbits routes signing through a `RemoteBunkerSigner` over NIP-46 (the - protocol-over-loopback boundary chosen deliberately over a Unix socket), and every - nsec — operator *and* server identity — is retired from the LNbits host. - -- **`lnbits/nostr_bunker` is the convenience inversion we're explicitly avoiding.** - Useful prior art for per-URL policy ergonomics, but adopting it as the *signer - location* would reintroduce plaintext nsec-at-rest on the payments host — the precise - thing `#18` is designed to kill. - -## Gaps to track on our side - -1. **OAuth-created keys are stored recoverable, not encrypted.** - `create_account.ts` writes `currentConfig.keys[keyName] = { key: key.privateKey }`, - unlike the passphrase-encrypted path the SECURITY-MODEL doc describes for - manually-added keys. The doc promises non-exfiltratable keys; the OAuth path doesn't - meet that bar. (We're still strictly better than upstream, which stores *all* nsecs - plaintext — but the doc/behavior gap is real.) - -2. **No per-grant rate limiting.** Upstream's `post_rate_limit_per_day` is a clean - primitive we lack. Worth considering as a `PolicyRule` field. diff --git a/docs/acl-prior-art-survey.md b/docs/acl-prior-art-survey.md deleted file mode 100644 index 01a9f57..0000000 --- a/docs/acl-prior-art-survey.md +++ /dev/null @@ -1,297 +0,0 @@ -# ACL prior-art survey — NIP-46 bunker implementations - -Source-verified survey of how other open-source NIP-46 remote signers model -authorization and grant lifecycle, run to inform the #25 ACL redesign (enforce -token + grant lifecycle live at sign time instead of via a materialized cache). - -> **Verification status.** Every claim below was read against actual source on -> 2026-06-19 (clones at the commits noted per project). An initial automated -> survey overstated several implementations (notably "Signet enforces all -> lifecycle live" — false); the corrections are called out inline. Treat the -> file:line citations as the authority, not the prose summaries. - -## TL;DR for the redesign - -- **Amber** is the one *positive* live-lifecycle template: store the absolute - deadline on the grant row, recompute the verdict against `now()` on every - request, treat the periodic sweep as cleanup only. It also time-boxes - *denials*, not just grants. -- **Signet** (a fork of our own codebase) re-shipped our #24 bug — proof that - materializing a policy photocopy without a live join cannot enforce - grant-level TTL/usage. Its schema is still the best reference for the - token/policy decomposition (minus the one `applyToken` materialization line). -- **FROSTR** has the cleanest *revocation decomposition* (3 independent layers) - and a good auditable-credential table — but enforces **no** live expiry - anywhere. -- **promenade** confirms the **revoke = re-key** anti-pattern to avoid, and - debunks "FROST can't decrypt DMs" (it's a design choice, not a math limit). -- **NDK** (which we embed) is a deliberately *blank* permit seam: we own 100% of - policy — and `get_public_key` bypasses the seam entirely (see #26). - -Decision unchanged: **Option D, leaning D1.** Amber = live-evaluation reference; -Signet = schema reference; FROSTR = revocation-decomposition reference; NDK = -confirmed blank seam. - -## Strategic decision: keep our fork, treat Signet as a parts donor (2026-06-19) - -Signet is a fork/re-architecture of the same kind-0/nsecbunkerd lineage we -maintain, and is feature-richer on the standalone-operator surface (trust dial, -suspension, NIP-49 at-rest, two-tier tokens, kill-switch, React dashboard, -Android companion). We considered adopting it wholesale. **Decision: no — keep -our fork as what we ship; lift Signet's patterns as needed.** - -Why: -- **Replacing doesn't solve #25.** Signet re-ships our exact #24 (materialized - photocopy, no live grant-level join). We'd still have to do the live-join work - — after paying a migration cost. -- **We'd lose the integration that makes it ours.** LNbits wallet provisioning - (`usermanager` + nostdress), the OAuth-like `create_account` flow, and being - the signer target for the #18 `RemoteBunkerSigner` endgame. Porting those into - Signet just means maintaining a fork of a more opinionated upstream. -- **Lineage/bus-factor.** Our `master` tracks the canonical kind-0 upstream; - Signet is a solo-maintainer rewrite with choices we may not want (removed JWT - auth, Android surface). For a security-load-bearing component that's more risk, - not less. - -Why it's low-stakes either way: LNbits ↔ bunker is **NIP-46 over the wire** (the -deliberate protocol-over-IPC choice), so the signer is substitutable by design. -If our fork ever becomes a maintenance burden we can drop in any conformant -NIP-46 signer (Signet, Amber-as-bunker, HSM-backed) with config-only changes — -**not a one-way door.** - -Escape hatch (option 3, parked): run Signet unmodified behind the protocol. Only -attractive if the LNbits provisioning/OAuth flows move out of the bunker into -LNbits proper (plausible under #18), which would shrink the integration gap -that's the main reason to stay. Revisit if #25 implementation reveals our -daemon's NDK/relay/ACL plumbing is materially rougher than Signet's. - ---- - -## A — daemon/server implementations with a real policy model - -### Signet — `Letdown2491/signet` (TS daemon + React UI + Kotlin companion) -MIT, very active (v1.11.0, 2026-06). An extensive re-architecture of the same -kind-0/nsecbunkerd codebase we maintain. - -- **Re-ships our #24.** `applyToken` (`nip46-backend.ts:807`) checks - `Token.expiresAt` once at redeem (`:895`), then materializes `policy.rules` - into lifecycle-free `SigningCondition` rows (`:845-862`); the sign-time path - (`acl.ts:checkRequestPermission`) never reads `Token` again. - `maxUsageCount`/`currentUsageCount` are touched only in the policy CRUD route — - never enforced. Same materialization-drift bug as ours. -- **What it adds over us:** a coarse-cache layer for **subject-level** state on - `KeyUser` — `revokedAt`, `suspendedAt`/`suspendUntil`, `trustLevel` — read live - per request and invalidated on change (`invalidateAclCache`). Genuinely fixes - live *revoke* (our sibling spirekeeper#22). Puts revoke on **`KeyUser`, not - `Token`** — corroborating our revoke=subject / expiry=grant split. -- **Trust dial** over a kind-risk classifier: `trustLevel ∈ {paranoid, - reasonable, full}`, `SAFE_KINDS` auto / `SENSITIVE_KINDS` (0/3/4/5/wallet/ - auth/NIP-04) forced manual (`acl.ts:129-161`). -- **Two-tier tokens:** one-time `ConnectionToken` (mandatory `expiresAt`, - validates connect but never auto-approves) vs policy-backed `Token` (atomic - claim `updateMany where redeemedAt:null`, `nip46-backend.ts:813`). -- **Key-at-rest:** NIP-49 ncryptsec + AES-256-GCM envelope (PBKDF2-SHA256 @600k). -- **Takeaway:** adopt its `KeyUser` subject-state + `Request` indexing; reject - its `applyToken` materialization; the `ConnectionToken`-vs-`Token` split *is* - D1 in schema form. - -### Amber — `greenart7c3/Amber` (Android, Kotlin/Room) ⭐ live-lifecycle reference -MIT, very active (last commit 2026-06-19). Android signer (NIP-55 intents **and** -NIP-46 over relays). Listed in tier A despite being mobile because its permission -model is the strongest of any surveyed. - -- **Grant schema** (`ApplicationPermissionsEntity.kt:18-41`): unique composite - index over `(pkKey, type, kind, relay)` — per-(app × method × kind × relay). - Columns include `acceptable: Boolean`, `rememberType: Int`, `acceptUntil: - Long`, `rejectUntil: Long`. -- **Expiry enforced LIVE** (the key finding): `IntentUtils.isRemembered()` - (`IntentUtils.kt:1087-1101`) is the per-request verdict and recomputes - `acceptUntil > TimeUtils.now()` / `rejectUntil > now()` fresh every call; - expired → returns `null` → falls through to a user prompt. Called on both the - NIP-46 relay path (`EventNotificationConsumer.kt:440-441`) and the NIP-55 - intent path (`SignerProviderQuery.kt:183` etc.). -- **The sweep is non-load-bearing.** `updateExpiredPermissions(time)` - (`ApplicationDao.kt:51`, exempts `rememberType <> 4`=ALWAYS) runs every 24h via - WorkManager — pure cleanup; correctness doesn't depend on it firing because the - decision is recomputed against `now()` on read. -- **Time-boxed denials too:** `rejectUntil` means "reject for 5 min" decays back - to a prompt rather than a permanent no — a nicer primitive than a single - allow/deny flag. -- **Wildcard-as-distinct-tier:** lookup ladder is exact-kind → all-kinds - (`kind IS NULL`, `getPermissionAllKinds`, `ApplicationDao.kt:87-91`); relay - wildcard matches `'*' OR '' OR NULL` in one query (`getWildcardRelayPermission`, - `:101-106`). Wildcard rows are explicitly queried, never an accidental - missing-WHERE match. -- **Read-through LRU caches rows, not verdicts** (`CachingApplicationDao`) — keeps - the live `now()` re-check on every cache hit; invalidation is write-driven and - coarse per-app. -- **Sign policies** (`ChooseSignPolicy.kt:32-45`, stored as `signPolicy: Int`): - `0` basic / `1` manual-per-new-app / `2` fully-auto (short-circuits to allow - before any row lookup, `IntentUtils.kt:1090`). -- **Key-at-rest** (`SecureCryptoHelper.kt`): Android Keystore AES-256-GCM, 96-bit - IV / 128-bit tag, StrongBox-backed when available with TEE fallback and a - MediaTek denylist; optional app-level biometric gate. -- **NIP-46 coverage** (`SignerType.kt`, `BunkerRequestUtils.kt:232-248`): connect, - sign_event, nip04/nip44 (+v3) encrypt/decrypt, get_public_key, - decrypt_zap_event, ping, switch_relays, sign_psbt, logout; both `bunker://` and - `nostrconnect://`. -- **Steal for us:** absolute-deadline-on-row + recompute-vs-now per request; - time-boxed denials; wildcard as a distinct explicitly-queried tier; cache rows - not answers. - -### FROSTR — `FROSTR-ORG/igloo-server` + `bifrost` (TS, FROST k-of-n) -MIT. igloo-server v1.2.0 (2026-05-28); bifrost v2.0.2 (2026-01-24). Threshold -Schnorr over Nostr; igloo-server exposes the NIP-46 endpoint, bifrost is the node -SDK. - -- **Three independent authorization layers** (the prize): - 1. **App NIP-46 policy** — `Nip46Policy { methods?, kinds? }` (`db/nip46.ts:8-11`), - sessions keyed `(user_id, client_pubkey)` (`:92`), checked live per request - (`service.ts:508-509, 766-795`). No TTL/expiry. Session revoke is **explicit** - (`status='revoked'`, `:792-826`); per-method/kind revoke is **implicit** (flip - boolean false, audited at `:722-790`). - 2. **Peer-transport policy** — per-peer directional `allowSend`/`allowReceive` - (`util/peer-policy.ts:3-9`, `docs/PEER_POLICIES.md`), enforced in bifrost - `_filter`/`get_recv_pubkeys` (`client.ts:226-245`). **Correction:** it's - *default-allow + explicit per-peer deny + last-layer-wins*, not "deny-override". - 3. **Operator API auth** — keys stored SHA-256 hash+prefix with `revoked_at` - (checked first, timing-safe) + `last_used_at/ip` (`migrations/..._api_keys.sql`, - `database.ts:815-1047`); Argon2id password hashing (`config/crypto.ts:26-31`). -- **No layer enforces live expiry.** `nip46_requests.expires_at` exists but is - never populated; the only time-based enforcement is the in-memory derived-key - vault (TTL + bounded reads + zeroize, `auth.ts:359-459`). -- **Key-at-rest:** DB mode AES-256-GCM in SQLite, PBKDF2-HMAC-SHA256 **@600k** - (corrected from "~200k", `config/crypto.ts:7-11`); headless mode = plaintext env - (`GROUP_CRED`/`SHARE_CRED`). -- **Distributed veto** is real at the participation level (a co-signer withholding - its partial below threshold blocks the sig) but the default signer auto-signs - (`middleware: {}`, `client.ts:55`) — realizing a veto needs a custom - `middleware.sign` not shipped by default. -- **Share rotation** (recover → re-split, same group npub, old shares can't - combine) exists as a **bifrost SDK primitive** (`generate_dealer_package`), **not** - as an igloo-server endpoint; recovery reconstructs the full nsec in memory and - `/api/recover` even returns it over HTTP (`routes/recovery.ts:147-157`). -- **Steal for us:** the 3-layer revocation decomposition; audit-event-on-grant- - change; `revoked_at`-checked-first + last-used credential table. - -### promenade — fiatjaf (Go, FROST coordinator + signer split) -Off GitHub; cloned from fiatjaf's nostr-git (`relay.ngit.dev/npub180c…/promenade.git`), -HEAD `70ff8439` 2026-06-18. NIP-46 method logic lives in the pinned dep -`fiatjaf.com/nostr` (`nip46.DynamicSigner`). - -- **Architecture:** khatru coordinator-relay doubles as the NIP-46 endpoint, runs - the FROST ceremony, holds a transport/handler key but **no shard** - (`account_registration.go:44` carries only `frost.PublicKeyShard`); separate - signer daemons each hold one shard; m-of-n with m≤20 (`:79`). Signing-ceremony - kinds 26430–26434; account registration is kind **16430** (replaceable). -- **No encrypted DMs — by choice, not by math.** `DynamicSigner` recognizes - `nip44_encrypt`/`nip44_decrypt`/`switch_relays` (`dynamic-signer.go`), but - promenade hardwires `AuthorizeEncryption → false` (`coordinator/nip46.go:167`) - and `GroupContext.Encrypt/Decrypt → "not implemented"` (`sign.go:288-302`). - README: *"destroyer of encryption."* **Correction:** threshold ECDH is NOT - impossible for FROST — `frost/ecdh.go` implements `CreateECDHShare` / - `AggregateECDHShards`; it's simply not plumbed in. -- **ACL:** `AuthorizeSigning` per sign_event (`coordinator/nip46.go:86`); named - profiles `["profile", name, secret, restrictions]` where restrictions is a - `nostr.Filter` but only `Kinds` + `Until` are enforced (`:139-159`). The secret - is a reusable bearer capability. -- **Lifecycle:** per-profile `Until` is the only time-bound; **no revoke API** — - dropping one capability means re-publishing the whole kind:16430 account signed - by the **master nsec**. The **revoke = re-key anti-pattern** to avoid. -- **Key-at-rest:** nsec sharded client-side (never whole), but shards stored - **plaintext** in each signer's BoltDB (`acceptor.go:209`); coordinator/signer - identity keys from plaintext env. -- **Relevance:** confirms (1) keep grant-revoke independent of key rotation, and - (2) for the #18 "bunker for everything" endgame, threshold-protecting the server - identity wouldn't *mathematically* preclude DM decryption — but keeping ECDH on a - separate non-threshold key is the cheaper path. - ---- - -## B — library/SDK signer seams - -### NDK — `nostr-dev-kit/ndk` (we embed this) @ `4b86acd` (2026-04-05) -nip46 under `core/src/signers/nip46/`. - -- Backend `NDKNip46Backend` (`backend/index.ts:58`), client `NDKNip46Signer` - (`index.ts:60`). -- Permit seam: `Nip46PermitCallback = (params: {id, pubkey, method, params?}) => - Promise` (`backend/index.ts:29-43`), invoked via overridable - `pubkeyAllowed()` (`:229-231`) from each strategy. -- **`get_public_key` bypasses the seam** — `backend/get-public-key.ts:3-11` - returns the pubkey with no `pubkeyAllowed` call. (rust-nostr's `approve()` wraps - every method including this one.) See #26. -- Signature verified before dispatch (`index.ts:181`); strategies swappable - (`setStrategy`, `:156-158`). -- `applyToken(pubkey, token)` default-throws (`:166-168`), invoked by the connect - handler when a token is present (`connect.ts:21-24`) — token policy is the - embedder's job. -- **No** built-in scoping/kinds/rate-limit/expiry/persistence — all policy lives - behind the one callback. We own 100% of the policy engine. - -### rust-nostr / nostr-sdk @ `e47b572` (v0.45.0-alpha.1) -- `NostrConnectRemoteSigner` (`signer.rs:39`) + `NostrConnect` client. -- Trait `NostrConnectSignerActions::approve(&self, public_key, req) -> bool` - (`signer.rs:342-345`), synchronous bool, wraps the **entire** request match in - `serve()` (`:201-202`) — gates every method **including** `get_public_key`. -- FFI (uniffi/wasm) exposes **only the client** `NostrConnect`, not the backend — - no non-Rust embedding of the signer side. - -### nak — `fiatjaf/nak` bunker subcommand @ `483bf94` -- Allow-list of client pubkeys (`BunkerConfig.Clients`), `--persist`s 0600 JSON. -- Once authorized, **signs everything** — no method/kind scoping, no expiry, no - rate limiting. Notably its underlying lib computes a `harmless` (connect/ - get_public_key/ping) vs dangerous (sign/encrypt/decrypt) hint that nak - **discards**. A bare always-sign baseline. - ---- - -## C — clients / extensions (less relevant; novel UX only) - -- **keys.band** — Svelte Chrome extension (NIP-07): the one browser signer with - *time-bounded* authorization grants (allow-for-N-minutes/session). Relevant to a - TTL-grant UX. -- **nos2x / nos2x-fox** (fiatjaf) — origin of the per-origin "remember / allow - this site" NIP-07 model; key stored ~plaintext in extension storage. -- **Gossip** (Rust desktop) — not a bunker, but best-in-class key-at-rest: - passphrase-encrypted on disk, startup unlock, memory zeroed before free. Clean - `LocalSigner` envelope reference. -- **Primal**, **nowser** (Flutter) — clients that also serve NIP-46/NIP-55; use the - standard `optional_requested_perms` per-method/per-kind grammar. - ---- - -## D — not bunkers / dead - -- **`Letdown2491/nip46-relay`** — a NIP-46 *transport relay* (forwards opaque - blobs), no signing/authz. Appears next to Signet; easy to mistake for a signer. -- **Keychat** — Signal-over-Nostr chat app; signs only its own events. -- **python-nostr** — abandoned 2022, no NIP-46. (No Python library offers a - signer-side permission abstraction; a Python bunker means hand-rolling the - kind-24133 loop or driving rust-nostr via FFI — and the FFI exposes only the - client.) - ---- - -## Patterns worth stealing — consolidated - -1. **Live evaluation (Amber):** absolute deadline on the grant row; verdict is a - pure function recomputed vs `now()` per request; sweep is cleanup-only. This is - Option D, proven in production. -2. **Time-box denials too (Amber `rejectUntil`):** a deny decays to a prompt. -3. **Wildcard as a distinct, explicitly-queried tier (Amber):** never a fuzzy - missing-WHERE match in the auto-decide path. -4. **Cache rows, never verdicts (Amber `CachingApplicationDao`, Signet coarse - cache):** keep the `now()` re-check on every hit; invalidate on write. -5. **Subject vs grant separation (Signet):** revoke/suspend/trust on `KeyUser` - (cheap, cache+invalidate); expiry/usage on `Token`/`Policy` (must join live). -6. **Usage = COUNT(Request) in window (lnbits/nostr_bunker), not a mutable - counter:** drop `currentUsageCount`; needs `Request.keyUserId` + index. -7. **Revocation decomposition (FROSTR):** app-grant revoke ≠ transport quarantine ≠ - key rotation. Never collapse grant-revoke into re-key (promenade anti-pattern). -8. **Auditable, revocable credentials (FROSTR):** `revoked_at` checked first + - last-used tracking; audit-event-on-grant-change decoupled from enforcement. -9. **Single predicate `grantIsLive(now)`** used at both redeem and sign time - (the discipline that prevents the original drift). -10. **NDK seam reality:** we own all policy; design around `get_public_key` - bypassing `pubkeyAllowed`. diff --git a/package.json b/package.json index f1f8732..4975b68 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,6 @@ "scripts": { "build": "tsup src/index.ts; tsup src/daemon/index.ts -d dist/daemon; tsup src/client.ts -d dist/client", "build:client": "tsup src/client.ts -d dist/client", - "test": "TS_NODE_TRANSPILE_ONLY=1 node -r ts-node/register --test tests/*.test.ts", "prisma:generate": "npx prisma generate", "prisma:migrate": "npx prisma migrate deploy", "prisma:create": "npx prisma db push --preview-feature", diff --git a/prisma/migrations/20260619125847_live_grant_lifecycle_schema/migration.sql b/prisma/migrations/20260619125847_live_grant_lifecycle_schema/migration.sql deleted file mode 100644 index 7608e6e..0000000 --- a/prisma/migrations/20260619125847_live_grant_lifecycle_schema/migration.sql +++ /dev/null @@ -1,37 +0,0 @@ --- RedefineTables -PRAGMA defer_foreign_keys=ON; -PRAGMA foreign_keys=OFF; -CREATE TABLE "new_Request" ( - "id" TEXT NOT NULL PRIMARY KEY, - "keyName" TEXT, - "createdAt" DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, - "requestId" TEXT NOT NULL, - "remotePubkey" TEXT NOT NULL, - "method" TEXT NOT NULL, - "params" TEXT, - "allowed" BOOLEAN, - "keyUserId" INTEGER, - CONSTRAINT "Request_keyUserId_fkey" FOREIGN KEY ("keyUserId") REFERENCES "KeyUser" ("id") ON DELETE SET NULL ON UPDATE CASCADE -); -INSERT INTO "new_Request" ("allowed", "createdAt", "id", "keyName", "method", "params", "remotePubkey", "requestId") SELECT "allowed", "createdAt", "id", "keyName", "method", "params", "remotePubkey", "requestId" FROM "Request"; -DROP TABLE "Request"; -ALTER TABLE "new_Request" RENAME TO "Request"; -CREATE INDEX "Request_keyUserId_method_idx" ON "Request"("keyUserId", "method"); -CREATE TABLE "new_SigningCondition" ( - "id" INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, - "method" TEXT, - "kind" TEXT, - "content" TEXT, - "keyUserKeyName" TEXT, - "allowed" BOOLEAN, - "keyUserId" INTEGER, - "createdAt" DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, - "expiresAt" DATETIME, - "revokedAt" DATETIME, - CONSTRAINT "SigningCondition_keyUserId_fkey" FOREIGN KEY ("keyUserId") REFERENCES "KeyUser" ("id") ON DELETE SET NULL ON UPDATE CASCADE -); -INSERT INTO "new_SigningCondition" ("allowed", "content", "id", "keyUserId", "keyUserKeyName", "kind", "method") SELECT "allowed", "content", "id", "keyUserId", "keyUserKeyName", "kind", "method" FROM "SigningCondition"; -DROP TABLE "SigningCondition"; -ALTER TABLE "new_SigningCondition" RENAME TO "SigningCondition"; -PRAGMA foreign_keys=ON; -PRAGMA defer_foreign_keys=OFF; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 73f54df..f0072ea 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -17,14 +17,6 @@ model Request { method String params String? allowed Boolean? - // Bind each request to the KeyUser it was evaluated against so usage - // caps can be derived live by COUNTing allowed Requests, instead of - // maintaining a mutable PolicyRule.currentUsageCount that drifts. - // See aiolabs/nsecbunkerd#25 (Option D, derive-don't-count). - keyUserId Int? - KeyUser KeyUser? @relation(fields: [keyUserId], references: [id]) - - @@index([keyUserId, method]) } model KeyUser { @@ -39,7 +31,6 @@ model KeyUser { logs Log[] signingConditions SigningCondition[] Token Token[] - requests Request[] @@unique([keyName, userPubkey], name: "unique_key_user") } @@ -65,25 +56,15 @@ model User { pubkey String } -// The SigningCondition layer is the MANUAL-OVERRIDE source of truth -// (web-approval / add_signing_condition / create_account bootstrap) — it is -// no longer materialized from token policies (see aiolabs/nsecbunkerd#25: -// applyToken stopped photocopying; token grants are evaluated live off -// Token -> Policy -> PolicyRule). Under D1 the override layer carries its -// own lifecycle so it runs through the same grantIsLive(now) predicate as -// token grants. model SigningCondition { - id Int @id @default(autoincrement()) + id Int @id @default(autoincrement()) method String? kind String? content String? keyUserKeyName String? allowed Boolean? keyUserId Int? - createdAt DateTime @default(now()) - expiresAt DateTime? - revokedAt DateTime? - KeyUser KeyUser? @relation(fields: [keyUserId], references: [id]) + KeyUser KeyUser? @relation(fields: [keyUserId], references: [id]) } model Log { diff --git a/src/daemon/backend/index.ts b/src/daemon/backend/index.ts index 91f2f58..5d05a8a 100644 --- a/src/daemon/backend/index.ts +++ b/src/daemon/backend/index.ts @@ -1,7 +1,6 @@ import NDK, { NDKNip46Backend, NDKPrivateKeySigner, Nip46PermitCallback } from '@nostr-dev-kit/ndk'; import prisma from '../../db.js'; import type {FastifyInstance} from "fastify"; -import { grantIsLive } from '../lib/acl/index.js'; export class Backend extends NDKNip46Backend { public baseUrl?: string; @@ -92,10 +91,7 @@ export class Backend extends NDKNip46Backend { if (!tokenRecord) throw new Error("Token not found"); if (tokenRecord.redeemedAt) throw new Error("Token already redeemed"); if (!tokenRecord.policy) throw new Error("Policy not found"); - // Revoke + expiry via the single grantIsLive predicate — the exact - // check the sign-time ACL uses, so redeem-time and sign-time cannot - // drift (the root of #24). See aiolabs/nsecbunkerd#25. - if (!grantIsLive(tokenRecord)) throw new Error("Token expired or revoked"); + if (tokenRecord.expiresAt && tokenRecord.expiresAt < new Date()) throw new Error("Token expired"); return tokenRecord; } @@ -104,20 +100,39 @@ export class Backend extends NDKNip46Backend { const tokenRecord = await this.validateToken(token); const keyName = tokenRecord.keyName; - // Record ONLY the binding (KeyUser <- Token). Under #25 the token's - // policy is evaluated live at sign time (checkIfPubkeyAllowed step 4) - // off Token -> Policy -> PolicyRule, NOT photocopied into - // SigningCondition rows here. That photocopy was the root of #24: the - // copy carried no expiry/revoke and short-circuited the live check, so - // an expired or revoked token kept signing forever. With no copy, the - // token's lifecycle is re-checked on every request and there is nothing - // to keep in sync. + // Upsert the KeyUser with the given remotePubkey const upsertedUser = await prisma.keyUser.upsert({ where: { unique_key_user: { keyName, userPubkey } }, update: { }, create: { keyName, userPubkey, description: tokenRecord.clientName }, }); + await prisma.signingCondition.create({ + data: { + keyUserId: upsertedUser.id, + method: 'connect', + allowed: true, + } + }); + + // Go through the rules of this policy and apply them to the user + for (const rule of tokenRecord!.policy!.rules) { + const signingConditionQuery: any = { method: rule.method }; + + if (rule && rule.kind) { + signingConditionQuery.kind = rule.kind.toString(); + } + + await prisma.signingCondition.create({ + data: { + keyUserId: upsertedUser.id, + method: rule.method, + allowed: true, + ...signingConditionQuery, + } + }); + } + await prisma.token.update({ where: { id: tokenRecord.id }, data: { diff --git a/src/daemon/lib/acl/index.ts b/src/daemon/lib/acl/index.ts index d693e72..f621c98 100644 --- a/src/daemon/lib/acl/index.ts +++ b/src/daemon/lib/acl/index.ts @@ -1,45 +1,31 @@ -import { NostrEvent, NIP46Method } from '@nostr-dev-kit/ndk'; +import { NDKEvent, NostrEvent, NIP46Method } from '@nostr-dev-kit/ndk'; import prisma from '../../../db.js'; -import { liveWhere } from './lifecycle.js'; - -// Re-export the single lifecycle predicate so callers (e.g. -// Backend.validateToken) import it from the ACL module. The implementation -// lives in ./lifecycle.ts so it can be unit-tested without a database. -export { grantIsLive } from './lifecycle.js'; /** - * Layered authorization check. Order matters (denials beat grants): + * Layered authorization check. Order matters: * * 1. fetch KeyUser; if missing → undefined (no binding exists) - * 2. KeyUser.revokedAt set → false (subject-level ban beats everything) - * 3. manual-override layer (LIVE SigningConditions only): - * - live explicit reject (method='*', allowed=false) → false - * - live matching per-(method,kind) deny → false - * - live matching per-(method,kind) grant → true - * 4. live token grant: a redeemed Token bound to this KeyUser that is - * neither revoked nor expired pairs the user (`connect`) outright and, - * via its policy, governs signing. Token expiry/revoke are evaluated - * HERE, every request — not photocopied at redeem (#24). - * 5. else → undefined (caller's requestPermission flow may prompt an admin) + * 2. if KeyUser.revokedAt set → false (binary user revoke beats everything) + * 3. SigningCondition override layer (per-user grants/denies): + * - explicit reject (method='*', allowed=false) → false + * - matching per-(method,kind) row → return row.allowed + * 4. Live policy join over KeyUser → Token → Policy → PolicyRule + * with Token.revokedAt IS NULL and a matching rule → true + * 5. else → undefined (denied) * - * Unlike the pre-#25 algorithm, token grants are no longer materialized into - * SigningCondition rows at redeem (Backend.applyToken stopped photocopying), - * so step 4 is the live source of truth for token lifecycle. The override - * layer (step 3) is manual-only and now carries its own lifecycle, so an - * expired/revoked override stops granting too. + * Step 3 must precede step 4: per-user denies override the policy, and + * per-user grants extend beyond the policy. Step 2 must precede step 3: + * a revoked KeyUser stays revoked regardless of conditions or policy. * - * Supersedes the #11 algorithm; closes the materialization-drift family - * behind #24. See aiolabs/nsecbunkerd#25. + * See aiolabs/nsecbunkerd#11 and the issue comment that ratified the + * algorithm (https://git.atitlan.io/aiolabs/nsecbunkerd/issues/11#issuecomment-1473). */ export async function checkIfPubkeyAllowed( keyName: string, remotePubkey: string, method: IMethod, - payload?: string | NostrEvent, + payload?: string | NostrEvent ): Promise { - // One clock reading for the whole decision. - const now = new Date(); - // Step 1: find KeyUser. const keyUser = await prisma.keyUser.findUnique({ where: { unique_key_user: { keyName, userPubkey: remotePubkey } }, @@ -49,95 +35,81 @@ export async function checkIfPubkeyAllowed( return undefined; } - // Step 2: subject-level revoke (sticky ban, beats everything). + // Step 2: binary user revoke. if (keyUser.revokedAt) { return false; } - const live = liveWhere(now); - - // Step 3a: live explicit reject. + // Step 3a: explicit-reject override (rejectAllRequestsFromKey writes this). const explicitReject = await prisma.signingCondition.findFirst({ - where: { keyUserId: keyUser.id, method: '*', allowed: false, ...live }, + where: { + keyUserId: keyUser.id, + method: '*', + allowed: false, + } }); if (explicitReject) { + console.log(`explicit reject`, explicitReject); return false; } - // Step 3b: live matching per-(method, kind) override — deny beats grant. + // Step 3b: matching per-(method, kind) override. const signingConditionQuery = requestToSigningConditionQuery(method, payload); - const liveDeny = await prisma.signingCondition.findFirst({ - where: { keyUserId: keyUser.id, ...signingConditionQuery, allowed: false, ...live }, + const signingCondition = await prisma.signingCondition.findFirst({ + where: { + keyUserId: keyUser.id, + ...signingConditionQuery, + } }); - if (liveDeny) { - return false; + if (signingCondition && (signingCondition.allowed === true || signingCondition.allowed === false)) { + console.log(`found signing condition`, signingCondition); + return signingCondition.allowed; } - const liveGrant = await prisma.signingCondition.findFirst({ - where: { keyUserId: keyUser.id, ...signingConditionQuery, allowed: true, ...live }, - }); - - if (liveGrant) { - return true; - } - - // Step 4: live token grant. + // Step 4: live policy join. Walk every non-revoked Token bound to this + // KeyUser; if any of their policies has a matching PolicyRule, allow. // - // A redeemed token that is live (not revoked, not past expiry) grants - // `connect` (the pairing) outright, and grants other methods when its - // policy has a matching PolicyRule. The live filter is what closes #24: - // an expired or revoked token simply stops matching here, every request, - // with no photocopy to outlive it. - if (method === 'connect') { - const liveToken = await prisma.token.findFirst({ - where: { keyUserId: keyUser.id, ...live }, - }); + // PolicyRule.kind matching: + // - exact match against payload kind (stringified — matches the + // create_new_policy.ts:23 storage format `rule.kind.toString()`) + // - 'all' literal matches any kind (parity with the override-layer + // allowScopeToSigningConditionQuery convention) + // - NULL kind is a defensive branch — no current code path inserts + // PolicyRules with null kind, but if one ever appears (raw SQL, + // future code, schema migration) we treat it as a wildcard rather + // than failing closed silently. + const payloadKindString = (method === 'sign_event' && typeof payload === 'object' && payload?.kind !== undefined) + ? payload.kind.toString() + : undefined; - if (liveToken) { - return true; - } - } else { - // PolicyRule.kind matching: - // - exact match against the stringified payload kind (matches the - // create_new_policy.ts storage format `rule.kind.toString()`) - // - 'all' literal matches any kind - // - NULL kind is a defensive wildcard — no current writer emits a - // null-kind rule, but treat it as a wildcard rather than failing - // closed silently if one ever appears (raw SQL, future code). - const payloadKindString = - method === 'sign_event' && typeof payload === 'object' && payload?.kind !== undefined - ? payload.kind.toString() - : undefined; + const kindMatchers: Array<{ kind: string | null }> = [{ kind: null }, { kind: 'all' }]; + if (payloadKindString !== undefined) { + kindMatchers.push({ kind: payloadKindString }); + } - const kindMatchers: Array<{ kind: string | null }> = [{ kind: null }, { kind: 'all' }]; - if (payloadKindString !== undefined) { - kindMatchers.push({ kind: payloadKindString }); - } - - const policyAllowance = await prisma.token.findFirst({ - where: { - keyUserId: keyUser.id, - ...live, - policy: { - rules: { - some: { - method, - OR: kindMatchers, - }, + const policyAllowance = await prisma.token.findFirst({ + where: { + keyUserId: keyUser.id, + revokedAt: null, + policy: { + rules: { + some: { + method, + OR: kindMatchers, }, }, }, - }); + }, + }); - if (policyAllowance) { - return true; - } + if (policyAllowance) { + return true; } - // Step 5: no live override and no live token grant matched. Caller's + // Step 5: no override granted, no policy rule matched. Caller's // requestPermission flow may still prompt the admin out-of-band. return undefined; } @@ -239,4 +211,4 @@ export async function rejectAllRequestsFromKey(remotePubkey: string, keyName: st keyUserId: upsertedUser.id, }, }); -} +} \ No newline at end of file diff --git a/src/daemon/lib/acl/lifecycle.ts b/src/daemon/lib/acl/lifecycle.ts deleted file mode 100644 index 783dd84..0000000 --- a/src/daemon/lib/acl/lifecycle.ts +++ /dev/null @@ -1,40 +0,0 @@ -/** - * Pure grant-lifecycle logic, extracted from the ACL so it can be unit-tested - * without a database and reused verbatim at redeem time and sign time. - * - * The original #24 bug was possible because redeem-time checked expiry and - * sign-time didn't — two definitions of "valid" that drifted. Defining "is - * this grant valid right now?" exactly once makes them impossible to disagree. - * See aiolabs/nsecbunkerd#25. - */ - -/** The lifecycle fields every grant (Token, SigningCondition) carries. */ -export type Lifecycle = { - revokedAt?: Date | null; - expiresAt?: Date | null; -}; - -/** - * "Is this grant valid right now?" — the single lifecycle predicate. A grant - * is live iff it has not been revoked and its expiry (if any) is still in the - * future. Expiry is treated as exclusive at the boundary: a grant whose - * `expiresAt` equals `now` is already dead. - */ -export function grantIsLive(grant: Lifecycle, now: Date = new Date()): boolean { - if (grant.revokedAt) return false; - if (grant.expiresAt && grant.expiresAt.getTime() <= now.getTime()) return false; - return true; -} - -/** - * `grantIsLive` expressed as a Prisma `where` fragment, so the live filter - * runs in the query rather than in app code after the fetch. `now` is threaded - * in explicitly so a single request evaluates every row against one clock - * reading. Kept in lockstep with `grantIsLive` (see lifecycle.test.ts). - */ -export function liveWhere(now: Date) { - return { - revokedAt: null, - OR: [{ expiresAt: null }, { expiresAt: { gt: now } }], - }; -} diff --git a/tests/lifecycle.test.ts b/tests/lifecycle.test.ts deleted file mode 100644 index 4bbe5f1..0000000 --- a/tests/lifecycle.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { test } from 'node:test'; -import assert from 'node:assert/strict'; -import { grantIsLive, liveWhere } from '../src/daemon/lib/acl/lifecycle'; - -// Fixed reference clock so the assertions don't depend on wall time. -const now = new Date('2026-06-19T12:00:00.000Z'); -const past = new Date(now.getTime() - 60_000); -const future = new Date(now.getTime() + 60_000); - -test('grantIsLive: no revoke, no expiry -> live', () => { - assert.equal(grantIsLive({}, now), true); - assert.equal(grantIsLive({ revokedAt: null, expiresAt: null }, now), true); -}); - -test('grantIsLive: future expiry -> live', () => { - assert.equal(grantIsLive({ expiresAt: future }, now), true); -}); - -test('grantIsLive: past expiry -> dead (the #24 case the old code missed at sign time)', () => { - assert.equal(grantIsLive({ expiresAt: past }, now), false); -}); - -test('grantIsLive: expiry exactly now -> dead (boundary is exclusive)', () => { - assert.equal(grantIsLive({ expiresAt: new Date(now.getTime()) }, now), false); -}); - -test('grantIsLive: revoked -> dead even with a future expiry (revoke wins)', () => { - assert.equal(grantIsLive({ revokedAt: past, expiresAt: future }, now), false); -}); - -test('grantIsLive: defaults now to the current time', () => { - assert.equal(grantIsLive({ expiresAt: new Date(Date.now() + 3_600_000) }), true); - assert.equal(grantIsLive({ expiresAt: new Date(Date.now() - 3_600_000) }), false); -}); - -// liveWhere is the SQL mirror of grantIsLive; pin its shape so the two -// can't silently drift (a drift would re-open the redeem-vs-sign gap #25 -// exists to close). -test('liveWhere: mirrors grantIsLive as a prisma where-fragment', () => { - assert.deepEqual(liveWhere(now), { - revokedAt: null, - OR: [{ expiresAt: null }, { expiresAt: { gt: now } }], - }); -});