2 changed files with 133 additions and 94 deletions
fix(acl)(#24,#25): enforce token expiry+revoke live at sign time
The bug (#24): applyToken photocopied a token's policy rules into SigningCondition rows at redeem; checkIfPubkeyAllowed matched those rows (step 3) and short-circuited before the live Token join (step 4), so an expired or revoked token kept signing forever — the copy carried no lifecycle. Same cause re-shipped by upstream Signet (see docs survey). Option D fix: - grantIsLive(grant, now): the single 'valid right now?' predicate (revokedAt null AND not past expiresAt), used identically at redeem (Backend.validateToken) and sign (checkIfPubkeyAllowed). Redeem and sign can no longer disagree. - Backend.applyToken records ONLY the KeyUser<-Token binding; it no longer materializes SigningCondition rows. Token policy is evaluated live every request. - checkIfPubkeyAllowed step 4 filters tokens through liveWhere(now) (revoke + expiry) and grants connect off a live bound token; the manual-override layer (step 3) now honors SigningCondition expiresAt/revokedAt too (denials beat grants). Closes the materialization-drift family: a new lifecycle rule is one more predicate, never a forgotten photocopy. Token-revoke sibling (spirekeeper#22) falls out of the same seam. Usage caps deferred (no durable signing log exists yet to count) — follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
commit
85e781dfa9
|
|
@ -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: {
|
||||
|
|
|
|||
|
|
@ -1,31 +1,71 @@
|
|||
import { NDKEvent, NostrEvent, NIP46Method } from '@nostr-dev-kit/ndk';
|
||||
import { NostrEvent, NIP46Method } from '@nostr-dev-kit/ndk';
|
||||
import prisma from '../../../db.js';
|
||||
|
||||
/**
|
||||
* Layered authorization check. Order matters:
|
||||
* "Is this grant valid right now?" — the single lifecycle predicate, used
|
||||
* identically at redeem time (Backend.validateToken) and sign time
|
||||
* (checkIfPubkeyAllowed). Both Token and SigningCondition carry `revokedAt`
|
||||
* + `expiresAt`, so one shape governs token grants and manual overrides.
|
||||
*
|
||||
* The original #24 bug was possible because redeem-time checked expiry and
|
||||
* sign-time didn't — two definitions of "valid" that drifted. Defining it
|
||||
* once makes them impossible to disagree. See aiolabs/nsecbunkerd#25.
|
||||
*/
|
||||
export function grantIsLive(
|
||||
grant: { revokedAt?: Date | null; expiresAt?: Date | null },
|
||||
now: Date = new Date(),
|
||||
): boolean {
|
||||
if (grant.revokedAt) return false;
|
||||
if (grant.expiresAt && grant.expiresAt.getTime() <= now.getTime()) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Prisma `where` fragment selecting only lifecycle-live rows (not revoked,
|
||||
* not past expiry) — grantIsLive expressed in SQL so the live filter happens
|
||||
* in the query, not in app code after the fact. `now` is threaded in so a
|
||||
* single request evaluates every row against one clock reading.
|
||||
*/
|
||||
function liveWhere(now: Date) {
|
||||
return {
|
||||
revokedAt: null,
|
||||
OR: [{ expiresAt: null }, { expiresAt: { gt: now } }],
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 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)
|
||||
*
|
||||
* 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<boolean | undefined> {
|
||||
// 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,53 +75,66 @@ 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 live = liveWhere(now);
|
||||
|
||||
// Step 3a: live explicit reject.
|
||||
const explicitReject = await prisma.signingCondition.findFirst({
|
||||
where: {
|
||||
keyUserId: keyUser.id,
|
||||
method: '*',
|
||||
allowed: false,
|
||||
}
|
||||
where: { keyUserId: keyUser.id, method: '*', allowed: false, ...live },
|
||||
});
|
||||
|
||||
if (explicitReject) {
|
||||
console.log(`explicit reject`, explicitReject);
|
||||
return false;
|
||||
}
|
||||
|
||||
// Step 3b: matching per-(method, kind) override.
|
||||
// Step 3b: live matching per-(method, kind) override — deny beats grant.
|
||||
const signingConditionQuery = requestToSigningConditionQuery(method, payload);
|
||||
|
||||
const signingCondition = await prisma.signingCondition.findFirst({
|
||||
where: {
|
||||
keyUserId: keyUser.id,
|
||||
...signingConditionQuery,
|
||||
}
|
||||
const liveDeny = await prisma.signingCondition.findFirst({
|
||||
where: { keyUserId: keyUser.id, ...signingConditionQuery, allowed: false, ...live },
|
||||
});
|
||||
|
||||
if (signingCondition && (signingCondition.allowed === true || signingCondition.allowed === false)) {
|
||||
console.log(`found signing condition`, signingCondition);
|
||||
return signingCondition.allowed;
|
||||
if (liveDeny) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Step 4: live policy join. Walk every non-revoked Token bound to this
|
||||
// KeyUser; if any of their policies has a matching PolicyRule, allow.
|
||||
const liveGrant = await prisma.signingCondition.findFirst({
|
||||
where: { keyUserId: keyUser.id, ...signingConditionQuery, allowed: true, ...live },
|
||||
});
|
||||
|
||||
if (liveGrant) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// 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 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)
|
||||
// - 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;
|
||||
|
||||
|
|
@ -93,7 +146,7 @@ export async function checkIfPubkeyAllowed(
|
|||
const policyAllowance = await prisma.token.findFirst({
|
||||
where: {
|
||||
keyUserId: keyUser.id,
|
||||
revokedAt: null,
|
||||
...live,
|
||||
policy: {
|
||||
rules: {
|
||||
some: {
|
||||
|
|
@ -108,8 +161,9 @@ export async function checkIfPubkeyAllowed(
|
|||
if (policyAllowance) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// Step 5: no override granted, no policy rule matched. Caller's
|
||||
// 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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue