فهرست منبع

fix: run password_verify on every local-login attempt for constant-time response (SEC_REVIEW F7)

Precompute a dummy Argon2id hash once per worker in the LocalLoginController
constructor and route password_verify to either the configured admin hash
(when set) or the dummy hash (when unset / on a username miss). The verify
result is ANDed with the username check and the "configured" check, so a
miss still fails closed.

Closes the timing channel where an empty LOCAL_ADMIN_PASSWORD_HASH (or any
future regression that short-circuits the hot path on a username miss)
would let an unauthenticated attacker probe whether a local-admin account
is configured by measuring response latency.

Regression tests:
- LocalLoginTest::testWrongUsernameTriggersPasswordVerify — wrong username
  POST takes >10ms, well above any path that skips Argon2id work.
- LocalLoginTest::testUnconfiguredLocalPasswordHashStillRunsPasswordVerify
  — unconfigured-hash POST also takes >10ms via the dummy hash.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 5 روز پیش
والد
کامیت
84238e6592
2فایلهای تغییر یافته به همراه88 افزوده شده و 1 حذف شده
  1. 29 1
      ui/src/Auth/LocalLoginController.php
  2. 59 0
      ui/tests/Integration/Auth/LocalLoginTest.php

+ 29 - 1
ui/src/Auth/LocalLoginController.php

@@ -29,6 +29,13 @@ use Slim\Views\Twig;
  */
 final class LocalLoginController
 {
+    /**
+     * Precomputed Argon2id hash used as a constant-time `password_verify`
+     * target on the failure path (SEC_REVIEW F7). Built once per worker in
+     * the constructor; never on the request hot path.
+     */
+    private readonly string $dummyPasswordHash;
+
     public function __construct(
         private readonly Twig $twig,
         private readonly SessionManager $sessions,
@@ -41,6 +48,10 @@ final class LocalLoginController
         private readonly string $localUsername,
         private readonly string $localPasswordHash,
     ) {
+        $this->dummyPasswordHash = password_hash(
+            bin2hex(random_bytes(16)),
+            PASSWORD_ARGON2ID,
+        );
     }
 
     public function showLogin(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
@@ -75,7 +86,24 @@ final class LocalLoginController
         }
 
         $usernameOk = hash_equals($this->localUsername, $username);
-        $passwordOk = $this->localPasswordHash !== '' && password_verify($password, $this->localPasswordHash);
+
+        // SEC_REVIEW F7: always run `password_verify` against an Argon2id
+        // hash so the request takes the same time on a username miss as on
+        // a hit — and even when the local-admin password is unconfigured.
+        // We compare against the configured admin hash whenever it is set
+        // (so the timing target matches whatever cost params the operator
+        // chose); when it is unset we fall back to a precomputed dummy
+        // hash. The verify result is then ANDed with the username check
+        // and the "configured" check, so a username miss still fails
+        // closed.
+        $hashToCompare = $this->localPasswordHash !== ''
+            ? $this->localPasswordHash
+            : $this->dummyPasswordHash;
+        $verifyOk = password_verify($password, $hashToCompare);
+
+        $passwordOk = $usernameOk
+            && $this->localPasswordHash !== ''
+            && $verifyOk;
 
         if (!$usernameOk || !$passwordOk) {
             $this->throttle->recordFailure($username, $sourceIp);

+ 59 - 0
ui/tests/Integration/Auth/LocalLoginTest.php

@@ -88,6 +88,65 @@ final class LocalLoginTest extends AppTestCase
         self::assertFalse($throttle->isLocked('someone', ''));
     }
 
+    public function testWrongUsernameTriggersPasswordVerify(): void
+    {
+        // SEC_REVIEW F7: a wrong username must still go through
+        // password_verify against an Argon2id hash, so timing does not
+        // distinguish "username matches" from "username does not match".
+        // We assert a generous lower bound (10 ms) — well above the
+        // microsecond cost of a path that skips password_verify, and
+        // well below PHP's default PASSWORD_ARGON2ID cost (~50 ms+).
+        $this->request('GET', '/login');
+        $token = (string) ($_SESSION[CsrfMiddleware::SESSION_KEY] ?? '');
+
+        $body = http_build_query([
+            'csrf_token' => $token,
+            'username' => 'definitely_not_the_admin',
+            'password' => 'irrelevant',
+        ]);
+
+        $start = hrtime(true);
+        $response = $this->request('POST', '/login/local', [], $body, 'application/x-www-form-urlencoded');
+        $elapsedNs = hrtime(true) - $start;
+
+        self::assertSame(303, $response->getStatusCode());
+        self::assertGreaterThan(
+            10_000_000,
+            $elapsedNs,
+            'wrong-username path took <10ms; password_verify likely skipped (F7 regression)',
+        );
+    }
+
+    public function testUnconfiguredLocalPasswordHashStillRunsPasswordVerify(): void
+    {
+        // SEC_REVIEW F7 defence-in-depth: even when LOCAL_ADMIN_PASSWORD_HASH
+        // is empty, the login path must run password_verify against the
+        // dummy Argon2id hash so an attacker cannot probe whether a local
+        // admin password is configured by timing.
+        $this->bootApp(['local_admin_password_hash' => '']);
+
+        $this->request('GET', '/login');
+        $token = (string) ($_SESSION[CsrfMiddleware::SESSION_KEY] ?? '');
+
+        $body = http_build_query([
+            'csrf_token' => $token,
+            'username' => 'admin',
+            'password' => 'whatever',
+        ]);
+
+        $start = hrtime(true);
+        $response = $this->request('POST', '/login/local', [], $body, 'application/x-www-form-urlencoded');
+        $elapsedNs = hrtime(true) - $start;
+
+        self::assertSame(303, $response->getStatusCode());
+        self::assertSame('/login', $response->getHeaderLine('Location'));
+        self::assertGreaterThan(
+            10_000_000,
+            $elapsedNs,
+            'unconfigured-hash path took <10ms; dummy password_verify likely skipped (F7 regression)',
+        );
+    }
+
     public function testCsrfMissingIs403(): void
     {
         $this->request('GET', '/login');