From 7596d9e8240806bb0ef2ae1af23d7bbce03daa27 Mon Sep 17 00:00:00 2001 From: Andreas Maros Date: Wed, 8 Dec 2021 14:40:47 +0100 Subject: [PATCH] Allow using hashed IP addresses (#9) * [fix] Use correct psr-4 root for test sources * [feat] Add option to hash IP addresses before storing them * [refactor] Separate test setup and tests * [chore] Add tests for IP hashing * [chore] Update documentation (add `hash_ips`) --- README.md | 7 +++ composer.json | 2 +- src/RateLimitMiddleware.php | 14 +++++- src/RateLimitOptions.php | 2 + test/HashIpsTest.php | 54 ++++++++++++++++++++++ test/RateLimitTest.php | 70 +++++++--------------------- test/TestSetup.php | 92 +++++++++++++++++++++++++++++++++++++ 7 files changed, 185 insertions(+), 56 deletions(-) create mode 100644 test/HashIpsTest.php create mode 100644 test/TestSetup.php diff --git a/README.md b/README.md index ef55280..3a1872b 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,8 @@ composer require los/los-rate-limit 'reset_time' => 3600, ], ], + 'hash_ips' => false, + 'hash_salt' => 'Los%Rate', ] ``` @@ -71,6 +73,11 @@ composer require los/los-rate-limit * `forwarded_ip_index` If null (default), the first plausible IP in an XFF header (reading left to right) is used. If numeric, only a specific index of IP is used. Use `-2` to get the penultimate IP from the list, which could make sense if the header always ends `..., `. Or use `0` to use only the first IP (stopping if it's not valid). Like `prefer_forwarded`, this only makes sense if your app's always reached through a predictable hop that controls the header - remember these are easily spoofed on the initial request. * `keys` Specify different max_requests/reset_time per api key * `ips` Specify different max_requests/reset_time per IP +* `hash_ips` Enable the hashing of IP addresses before storing them. This is particularly useful when using a + filesystem-based cache implementation and working with IPv6 addresses. A salted MD5-hash will be used if you set + this to `true`. +* `hash_salt' This setting allows you to optionally define a custom salt when using hashed IP addresses. Only + effective when `hash_ips` is `true`. The values above indicate that the user can trigger 100 requests per hour. diff --git a/composer.json b/composer.json index da7cf00..75cca5f 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,7 @@ ], "autoload-dev": { "psr-4": { - "LosMiddlewareTest\\RateLimit\\": "tests/" + "LosMiddlewareTest\\RateLimit\\": "test/" } }, "autoload": { diff --git a/src/RateLimitMiddleware.php b/src/RateLimitMiddleware.php index 128fe3e..e611806 100644 --- a/src/RateLimitMiddleware.php +++ b/src/RateLimitMiddleware.php @@ -21,6 +21,7 @@ use function explode; use function filter_var; use function is_array; +use function md5; use function str_replace; use function time; @@ -74,7 +75,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface throw new MissingRequirement('Could not detect the client IP'); } - $key = str_replace('.', '-', $key); + if ($this->options['hash_ips'] === true) { + $key = $this->hashIp($key); + } else { + $key = str_replace('.', '-', $key); + } } $data = $this->storage->get($key); @@ -173,6 +178,13 @@ private function getClientIp(ServerRequestInterface $request) return null; } + private function hashIp(string $ip) : string + { + $salt = $this->options['hash_salt']; + + return md5($salt . $ip); + } + /** * @param mixed $possibleIp */ diff --git a/src/RateLimitOptions.php b/src/RateLimitOptions.php index 7b1ebd4..956be32 100644 --- a/src/RateLimitOptions.php +++ b/src/RateLimitOptions.php @@ -34,6 +34,8 @@ class RateLimitOptions extends ArrayObject ], 'keys' => [], 'ips' => [], + 'hash_ips' => false, + 'hash_salt' => 'Los%Rate', ]; /** diff --git a/test/HashIpsTest.php b/test/HashIpsTest.php new file mode 100644 index 0000000..2803980 --- /dev/null +++ b/test/HashIpsTest.php @@ -0,0 +1,54 @@ + 2, + 'reset_time' => 10, + 'ip_max_requests' => 2, + 'ip_reset_time' => 10, + 'api_header' => 'X-Api-Key', + 'trust_forwarded' => true, + 'prefer_forwarded' => false, + 'forwarded_headers_allowed' => [ + 'Client-Ip', + 'Forwarded', + 'Forwarded-For', + 'X-Cluster-Client-Ip', + 'X-Forwarded', + 'X-Forwarded-For', + ], + 'forwarded_ip_index' => null, + 'hash_ips' => true, + ]); + + $problemResponse = $this->getMockProblemResponse(); + $storage = $this->getMockStorage(); + $this->middleware = new RateLimitMiddleware($storage, $problemResponse, $options); + } + + public function testHashIp() + { + $defaultSalt = 'Los%Rate'; + $clientIp = '192.168.1.1'; + + $request = new ServerRequest(['REMOTE_ADDR' => '127.0.0.1']); + $request = $request->withHeader('X-Forwarded-For', $clientIp); + + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->method('handle')->willReturn(new JsonResponse([])); + $this->middleware->process($request, $handler); + + $this->assertArrayHasKey(\md5($defaultSalt . $clientIp), $this->cache); + } +} diff --git a/test/RateLimitTest.php b/test/RateLimitTest.php index a78b609..cac660a 100644 --- a/test/RateLimitTest.php +++ b/test/RateLimitTest.php @@ -12,44 +12,8 @@ use Laminas\Diactoros\ServerRequest; use Mezzio\ProblemDetails\ProblemDetailsResponseFactory; -class RateLimitTest extends TestCase +class RateLimitTest extends TestSetup { - /** @var array */ - private $cache = []; - /** @var RateLimitMiddleware */ - private $middleware; - - protected function setUp() : void - { - $options = new RateLimitOptions([ - 'max_requests' => 2, - 'reset_time' => 10, - 'ip_max_requests' => 2, - 'ip_reset_time' => 10, - 'api_header' => 'X-Api-Key', - 'trust_forwarded' => true, - 'prefer_forwarded' => false, - 'forwarded_headers_allowed' => [ - 'Client-Ip', - 'Forwarded', - 'Forwarded-For', - 'X-Cluster-Client-Ip', - 'X-Forwarded', - 'X-Forwarded-For', - ], - 'forwarded_ip_index' => null, - ]); - - $problemResponse = $this->createMock(ProblemDetailsResponseFactory::class); - $problemResponse->method('createResponseFromThrowable')->willReturn(new JsonResponse([], 429)); - - $storage = $this->createMock(CacheInterface::class); - $storage->method('get')->will($this->returnCallback([$this, 'getCache'])); - $storage->method('set')->will($this->returnCallback([$this, 'setCache'])); - - $this->middleware = new RateLimitMiddleware($storage, $problemResponse, $options); - } - public function testNeedIpOuApiKey() { $request = new ServerRequest(); @@ -135,6 +99,21 @@ public function testGenerate429() $this->assertSame(429, $result->getStatusCode()); } + public function testStoresUnhashedIps() + { + $clientIp = '192.168.1.1'; + $expectedCacheKey = '192-168-1-1'; + + $request = new ServerRequest(['REMOTE_ADDR' => '127.0.0.1']); + $request = $request->withHeader('X-Forwarded-For', $clientIp); + + $handler = $this->createMock(RequestHandlerInterface::class); + $handler->method('handle')->willReturn(new JsonResponse([])); + $this->middleware->process($request, $handler); + + $this->assertArrayHasKey($expectedCacheKey, $this->cache); + } + public function testReset() { // $container = new Container('LosRateLimit'); @@ -157,21 +136,4 @@ public function testReset() $this->assertLessThanOrEqual(10, $result->getHeader(RateLimitMiddleware::HEADER_RESET)[0]); $this->assertSame(200, $result->getStatusCode()); } - - /** - * @param null|mixed $default - * @return null|mixed - */ - public function getCache(string $key, $default = null) - { - return $this->cache[$key] ?? $default; - } - - /** - * @param mixed $value - */ - public function setCache(string $key, $value) : void - { - $this->cache[$key] = $value; - } } diff --git a/test/TestSetup.php b/test/TestSetup.php new file mode 100644 index 0000000..99f33e8 --- /dev/null +++ b/test/TestSetup.php @@ -0,0 +1,92 @@ + 2, + 'reset_time' => 10, + 'ip_max_requests' => 2, + 'ip_reset_time' => 10, + 'api_header' => 'X-Api-Key', + 'trust_forwarded' => true, + 'prefer_forwarded' => false, + 'forwarded_headers_allowed' => [ + 'Client-Ip', + 'Forwarded', + 'Forwarded-For', + 'X-Cluster-Client-Ip', + 'X-Forwarded', + 'X-Forwarded-For', + ], + 'forwarded_ip_index' => null, + ]); + + $problemResponse = $this->getMockProblemResponse(); + $storage = $this->getMockStorage(); + $this->middleware = new RateLimitMiddleware( + $storage, + $problemResponse, + $options + ); + } + + /** + * @param null|mixed $default + * + * @return null|mixed + */ + public function getCache(string $key, $default = null) + { + return $this->cache[$key] ?? $default; + } + + /** + * @param mixed $value + */ + public function setCache(string $key, $value): void + { + $this->cache[$key] = $value; + } + + protected function getMockProblemResponse() + { + $problemResponse = $this->createMock( + ProblemDetailsResponseFactory::class + ); + $problemResponse->method('createResponseFromThrowable')->willReturn( + new JsonResponse([], 429) + ); + + return $problemResponse; + } + + protected function getMockStorage() + { + $storage = $this->createMock(CacheInterface::class); + $storage->method('get')->will( + $this->returnCallback([$this, 'getCache']) + ); + $storage->method('set')->will( + $this->returnCallback([$this, 'setCache']) + ); + + return $storage; + } +}