fix(firewall): serialize automatic brute-force blocking
Signed-off-by: Sebastian Krupinski <krupinski01@gmail.com>
This commit is contained in:
@@ -164,7 +164,22 @@ class FirewallService
|
|||||||
);
|
);
|
||||||
|
|
||||||
if ($failureCount >= $maxFailures) {
|
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 $tenantId,
|
||||||
string $ipAddress,
|
string $ipAddress,
|
||||||
int $failureCount,
|
int $failureCount,
|
||||||
int $windowSeconds
|
int $windowSeconds,
|
||||||
|
int $blockDuration
|
||||||
): void {
|
): void {
|
||||||
// Publish brute force event
|
// Publish brute force event
|
||||||
$event = SecurityEvent::bruteForceDetected($ipAddress, $failureCount, $windowSeconds);
|
$event = SecurityEvent::bruteForceDetected($ipAddress, $failureCount, $windowSeconds);
|
||||||
$event->setTenantId($tenantId);
|
$event->setTenantId($tenantId);
|
||||||
$this->events->dispatch($event);
|
$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(
|
$this->rules->blockIp(
|
||||||
FirewallRuleScope::tenant($tenantId),
|
FirewallRuleScope::tenant($tenantId),
|
||||||
$ipAddress,
|
$ipAddress,
|
||||||
@@ -334,10 +343,12 @@ class FirewallService
|
|||||||
{
|
{
|
||||||
$expiredRules = $this->store->cleanupExpiredRules();
|
$expiredRules = $this->store->cleanupExpiredRules();
|
||||||
$oldLogs = $this->store->cleanupOldLogs(30);
|
$oldLogs = $this->store->cleanupOldLogs(30);
|
||||||
|
$expiredClaims = $this->store->cleanupExpiredBruteForceClaims();
|
||||||
|
|
||||||
return [
|
return [
|
||||||
'expiredRules' => $expiredRules,
|
'expiredRules' => $expiredRules,
|
||||||
'oldLogs' => $oldLogs,
|
'oldLogs' => $oldLogs,
|
||||||
|
'expiredBruteForceClaims' => $expiredClaims,
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ class FirewallStore
|
|||||||
{
|
{
|
||||||
protected const RULES_COLLECTION = 'firewall_rules';
|
protected const RULES_COLLECTION = 'firewall_rules';
|
||||||
protected const LOGS_COLLECTION = 'firewall_logs';
|
protected const LOGS_COLLECTION = 'firewall_logs';
|
||||||
|
protected const BRUTE_FORCE_CLAIMS_COLLECTION = 'firewall_brute_force_claims';
|
||||||
|
|
||||||
public function __construct(
|
public function __construct(
|
||||||
protected readonly DataStore $dataStore
|
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
|
* Get blocked requests count for dashboard
|
||||||
*/
|
*/
|
||||||
@@ -375,4 +418,15 @@ class FirewallStore
|
|||||||
|
|
||||||
return $result->getDeletedCount();
|
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();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -238,6 +238,33 @@ class FirewallStoreTest extends TestCase
|
|||||||
self::assertSame('event-123', $logs[0]->getEventId());
|
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(
|
private function rule(
|
||||||
string $reason,
|
string $reason,
|
||||||
string $scope,
|
string $scope,
|
||||||
|
|||||||
@@ -392,6 +392,10 @@ class FirewallServiceTest extends TestCase
|
|||||||
->method('countRecentFailures')
|
->method('countRecentFailures')
|
||||||
->with('tenant-event', '203.0.113.10', 300)
|
->with('tenant-event', '203.0.113.10', 300)
|
||||||
->willReturn(5);
|
->willReturn(5);
|
||||||
|
$this->store->expects($this->once())
|
||||||
|
->method('claimBruteForce')
|
||||||
|
->with('tenant-event', '203.0.113.10', 3600)
|
||||||
|
->willReturn(true);
|
||||||
$this->store->expects($this->once())
|
$this->store->expects($this->once())
|
||||||
->method('findExactIpRule')
|
->method('findExactIpRule')
|
||||||
->with(
|
->with(
|
||||||
@@ -431,6 +435,26 @@ class FirewallServiceTest extends TestCase
|
|||||||
self::assertSame(FirewallRuleManager::ORIGIN_AUTOMATIC, $lifecycleOrigin);
|
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')]
|
#[TestDox('Authentication events without a tenant use the current tenant')]
|
||||||
public function testAutomaticBlockTenantFallback(): void
|
public function testAutomaticBlockTenantFallback(): void
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user