Merge pull request #1382 from michalsn/fix/harden-auth-timing

fix: reduce timing-based user enumeration

Author: datamweb

UPGRADING.md

@@ -10,6 +10,22 @@
 - `CodeIgniter\Shield\Config\BaseAuthToken` is now `abstract`. Code that
   instantiated it directly with `new BaseAuthToken()` will now throw an `Error`.
 
+### Login Timing Hardening
+
+Failed password logins now return `Auth.badAttempt` for both unknown users and
+incorrect passwords. `Auth.invalidPassword` remains in the language files for
+application code that uses it, so no new translations are required. Update any
+custom views or tests that expect the old incorrect-password message.
+
+Unknown-user logins now verify against `Auth::$dummyPasswordHash`. The default
+hash uses bcrypt cost 12. If your application changes the password algorithm or
+cost, run `php spark shield:generate-dummy-hash` with its current configuration.
+Copy the printed property declaration into **app/Config/Auth.php**, or set
+`auth.dummyPasswordHash = '<generated hash>'` in **.env** using only the hash
+value. The command does not modify either file. Accounts with older password
+hash settings can still have different verification times until their hashes
+are upgraded.
+
 ## Version 1.2 to 1.3
 
 ### JWT: Minimum Key Length Now Enforced

docs/guides/strengthen_password.md

@@ -99,6 +99,31 @@ public int $hashTimeCost = PASSWORD_ARGON2_DEFAULT_TIME_COST;
 public int $hashThreads  = PASSWORD_ARGON2_DEFAULT_THREADS;
 ```
 
+### Update the Dummy Password Hash
+
+When you change the password hashing algorithm or its cost settings, update
+`Auth::$dummyPasswordHash` as well. Shield uses this hash to
+verify a password when the login identifier does not match an account, reducing
+timing differences between failed login attempts. The default dummy hash uses
+bcrypt with cost 12.
+
+After applying the new hash settings in the target environment, run:
+
+```sh
+php spark shield:generate-dummy-hash
+```
+
+Copy the printed property declaration into `app/Config/Auth.php`, or put only the
+generated hash in your `.env` file:
+
+```ini
+auth.dummyPasswordHash = '<generated hash>'
+```
+
+The command reads the current hash settings and does not edit either file.
+Existing user hashes may still use older settings until they are rehashed after
+a successful login.
+
 ## Maximum Password Length
 
 By default, Shield has the validation rules for maximum password length.

src/Auth.php

@@ -111,7 +111,7 @@ public function id()
     {
         $user = $this->user();
 
-        return ($user !== null) ? $user->id : null;
+        return $user?->id;
     }
 
     public function authenticate(array $credentials): Result

src/Authentication/Authenticators/HmacSha256.php

@@ -141,8 +141,17 @@ public function check(array $credentials): Result
             $credentials['token'] = trim(substr((string) $credentials['token'], 11)); // HMAC-SHA256
         }
 
+        $authTokens = $this->getHmacAuthTokens($credentials['token']);
+
+        if ($authTokens === null || count($authTokens) !== 2) {
+            return new Result([
+                'success' => false,
+                'reason'  => lang('Auth.badToken'),
+            ]);
+        }
+
         // Extract UserToken and HMACSHA256 Signature from Authorization token
-        [$userToken, $signature] = $this->getHmacAuthTokens($credentials['token']);
+        [$userToken, $signature] = $authTokens;
 
         $identityModel = model(UserIdentityModel::class);
 
@@ -160,7 +169,7 @@ public function check(array $credentials): Result
 
         // Check signature...
         $hash = hash_hmac('sha256', (string) $credentials['body'], $secretKey);
-        if ($hash !== $signature) {
+        if (! hash_equals($hash, $signature)) {
             return new Result([
                 'success' => false,
                 'reason'  => lang('Auth.badToken'),
@@ -307,7 +316,7 @@ public function getHmacAuthTokens(?string $fullToken = null): ?array
      */
     public function getHmacKeyFromToken(): ?string
     {
-        [$key, $secretKey] = $this->getHmacAuthTokens();
+        [$key] = $this->getHmacAuthTokens();
 
         return $key;
     }
@@ -319,7 +328,7 @@ public function getHmacKeyFromToken(): ?string
      */
     public function getHmacHashFromToken(): ?string
     {
-        [$key, $hash] = $this->getHmacAuthTokens();
+        [, $hash] = $this->getHmacAuthTokens();
 
         return $hash;
     }

src/Authentication/Authenticators/Session.php

@@ -245,7 +245,7 @@ public function checkAction(UserIdentity $identity, string $token): bool
             throw new LogicException('Cannot get the User.');
         }
 
-        if ($token === '' || $token !== $identity->secret) {
+        if ($token === '' || ! hash_equals((string) $identity->secret, $token)) {
             return false;
         }
 
@@ -365,21 +365,25 @@ public function check(array $credentials): Result
         // Find the existing user
         $user = $this->provider->findByCredentials($credentials);
 
+        /** @var Passwords $passwords */
+        $passwords = service('passwords');
+
         if ($user === null) {
+            // Verify against a fixed hash so an unknown identifier still does
+            // the expensive password check. The result is never accepted.
+            $passwords->verify($givenPassword, config('Auth')->dummyPasswordHash);
+
             return new Result([
                 'success' => false,
                 'reason'  => lang('Auth.badAttempt'),
             ]);
         }
 
-        /** @var Passwords $passwords */
-        $passwords = service('passwords');
-
         // Now, try matching the passwords.
         if (! $passwords->verify($givenPassword, $user->password_hash)) {
             return new Result([
                 'success' => false,
-                'reason'  => lang('Auth.invalidPassword'),
+                'reason'  => lang('Auth.badAttempt'),
             ]);
         }
 

src/Commands/GenerateDummyPasswordHash.php

@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of CodeIgniter Shield.
+ *
+ * (c) CodeIgniter Foundation <admin@codeigniter.com>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace CodeIgniter\Shield\Commands;
+
+class GenerateDummyPasswordHash extends BaseCommand
+{
+    protected $name        = 'shield:generate-dummy-hash';
+    protected $description = 'Generate a dummy password hash using the current Auth password settings.';
+    protected $usage       = 'shield:generate-dummy-hash';
+
+    public function run(array $params): int
+    {
+        $hash = service('passwords')->hash(bin2hex(random_bytes(32)));
+
+        if (! is_string($hash)) {
+            $this->error('Could not generate a dummy password hash.');
+
+            return EXIT_ERROR;
+        }
+
+        $this->write('public string $dummyPasswordHash = \'' . $hash . '\';');
+
+        return EXIT_SUCCESS;
+    }
+}

src/Config/Auth.php

@@ -379,6 +379,15 @@ class Auth extends BaseConfig
      */
     public int $hashCost = 12;
 
+    /**
+     * Hash used when a login identifier does not match an account. It must use
+     * the same password algorithm and cost as real accounts so that failed
+     * logins take comparable time. Override this when changing hash settings.
+     * Run `php spark shield:generate-dummy-hash` to generate a matching hash.
+     * The corresponding plaintext password is never accepted.
+     */
+    public string $dummyPasswordHash = '$2y$12$lGwEWbbJHNlNehaFdwrcH.e7c0S3L2sMFVZr6LX/IHjU0ONwWzogq';
+
     /*
      * ////////////////////////////////////////////////////////////////////
      * OTHER SETTINGS

tests/Authentication/Authenticators/AccessTokenAuthenticatorTest.php

@@ -168,7 +168,7 @@ public function testCheckSuccess(): void
         $this->assertSame($user->id, $result->extraInfo()->id);
 
         $updatedToken = $result->extraInfo()->currentAccessToken();
-        $this->assertNotEmpty($updatedToken->last_used_at);
+        $this->assertInstanceOf(Time::class, $updatedToken->last_used_at);
 
         // Checking token in the same second does not throw "DataException : There is no data to update."
         $this->auth->check(['token' => $token->raw_token]);

tests/Authentication/Authenticators/HmacAuthenticatorTest.php

@@ -155,6 +155,17 @@ public function testCheckBadSignature(): void
         $this->assertSame(lang('Auth.badToken'), $result->reason());
     }
 
+    public function testCheckMalformedToken(): void
+    {
+        $result = $this->auth->check([
+            'token' => 'abc123',
+            'body'  => 'bar',
+        ]);
+
+        $this->assertFalse($result->isOK());
+        $this->assertSame(lang('Auth.badToken'), $result->reason());
+    }
+
     public function testCheckOldToken(): void
     {
         $user       = fake(UserModel::class);
@@ -198,7 +209,7 @@ public function testCheckSuccess(): void
         $this->assertSame($user->id, $result->extraInfo()->id);
 
         $updatedToken = $result->extraInfo()->currentHmacToken();
-        $this->assertNotEmpty($updatedToken->last_used_at);
+        $this->assertInstanceOf(Time::class, $updatedToken->last_used_at);
 
         // Checking token in the same second does not throw "DataException : There is no data to update."
         $this->auth->check(['token' => $rawToken, 'body' => 'bar']);

tests/Authentication/Authenticators/SessionAuthenticatorTest.php

@@ -17,6 +17,7 @@
 use CodeIgniter\Shield\Authentication\Authentication;
 use CodeIgniter\Shield\Authentication\AuthenticationException;
 use CodeIgniter\Shield\Authentication\Authenticators\Session;
+use CodeIgniter\Shield\Authentication\Passwords;
 use CodeIgniter\Shield\Config\Auth;
 use CodeIgniter\Shield\Entities\User;
 use CodeIgniter\Shield\Exceptions\LogicException;
@@ -272,6 +273,29 @@ public function testCheckCannotFindUser(): void
         $this->assertSame(lang('Auth.badAttempt'), $result->reason());
     }
 
+    public function testCheckUnknownUserStillVerifiesPassword(): void
+    {
+        $passwords = new class (new Auth()) extends Passwords {
+            public array $checks = [];
+
+            public function verify(string $password, string $hash): bool
+            {
+                $this->checks[] = [$password, $hash];
+
+                return false;
+            }
+        };
+        Services::injectMock('passwords', $passwords);
+
+        $result = $this->auth->check([
+            'email'    => 'unknown@example.com',
+            'password' => 'secret',
+        ]);
+
+        $this->assertFalse($result->isOK());
+        $this->assertSame([['secret', config('Auth')->dummyPasswordHash]], $passwords->checks);
+    }
+
     public function testCheckBadPassword(): void
     {
         $this->user->createEmailIdentity([
@@ -286,7 +310,7 @@ public function testCheckBadPassword(): void
 
         $this->assertInstanceOf(Result::class, $result);
         $this->assertFalse($result->isOK());
-        $this->assertSame(lang('Auth.invalidPassword'), $result->reason());
+        $this->assertSame(lang('Auth.badAttempt'), $result->reason());
     }
 
     public function testCheckSuccess(): void