Browse Source

docs: mark SEC_REVIEW F7 as fixed in 84238e6

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 5 days ago
parent
commit
ba4072b01e
1 changed files with 14 additions and 1 deletions
  1. 14 1
      doc/SEC_REVIEW.md

+ 14 - 1
doc/SEC_REVIEW.md

@@ -11,7 +11,7 @@
 >
 > Each finding is referenced as **F<N>** for later citation.
 >
-> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (1 fixed, 26 open), 42 sev-1.
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (2 fixed, 25 open), 42 sev-1.
 
 ---
 
@@ -186,6 +186,19 @@
   Mitigation: always run a dummy `password_verify` against a fixed
   hash regardless of username match.
 - **Severity: 2**
+- **Status:** Fixed in `84238e6`. `LocalLoginController` now precomputes a
+  dummy Argon2id hash once per worker in its constructor and routes
+  `password_verify` to either the configured admin hash (when set) or the
+  dummy hash (when unset). The verify result is ANDed with the username
+  check and the "configured" check, so a username miss still fails closed
+  but consumes Argon2id time. Also closes the related timing channel where
+  an empty `LOCAL_ADMIN_PASSWORD_HASH` previously skipped `password_verify`
+  entirely. Regression tests in
+  `ui/tests/Integration/Auth/LocalLoginTest.php`
+  (`testWrongUsernameTriggersPasswordVerify`,
+  `testUnconfiguredLocalPasswordHashStillRunsPasswordVerify`) assert the
+  failure path takes more than 10 ms — well above any path that would
+  skip an Argon2id compare.
 
 ### F8 — `headers_sent()` short-circuit silently skips session regeneration / clear
 - **Files:** `ui/src/Auth/SessionManager.php:43-53, 67-75, 101-107`