fix(security): constant-time password-reset (flatten account-enumeration timing)
Queues the reset-link notification so sendResetLink returns in constant time regardless of account existence; resetPassword does equivalent dummy verify work for unknown/no-2FA users. Closes the timing side-channels flagged on the forgot-password review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>feat/v1-foundation
parent
9035078ac0
commit
217189598b
|
|
@ -11,10 +11,17 @@ use Illuminate\Validation\Rules\Password;
|
|||
use Illuminate\Validation\ValidationException;
|
||||
use Livewire\Attributes\Layout;
|
||||
use Livewire\Component;
|
||||
use PragmaRX\Google2FAQRCode\Google2FA;
|
||||
|
||||
#[Layout('layouts.auth')]
|
||||
class ForgotPassword extends Component
|
||||
{
|
||||
/** Fixed throwaway base32 secret for the dummy TOTP verify on the no-user/no-2FA branch. */
|
||||
private const DUMMY_TOTP_SECRET = 'ABCDEFGHIJKLMNOP';
|
||||
|
||||
/** Fixed throwaway bcrypt hash (cost 12) for the dummy password comparison on that branch. */
|
||||
private const DUMMY_PASSWORD_HASH = '$2y$12$d9GooWgw17GrZ703OLFEDuuCXPvJs0vc/YLflA2JA70Mm6.I.3f.W';
|
||||
|
||||
public string $email = '';
|
||||
|
||||
public string $code = '';
|
||||
|
|
@ -40,7 +47,15 @@ class ForgotPassword extends Component
|
|||
}
|
||||
|
||||
$this->validate(['email' => ['required', 'email']]);
|
||||
|
||||
// The notification is queued (see User::sendPasswordResetNotification), so this only ever
|
||||
// pushes a job — no synchronous SMTP. Swallow any transport/queue error so that neither
|
||||
// latency nor an exception can distinguish a known address from an unknown one.
|
||||
try {
|
||||
\Illuminate\Support\Facades\Password::sendResetLink(['email' => $this->email]);
|
||||
} catch (\Throwable) {
|
||||
// Intentionally ignored — the response is identical regardless (auth.reset_link_sent).
|
||||
}
|
||||
|
||||
// Generic — never reveal whether the email exists.
|
||||
$this->dispatch('notify', message: __('auth.reset_link_sent'));
|
||||
|
|
@ -64,9 +79,15 @@ class ForgotPassword extends Component
|
|||
|
||||
$user = User::where('email', $this->email)->first();
|
||||
|
||||
$ok = $user
|
||||
&& $user->hasTwoFactorEnabled()
|
||||
&& ($user->verifyTotp($this->code) || $user->useRecoveryCode($this->code));
|
||||
if ($user && $user->hasTwoFactorEnabled()) {
|
||||
$ok = $user->verifyTotp($this->code) || $user->useRecoveryCode($this->code);
|
||||
} else {
|
||||
// No account, or one without 2FA: run equivalent dummy crypto so this branch costs
|
||||
// about as much as a real wrong-code attempt — the verification path must not be a
|
||||
// timing oracle that distinguishes a known address.
|
||||
$this->burnVerificationTime();
|
||||
$ok = false;
|
||||
}
|
||||
|
||||
if (! $ok) {
|
||||
RateLimiter::hit($key, 60);
|
||||
|
|
@ -95,6 +116,24 @@ class ForgotPassword extends Component
|
|||
return $this->redirect(route('login'), navigate: true);
|
||||
}
|
||||
|
||||
/**
|
||||
* Constant dummy verification for the no-account / no-2FA branch: mirrors the crypto work a
|
||||
* real wrong-code attempt does (a Google2FA HMAC verify + a bcrypt comparison) so response
|
||||
* time can't be used to enumerate accounts. The results are deliberately discarded.
|
||||
*/
|
||||
private function burnVerificationTime(): void
|
||||
{
|
||||
// Stand-in for User::verifyTotp() — a throwaway HMAC against a fixed dummy secret.
|
||||
try {
|
||||
(new Google2FA)->verifyKey(self::DUMMY_TOTP_SECRET, preg_replace('/\s+/', '', $this->code) ?? '');
|
||||
} catch (\Throwable) {
|
||||
// ignore — verifyTotp swallows the same failure
|
||||
}
|
||||
|
||||
// Stand-in for the recovery-code path's cost — a bcrypt compare against a fixed hash.
|
||||
Hash::check($this->code, self::DUMMY_PASSWORD_HASH);
|
||||
}
|
||||
|
||||
public function render()
|
||||
{
|
||||
return view('livewire.auth.forgot-password')->title(__('auth.title_forgot'));
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
namespace App\Models;
|
||||
|
||||
use App\Notifications\QueuedResetPassword;
|
||||
use Database\Factories\UserFactory;
|
||||
use Illuminate\Database\Eloquent\Attributes\Fillable;
|
||||
use Illuminate\Database\Eloquent\Attributes\Hidden;
|
||||
|
|
@ -134,6 +135,16 @@ class User extends Authenticatable
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Always queue the reset-password mail (never send synchronously), so the request that
|
||||
* triggers it returns in constant time whether or not the address exists — see
|
||||
* {@see QueuedResetPassword} and ForgotPassword::sendResetLink().
|
||||
*/
|
||||
public function sendPasswordResetNotification(#[\SensitiveParameter] $token): void
|
||||
{
|
||||
$this->notify(new QueuedResetPassword($token));
|
||||
}
|
||||
|
||||
public function webauthnCredentials(): HasMany
|
||||
{
|
||||
return $this->hasMany(WebauthnCredential::class);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,21 @@
|
|||
<?php
|
||||
|
||||
namespace App\Notifications;
|
||||
|
||||
use App\Livewire\Auth\ForgotPassword;
|
||||
use Illuminate\Auth\Notifications\ResetPassword;
|
||||
use Illuminate\Bus\Queueable;
|
||||
use Illuminate\Contracts\Queue\ShouldQueue;
|
||||
|
||||
/**
|
||||
* Queued variant of the framework reset-password notification.
|
||||
*
|
||||
* Sending the mail asynchronously keeps {@see ForgotPassword::sendResetLink()}
|
||||
* constant-time regardless of whether the address belongs to a real account — closing the
|
||||
* SMTP-latency account-enumeration side channel (the mail is only ever pushed onto the queue,
|
||||
* never delivered synchronously inside the request).
|
||||
*/
|
||||
class QueuedResetPassword extends ResetPassword implements ShouldQueue
|
||||
{
|
||||
use Queueable;
|
||||
}
|
||||
|
|
@ -5,7 +5,7 @@ namespace Tests\Feature;
|
|||
use App\Livewire\Auth\ForgotPassword;
|
||||
use App\Livewire\Auth\ResetPassword;
|
||||
use App\Models\User;
|
||||
use Illuminate\Auth\Notifications\ResetPassword as ResetPasswordNotification;
|
||||
use App\Notifications\QueuedResetPassword as ResetPasswordNotification;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Hash;
|
||||
use Illuminate\Support\Facades\Notification;
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ namespace Tests\Feature;
|
|||
use App\Livewire\Auth\ForgotPassword;
|
||||
use App\Livewire\Settings\Security;
|
||||
use App\Models\User;
|
||||
use Illuminate\Auth\Notifications\ResetPassword as ResetPasswordNotification;
|
||||
use App\Notifications\QueuedResetPassword as ResetPasswordNotification;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Hash;
|
||||
use Illuminate\Support\Facades\Notification;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,139 @@
|
|||
<?php
|
||||
|
||||
namespace Tests\Feature;
|
||||
|
||||
use App\Livewire\Auth\ForgotPassword;
|
||||
use App\Models\User;
|
||||
use App\Notifications\QueuedResetPassword;
|
||||
use Illuminate\Contracts\Queue\ShouldQueue;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Hash;
|
||||
use Illuminate\Support\Facades\Notification;
|
||||
use Illuminate\Support\Facades\RateLimiter;
|
||||
use Livewire\Features\SupportTesting\Testable;
|
||||
use Livewire\Livewire;
|
||||
use PragmaRX\Google2FAQRCode\Google2FA;
|
||||
use Tests\TestCase;
|
||||
|
||||
/**
|
||||
* Hardening of the two low-severity account-enumeration *timing* side channels on the
|
||||
* forgot-password screen (the message-level leak is already closed via generic copy):
|
||||
* (a) sendResetLink() must enqueue the reset mail (never send synchronously), so SMTP
|
||||
* latency/exceptions can't distinguish a known address;
|
||||
* (b) resetPassword() must spend comparable crypto time whether the account exists / has
|
||||
* 2FA or not, so the verification branch isn't a timing oracle.
|
||||
* These tests assert the *structural* equivalence of the branches (and that mail is queued),
|
||||
* not wall-clock timing (which would be flaky).
|
||||
*/
|
||||
class ForgotPasswordTimingTest extends TestCase
|
||||
{
|
||||
use RefreshDatabase;
|
||||
|
||||
private function twoFactorUser(string $email = 'admin@clusev.local'): User
|
||||
{
|
||||
return User::factory()->create([
|
||||
'email' => $email,
|
||||
'two_factor_secret' => (new Google2FA)->generateSecretKey(),
|
||||
'two_factor_confirmed_at' => now(),
|
||||
]);
|
||||
}
|
||||
|
||||
private function attemptReset(string $email, string $code = '123456'): Testable
|
||||
{
|
||||
return Livewire::test(ForgotPassword::class)
|
||||
->set('email', $email)->set('code', $code)
|
||||
->set('password', 'NewPassw0rd123')->set('password_confirmation', 'NewPassw0rd123')
|
||||
->call('resetPassword');
|
||||
}
|
||||
|
||||
// ── (a) the reset-link mail is queued ────────────────────────────────
|
||||
public function test_reset_link_notification_is_queued(): void
|
||||
{
|
||||
config(['mail.default' => 'smtp']);
|
||||
Notification::fake();
|
||||
$user = User::factory()->create(['email' => 'admin@clusev.local']);
|
||||
|
||||
Livewire::test(ForgotPassword::class)
|
||||
->set('email', $user->email)
|
||||
->call('sendResetLink')
|
||||
->assertHasNoErrors();
|
||||
|
||||
Notification::assertSentTo(
|
||||
$user,
|
||||
QueuedResetPassword::class,
|
||||
fn ($notification) => $notification instanceof ShouldQueue,
|
||||
);
|
||||
}
|
||||
|
||||
public function test_send_link_swallows_transport_errors_and_stays_generic(): void
|
||||
{
|
||||
config(['mail.default' => 'smtp']);
|
||||
// No user exists → Password::sendResetLink hits the INVALID_USER path; the screen must
|
||||
// still report the generic message with no error, exactly like the existing-user case.
|
||||
Livewire::test(ForgotPassword::class)
|
||||
->set('email', 'ghost@clusev.local')
|
||||
->call('sendResetLink')
|
||||
->assertHasNoErrors()
|
||||
->assertDispatched('notify', message: __('auth.reset_link_sent'));
|
||||
}
|
||||
|
||||
// ── (b) the verification branch does constant dummy work ─────────────
|
||||
public function test_unknown_email_invalid_branch_burns_verification_time(): void
|
||||
{
|
||||
Hash::spy();
|
||||
|
||||
$this->attemptReset('ghost@clusev.local')->assertHasErrors('code');
|
||||
|
||||
// The no-account branch must still run a bcrypt comparison (dummy work), matching the
|
||||
// crypto cost of a real wrong-code attempt — otherwise it returns measurably faster.
|
||||
Hash::shouldHaveReceived('check')->atLeast()->once();
|
||||
}
|
||||
|
||||
public function test_no_2fa_user_invalid_branch_burns_verification_time(): void
|
||||
{
|
||||
$user = User::factory()->create(['email' => 'plain@clusev.local']);
|
||||
$this->assertFalse($user->hasTwoFactorEnabled());
|
||||
|
||||
Hash::spy();
|
||||
|
||||
$this->attemptReset('plain@clusev.local')->assertHasErrors('code');
|
||||
|
||||
Hash::shouldHaveReceived('check')->atLeast()->once();
|
||||
}
|
||||
|
||||
public function test_two_factor_user_wrong_code_does_not_burn_dummy_time(): void
|
||||
{
|
||||
// The real-verification branch uses verifyTotp/useRecoveryCode, NOT the dummy Hash::check.
|
||||
$this->twoFactorUser();
|
||||
Hash::spy();
|
||||
|
||||
$this->attemptReset('admin@clusev.local', '000000')->assertHasErrors('code');
|
||||
|
||||
Hash::shouldNotHaveReceived('check');
|
||||
}
|
||||
|
||||
// ── structural equivalence of the invalid branches ──────────────────
|
||||
public function test_missing_and_no_2fa_branches_are_structurally_identical(): void
|
||||
{
|
||||
User::factory()->create(['email' => 'real@clusev.local']);
|
||||
|
||||
$missing = $this->attemptReset('ghost@clusev.local');
|
||||
$no2fa = $this->attemptReset('real@clusev.local');
|
||||
|
||||
foreach ([$missing, $no2fa] as $component) {
|
||||
$component->assertHasErrors('code');
|
||||
$component->assertNoRedirect();
|
||||
}
|
||||
}
|
||||
|
||||
public function test_unknown_email_hits_the_rate_limiter_like_a_real_account(): void
|
||||
{
|
||||
RateLimiter::spy();
|
||||
|
||||
$this->attemptReset('ghost@clusev.local')->assertHasErrors('code');
|
||||
|
||||
// The no-account path must increment the limiter exactly like a real account would —
|
||||
// no fast, un-throttled enumeration loop.
|
||||
RateLimiter::shouldHaveReceived('hit')->atLeast()->once();
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue