diff --git a/README.md b/README.md index c65f450..522443b 100644 --- a/README.md +++ b/README.md @@ -95,9 +95,17 @@ If required, update your `.env` file with the environmental variables found in ` ## Testing -* Code style: ``$ vendor/bin/phpcs`` -* Unit tests: ``$ vendor/bin/phpunit`` -* Code coverage: ``$ vendor/bin/phpunit --coverage-html ./build`` +* Code style: `$ vendor/bin/phpcs` +* Fix style: `$ vendor/bin/phpcbf` +* Unit tests: `$ vendor/bin/phpunit` +* Code coverage: `$ vendor/bin/phpunit --coverage-html ./build` + +You can also use Composer scripts: + +* Check both: `$ composer check` +* Code style: `$ composer cs` +* Fix style: `$ composer cs-fix` +* Unit tests: `$ composer test` [Master]: https://travis-ci.org/akrabat/ip-address-middleware diff --git a/composer.json b/composer.json index d3ab097..25fb096 100644 --- a/composer.json +++ b/composer.json @@ -37,5 +37,15 @@ "laminas": { "config-provider": "RKA\\Middleware\\Mezzio\\ConfigProvider" } + }, + "scripts": { + "test": "phpunit", + "cs": "phpcs", + "cs-fix": "phpcbf", + "code-coverage": "phpunit --coverage-html=coverage ./build", + "check": [ + "@cs", + "@test" + ] } } diff --git a/src/IpAddress.php b/src/IpAddress.php index 4216a08..102d2ef 100644 --- a/src/IpAddress.php +++ b/src/IpAddress.php @@ -27,7 +27,7 @@ class IpAddress implements MiddlewareInterface * * @var array */ - protected $trustedProxies; + protected $trustedProxies = []; /** * List of trusted proxy IP wildcard ranges @@ -83,7 +83,7 @@ public function __construct( $this->checkProxyHeaders = $checkProxyHeaders; - if ($trustedProxies) { + if (is_array($trustedProxies)) { foreach ($trustedProxies as $proxy) { if (strpos($proxy, '*') !== false) { // Wildcard IP address @@ -175,26 +175,40 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res * @param ServerRequestInterface $request PSR-7 Request * @return string */ - protected function determineClientIpAddress($request) + protected function determineClientIpAddress($request): ?string { - $ipAddress = ''; + $ipAddress = null; $serverParams = $request->getServerParams(); if (isset($serverParams['REMOTE_ADDR'])) { $remoteAddr = $this->extractIpAddress($serverParams['REMOTE_ADDR']); - if ($this->isValidIpAddress($remoteAddr)) { + if (filter_var($remoteAddr, FILTER_VALIDATE_IP)) { $ipAddress = $remoteAddr; } } + if (!$this->checkProxyHeaders) { + // do not check if configured to not check + return $ipAddress; + } - if ($this->shouldCheckProxyHeaders($ipAddress)) { - foreach ($this->headersToInspect as $header) { - if ($request->hasHeader($header)) { - $ip = $this->getFirstIpAddressFromHeader($request, $header); - if ($this->isValidIpAddress($ip)) { - $ipAddress = $ip; - break; - } + // If trustedProxies is empty, then the remote address is the trusted proxy + $trustedProxies = $this->trustedProxies; + if (empty($trustedProxies) && empty($this->trustedWildcards) && empty($this->trustedCidrs)) { + $trustedProxies[] = $ipAddress; + } + + // find the first non-empty header from the headersToInspect list and use just that one + foreach ($this->headersToInspect as $header) { + if ($request->hasHeader($header)) { + $headerValue = $request->getHeaderLine($header); + if (!empty($headerValue)) { + $ipAddress = $this->getIpAddressFromHeader( + $header, + $headerValue, + $ipAddress, + $trustedProxies + ); + break; } } } @@ -202,27 +216,61 @@ protected function determineClientIpAddress($request) return empty($ipAddress) ? null : $ipAddress; } - /** - * Determine whether we should check proxy headers for specified ip address - */ - protected function shouldCheckProxyHeaders(string $ipAddress): bool - { - //do not check if configured to not check - if (!$this->checkProxyHeaders) { - return false; + public function getIpAddressFromHeader( + string $headerName, + string $headerValue, + string $ipAddress, + array $trustedProxies + ) { + if (strtolower($headerName) == 'forwarded') { + // The Forwarded header is different, so we need to extract the for= values. Note that we perform a + // simple extraction here, and do not support the full RFC 7239 specification. + preg_match_all('/for=([^,;]+)/i', $headerValue, $matches); + $ipList = $matches[1]; + + // If any of the items in the list are not an IP address, then we ignore the entire list for now + foreach ($ipList as $ip) { + $ip = $this->extractIpAddress($ip); + if (!filter_var($ip, FILTER_VALIDATE_IP)) { + return $ipAddress; + } + } + } else { + $ipList = explode(',', $headerValue); } + $ipList[] = $ipAddress; - //if configured to check but no constraints - if (!$this->trustedProxies && !$this->trustedWildcards && !$this->trustedCidrs) { - return true; + // Remove port from each item in the list + $ipList = array_map(function ($ip) { + return $this->extractIpAddress(trim($ip)); + }, $ipList); + + // Ensure all IPs are valid and return $ipAddress if not + foreach ($ipList as $ip) { + if (!filter_var($ip, FILTER_VALIDATE_IP)) { + return $ipAddress; + } } - // Exact Match for trusted proxies - if ($this->trustedProxies && in_array($ipAddress, $this->trustedProxies)) { + // walk list from right to left removing known proxy IP addresses. + $ipList = array_reverse($ipList); + foreach ($ipList as $ip) { + $ip = trim($ip); + if (!empty($ip) && !$this->isTrustedProxy($ip, $trustedProxies)) { + return $ip; + } + } + + return $ipAddress; + } + + protected function isTrustedProxy(string $ipAddress, array $trustedProxies): bool + { + if (in_array($ipAddress, $trustedProxies)) { return true; } - // Wildcard Match + // Do we match a wildcard? if ($this->trustedWildcards) { // IPv4 has 4 parts separated by '.' // IPv6 has 8 parts separated by ':' @@ -252,7 +300,7 @@ protected function shouldCheckProxyHeaders(string $ipAddress): bool } } - // CIDR Match + // Do we match a CIDR address? if ($this->trustedCidrs) { // Only IPv4 is supported for CIDR matching $ipAsLong = ip2long($ipAddress); @@ -265,7 +313,6 @@ protected function shouldCheckProxyHeaders(string $ipAddress): bool } } - //default - not check return false; } @@ -280,48 +327,25 @@ protected function shouldCheckProxyHeaders(string $ipAddress): bool protected function extractIpAddress($ipAddress) { $parts = explode(':', $ipAddress); + if (count($parts) == 1) { + return $ipAddress; + } if (count($parts) == 2) { if (filter_var($parts[0], FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false) { return $parts[0]; } } - return $ipAddress; - } - - /** - * Check that a given string is a valid IP address - * - * @param string $ip - * @return boolean - */ - protected function isValidIpAddress(string $ip): bool - { - return filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4 | FILTER_FLAG_IPV6) !== false; - } - - /** - * Find out the client's IP address from the headers available to us - * - * @param ServerRequestInterface $request PSR-7 Request - * @param string $header Header name - * @return string - */ - private function getFirstIpAddressFromHeader(MessageInterface $request, string $header): string - { - $items = explode(',', $request->getHeaderLine($header)); - $headerValue = trim(reset($items)); - - if (ucfirst($header) == 'Forwarded') { - foreach (explode(';', $headerValue) as $headerPart) { - if (strtolower(substr($headerPart, 0, 4)) == 'for=') { - $for = explode(']', $headerPart); - $headerValue = trim(substr(reset($for), 4), " \t\n\r\0\x0B" . "\"[]"); - break; - } - } + // If the $ipAddress starts with a [ and ends with ] or ]:port, then it is an IPv6 address and + // we can extract the IP address + $ipAddress = trim($ipAddress, '"\''); + if (substr($ipAddress, 0, 1) === '[' + && (substr($ipAddress, -1) === ']' || preg_match('/\]:\d+$/', $ipAddress))) { + // Extract IPv6 address between brackets + preg_match('/\[(.*?)\]/', $ipAddress, $matches); + $ipAddress = $matches[1]; } - return $this->extractIpAddress($headerValue); + return $ipAddress; } } diff --git a/tests/IpAddressTest.php b/tests/IpAddressTest.php index 948ecdb..5cc81c6 100644 --- a/tests/IpAddressTest.php +++ b/tests/IpAddressTest.php @@ -22,24 +22,26 @@ private function simpleRequest(IPAddress $middleware, $env, $attrName = 'ip_addr return $attributeValue; } - public function testIpSetByRemoteAddr() + public function testIpAddressIsSetByRemoteAddrIfCheckProxyHeadersIsFalse() { $middleware = new IPAddress(false, [], 'IP'); $env = [ 'REMOTE_ADDR' => '192.168.1.1', + 'HTTP_X_FORWARDED_FOR' => '123.4.5.6', ]; $ipAddress = $this->simpleRequest($middleware, $env, 'IP'); $this->assertSame('192.168.1.1', $ipAddress); } - public function testIpWithPortSetByRemoteAddr() + public function testPortIsStrippedFromRemoteAddr() { - $middleware = new IPAddress(false, [], 'IP'); + $middleware = new IPAddress(false, []); $env = [ 'REMOTE_ADDR' => '192.168.1.1:80', + 'HTTP_X_FORWARDED_FOR' => '123.4.5.6', ]; - $ipAddress = $this->simpleRequest($middleware, $env, 'IP'); + $ipAddress = $this->simpleRequest($middleware, $env); $this->assertSame('192.168.1.1', $ipAddress); } @@ -138,43 +140,43 @@ public function testIpIsNullIfMissingAndProxiesAreConfigured() $this->assertSame(null, $ipAddress); } - public function testXForwardedForIp() + public function testIpAddressIsLastInXForwardedForListIfTrustedProxiesIsEmpty() { $middleware = new IPAddress(true, []); $env = [ 'REMOTE_ADDR' => '192.168.1.1', - 'HTTP_X_FORWARDED_FOR' => '192.168.1.3, 192.168.1.2, 192.168.1.1' + 'HTTP_X_FORWARDED_FOR' => '192.168.1.4, 192.168.1.3, 192.168.1.2' ]; $ipAddress = $this->simpleRequest($middleware, $env); - $this->assertSame('192.168.1.3', $ipAddress); + $this->assertSame('192.168.1.2', $ipAddress); } - public function testXForwardedForIpWithPort() + public function testPortsAreStrippedOnXForwardedFor() { $middleware = new IPAddress(true, ['192.168.1.1']); $env = [ 'REMOTE_ADDR' => '192.168.1.1:81', - 'HTTP_X_FORWARDED_FOR' => '192.168.1.3:81, 192.168.1.2:81, 192.168.1.1:81' + 'HTTP_X_FORWARDED_FOR' => '192.168.1.4:81, 192.168.1.3:81, 192.168.1.2:81' ]; $ipAddress = $this->simpleRequest($middleware, $env); - $this->assertSame('192.168.1.3', $ipAddress); + $this->assertSame('192.168.1.2', $ipAddress); } - public function testProxyIpIsIgnoredWhenNoArgumentsProvided() + public function testProxyHeaderIsIgnoredWhenNoArgumentsProvided() { $middleware = new IPAddress(); $env = [ - 'REMOTE_ADDR' => '192.168.0.1', - 'HTTP_X_FORWARDED_FOR' => '192.168.1.3, 192.168.1.2, 192.168.1.1' + 'REMOTE_ADDR' => '192.168.1.1', + 'HTTP_X_FORWARDED_FOR' => '192.168.1.4, 192.168.1.3, 192.168.1.2' ]; $ipAddress = $this->simpleRequest($middleware, $env); - $this->assertSame('192.168.0.1', $ipAddress); + $this->assertSame('192.168.1.1', $ipAddress); } - public function testHttpClientIp() + public function testHttpClientIpHeaderIsUsedWhenAvailable() { $middleware = new IPAddress(true, []); $env = [ @@ -198,19 +200,19 @@ public function testXForwardedForIpV6() $this->assertSame('001:DB8::21f:5bff:febf:ce22:8a2e', $ipAddress); } - public function testXForwardedForWithInvalidIp() + public function testXForwardedForIsIgnoredWhenItContainsAnInvalidIp() { $middleware = new IPAddress(true, []); $env = [ 'REMOTE_ADDR' => '192.168.1.1', - 'HTTP_X_FORWARDED_FOR' => 'foo-bar' + 'HTTP_X_FORWARDED_FOR' => 'foo-bar 192.168.1.2' ]; $ipAddress = $this->simpleRequest($middleware, $env); $this->assertSame('192.168.1.1', $ipAddress); } - public function testXForwardedForIpWithTrustedProxy() + public function testXForwardedForIpWithOneTrustedProxyInList() { $middleware = new IPAddress(true, ['192.168.0.1', '192.168.0.2']); $env = [ @@ -219,9 +221,24 @@ public function testXForwardedForIpWithTrustedProxy() ]; $ipAddress = $this->simpleRequest($middleware, $env); - $this->assertSame('192.168.1.3', $ipAddress); + $this->assertSame('192.168.1.1', $ipAddress); + } + + public function testXForwardedForIpWithTwoTrustedProxiesInList() + { + $middleware = new IPAddress(true, ['192.168.0.1', '192.168.0.2']); + $env = [ + 'REMOTE_ADDR' => '192.168.0.2', + 'HTTP_X_FORWARDED_FOR' => '192.168.1.3, 192.168.1.2, 192.168.0.1' + ]; + $ipAddress = $this->simpleRequest($middleware, $env); + + $this->assertSame('192.168.1.2', $ipAddress); } + /** + * If the proxy is untrusted, then we do not use the proxy list and return the REMOTE_ADDR + */ public function testXForwardedForIpWithUntrustedProxy() { $middleware = new IPAddress(true, ['192.168.0.1']); @@ -243,9 +260,13 @@ public function testForwardedWithMultipleFor() ]; $ipAddress = $this->simpleRequest($middleware, $env); - $this->assertSame('192.0.2.43', $ipAddress); + $this->assertSame('198.51.100.17', $ipAddress); } + /** + * With no trusted proxies listed, REMOTE_ADDR is assumed to be a trusted proxy and + * so the last for= entry is returned. + */ public function testForwardedWithAllOptions() { $middleware = new IPAddress(true, []); @@ -255,21 +276,40 @@ public function testForwardedWithAllOptions() ]; $ipAddress = $this->simpleRequest($middleware, $env); - $this->assertSame('192.0.2.60', $ipAddress); + $this->assertSame('192.0.2.61', $ipAddress); } + /** + * Test that we support IPV6 with a port number as the for= value + */ public function testForwardedWithWithIpV6() { $middleware = new IPAddress(true, []); $env = [ 'REMOTE_ADDR' => '192.168.1.1', - 'HTTP_FORWARDED' => 'For="[2001:db8:cafe::17]:4711", for=_internalProxy', + 'HTTP_FORWARDED' => 'For="[2001:db8:cafe::17]:4711", host=_internalProxy', ]; $ipAddress = $this->simpleRequest($middleware, $env); $this->assertSame('2001:db8:cafe::17', $ipAddress); } + /** + * If one of the for= entries is an internal proxy, then we do not support the entire + * header at this time. This could be fixed if we support a count of proxy hops. + */ + public function testForwardedWithWithAnInternalProxy() + { + $middleware = new IPAddress(true, []); + $env = [ + 'REMOTE_ADDR' => '192.168.1.1', + 'HTTP_FORWARDED' => 'For="[2001:db8:cafe::17]:4711", for=_internalProxy', + ]; + $ipAddress = $this->simpleRequest($middleware, $env); + + $this->assertSame('192.168.1.1', $ipAddress); + } + public function testCustomHeader() { $headersToInspect = [ @@ -366,4 +406,20 @@ public function testNotGivingAProxyListShouldThrowException() $this->expectException(\InvalidArgumentException::class); new IpAddress(true); } + + /** + * If the trusted proxy is not rightmost, then we ignore it and pick the rightmost + * IP address after the trusted proxies in the right position + */ + public function testThatATrustedProxiesInWrongPlaceIsIgnored() + { + $middleware = new IPAddress(true, ['192.168.0.1', '192.168.0.2']); + $env = [ + 'REMOTE_ADDR' => '192.168.0.2', + 'HTTP_X_FORWARDED_FOR' => '192.168.1.3, 192.168.0.1, 192.168.1.2' + ]; + $ipAddress = $this->simpleRequest($middleware, $env); + + $this->assertSame('192.168.1.2', $ipAddress); + } }