fix(security): bound concurrent board-key auth work
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
f0ba1c96d0
commit
61bb57d5ad
|
|
@ -34,6 +34,8 @@ function createDbState() {
|
|||
revoked: false,
|
||||
revokeOnTouch: false,
|
||||
lastUsedAt: null as Date | null,
|
||||
boardKeyLookupBarrier: null as Promise<void> | null,
|
||||
boardKeyLookupStarts: 0,
|
||||
};
|
||||
const audits: Array<Record<string, unknown>> = [];
|
||||
const key = () => ({
|
||||
|
|
@ -71,7 +73,13 @@ function createDbState() {
|
|||
? []
|
||||
: [];
|
||||
return {
|
||||
where: () => Promise.resolve(rows),
|
||||
where: async () => {
|
||||
if (table === boardApiKeys) {
|
||||
state.boardKeyLookupStarts += 1;
|
||||
await state.boardKeyLookupBarrier;
|
||||
}
|
||||
return rows;
|
||||
},
|
||||
then: (resolve: (value: unknown[]) => unknown) => Promise.resolve(rows).then(resolve),
|
||||
};
|
||||
},
|
||||
|
|
@ -201,6 +209,8 @@ describe("board-key authentication middleware", () => {
|
|||
globalLimit: 10_000,
|
||||
credentialMaxEntries: 7,
|
||||
sourceMaxEntries: 5,
|
||||
sourceInFlightLimit: 2,
|
||||
globalInFlightLimit: 3,
|
||||
});
|
||||
|
||||
for (let index = 0; index < 100; index += 1) {
|
||||
|
|
@ -215,6 +225,8 @@ describe("board-key authentication middleware", () => {
|
|||
credentialEntries: 7,
|
||||
sourceEntries: 5,
|
||||
globalEntries: 1,
|
||||
sourceInFlightEntries: 0,
|
||||
globalInFlight: 0,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -226,6 +238,8 @@ describe("board-key authentication middleware", () => {
|
|||
globalLimit: 3,
|
||||
credentialMaxEntries: 10,
|
||||
sourceMaxEntries: 10,
|
||||
sourceInFlightLimit: 10,
|
||||
globalInFlightLimit: 10,
|
||||
});
|
||||
|
||||
for (let index = 0; index < 3; index += 1) {
|
||||
|
|
@ -235,6 +249,34 @@ describe("board-key authentication middleware", () => {
|
|||
expect(limiter.isLimited({ credentialId: "novel-credential", sourceId: "novel-source", now: 1 })).toBe(true);
|
||||
});
|
||||
|
||||
it("bounds concurrent database authentication starts with pre-lookup admission", async () => {
|
||||
const { db, state } = createDbState();
|
||||
state.keyExists = false;
|
||||
let releaseLookups!: () => void;
|
||||
state.boardKeyLookupBarrier = new Promise<void>((resolve) => {
|
||||
releaseLookups = resolve;
|
||||
});
|
||||
const { app } = createApp(db);
|
||||
const inFlightLimit = BOARD_KEY_AUTH_FAILURE_RATE_LIMIT_DEFAULTS.sourceInFlightLimit;
|
||||
|
||||
const requests = Array.from({ length: inFlightLimit + 6 }, (_, index) =>
|
||||
request(app)
|
||||
.get("/actor")
|
||||
.set("Authorization", `Bearer pcp_board_concurrent_${index}`)
|
||||
.then((response) => response),
|
||||
);
|
||||
|
||||
await vi.waitFor(() => expect(state.boardKeyLookupStarts).toBe(inFlightLimit));
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
expect(state.boardKeyLookupStarts).toBe(inFlightLimit);
|
||||
releaseLookups();
|
||||
|
||||
const responses = await Promise.all(requests);
|
||||
expect(responses.filter((response) => response.status === 401)).toHaveLength(inFlightLimit);
|
||||
expect(responses.filter((response) => response.status === 429)).toHaveLength(6);
|
||||
expect(state.boardKeyLookupStarts).toBe(inFlightLimit);
|
||||
});
|
||||
|
||||
it("does not resurrect last-used state when revocation wins the authentication race", async () => {
|
||||
const { db, state } = createDbState();
|
||||
state.revokeOnTouch = true;
|
||||
|
|
|
|||
|
|
@ -352,16 +352,29 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa
|
|||
next(tooManyRequests("Too many authentication failures"));
|
||||
return;
|
||||
}
|
||||
const authentication = await boardAuth.authenticateBoardApiKey(token);
|
||||
if (!authentication.ok) {
|
||||
await auditBoardKeyAuthenticationFailure(db, req, authentication);
|
||||
next(
|
||||
boardKeyAuthFailureRateLimiter.recordFailure(failureIdentity)
|
||||
? tooManyRequests("Too many authentication failures")
|
||||
: unauthorized(),
|
||||
);
|
||||
const admission = boardKeyAuthFailureRateLimiter.tryAcquire({ sourceId: failureIdentity.sourceId });
|
||||
if (!admission) {
|
||||
next(tooManyRequests("Too many authentication failures"));
|
||||
return;
|
||||
}
|
||||
let authentication: Awaited<ReturnType<typeof boardAuth.authenticateBoardApiKey>>;
|
||||
try {
|
||||
authentication = await boardAuth.authenticateBoardApiKey(token);
|
||||
} catch (error) {
|
||||
admission.release();
|
||||
throw error;
|
||||
}
|
||||
if (!authentication.ok) {
|
||||
const limited = boardKeyAuthFailureRateLimiter.recordFailure(failureIdentity);
|
||||
try {
|
||||
await auditBoardKeyAuthenticationFailure(db, req, authentication);
|
||||
} finally {
|
||||
admission.release();
|
||||
}
|
||||
next(limited ? tooManyRequests("Too many authentication failures") : unauthorized());
|
||||
return;
|
||||
}
|
||||
admission.release();
|
||||
const { key: boardKey, access, scopeConfig } = authentication;
|
||||
const effectiveCompanyIds = scopeConfig
|
||||
? scopeConfig.companyIds.filter((companyId) => access.companyIds.includes(companyId))
|
||||
|
|
|
|||
|
|
@ -5,6 +5,8 @@ export type BoardKeyAuthFailureRateLimitConfig = {
|
|||
globalLimit: number;
|
||||
credentialMaxEntries: number;
|
||||
sourceMaxEntries: number;
|
||||
sourceInFlightLimit: number;
|
||||
globalInFlightLimit: number;
|
||||
};
|
||||
|
||||
export const BOARD_KEY_AUTH_FAILURE_RATE_LIMIT_DEFAULTS = {
|
||||
|
|
@ -14,6 +16,8 @@ export const BOARD_KEY_AUTH_FAILURE_RATE_LIMIT_DEFAULTS = {
|
|||
globalLimit: 1_000,
|
||||
credentialMaxEntries: 4_096,
|
||||
sourceMaxEntries: 1_024,
|
||||
sourceInFlightLimit: 8,
|
||||
globalInFlightLimit: 128,
|
||||
} satisfies BoardKeyAuthFailureRateLimitConfig;
|
||||
|
||||
type FailureEntry = { count: number; resetAt: number };
|
||||
|
|
@ -86,6 +90,8 @@ export function createBoardKeyAuthFailureRateLimiter(
|
|||
config.sourceMaxEntries,
|
||||
);
|
||||
const global = new BoundedFixedWindowFailures(config.globalLimit, config.windowMs, 1);
|
||||
const sourceInFlight = new Map<string, number>();
|
||||
let globalInFlight = 0;
|
||||
|
||||
return {
|
||||
isLimited(input: { credentialId: string; sourceId: string; now?: number }) {
|
||||
|
|
@ -103,10 +109,39 @@ export function createBoardKeyAuthFailureRateLimiter(
|
|||
return globalLimited || sourceLimited || credentialLimited;
|
||||
},
|
||||
|
||||
tryAcquire(input: { sourceId: string }) {
|
||||
const currentSourceInFlight = sourceInFlight.get(input.sourceId) ?? 0;
|
||||
const newSourceWouldExceedStorage = currentSourceInFlight === 0
|
||||
&& sourceInFlight.size >= config.sourceMaxEntries;
|
||||
if (
|
||||
globalInFlight >= config.globalInFlightLimit
|
||||
|| currentSourceInFlight >= config.sourceInFlightLimit
|
||||
|| newSourceWouldExceedStorage
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
globalInFlight += 1;
|
||||
sourceInFlight.set(input.sourceId, currentSourceInFlight + 1);
|
||||
let released = false;
|
||||
return {
|
||||
release() {
|
||||
if (released) return;
|
||||
released = true;
|
||||
globalInFlight = Math.max(0, globalInFlight - 1);
|
||||
const remainingForSource = (sourceInFlight.get(input.sourceId) ?? 1) - 1;
|
||||
if (remainingForSource <= 0) sourceInFlight.delete(input.sourceId);
|
||||
else sourceInFlight.set(input.sourceId, remainingForSource);
|
||||
},
|
||||
};
|
||||
},
|
||||
|
||||
reset() {
|
||||
credentials.clear();
|
||||
sources.clear();
|
||||
global.clear();
|
||||
sourceInFlight.clear();
|
||||
globalInFlight = 0;
|
||||
},
|
||||
|
||||
snapshot() {
|
||||
|
|
@ -114,6 +149,8 @@ export function createBoardKeyAuthFailureRateLimiter(
|
|||
credentialEntries: credentials.size,
|
||||
sourceEntries: sources.size,
|
||||
globalEntries: global.size,
|
||||
sourceInFlightEntries: sourceInFlight.size,
|
||||
globalInFlight,
|
||||
};
|
||||
},
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in New Issue