diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 1d5a29f74f..2c745973ed 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -598,6 +598,44 @@ class Request implements \ArrayAccess, \Countable, IRequest { return $this->requestId; } + /** + * Checks if given $remoteAddress matches given $trustedProxy. + * If $trustedProxy is an IPv4 IP range given in CIDR notation, true will be returned if + * $remoteAddress is an IPv4 address within that IP range. + * Otherwise $remoteAddress will be compared to $trustedProxy literally and the result + * will be returned. + * @return boolean true if $remoteAddress matches $trustedProxy, false otherwise + */ + protected function matchesTrustedProxy($trustedProxy, $remoteAddress) { + $cidrre = '/^([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})\/([0-9]{1,2})$/'; + + if (preg_match($cidrre, $trustedProxy, $match)) { + $net = $match[1]; + $shiftbits = min(32, max(0, 32 - intval($match[2]))); + $netnum = ip2long($net) >> $shiftbits; + $ipnum = ip2long($remoteAddress) >> $shiftbits; + + return $ipnum === $netnum; + } + + return $trustedProxy === $remoteAddress; + } + + /** + * Checks if given $remoteAddress matches any entry in the given array $trustedProxies. + * For details regarding what "match" means, refer to `matchesTrustedProxy`. + * @return boolean true if $remoteAddress matches any entry in $trustedProxies, false otherwise + */ + protected function isTrustedProxy($trustedProxies, $remoteAddress) { + foreach ($trustedProxies as $tp) { + if ($this->matchesTrustedProxy($tp, $remoteAddress)) { + return true; + } + } + + return false; + } + /** * Returns the remote address, if the connection came from a trusted proxy * and `forwarded_for_headers` has been configured then the IP address @@ -609,7 +647,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { $remoteAddress = isset($this->server['REMOTE_ADDR']) ? $this->server['REMOTE_ADDR'] : ''; $trustedProxies = $this->config->getSystemValue('trusted_proxies', []); - if(\is_array($trustedProxies) && \in_array($remoteAddress, $trustedProxies)) { + if(\is_array($trustedProxies) && $this->isTrustedProxy($trustedProxies, $remoteAddress)) { $forwardedForHeaders = $this->config->getSystemValue('forwarded_for_headers', [ 'HTTP_X_FORWARDED_FOR' // only have one default, so we cannot ship an insecure product out of the box diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index a715eaa959..c0e8dc97ef 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -486,6 +486,35 @@ class RequestTest extends \Test\TestCase { $this->assertSame('10.4.0.5', $request->getRemoteAddress()); } + public function testGetRemoteAddressIPv6WithSingleTrustedRemote() { + $this->config + ->expects($this->at(0)) + ->method('getSystemValue') + ->with('trusted_proxies') + ->will($this->returnValue(['2001:db8:85a3:8d3:1319:8a2e:370:7348'])); + $this->config + ->expects($this->at(1)) + ->method('getSystemValue') + ->with('forwarded_for_headers') + ->will($this->returnValue(['HTTP_X_FORWARDED'])); + + $request = new Request( + [ + 'server' => [ + 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a2e:370:7348', + 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', + 'HTTP_X_FORWARDED_FOR' => '192.168.0.233' + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ); + + $this->assertSame('10.4.0.5', $request->getRemoteAddress()); + } + public function testGetRemoteAddressVerifyPriorityHeader() { $this->config ->expects($this->at(0)) @@ -519,6 +548,92 @@ class RequestTest extends \Test\TestCase { $this->assertSame('192.168.0.233', $request->getRemoteAddress()); } + public function testGetRemoteAddressIPv6VerifyPriorityHeader() { + $this->config + ->expects($this->at(0)) + ->method('getSystemValue') + ->with('trusted_proxies') + ->will($this->returnValue(['2001:db8:85a3:8d3:1319:8a2e:370:7348'])); + $this->config + ->expects($this->at(1)) + ->method('getSystemValue') + ->with('forwarded_for_headers') + ->will($this->returnValue([ + 'HTTP_CLIENT_IP', + 'HTTP_X_FORWARDED_FOR', + 'HTTP_X_FORWARDED' + ])); + + $request = new Request( + [ + 'server' => [ + 'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a2e:370:7348', + 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', + 'HTTP_X_FORWARDED_FOR' => '192.168.0.233' + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ); + + $this->assertSame('192.168.0.233', $request->getRemoteAddress()); + } + + public function testGetRemoteAddressWithMatchingCidrTrustedRemote() { + $this->config + ->expects($this->at(0)) + ->method('getSystemValue') + ->with('trusted_proxies') + ->will($this->returnValue(['192.168.2.0/24'])); + $this->config + ->expects($this->at(1)) + ->method('getSystemValue') + ->with('forwarded_for_headers') + ->will($this->returnValue(['HTTP_X_FORWARDED_FOR'])); + + $request = new Request( + [ + 'server' => [ + 'REMOTE_ADDR' => '192.168.2.99', + 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', + 'HTTP_X_FORWARDED_FOR' => '192.168.0.233' + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ); + + $this->assertSame('192.168.0.233', $request->getRemoteAddress()); + } + + public function testGetRemoteAddressWithNotMatchingCidrTrustedRemote() { + $this->config + ->expects($this->once()) + ->method('getSystemValue') + ->with('trusted_proxies') + ->will($this->returnValue(['192.168.2.0/24'])); + + $request = new Request( + [ + 'server' => [ + 'REMOTE_ADDR' => '192.168.3.99', + 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4', + 'HTTP_X_FORWARDED_FOR' => '192.168.0.233' + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ); + + $this->assertSame('192.168.3.99', $request->getRemoteAddress()); + } + /** * @return array */