From 7828f9c419664c8b26b200cdd332f369a1ce56f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9C=A0=EC=B0=BD=EC=9A=B1?= Date: Sun, 12 Jul 2026 09:09:31 +0900 Subject: [PATCH] fix(cloud): move ingest per-device rate limit post-auth, add pre-auth IP limit (H1) An unauthenticated attacker could guess a real device_id and 429-lock that camera by hammering /api/events before HMAC verification ran, dropping its accident events. Add a cheap pre-auth per-IP limiter (Config::ingestIpRateLimitPerMin, default 300/min) that fires before any device-secret lookup, and move the existing per-device limiter to run only after HMAC + timestamp + nonce checks succeed, so only authenticated requests consume a device's own quota. --- cloud/public/index.php | 2 +- cloud/src/Config.php | 1 + cloud/src/Ingest/IngestController.php | 15 ++++++++++++-- cloud/tests/IngestControllerTest.php | 30 +++++++++++++++++++++------ 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/cloud/public/index.php b/cloud/public/index.php index d3eb839..f5b9ba6 100644 --- a/cloud/public/index.php +++ b/cloud/public/index.php @@ -44,7 +44,7 @@ $render = function (string $view): Response { }; $r = new Router(); -$r->add('POST', '/api/events', fn(Request $q) => $ingest->handle($q, $now)); +$r->add('POST', '/api/events', fn(Request $q) => $ingest->handle($q, $now, (string)($_SERVER['REMOTE_ADDR'] ?? ''))); $r->add('POST', '/api/heartbeat', fn(Request $q) => $heartbeat->post($q, $now)); $r->add('GET', '/api/config', fn(Request $q) => $config->get($q)); $r->add('POST', '/login', fn(Request $q) => $auth->login($q, $sess, $now)); diff --git a/cloud/src/Config.php b/cloud/src/Config.php index 61fee96..4aa11c2 100644 --- a/cloud/src/Config.php +++ b/cloud/src/Config.php @@ -15,4 +15,5 @@ final class Config public function nonceTtl(): int { return (int)($this->a['nonce_ttl'] ?? 86400); } public function timestampWindow(): int { return (int)($this->a['timestamp_window'] ?? 300); } public function rateLimitPerMin(): int { return (int)($this->a['rate_limit_per_min'] ?? 60); } + public function ingestIpRateLimitPerMin(): int { return (int)($this->a['ingest_ip_rate_limit_per_min'] ?? 300); } } diff --git a/cloud/src/Ingest/IngestController.php b/cloud/src/Ingest/IngestController.php index 18d35aa..887d7a8 100644 --- a/cloud/src/Ingest/IngestController.php +++ b/cloud/src/Ingest/IngestController.php @@ -19,12 +19,16 @@ final class IngestController private SnapshotRepository $snapshots, ) {} - public function handle(Request $req, int $now): Response + public function handle(Request $req, int $now, string $clientIp): Response { $payload = $req->json(); $deviceId = (string)($payload['device_id'] ?? ''); $window = intdiv($now, 60); - if ($deviceId !== '' && $this->rateLimiter->hit($deviceId, $window) > $this->cfg->rateLimitPerMin()) { + + // Pre-auth IP limit (cheap, anti-flood): guards the endpoint before any + // HMAC/device-secret work, so an unauthenticated attacker can't burn + // a real device's own quota (see per-device limit below, moved post-auth). + if ($clientIp !== '' && $this->rateLimiter->hit('ip:' . $clientIp, $window) > $this->cfg->ingestIpRateLimitPerMin()) { return Response::json(['error' => 'rate_limited'], 429); } @@ -46,6 +50,13 @@ final class IngestController } $this->nonces->remember($nonce, $now + $this->cfg->nonceTtl()); + // Post-auth per-device limit: only authenticated (HMAC-verified) requests + // count toward a device's quota, so an attacker guessing device_id can no + // longer 429-lock a real camera. + if ($this->rateLimiter->hit($deviceId, $window) > $this->cfg->rateLimitPerMin()) { + return Response::json(['error' => 'rate_limited'], 429); + } + $v = EventValidator::validate($payload); if (!$v['ok']) return Response::json(['error' => 'invalid', 'details' => $v['errors']], 400); diff --git a/cloud/tests/IngestControllerTest.php b/cloud/tests/IngestControllerTest.php index 4a3ea7b..7355c65 100644 --- a/cloud/tests/IngestControllerTest.php +++ b/cloud/tests/IngestControllerTest.php @@ -39,7 +39,7 @@ final class IngestControllerTest extends TestCase public function testAcceptsValidSignedEvent(): void { [$c, $events, $snaps] = $this->make(); - $res = $c->handle($this->signedReq('sekret', $this->payload()), now: 1000); + $res = $c->handle($this->signedReq('sekret', $this->payload()), now: 1000, clientIp: '1.2.3.4'); $this->assertSame(201, $res->status()); $this->assertCount(1, $events->rows); $id = json_decode($res->body(), true)['event_id']; @@ -48,21 +48,39 @@ final class IngestControllerTest extends TestCase public function testRejectsBadSignature(): void { [$c] = $this->make(); $req = $this->signedReq('WRONG', $this->payload()); - $this->assertSame(401, $c->handle($req, now: 1000)->status()); + $this->assertSame(401, $c->handle($req, now: 1000, clientIp: '1.2.3.4')->status()); } public function testRejectsReplayedNonce(): void { [$c] = $this->make(); $req = $this->signedReq('sekret', $this->payload()); - $this->assertSame(201, $c->handle($req, now: 1000)->status()); - $this->assertSame(401, $c->handle($req, now: 1000)->status()); // same nonce again + $this->assertSame(201, $c->handle($req, now: 1000, clientIp: '1.2.3.4')->status()); + $this->assertSame(401, $c->handle($req, now: 1000, clientIp: '1.2.3.4')->status()); // same nonce again } public function testRejectsStaleTimestamp(): void { [$c] = $this->make(); $req = $this->signedReq('sekret', $this->payload(), ts: '1000'); - $this->assertSame(401, $c->handle($req, now: 5000)->status()); // 4000s > 300 window + $this->assertSame(401, $c->handle($req, now: 5000, clientIp: '1.2.3.4')->status()); // 4000s > 300 window } public function testRejectsUnknownDevice(): void { [$c] = $this->make([]); // no secrets - $this->assertSame(401, $c->handle($this->signedReq('x', $this->payload()), now: 1000)->status()); + $this->assertSame(401, $c->handle($this->signedReq('x', $this->payload()), now: 1000, clientIp: '1.2.3.4')->status()); + } + public function testIpFloodRateLimited(): void { + // Attacker without a valid device secret floods from one IP; the pre-auth + // IP limiter must trip BEFORE any HMAC/device-secret work, protecting + // real devices from having their own per-device quota consumed. + $cfg = Config::fromArray(['snapshot_dir' => $this->dir, 'timestamp_window' => 300, 'ingest_ip_rate_limit_per_min' => 2]); + $dev = new InMemoryDeviceRepository(['pi-1' => 'sekret']); + $nonce = new InMemoryNonceRepository(); + $rl = new InMemoryRateLimiter(); + $events = new InMemoryEventRepository(); + $snaps = new InMemorySnapshotRepository(); + $c = new IngestController($cfg, $dev, $nonce, $rl, $events, $snaps); + + $garbage = new Request('POST', '/api/events', [], 'not-json', [], []); + $this->assertSame(401, $c->handle($garbage, now: 1000, clientIp: '9.9.9.9')->status()); + $this->assertSame(401, $c->handle($garbage, now: 1000, clientIp: '9.9.9.9')->status()); + $res = $c->handle($garbage, now: 1000, clientIp: '9.9.9.9'); + $this->assertSame(429, $res->status()); // 3rd request from same IP: rate limited pre-auth } }