Merge pull request #32615 from sleiner/feature/ipv6-cidr-for-trusted-proxies

Support specifying IPv6 proxies in CIDR notation

2 files changed, 80 insertions, 30 deletions

diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php
index 35cd46bf68a..496a845dd4a 100644
--- a/lib/private/AppFramework/Http/Request.php
+++ b/lib/private/AppFramework/Http/Request.php
@@ -25,6 +25,7 @@ declare(strict_types=1);
  * @author Thomas Müller <thomas.mueller@tmit.eu>
  * @author Thomas Tanghus <thomas@tanghus.net>
  * @author Vincent Petry <vincent@nextcloud.com>
+ * @author Simon Leiner <simon@leiner.me>
  *
  * @license AGPL-3.0
  *
@@ -50,6 +51,7 @@ use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IRequestId;
 use OCP\Security\ICrypto;
+use Symfony\Component\HttpFoundation\IpUtils;
 
 /**
  * Class for accessing variables in the request.
@@ -573,41 +575,12 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	}
 
 	/**
-	 * 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;
+		return IpUtils::checkIp($remoteAddress, $trustedProxies);
 	}
 
 	/**
diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php
index 3289a373a12..cf5ebdca2f0 100644
--- a/tests/lib/AppFramework/Http/RequestTest.php
+++ b/tests/lib/AppFramework/Http/RequestTest.php
@@ -585,6 +585,83 @@ class RequestTest extends \Test\TestCase {
 		$this->assertSame('192.168.3.99', $request->getRemoteAddress());
 	}
 
+	public function testGetRemoteIpv6AddressWithMatchingIpv6CidrTrustedRemote() {
+		$this->config
+			->expects($this->exactly(2))
+			->method('getSystemValue')
+			->withConsecutive(
+				['trusted_proxies'],
+				['forwarded_for_headers']
+			)->willReturnOnConsecutiveCalls(
+				['2001:db8:85a3:8d3:1319:8a20::/95'],
+				['HTTP_X_FORWARDED_FOR']
+			);
+
+		$request = new Request(
+			[
+				'server' => [
+					'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a21:370:7348',
+					'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4',
+					'HTTP_X_FORWARDED_FOR' => '192.168.0.233'
+				],
+			],
+			$this->requestId,
+			$this->config,
+			$this->csrfTokenManager,
+			$this->stream
+		);
+
+		$this->assertSame('192.168.0.233', $request->getRemoteAddress());
+	}
+
+	public function testGetRemoteAddressIpv6WithNotMatchingCidrTrustedRemote() {
+		$this->config
+			->expects($this->once())
+			->method('getSystemValue')
+			->with('trusted_proxies')
+			->willReturn(['fd::/8']);
+
+		$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->requestId,
+			$this->config,
+			$this->csrfTokenManager,
+			$this->stream
+		);
+
+		$this->assertSame('2001:db8:85a3:8d3:1319:8a2e:370:7348', $request->getRemoteAddress());
+	}
+
+	public function testGetRemoteAddressIpv6WithInvalidTrustedProxy() {
+		$this->config
+			->expects($this->once())
+			->method('getSystemValue')
+			->with('trusted_proxies')
+			->willReturn(['fx::/8']);
+
+		$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->requestId,
+			$this->config,
+			$this->csrfTokenManager,
+			$this->stream
+		);
+
+		$this->assertSame('2001:db8:85a3:8d3:1319:8a2e:370:7348', $request->getRemoteAddress());
+	}
+
 	public function testGetRemoteAddressWithXForwardedForIPv6() {
 		$this->config
 			->expects($this->exactly(2))