fix(api-keys): gültige Schlüssel nie durch die Fehlversuchs-Sperre abweisen
Befunde aus dem Sicherheitsreview des Branches: - MEDIUM: Die Sperre lief vor der Schlüsselsuche und zählte auch falsch aufgebaute Tokens. Wer die gemeinsame Adresse (Cluster-Ausgang, Proxy) nutzt oder per X-Forwarded-For vortäuscht, hätte ERPNext und den Shop 15 Minuten aussperren können. Jetzt: erst Suche über den Hash; ein gültiger, nicht widerrufener, nicht abgelaufener Schlüssel kommt immer durch; 429 nur für unbekannte, widerrufene und abgelaufene Schlüssel; falsch aufgebaute Tokens sofort 401 ohne Zählung. Beim Start warnt ApiCore, wenn TRUST_PROXY in Produktion fehlt (im Cluster gesetzt). - LOW: Bei vollem Speicher verdrängt der Limiter zuerst nicht gesperrte Einträge, gesperrte erst zuletzt. - LOW: Bewegungs-Cursor mit Jahr außerhalb 2000-2100 ergab 500, jetzt 400. - LOW/INFO: Vertrag legt fest, dass ERPNext Produkte und Chargen bei jedem Lauf vollständig neu liest (Voll-Spiegel) und Bewegungen inkrementell; remainingQuantity ist live, Bewegungen kommen 5 Minuten verzögert. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
Nick SzaunigandClaude Opus 5.5 committed
1 parent
e28cc8e0f3
commit
fe7b61c3c1
11 files changed
+300
-81
No files matched your search
@@ -39,12 +39,13 @@ const LAST_USED_RESOLUTION_MS = 5 * 60 * 1000;
|
||||
const LOGGABLE_PREFIX_LENGTH = 12;
|
||||
|
||||
/**
|
||||
* Failed key attempts per client IP before that IP is refused with 429 for
|
||||
* the rest of the window - the same budget as the shop's ERP export. A key
|
||||
* holds 256 random bits, so this is not what keeps it safe; it keeps a
|
||||
* misconfigured or hostile client from hammering the key lookup, and the
|
||||
* block applies to every key from that address, so a correct key cannot be
|
||||
* told apart from a wrong one while it lasts.
|
||||
* Unknown, revoked or expired keys per client network before that network
|
||||
* gets 429 for further invalid keys for the rest of the window - the same
|
||||
* budget as the shop's ERP export. It slows down guessing and a
|
||||
* misconfigured client; a valid key is never refused by it. Otherwise
|
||||
* anyone able to send requests from (or forge) a shared address - the
|
||||
* cluster egress, a proxy - could lock ERPNext or the shop out, and a key
|
||||
* holds 256 random bits, so letting it through costs nothing.
|
||||
*/
|
||||
export const API_KEY_FAILURE_LIMIT = {
|
||||
maxFailures: 20,
|
||||
@@ -52,7 +53,7 @@ export const API_KEY_FAILURE_LIMIT = {
|
||||
maxTrackedSources: 10_000,
|
||||
} as const;
|
||||
|
||||
type RejectReason = 'malformed' | 'unknown' | 'revoked' | 'expired';
|
||||
type RejectReason = 'unknown' | 'revoked' | 'expired';
|
||||
|
||||
/** What a request authenticated by an API key may do - it is not a user. */
|
||||
export interface ApiKeyPrincipal {
|
||||
@@ -199,19 +200,40 @@ export class ApiKeysService {
|
||||
|
||||
/**
|
||||
* Resolves a presented key to what it may do, or null for anything that
|
||||
* must not pass: malformed, unknown, revoked or expired. Each of those
|
||||
* counts as a failed attempt of the client IP; an IP over the limit gets
|
||||
* 429 before any lookup.
|
||||
* must not pass: malformed, unknown, revoked or expired.
|
||||
*
|
||||
* The key is found by the SHA-256 of the presented token through a unique
|
||||
* index - never by comparing secrets byte by byte, so response timing says
|
||||
* nothing about the stored key.
|
||||
* nothing about the stored key. Order matters: the lookup comes first, and
|
||||
* only an invalid key counts against (or is stopped by) the failure limit
|
||||
* of the client network. A malformed token is not a guess at a key and is
|
||||
* refused without lookup and without counting.
|
||||
*/
|
||||
async authenticate(
|
||||
token: string,
|
||||
clientIp?: string,
|
||||
): Promise<ApiKeyPrincipal | null> {
|
||||
const source = clientIp ?? 'unknown';
|
||||
if (!TOKEN_PATTERN.test(token)) {
|
||||
return null;
|
||||
}
|
||||
const apiKey = await this.prisma.apiKey.findUnique({
|
||||
where: { keyHash: hashToken(token) },
|
||||
});
|
||||
const now = new Date();
|
||||
const reason = rejectReason(apiKey, now);
|
||||
if (reason === null && apiKey) {
|
||||
return this.accept(apiKey, clientIp, now);
|
||||
}
|
||||
this.recordRejection(token, reason ?? 'unknown', clientIp);
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Counts an invalid key; 429 once the client network is over the limit. */
|
||||
private recordRejection(
|
||||
token: string,
|
||||
reason: RejectReason,
|
||||
clientIp: string | undefined,
|
||||
): void {
|
||||
// Counted per client network like the throttler (IPv6 /64), so rotating
|
||||
// addresses inside one's own prefix does not reset the budget.
|
||||
const subject = clientIp ? rateLimitSubject(clientIp) : 'unknown';
|
||||
@@ -228,46 +250,22 @@ export class ApiKeysService {
|
||||
HttpStatus.TOO_MANY_REQUESTS,
|
||||
);
|
||||
}
|
||||
const result = await this.resolve(token, clientIp);
|
||||
if ('principal' in result) {
|
||||
return result.principal;
|
||||
}
|
||||
// Only the public prefix is ever logged, never the key itself.
|
||||
const prefix =
|
||||
result.reason === 'malformed'
|
||||
? '(malformed)'
|
||||
: token.slice(0, LOGGABLE_PREFIX_LENGTH);
|
||||
this.logger.warn(
|
||||
`API key rejected (${result.reason}): ${prefix} from ${source}`,
|
||||
`API key rejected (${reason}): ${token.slice(0, LOGGABLE_PREFIX_LENGTH)} from ${clientIp ?? 'unknown'}`,
|
||||
);
|
||||
if (this.failures.recordFailure(subject)) {
|
||||
this.logger.warn(
|
||||
`API key attempts from ${subject} blocked for ${API_KEY_FAILURE_LIMIT.windowMs / 60_000} minutes after ${API_KEY_FAILURE_LIMIT.maxFailures} failures`,
|
||||
`Invalid API keys from ${subject} refused for ${API_KEY_FAILURE_LIMIT.windowMs / 60_000} minutes after ${API_KEY_FAILURE_LIMIT.maxFailures} failures`,
|
||||
);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
private async resolve(
|
||||
token: string,
|
||||
private async accept(
|
||||
apiKey: ApiKey,
|
||||
clientIp: string | undefined,
|
||||
): Promise<{ principal: ApiKeyPrincipal } | { reason: RejectReason }> {
|
||||
if (!TOKEN_PATTERN.test(token)) {
|
||||
return { reason: 'malformed' };
|
||||
}
|
||||
const apiKey = await this.prisma.apiKey.findUnique({
|
||||
where: { keyHash: hashToken(token) },
|
||||
});
|
||||
const now = new Date();
|
||||
if (!apiKey) {
|
||||
return { reason: 'unknown' };
|
||||
}
|
||||
if (apiKey.revokedAt) {
|
||||
return { reason: 'revoked' };
|
||||
}
|
||||
if (apiKey.expiresAt && apiKey.expiresAt <= now) {
|
||||
return { reason: 'expired' };
|
||||
}
|
||||
now: Date,
|
||||
): Promise<ApiKeyPrincipal> {
|
||||
if (!isIpAllowed(clientIp, apiKey.allowedIps)) {
|
||||
// The key itself is right: say so, so the admin finds the cause - a
|
||||
// thief learns nothing new, they already hold the key.
|
||||
@@ -290,12 +288,17 @@ export class ApiKeysService {
|
||||
}
|
||||
// Intersect with today's allowlist: narrowing it later also narrows
|
||||
// keys issued before.
|
||||
return {
|
||||
principal: { id: apiKey.id, scopes: apiKey.scopes.filter(isApiKeyScope) },
|
||||
};
|
||||
return { id: apiKey.id, scopes: apiKey.scopes.filter(isApiKeyScope) };
|
||||
}
|
||||
}
|
||||
|
||||
function rejectReason(apiKey: ApiKey | null, now: Date): RejectReason | null {
|
||||
if (!apiKey) return 'unknown';
|
||||
if (apiKey.revokedAt) return 'revoked';
|
||||
if (apiKey.expiresAt && apiKey.expiresAt <= now) return 'expired';
|
||||
return null;
|
||||
}
|
||||
|
||||
/** The key from an `Authorization: Bearer ack_…` header, or null. */
|
||||
export function extractApiKeyToken(
|
||||
authorization: string | undefined,
|
||||
|
||||
Reference in new issue
Block a user