fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(manager-core): F-S-006 cap API-key candidate set at 16 (Argon2 CPU amplifier)

Browse Source 
verify_api_key Argon2-verifies every candidate sharing the 8-char
prefix. With unlimited candidates, an attacker who can provision many
keys against one indexed prefix could force the server into per-request
M×Argon2 — a few hundred concurrent connections trivially DoSes the
manager since the verify runs before any concurrency cap.

Cap the candidate set at MAX_API_KEY_CANDIDATES = 16; log a warn when
truncation kicks in so operators can spot it. With 32-byte random key
bodies the natural collision rate is negligible, so truncation under
benign load shouldn't happen — its presence is the alarm.

Switching the verify hash from Argon2 to SHA-256 (the audit's other
suggested fix) is a larger refactor (new hash column, backfill, dual-
hash verify during migration) and is left for a follow-up.

AUDIT.md anchor: F-S-006 (cap; hash swap deferred).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-07 20:40:52 +02:00
parent 7d045b2a0b
commit f4189fc52f
1 changed files with 18 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
crates/manager-core/src/auth_middleware.rs
View File

@@ -263,7 +263,7 @@ async fn verify_api_key(state: &AuthState, rest: &str) -> Result<Option<Principa
}
let prefix = &rest[..API_KEY_PREFIX_LEN];
let candidates = match state.keys.find_active_by_prefix(prefix).await {
let mut candidates = match state.keys.find_active_by_prefix(prefix).await {
Ok(v) => v,
Err(err) => {
tracing::error!(?err, "api_keys lookup failed");
@@ -271,6 +271,23 @@ async fn verify_api_key(state: &AuthState, rest: &str) -> Result<Option<Principa
}
};
// F-S-006: bound the candidate set. Prefix collisions are
// statistically rare for a 32-byte random body, but the 8-char
// index space is small enough that an attacker who provisions
// many keys against one indexed prefix could amplify each public
// bearer-tagged request into M×Argon2 verifies. Cap at MAX and
// log the truncation so operators can spot it.
const MAX_API_KEY_CANDIDATES: usize = 16;
if candidates.len() > MAX_API_KEY_CANDIDATES {
tracing::warn!(
prefix,
total = candidates.len(),
kept = MAX_API_KEY_CANDIDATES,
"api_keys prefix-bucket overflow; truncating candidate verify set"
);
candidates.truncate(MAX_API_KEY_CANDIDATES);
}
// F-P-002: Argon2id verify is CPU-bound (~tens of ms each at OWASP
// defaults). Move it off the Tokio async worker so a hot user with
// N prefix-colliding keys doesn't park the worker for N×Argon2.