boban
/
clusev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

harden(auth): BruteforceGuard review fixes (hit count, mask bounds, once-guard, mapped loopback) 

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/v1-foundation
boban 2026-06-20 17:39:01 +02:00
parent ce98fbdba0
commit 49d5c0aed3
1 changed files with 23 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
app/Services/BruteforceGuard.php
View File

@ -16,11 +16,12 @@ use Illuminate\Support\Facades\RateLimiter;
class BruteforceGuard class BruteforceGuard
{ {
/** Always exempt — hard, non-configurable (prevents loopback/health self-lockout). */ /** Always exempt — hard, non-configurable (prevents loopback/health self-lockout). */
private const LOOPBACK = ['127.0.0.0/8', '::1', '::ffff:127.0.0.1']; private const LOOPBACK = ['127.0.0.0/8', '::1', '::ffff:127.0.0.0/104'];
/** Default operator whitelist: private ranges (covers LAN/VPN + the Docker/proxy net). */ /** Default operator whitelist: private ranges (covers LAN/VPN + the Docker/proxy net). */
private const DEFAULT_WHITELIST = "10.0.0.0/8\n172.16.0.0/12\n192.168.0.0/16"; private const DEFAULT_WHITELIST = "10.0.0.0/8\n172.16.0.0/12\n192.168.0.0/16";
/** Ban-check cache TTL (seconds). Stale window on natural expiry; keep <= bantime()/2. */
private const CHECK_TTL = 30; private const CHECK_TTL = 30;
public function enabled(): bool public function enabled(): bool
@ -76,23 +77,29 @@ class BruteforceGuard
$canonical = $this->canonical($ip); $canonical = $this->canonical($ip);
$key = 'bruteforce:'.md5($canonical); $key = 'bruteforce:'.md5($canonical);
RateLimiter::hit($key, $this->findtime() * 60); // minutes → seconds $hits = RateLimiter::hit($key, $this->findtime() * 60); // minutes → seconds; hit() returns the count
$hits = RateLimiter::attempts($key);
if ($hits < $this->maxretry()) { if ($hits < $this->maxretry()) {
return; return;
} }
// Atomic upsert: two concurrent requests at the threshold must not collide on the // Ban + audit at most once per IP per window even if two requests cross the threshold
// unique ip index. Eloquent upsert does not auto-manage timestamps for bulk ops. // concurrently (the upsert is idempotent; this also de-dupes the audit row).
if (! Cache::add('bruteforce:banning:'.$canonical, true, 60)) {
return;
}
// Atomic upsert keyed on the unique ip index. Eloquent upsert needs explicit timestamps
// for the bulk path; a single $now keeps created_at/updated_at identical.
$now = now();
BannedIp::upsert( BannedIp::upsert(
[[ [[
'ip' => $canonical, 'ip' => $canonical,
'banned_until' => now()->addMinutes($this->bantime()), 'banned_until' => $now->copy()->addMinutes($this->bantime()),
'reason' => $reason, 'reason' => $reason,
'attempts' => $hits, 'attempts' => $hits,
'created_at' => now(), 'created_at' => $now,
'updated_at' => now(), 'updated_at' => $now,
]], ]],
['ip'], ['ip'],
['banned_until', 'reason', 'attempts', 'updated_at'], ['banned_until', 'reason', 'attempts', 'updated_at'],
@ -110,6 +117,11 @@ class BruteforceGuard
]); ]);
} }
/**
* Whether $ip has an active ban. Does NOT check enabled() — the BlockBannedIp middleware
* gates on enabled() before calling this, so disabling the feature stops enforcement while
* existing rows persist and resume if it is re-enabled (spec §7).
*/
public function isBanned(string $ip): bool public function isBanned(string $ip): bool
{ {
if ($this->isExempt($ip)) { if ($this->isExempt($ip)) {
@ -149,6 +161,9 @@ class BruteforceGuard
} }
$mask = (int) $maskStr; $mask = (int) $maskStr;
if ($mask > strlen($pip) * 8) {
return false; // mask wider than the address family (e.g. IPv4 /33)
}
$bytes = intdiv($mask, 8); $bytes = intdiv($mask, 8);
$rem = $mask % 8; $rem = $mask % 8;
if ($bytes > 0 && substr($pip, 0, $bytes) !== substr($psub, 0, $bytes)) { if ($bytes > 0 && substr($pip, 0, $bytes) !== substr($psub, 0, $bytes)) {