diff --git a/CHANGELOG.md b/CHANGELOG.md index a63660aa..5c97a1fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Changed -* *Nothing* +* [#2522](https://github.com/shlinkio/shlink/issues/2522) Shlink no longer tries to detect trusted proxies automatically, when resolving the visitor's IP address, as this is a potential security issue. + + Instead, if you have more than 1 proxy in front of Shlink, you should provide `TRUSTED_PROXIES` env var, with either a comma-separated list of the IP addresses of your proxies, or a number indicating how many proxies are there in front of Shlink. ### Deprecated * *Nothing* diff --git a/config/autoload/ip-address.global.php b/config/autoload/ip-address.global.php index 11902091..83766171 100644 --- a/config/autoload/ip-address.global.php +++ b/config/autoload/ip-address.global.php @@ -5,7 +5,6 @@ declare(strict_types=1); use RKA\Middleware\IpAddress; use RKA\Middleware\Mezzio\IpAddressFactory; use Shlinkio\Shlink\Core\Config\EnvVars; -use Shlinkio\Shlink\Core\Middleware\ReverseForwardedAddressesMiddlewareDecorator; use function Shlinkio\Shlink\Core\splitByComma; @@ -43,18 +42,6 @@ return (static function (): array { 'factories' => [ IpAddress::class => IpAddressFactory::class, ], - 'delegators' => [ - // Make middleware decoration transparent to other parts of the code - IpAddress::class => [ - fn ($c, $n, callable $callback) => - // If trusted proxies have been provided, use original middleware verbatim, otherwise decorate - // with workaround - $trustedProxies !== null - ? $callback() - : new ReverseForwardedAddressesMiddlewareDecorator($callback()), - ], - ], - ], ]; diff --git a/module/Core/src/Middleware/ReverseForwardedAddressesMiddlewareDecorator.php b/module/Core/src/Middleware/ReverseForwardedAddressesMiddlewareDecorator.php deleted file mode 100644 index 3a86b129..00000000 --- a/module/Core/src/Middleware/ReverseForwardedAddressesMiddlewareDecorator.php +++ /dev/null @@ -1,51 +0,0 @@ -hasHeader(self::FORWARDED_FOR_HEADER)) { - $request = $request->withHeader( - self::FORWARDED_FOR_HEADER, - implode(',', array_reverse(explode(',', $request->getHeaderLine(self::FORWARDED_FOR_HEADER)))), - ); - } - - return $this->wrappedMiddleware->process($request, $handler); - } -} diff --git a/module/Core/test/Middleware/ReverseForwardedAddressesMiddlewareDecoratorTest.php b/module/Core/test/Middleware/ReverseForwardedAddressesMiddlewareDecoratorTest.php deleted file mode 100644 index d3e1bd5e..00000000 --- a/module/Core/test/Middleware/ReverseForwardedAddressesMiddlewareDecoratorTest.php +++ /dev/null @@ -1,59 +0,0 @@ -decoratedMiddleware = $this->createMock(MiddlewareInterface::class); - $this->requestHandler = $this->createMock(RequestHandlerInterface::class); - $this->middleware = new ReverseForwardedAddressesMiddlewareDecorator($this->decoratedMiddleware); - } - - #[Test] - public function processesRequestAsIsWhenHeadersIsNotFound(): void - { - $request = ServerRequestFactory::fromGlobals(); - $this->decoratedMiddleware->expects($this->once())->method('process')->with( - $request, - $this->requestHandler, - )->willReturn(new Response()); - - $this->middleware->process($request, $this->requestHandler); - } - - #[Test] - public function revertsListOfAddressesWhenHeaderIsFound(): void - { - $request = ServerRequestFactory::fromGlobals()->withHeader( - ReverseForwardedAddressesMiddlewareDecorator::FORWARDED_FOR_HEADER, - '1.2.3.4,5.6.7.8,9.10.11.12', - ); - - $this->decoratedMiddleware->expects($this->once())->method('process')->with( - $this->callback(fn (ServerRequestInterface $req): bool => $req->getHeaderLine( - ReverseForwardedAddressesMiddlewareDecorator::FORWARDED_FOR_HEADER, - ) === '9.10.11.12,5.6.7.8,1.2.3.4'), - $this->requestHandler, - )->willReturn(new Response()); - - $this->middleware->process($request, $this->requestHandler); - } -}