diff --git a/docs/COMPARISON-lnbits-nostr_bunker.md b/docs/COMPARISON-lnbits-nostr_bunker.md new file mode 100644 index 0000000..5aa6d77 --- /dev/null +++ b/docs/COMPARISON-lnbits-nostr_bunker.md @@ -0,0 +1,108 @@ +# 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 new file mode 100644 index 0000000..01a9f57 --- /dev/null +++ b/docs/acl-prior-art-survey.md @@ -0,0 +1,297 @@ +# 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 4975b68..f1f8732 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "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 new file mode 100644 index 0000000..7608e6e --- /dev/null +++ b/prisma/migrations/20260619125847_live_grant_lifecycle_schema/migration.sql @@ -0,0 +1,37 @@ +-- 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 f0072ea..73f54df 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -17,6 +17,14 @@ 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 { @@ -31,6 +39,7 @@ model KeyUser { logs Log[] signingConditions SigningCondition[] Token Token[] + requests Request[] @@unique([keyName, userPubkey], name: "unique_key_user") } @@ -56,15 +65,25 @@ 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? - KeyUser KeyUser? @relation(fields: [keyUserId], references: [id]) + createdAt DateTime @default(now()) + expiresAt DateTime? + revokedAt DateTime? + KeyUser KeyUser? @relation(fields: [keyUserId], references: [id]) } model Log { diff --git a/src/daemon/backend/index.ts b/src/daemon/backend/index.ts index 5d05a8a..91f2f58 100644 --- a/src/daemon/backend/index.ts +++ b/src/daemon/backend/index.ts @@ -1,6 +1,7 @@ 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; @@ -91,7 +92,10 @@ 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"); - if (tokenRecord.expiresAt && tokenRecord.expiresAt < new Date()) throw new Error("Token expired"); + // 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"); return tokenRecord; } @@ -100,39 +104,20 @@ export class Backend extends NDKNip46Backend { const tokenRecord = await this.validateToken(token); const keyName = tokenRecord.keyName; - // Upsert the KeyUser with the given remotePubkey + // 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. 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 f621c98..1c7912d 100644 --- a/src/daemon/lib/acl/index.ts +++ b/src/daemon/lib/acl/index.ts @@ -1,31 +1,44 @@ -import { NDKEvent, NostrEvent, NIP46Method } from '@nostr-dev-kit/ndk'; +import { 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: + * Layered authorization check. Order matters (denials beat grants): * * 1. fetch KeyUser; if missing → undefined (no binding exists) - * 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) + * 2. KeyUser.revokedAt set → false (subject-level ban beats everything) + * 3. manual-override layer (LIVE SigningConditions only): + * - 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) * - * 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. + * 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. * - * See aiolabs/nsecbunkerd#11 and the issue comment that ratified the - * algorithm (https://git.atitlan.io/aiolabs/nsecbunkerd/issues/11#issuecomment-1473). + * Supersedes the #11 algorithm; closes the materialization-drift family + * behind #24. See aiolabs/nsecbunkerd#25. */ 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 } }, @@ -35,81 +48,89 @@ export async function checkIfPubkeyAllowed( return undefined; } - // Step 2: binary user revoke. + // Step 2: subject-level revoke (sticky ban, beats everything). if (keyUser.revokedAt) { return false; } - // Step 3a: explicit-reject override (rejectAllRequestsFromKey writes this). - const explicitReject = await prisma.signingCondition.findFirst({ - where: { - keyUserId: keyUser.id, - method: '*', - allowed: false, - } + const live = liveWhere(now); + + // Step 3: live matching per-(method, kind) override — deny beats grant. + // (Subject-level "reject all from this user" is KeyUser.revokedAt, applied + // at step 2 via the revoke_user admin command. There is no method='*' + // SigningCondition sentinel — nothing writes one.) + const signingConditionQuery = requestToSigningConditionQuery(method, payload); + + const liveDeny = await prisma.signingCondition.findFirst({ + where: { keyUserId: keyUser.id, ...signingConditionQuery, allowed: false, ...live }, }); - if (explicitReject) { - console.log(`explicit reject`, explicitReject); + if (liveDeny) { return false; } - // Step 3b: matching per-(method, kind) override. - const signingConditionQuery = requestToSigningConditionQuery(method, payload); - - const signingCondition = await prisma.signingCondition.findFirst({ - where: { - keyUserId: keyUser.id, - ...signingConditionQuery, - } + const liveGrant = await prisma.signingCondition.findFirst({ + where: { keyUserId: keyUser.id, ...signingConditionQuery, allowed: true, ...live }, }); - if (signingCondition && (signingCondition.allowed === true || signingCondition.allowed === false)) { - console.log(`found signing condition`, signingCondition); - return signingCondition.allowed; - } - - // Step 4: live policy join. Walk every non-revoked Token bound to this - // KeyUser; if any of their policies has a matching PolicyRule, allow. - // - // 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; - - 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, - revokedAt: null, - policy: { - rules: { - some: { - method, - OR: kindMatchers, - }, - }, - }, - }, - }); - - if (policyAllowance) { + if (liveGrant) { return true; } - // Step 5: no override granted, no policy rule matched. Caller's + // Step 4: live token grant. + // + // 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 }, + }); + + 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 policyAllowance = await prisma.token.findFirst({ + where: { + keyUserId: keyUser.id, + ...live, + policy: { + rules: { + some: { + method, + OR: kindMatchers, + }, + }, + }, + }, + }); + + if (policyAllowance) { + return true; + } + } + + // Step 5: no live override and no live token grant matched. Caller's // requestPermission flow may still prompt the admin out-of-band. return undefined; } @@ -195,20 +216,3 @@ export async function allowAllRequestsFromKey( console.log('allowAllRequestsFromKey', e); } } - -export async function rejectAllRequestsFromKey(remotePubkey: string, keyName: string): Promise { - // Upsert the KeyUser with the given remotePubkey - const upsertedUser = await prisma.keyUser.upsert({ - where: { unique_key_user: { keyName, userPubkey: remotePubkey } }, - update: { }, - create: { keyName, userPubkey: remotePubkey }, - }); - - // Create a new SigningCondition for the given KeyUser and set allowed to false - await prisma.signingCondition.create({ - data: { - allowed: false, - 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 new file mode 100644 index 0000000..783dd84 --- /dev/null +++ b/src/daemon/lib/acl/lifecycle.ts @@ -0,0 +1,40 @@ +/** + * 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 new file mode 100644 index 0000000..4bbe5f1 --- /dev/null +++ b/tests/lifecycle.test.ts @@ -0,0 +1,44 @@ +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 } }], + }); +});