fix(webauthn): counter zero only when stored is zero + CLI clears keys

- ZeroTolerantCounterChecker only tolerates a 0 counter when the stored counter is
  also 0; a 0 after a non-zero counter is a rollback and is rejected.
- clusev:reset-admin --disable-2fa now also deletes the user's security keys, so a
  compromised key can't revive when 2FA is re-enabled.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feat/v1-foundation
boban 2026-06-14 18:47:07 +02:00
parent 44b87d78ba
commit 2b04a18023
3 changed files with 10 additions and 2 deletions

View File

@ -48,6 +48,9 @@ class ResetAdmin extends Command
'two_factor_confirmed_at' => null, 'two_factor_confirmed_at' => null,
'two_factor_recovery_codes' => null, 'two_factor_recovery_codes' => null,
]); ]);
// Drop security keys too — a key that prompted the emergency reset must not
// silently revive when 2FA is re-enabled.
$user->webauthnCredentials()->delete();
} }
$user->save(); $user->save();

View File

@ -16,8 +16,10 @@ class ZeroTolerantCounterChecker implements CounterChecker
{ {
public function check(CredentialRecord $credentialRecord, int $currentCounter): void public function check(CredentialRecord $credentialRecord, int $currentCounter): void
{ {
if ($currentCounter === 0) { // Tolerate 0 ONLY when the stored counter is also 0 (authenticator never counts).
return; // counter unsupported by this authenticator // A 0 after a previously non-zero counter is a rollback/clone → reject.
if ($currentCounter === 0 && $credentialRecord->counter === 0) {
return;
} }
if ($currentCounter <= $credentialRecord->counter) { if ($currentCounter <= $credentialRecord->counter) {

View File

@ -3,6 +3,7 @@
namespace Tests\Feature; namespace Tests\Feature;
use App\Models\User; use App\Models\User;
use App\Models\WebauthnCredential;
use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Hash;
use Tests\TestCase; use Tests\TestCase;
@ -18,6 +19,7 @@ class ResetAdminCommandTest extends TestCase
'two_factor_secret' => 'x', 'two_factor_confirmed_at' => now(), 'two_factor_secret' => 'x', 'two_factor_confirmed_at' => now(),
]); ]);
$user->replaceRecoveryCodes(); $user->replaceRecoveryCodes();
WebauthnCredential::create(['user_id' => $user->id, 'name' => 'k', 'credential_id' => 'cid', 'public_key' => '{}', 'sign_count' => 0]);
$this->artisan('clusev:reset-admin', ['--email' => 'admin@clusev.local', '--password' => 'BrandNew1234', '--disable-2fa' => true]) $this->artisan('clusev:reset-admin', ['--email' => 'admin@clusev.local', '--password' => 'BrandNew1234', '--disable-2fa' => true])
->assertExitCode(0); ->assertExitCode(0);
@ -27,6 +29,7 @@ class ResetAdminCommandTest extends TestCase
$this->assertNull($user->two_factor_secret); $this->assertNull($user->two_factor_secret);
$this->assertNull($user->two_factor_confirmed_at); $this->assertNull($user->two_factor_confirmed_at);
$this->assertFalse($user->hasRecoveryCodes()); $this->assertFalse($user->hasRecoveryCodes());
$this->assertFalse($user->hasWebauthnCredentials());
} }
public function test_unknown_email_fails(): void public function test_unknown_email_fails(): void