aiolabs/spirekeeper
8
0
Fork 0
Code Issues 23 Pull requests Projects Releases Packages Wiki Activity Actions

pair/revoke endpoint must call revoke_key_user, not revoke_key_token (revoke-one-spire is a no-op otherwise) #22

New issue
Closed
opened 2026-06-18 16:01:09 +00:00 by padreug · 1 comment
padreug commented 2026-06-18 16:01:09 +00:00
Owner
Copy link

Summary

The spire revoke path (the "Revoke spire/ATM access" acceptance criterion in #9/#12, surfaced while building #21) must use the bunker admin client's revoke_key_user(key_name), NOT revoke_token / revoke_key_token. Using token revocation will appear to succeed but will not actually stop the spire from signing — a silent security bug.

Why (verified live against nsecbunkerd@dev, 2026-06-18)

On NIP-46 connect, the bunker redeems the connect token and materializes the token's policy into standing per-user SigningCondition grants on the KeyUser. The sign-time ACL (nsecbunkerd src/daemon/lib/acl/index.ts) checks those grants before the Token.revokedAt IS NULL filter:

  1. KeyUser missing → undefined
  2. KeyUser.revokedAt set → deny ("binary user revoke beats everything")
  3. matching per-user SigningCondition → return its allowed ← grants live here after connect
  4. live-policy join with Token.revokedAt IS NULL → allow
  5. else → deny

lnbits eager-binds the spire key at provision time (the aiolabs/lnbits#32 fix redeems the token immediately), so the token is always already redeemed. Revoking the token (step 4) is bypassed by the materialized grants (step 3) → the spire keeps signing. Only revoke_user (step 2, KeyUser.revokedAt) actually cuts it off.

Daemon log from the failing case (revoked the token, then signed):

found signing condition { id: 81, method: 'sign_event', kind: '1', allowed: true, keyUserId: 4 }
🔎 npub1n58j… is allowed to sign_event …

After revoke_user instead: sign_event → Not authorized. ✅

The fix on the lnbits side is already shipped

aiolabs/lnbits#55 (commit 05f43894) adds to NsecBunkerAdminClient:

  • get_key_users(key_name) — discovery path returning KeyUser.id(s)
  • revoke_key_user(key_name) — resolves key_name → KeyUser id(s) and revoke_users each; returns the count revoked. This is the effective "cut off this spire" op.

The revoke_token / revoke_key_token docstrings now carry the redeemed-token caveat.

Action for spirekeeper

  • The pair/revoke endpoint (model A1, pair_spire sibling) should call admin_client.revoke_key_user(<spire bunker_name / key_name>) and treat a return value >= 1 as "spire access revoked", 0 as "nothing was bound" (token minted but spire never connected).
  • Add an end-to-end test mirroring lnbits' test_live_bunker_revoked_user_cannot_sign: provision/pair → sign once → revoke_key_user → assert a subsequent sign fails closed.
  • Depends on aiolabs/lnbits#55 merging (the revoke_key_user method).

Related

  • aiolabs/lnbits#54 / #55 (admin-client revoke/expiry/policy-cache)
  • aiolabs/spirekeeper#9, #12 (revoke-one-spire UX), #21 (seed-URL pairing)
  • Upstream nit: nsecbunkerd's revoke_token.ts docstring wrongly claims it stops signing "for the associated KeyUser" — worth correcting.
## Summary The spire revoke path (the "Revoke spire/ATM access" acceptance criterion in #9/#12, surfaced while building #21) must use the bunker admin client's **`revoke_key_user(key_name)`**, NOT `revoke_token` / `revoke_key_token`. Using token revocation will appear to succeed but **will not actually stop the spire from signing** — a silent security bug. ## Why (verified live against `nsecbunkerd@dev`, 2026-06-18) On NIP-46 `connect`, the bunker *redeems* the connect token and **materializes the token's policy into standing per-user `SigningCondition` grants** on the `KeyUser`. The sign-time ACL (`nsecbunkerd src/daemon/lib/acl/index.ts`) checks those grants **before** the `Token.revokedAt IS NULL` filter: 1. KeyUser missing → undefined 2. `KeyUser.revokedAt` set → **deny** ("binary user revoke beats everything") 3. matching per-user `SigningCondition` → return its `allowed` ← **grants live here after connect** 4. live-policy join with `Token.revokedAt IS NULL` → allow 5. else → deny lnbits eager-binds the spire key at provision time (the `aiolabs/lnbits#32` fix redeems the token immediately), so the token is **always already redeemed**. Revoking the *token* (step 4) is bypassed by the materialized grants (step 3) → the spire keeps signing. Only `revoke_user` (step 2, `KeyUser.revokedAt`) actually cuts it off. Daemon log from the failing case (revoked the token, then signed): ``` found signing condition { id: 81, method: 'sign_event', kind: '1', allowed: true, keyUserId: 4 } 🔎 npub1n58j… is allowed to sign_event … ``` After `revoke_user` instead: `sign_event` → `Not authorized`. ✅ ## The fix on the lnbits side is already shipped `aiolabs/lnbits#55` (commit `05f43894`) adds to `NsecBunkerAdminClient`: - `get_key_users(key_name)` — discovery path returning `KeyUser.id`(s) - `revoke_key_user(key_name)` — resolves `key_name` → KeyUser id(s) and `revoke_user`s each; returns the count revoked. This is the effective "cut off this spire" op. The `revoke_token` / `revoke_key_token` docstrings now carry the redeemed-token caveat. ## Action for spirekeeper - The pair/revoke endpoint (model A1, `pair_spire` sibling) should call `admin_client.revoke_key_user(<spire bunker_name / key_name>)` and treat a return value `>= 1` as "spire access revoked", `0` as "nothing was bound" (token minted but spire never connected). - Add an end-to-end test mirroring lnbits' `test_live_bunker_revoked_user_cannot_sign`: provision/pair → sign once → `revoke_key_user` → assert a subsequent sign fails closed. - Depends on `aiolabs/lnbits#55` merging (the `revoke_key_user` method). ## Related - `aiolabs/lnbits#54` / `#55` (admin-client revoke/expiry/policy-cache) - `aiolabs/spirekeeper#9`, `#12` (revoke-one-spire UX), `#21` (seed-URL pairing) - Upstream nit: nsecbunkerd's `revoke_token.ts` docstring wrongly claims it stops signing "for the associated KeyUser" — worth correcting.
padreug commented 2026-06-19 21:24:54 +00:00
Author
Owner
Copy link

Resolved bunker-side by nsecbunkerd#27 (deployed 2026-06-19)

This issue's core finding — token-revoke is a silent no-op once the token is redeemed, because nsecbunkerd photocopied policy grants into per-KeyUser SigningCondition rows that the ACL checked before the Token.revokedAt filter — no longer holds.

nsecbunkerd#27 (merge 992c6a8, Option D; closes nsecbunkerd#24/#25/#12):

  • applyToken stopped photocopying grants into SigningConditions;
  • checkIfPubkeyAllowed step 4 now joins Token through liveWhere(now) = { revokedAt: null, OR: [expiresAt null, expiresAt > now] }, evaluated every request.

So token-revoke (and token expiry) are now enforced live post-bind. Verified against the deployed dev tree.

Our code stays as-is: revoke_spire keeps calling revoke_key_user (sets KeyUser.revokedAt, the step-2 subject-level ban) because that cuts the whole binding regardless of how many tokens were issued — the correct "revoke this spire" semantics — whereas token-revoke severs only one token's grant. Docstring/test-comment corrected in #28.

Closing as resolved — the mechanism this issue guarded against is fixed upstream and our deliberate revoke_key_user choice is documented. (Will leave the actual close to @padreug per our manual-close convention.)

## Resolved bunker-side by nsecbunkerd#27 (deployed 2026-06-19) This issue's core finding — token-revoke is a silent no-op once the token is redeemed, because nsecbunkerd photocopied policy grants into per-`KeyUser` `SigningCondition` rows that the ACL checked before the `Token.revokedAt` filter — **no longer holds.** nsecbunkerd#27 (merge `992c6a8`, Option D; closes nsecbunkerd#24/#25/#12): - `applyToken` **stopped photocopying** grants into SigningConditions; - `checkIfPubkeyAllowed` step 4 now joins `Token` through `liveWhere(now)` = `{ revokedAt: null, OR: [expiresAt null, expiresAt > now] }`, evaluated every request. So token-revoke (and token expiry) are now enforced live post-bind. Verified against the deployed `dev` tree. **Our code stays as-is:** `revoke_spire` keeps calling `revoke_key_user` (sets `KeyUser.revokedAt`, the step-2 subject-level ban) because that cuts the **whole** binding regardless of how many tokens were issued — the correct "revoke this spire" semantics — whereas token-revoke severs only one token's grant. Docstring/test-comment corrected in #28. Closing as resolved — the mechanism this issue guarded against is fixed upstream and our deliberate `revoke_key_user` choice is documented. (Will leave the actual close to @padreug per our manual-close convention.)
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
v0.1.3
v0.1.2
v0.1.1
v0.1.0
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
aiolabs/spirekeeper#22
No description provided.