check_token() and handle_login() compared bearer tokens, session tokens,
and the password hash with plain ==, which short-circuits on the first
differing byte - a textbook remote timing side-channel against exactly
the long-lived secrets these gates exist to protect. Added subtle (already
in the dependency tree transitively via chacha20poly1305) as a direct
dependency and route every secret comparison through a small secure_eq()
wrapper over ConstantTimeEq. Username comparison in handle_login is left
as-is: it isn't treated as a secret in this threat model (one fixed admin
username), matching standard practice of only constant-timing the
password/token side of an auth check.
Added tests for secure_eq() itself (equal, different, different-length,
empty) alongside the existing check_token coverage.
GET/POST/DELETE /api/audit were the only three handlers in the whole
Management API that never called check_token() - every other endpoint
(status, users, rules, config) does. Concretely, with the panel's
credentials configured, an unauthenticated request could still:
- read the full audit log (GET)
- inject arbitrary forged entries, e.g. fake "success" events to cover
tracks (POST)
- wipe the entire audit log (DELETE) - the exact mechanism meant to
detect and investigate unauthorized actions, erasable with zero auth
Added the same check_token() gate the rest of the file uses, and fixed
these three handlers' raw .unwrap() on the audit_logs lock to the
poison-recovery pattern (unwrap_or_else(|e| e.into_inner())) used
everywhere else, for consistency.
Added focused unit tests on check_token() itself (missing header, correct/
wrong bearer, raw token, session token, and the documented open-panel
mode when no credentials are configured) - it's the single gate every
sensitive handler depends on, worth pinning down independently of any one
handler.
Two classes of issue:
- Hot-path/attacker-triggerable events logged at info/error with internal
detail: a per-handshake info! byte dump (raw_vec[0..6]) and a per-packet
error! on session-id mismatch that dumped expected/got session ids.
Both are log-flood + info-leak surfaces; downgraded to debug and
stripped of the sensitive detail. Close/Resume frame handling likewise
moved from info to debug.
- The access key (a shared secret) was written to logs verbatim in three
places (session drop, key-created UI event, API create-user) and as an
8-char prefix in one. Added key_fp() — a short SHA-256 fingerprint — and
routed all key logging through it so operators can still correlate
events without the secret ever hitting the log.
ospab
ospab