From 3b9a5cd5ab94644627270673bef564c2f7b729ba Mon Sep 17 00:00:00 2001 From: Sebastian Krupinski Date: Thu, 30 Jul 2026 21:54:23 -0400 Subject: [PATCH] feat(firewall): validate rules and add typed configuration Signed-off-by: Sebastian Krupinski --- .../lib/Models/Tenant/TenantConfiguration.php | 6 + core/lib/Models/Tenant/TenantFirewall.php | 35 ++++++ core/lib/Service/FirewallService.php | 84 ++++++++++++- .../php/Unit/Service/FirewallServiceTest.php | 113 +++++++++++++++++- 4 files changed, 231 insertions(+), 7 deletions(-) create mode 100644 core/lib/Models/Tenant/TenantFirewall.php diff --git a/core/lib/Models/Tenant/TenantConfiguration.php b/core/lib/Models/Tenant/TenantConfiguration.php index 182d91e..649e40e 100644 --- a/core/lib/Models/Tenant/TenantConfiguration.php +++ b/core/lib/Models/Tenant/TenantConfiguration.php @@ -11,11 +11,13 @@ class TenantConfiguration extends JsonSerializableObject { protected TenantAuthentication $authentication; protected TenantSecurity $security; + protected TenantFirewall $firewall; public function __construct() { $this->authentication = new TenantAuthentication(); $this->security = new TenantSecurity(); + $this->firewall = new TenantFirewall(); } public function authentication(): TenantAuthentication { @@ -26,4 +28,8 @@ class TenantConfiguration extends JsonSerializableObject return $this->security; } + public function firewall(): TenantFirewall { + return $this->firewall; + } + } diff --git a/core/lib/Models/Tenant/TenantFirewall.php b/core/lib/Models/Tenant/TenantFirewall.php new file mode 100644 index 0000000..57f90e1 --- /dev/null +++ b/core/lib/Models/Tenant/TenantFirewall.php @@ -0,0 +1,35 @@ +enabled; + } + + public function maxAuthFailures(): int + { + return $this->maxAuthFailures; + } + + public function authFailureWindow(): int + { + return $this->authFailureWindow; + } + + public function autoBlockDuration(): int + { + return $this->autoBlockDuration; + } +} diff --git a/core/lib/Service/FirewallService.php b/core/lib/Service/FirewallService.php index 8d56d2b..879a3cc 100644 --- a/core/lib/Service/FirewallService.php +++ b/core/lib/Service/FirewallService.php @@ -29,6 +29,10 @@ class FirewallService private const DEFAULT_MAX_AUTH_FAILURES = 5; private const DEFAULT_AUTH_FAILURE_WINDOW = 300; // 5 minutes private const DEFAULT_AUTO_BLOCK_DURATION = 3600; // 1 hour + private const MAX_AUTH_FAILURES = 1000; + private const MAX_AUTH_FAILURE_WINDOW = 86400; // 1 day + private const MAX_AUTO_BLOCK_DURATION = 31536000; // 1 year + private const MAX_DEVICE_FINGERPRINT_LENGTH = 512; // Configuration keys private const CONFIG_MAX_FAILURES = 'firewall.maxAuthFailures'; @@ -142,13 +146,15 @@ class FirewallService } // Check for brute force - $windowSeconds = $this->getConfig( + $windowSeconds = $this->getBoundedIntegerConfig( self::CONFIG_FAILURE_WINDOW, - self::DEFAULT_AUTH_FAILURE_WINDOW + self::DEFAULT_AUTH_FAILURE_WINDOW, + self::MAX_AUTH_FAILURE_WINDOW ); - $maxFailures = $this->getConfig( + $maxFailures = $this->getBoundedIntegerConfig( self::CONFIG_MAX_FAILURES, - self::DEFAULT_MAX_AUTH_FAILURES + self::DEFAULT_MAX_AUTH_FAILURES, + self::MAX_AUTH_FAILURES ); $failureCount = $this->store->countRecentFailures( @@ -179,9 +185,10 @@ class FirewallService $this->events->dispatch($event); // Auto-block the IP - $blockDuration = $this->getConfig( + $blockDuration = $this->getBoundedIntegerConfig( self::CONFIG_AUTO_BLOCK_DURATION, - self::DEFAULT_AUTO_BLOCK_DURATION + self::DEFAULT_AUTO_BLOCK_DURATION, + self::MAX_AUTO_BLOCK_DURATION ); $this->blockIp( @@ -277,6 +284,9 @@ class FirewallService ?string $createdBy = null, ?int $durationSeconds = null ): FirewallRuleObject { + $ipAddress = $this->validateIpAddress($ipAddress); + $this->validateDuration($durationSeconds); + $tenantId = $this->tenantContext->identifier(); if (!$tenantId) { throw new \RuntimeException('Cannot create firewall rule: no tenant configured'); @@ -331,6 +341,8 @@ class FirewallService ?string $reason = null, ?string $createdBy = null ): FirewallRuleObject { + $ipAddress = $this->validateIpAddress($ipAddress); + $tenantId = $this->tenantContext->identifier(); if (!$tenantId) { throw new \RuntimeException('Cannot create firewall rule: no tenant configured'); @@ -368,6 +380,8 @@ class FirewallService ?string $reason = null, ?string $createdBy = null ): FirewallRuleObject { + $cidr = $this->validateCidr($cidr); + $tenantId = $this->tenantContext->identifier(); if (!$tenantId) { throw new \RuntimeException('Cannot create firewall rule: no tenant configured'); @@ -399,6 +413,14 @@ class FirewallService ?string $createdBy = null, ?int $durationSeconds = null ): FirewallRuleObject { + $fingerprint = trim($fingerprint); + if ($fingerprint === '' || strlen($fingerprint) > self::MAX_DEVICE_FINGERPRINT_LENGTH) { + throw new \InvalidArgumentException( + sprintf('Device fingerprint must contain between 1 and %d bytes.', self::MAX_DEVICE_FINGERPRINT_LENGTH) + ); + } + $this->validateDuration($durationSeconds); + $tenantId = $this->tenantContext->identifier(); if (!$tenantId) { throw new \RuntimeException('Cannot create firewall rule: no tenant configured'); @@ -538,6 +560,9 @@ class FirewallService private function getConfig(string $key, mixed $default = null): mixed { $config = $this->tenantContext->configuration(); + if ($config instanceof \JsonSerializable) { + $config = $config->jsonSerialize(); + } $parts = explode('.', $key); foreach ($parts as $part) { @@ -550,6 +575,53 @@ class FirewallService return $config; } + private function getBoundedIntegerConfig(string $key, int $default, int $maximum): int + { + $value = $this->getConfig($key, $default); + if (!is_int($value) || $value < 1 || $value > $maximum) { + return $default; + } + + return $value; + } + + private function validateIpAddress(string $ipAddress): string + { + $ipAddress = trim($ipAddress); + if (filter_var($ipAddress, \FILTER_VALIDATE_IP) === false) { + throw new \InvalidArgumentException("Invalid IP address: {$ipAddress}"); + } + + return $ipAddress; + } + + private function validateCidr(string $cidr): string + { + $cidr = trim($cidr); + if (substr_count($cidr, '/') !== 1) { + throw new \InvalidArgumentException("Invalid CIDR range: {$cidr}"); + } + + [$address, $prefix] = explode('/', $cidr, 2); + if (filter_var($address, \FILTER_VALIDATE_IP) === false || !ctype_digit($prefix)) { + throw new \InvalidArgumentException("Invalid CIDR range: {$cidr}"); + } + + $maximumPrefix = str_contains($address, ':') ? 128 : 32; + if ((int)$prefix > $maximumPrefix) { + throw new \InvalidArgumentException("Invalid CIDR range: {$cidr}"); + } + + return $cidr; + } + + private function validateDuration(?int $durationSeconds): void + { + if ($durationSeconds !== null && $durationSeconds < 1) { + throw new \InvalidArgumentException('Firewall rule duration must be greater than zero.'); + } + } + /** * Get active rules (cached) * @return FirewallRuleObject[] diff --git a/tests/php/Unit/Service/FirewallServiceTest.php b/tests/php/Unit/Service/FirewallServiceTest.php index 14d28c3..712707c 100644 --- a/tests/php/Unit/Service/FirewallServiceTest.php +++ b/tests/php/Unit/Service/FirewallServiceTest.php @@ -6,6 +6,7 @@ namespace KTXT\Unit\Service; use KTXC\Context\TenantContextInterface; use KTXC\Models\Firewall\FirewallRuleObject; +use KTXC\Models\Tenant\TenantConfiguration; use KTXC\Service\FirewallService; use KTXC\Stores\FirewallStore; use KTXF\Event\EventDispatcherInterface; @@ -22,6 +23,7 @@ class FirewallServiceTest extends TestCase private EventDispatcherInterface&MockObject $events; private FirewallService $service; private string $currentTenant; + private ?TenantConfiguration $currentConfiguration; protected function setUp(): void { @@ -29,10 +31,13 @@ class FirewallServiceTest extends TestCase $this->tenantContext = $this->createMock(TenantContextInterface::class); $this->events = $this->createMock(EventDispatcherInterface::class); $this->currentTenant = 'tenant-a'; + $this->currentConfiguration = null; $this->tenantContext->method('identifier')->willReturnCallback( fn(): string => $this->currentTenant ); - $this->tenantContext->method('configuration')->willReturn(null); + $this->tenantContext->method('configuration')->willReturnCallback( + fn(): ?TenantConfiguration => $this->currentConfiguration + ); $this->service = new FirewallService($this->store, $this->tenantContext, $this->events); } @@ -127,6 +132,112 @@ class FirewallServiceTest extends TestCase self::assertSame('tenant-a', $rule->getTenantId()); } + #[TestDox('Malformed IP addresses are rejected before persistence')] + public function testIpValidation(): void + { + $this->store->expects($this->never())->method('depositRule'); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid IP address'); + + $this->service->blockIp('999.2.3.4'); + } + + #[TestDox('Valid IPv6 addresses can be blocked')] + public function testIpv6Validation(): void + { + $this->store->method('findExactIpRule')->willReturn(null); + $this->store->expects($this->once()) + ->method('depositRule') + ->with(self::callback(static fn(FirewallRuleObject $rule): bool => $rule->getValue() === '2001:db8::1')) + ->willReturnArgument(0); + + self::assertSame('2001:db8::1', $this->service->blockIp(' 2001:db8::1 ')->getValue()); + } + + #[TestDox('Malformed CIDR ranges are rejected before persistence')] + public function testCidrValidation(): void + { + $this->store->expects($this->never())->method('depositRule'); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid CIDR range'); + + $this->service->blockIpRange('2001:db8::/129'); + } + + #[TestDox('Valid IPv4 and IPv6 CIDR ranges are accepted')] + public function testValidCidrs(): void + { + $this->store->expects($this->exactly(2))->method('depositRule')->willReturnArgument(0); + + self::assertSame('192.0.2.0/24', $this->service->blockIpRange('192.0.2.0/24')->getValue()); + self::assertSame('2001:db8::/32', $this->service->blockIpRange('2001:db8::/32')->getValue()); + } + + #[TestDox('Temporary rules require a positive duration')] + public function testDurationValidation(): void + { + $this->store->expects($this->never())->method('depositRule'); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('greater than zero'); + + $this->service->blockIp('203.0.113.10', durationSeconds: 0); + } + + #[TestDox('Device fingerprints must be non-empty and bounded')] + public function testFingerprintValidation(): void + { + $this->store->expects($this->never())->method('depositRule'); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Device fingerprint'); + + $this->service->blockDevice(' '); + } + + #[TestDox('Typed tenant firewall settings drive brute-force thresholds')] + public function testFirewallConfiguration(): void + { + $this->currentConfiguration = (new TenantConfiguration())->jsonDeserialize([ + 'firewall' => [ + 'enabled' => true, + 'maxAuthFailures' => 8, + 'authFailureWindow' => 600, + 'autoBlockDuration' => 7200, + ], + ]); + $this->store->expects($this->once()) + ->method('countRecentFailures') + ->with('tenant-a', '203.0.113.10', 600) + ->willReturn(4); + $this->events->expects($this->never())->method('dispatch'); + + $event = \KTXF\Event\SecurityEvent::authFailure('203.0.113.10'); + $event->setTenantId('tenant-a'); + $this->service->handleAuthFailure($event); + + self::assertSame(8, $this->currentConfiguration->firewall()->maxAuthFailures()); + self::assertSame(7200, $this->currentConfiguration->firewall()->autoBlockDuration()); + } + + #[TestDox('Unsafe numeric firewall settings fall back to safe defaults')] + public function testConfigurationBounds(): void + { + $this->currentConfiguration = (new TenantConfiguration())->jsonDeserialize([ + 'firewall' => [ + 'maxAuthFailures' => 0, + 'authFailureWindow' => -1, + 'autoBlockDuration' => 0, + ], + ]); + $this->store->expects($this->once()) + ->method('countRecentFailures') + ->with('tenant-a', '203.0.113.10', 300) + ->willReturn(0); + + $event = \KTXF\Event\SecurityEvent::authFailure('203.0.113.10'); + $event->setTenantId('tenant-a'); + $this->service->handleAuthFailure($event); + } + private function rule( string $id, string $scope,