diff --git a/core/lib/Service/FirewallService.php b/core/lib/Service/FirewallService.php index cd06315..a2e7146 100644 --- a/core/lib/Service/FirewallService.php +++ b/core/lib/Service/FirewallService.php @@ -164,7 +164,22 @@ class FirewallService ); if ($failureCount >= $maxFailures) { - $this->handleBruteForce($tenantId, $ipAddress, $failureCount, $windowSeconds); + $blockDuration = $this->getBoundedIntegerConfig( + self::CONFIG_AUTO_BLOCK_DURATION, + self::DEFAULT_AUTO_BLOCK_DURATION, + self::MAX_AUTO_BLOCK_DURATION + ); + if (!$this->store->claimBruteForce($tenantId, $ipAddress, $blockDuration)) { + return; + } + + $this->handleBruteForce( + $tenantId, + $ipAddress, + $failureCount, + $windowSeconds, + $blockDuration + ); } } @@ -175,20 +190,14 @@ class FirewallService string $tenantId, string $ipAddress, int $failureCount, - int $windowSeconds + int $windowSeconds, + int $blockDuration ): void { // Publish brute force event $event = SecurityEvent::bruteForceDetected($ipAddress, $failureCount, $windowSeconds); $event->setTenantId($tenantId); $this->events->dispatch($event); - // Auto-block the IP - $blockDuration = $this->getBoundedIntegerConfig( - self::CONFIG_AUTO_BLOCK_DURATION, - self::DEFAULT_AUTO_BLOCK_DURATION, - self::MAX_AUTO_BLOCK_DURATION - ); - $this->rules->blockIp( FirewallRuleScope::tenant($tenantId), $ipAddress, @@ -334,10 +343,12 @@ class FirewallService { $expiredRules = $this->store->cleanupExpiredRules(); $oldLogs = $this->store->cleanupOldLogs(30); + $expiredClaims = $this->store->cleanupExpiredBruteForceClaims(); return [ 'expiredRules' => $expiredRules, 'oldLogs' => $oldLogs, + 'expiredBruteForceClaims' => $expiredClaims, ]; } } diff --git a/core/lib/Stores/FirewallStore.php b/core/lib/Stores/FirewallStore.php index e8a2a02..3e4cdec 100644 --- a/core/lib/Stores/FirewallStore.php +++ b/core/lib/Stores/FirewallStore.php @@ -15,6 +15,7 @@ class FirewallStore { protected const RULES_COLLECTION = 'firewall_rules'; protected const LOGS_COLLECTION = 'firewall_logs'; + protected const BRUTE_FORCE_CLAIMS_COLLECTION = 'firewall_brute_force_claims'; public function __construct( protected readonly DataStore $dataStore @@ -343,6 +344,48 @@ class FirewallStore ]); } + /** + * Atomically claim responsibility for responding to a tenant/IP brute-force incident. + */ + public function claimBruteForce( + string $tenantId, + string $ipAddress, + int $claimDurationSeconds + ): bool { + if ($claimDurationSeconds < 1) { + throw new \InvalidArgumentException('Brute-force claim duration must be greater than zero.'); + } + + $now = new \DateTimeImmutable(); + $claimId = hash('sha256', $tenantId."\0".$ipAddress); + $collection = $this->dataStore->selectCollection(self::BRUTE_FORCE_CLAIMS_COLLECTION); + + $collection->deleteOne([ + '_id' => $claimId, + 'expiresAt' => ['$lte' => $now->format(\DateTimeInterface::ATOM)], + ]); + + try { + $collection->insertOne([ + '_id' => $claimId, + 'tenantId' => $tenantId, + 'ipAddress' => $ipAddress, + 'createdAt' => $now->format(\DateTimeInterface::ATOM), + 'expiresAt' => $now + ->modify("+{$claimDurationSeconds} seconds") + ->format(\DateTimeInterface::ATOM), + ]); + } catch (\MongoDB\Driver\Exception\BulkWriteException $error) { + if ($error->getCode() === 11000) { + return false; + } + + throw $error; + } + + return true; + } + /** * Get blocked requests count for dashboard */ @@ -375,4 +418,15 @@ class FirewallStore return $result->getDeletedCount(); } + + public function cleanupExpiredBruteForceClaims(): int + { + $result = $this->dataStore + ->selectCollection(self::BRUTE_FORCE_CLAIMS_COLLECTION) + ->deleteMany([ + 'expiresAt' => ['$lte' => (new \DateTimeImmutable())->format(\DateTimeInterface::ATOM)], + ]); + + return $result->getDeletedCount(); + } } diff --git a/tests/php/Integration/Stores/FirewallStoreTest.php b/tests/php/Integration/Stores/FirewallStoreTest.php index 62fd600..986760f 100644 --- a/tests/php/Integration/Stores/FirewallStoreTest.php +++ b/tests/php/Integration/Stores/FirewallStoreTest.php @@ -238,6 +238,33 @@ class FirewallStoreTest extends TestCase self::assertSame('event-123', $logs[0]->getEventId()); } + #[TestDox('Only one worker can claim a tenant and IP brute-force response')] + public function testBruteForceClaim(): void + { + self::assertTrue($this->store->claimBruteForce('tenant-a', '203.0.113.10', 3600)); + self::assertFalse($this->store->claimBruteForce('tenant-a', '203.0.113.10', 3600)); + self::assertTrue($this->store->claimBruteForce('tenant-b', '203.0.113.10', 3600)); + self::assertTrue($this->store->claimBruteForce('tenant-a', '203.0.113.11', 3600)); + } + + #[TestDox('Expired brute-force claims can be acquired again and cleaned up')] + public function testExpiredBruteForceClaim(): void + { + self::assertTrue($this->store->claimBruteForce('tenant-a', '203.0.113.10', 3600)); + $claimId = hash('sha256', "tenant-a\0"."203.0.113.10"); + $this->dataStore->selectCollection('firewall_brute_force_claims')->updateOne( + ['_id' => $claimId], + ['$set' => ['expiresAt' => (new \DateTimeImmutable('-1 minute'))->format(\DateTimeInterface::ATOM)]] + ); + + self::assertTrue($this->store->claimBruteForce('tenant-a', '203.0.113.10', 3600)); + $this->dataStore->selectCollection('firewall_brute_force_claims')->updateOne( + ['_id' => $claimId], + ['$set' => ['expiresAt' => (new \DateTimeImmutable('-1 minute'))->format(\DateTimeInterface::ATOM)]] + ); + self::assertSame(1, $this->store->cleanupExpiredBruteForceClaims()); + } + private function rule( string $reason, string $scope, diff --git a/tests/php/Unit/Service/FirewallServiceTest.php b/tests/php/Unit/Service/FirewallServiceTest.php index 4ca1e42..e5c4917 100644 --- a/tests/php/Unit/Service/FirewallServiceTest.php +++ b/tests/php/Unit/Service/FirewallServiceTest.php @@ -392,6 +392,10 @@ class FirewallServiceTest extends TestCase ->method('countRecentFailures') ->with('tenant-event', '203.0.113.10', 300) ->willReturn(5); + $this->store->expects($this->once()) + ->method('claimBruteForce') + ->with('tenant-event', '203.0.113.10', 3600) + ->willReturn(true); $this->store->expects($this->once()) ->method('findExactIpRule') ->with( @@ -431,6 +435,26 @@ class FirewallServiceTest extends TestCase self::assertSame(FirewallRuleManager::ORIGIN_AUTOMATIC, $lifecycleOrigin); } + #[TestDox('Workers that lose the brute-force claim do not block or publish detection events')] + public function testAutomaticBlockClaimLoss(): void + { + $this->store->method('createLogOnce')->willReturn(true); + $this->store->expects($this->once()) + ->method('countRecentFailures') + ->with('tenant-a', '203.0.113.10', 300) + ->willReturn(5); + $this->store->expects($this->once()) + ->method('claimBruteForce') + ->with('tenant-a', '203.0.113.10', 3600) + ->willReturn(false); + $this->store->expects($this->never())->method('depositRule'); + $this->events->expects($this->never())->method('dispatch'); + + $this->service->handleAuthFailure( + \KTXF\Event\SecurityEvent::authFailure('203.0.113.10') + ); + } + #[TestDox('Authentication events without a tenant use the current tenant')] public function testAutomaticBlockTenantFallback(): void {