Merge pull request #2696 from nextcloud/bugfix/allow-list-ipv6

nextcloud · Jan 2, 2023 · d47ac0a · d47ac0a
2 parents 5bfd18d + ce2a935
commit d47ac0a
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 62 deletions.
diff --git a/lib/Middleware/WOPIMiddleware.php b/lib/Middleware/WOPIMiddleware.php
@@ -39,6 +39,7 @@
 use OCP\IConfig;
 use OCP\IRequest;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\HttpFoundation\IpUtils;
 
 class WOPIMiddleware extends Middleware {
 	/** @var IConfig */
@@ -98,71 +99,11 @@ public function isWOPIAllowed(): bool {
 		$allowedRanges = preg_split('/(\s|,|;|\|)+/', $allowedRanges);
 
 		$userIp = $this->request->getRemoteAddress();
-		foreach ($allowedRanges as $range) {
-			if ($this->matchCidr($userIp, $range)) {
-				return true;
-			}
+		if (IpUtils::checkIp($userIp, $allowedRanges)) {
+			return true;
 		}
 
 		$this->logger->info('WOPI request denied from ' . $userIp . ' as it does not match the configured ranges: ' . implode(', ', $allowedRanges));
 		return false;
 	}
-
-	/**
-	 * @copyright https://stackoverflow.com/questions/594112/matching-an-ip-to-a-cidr-mask-in-php-5/594134#594134
-	 * @copyright (IPv4) https://stackoverflow.com/questions/594112/matching-an-ip-to-a-cidr-mask-in-php-5/594134#594134
-	 * @copyright (IPv6) MW. https://stackoverflow.com/questions/7951061/matching-ipv6-address-to-a-cidr-subnet via
-	 */
-	private function matchCidr(string $ip, string $range): bool {
-		list($subnet, $bits) = array_pad(explode('/', $range), 2, null);
-		if ($bits === null) {
-			$bits = 32;
-		}
-		$bits = (int)$bits;
-
-		if ($this->isIpv4($ip) && $this->isIpv4($subnet)) {
-			$mask = -1 << (32 - $bits);
-
-			$ip = ip2long($ip);
-			$subnet = ip2long($subnet);
-			$subnet &= $mask;
-			return ($ip & $mask) === $subnet;
-		}
-
-		if ($this->isIpv6($ip) && $this->isIPv6($subnet)) {
-			$subnet = inet_pton($subnet);
-			$ip = inet_pton($ip);
-
-			$binMask = str_repeat("f", $bits / 4);
-			switch ($bits % 4) {
-				case 0:
-					break;
-				case 1:
-					$binMask .= "8";
-					break;
-				case 2:
-					$binMask .= "c";
-					break;
-				case 3:
-					$binMask .= "e";
-					break;
-			}
-
-			$binMask = str_pad($binMask, 32, '0');
-			$binMask = pack("H*", $binMask);
-
-			if (($ip & $binMask) === $subnet) {
-				return true;
-			}
-		}
-		return false;
-	}
-
-	private function isIpv4($ip) {
-		return filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4);
-	}
-
-	private function isIpv6($ip) {
-		return filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6);
-	}
 }
diff --git a/tests/lib/Middleware/WOPIMiddlewareTest.php b/tests/lib/Middleware/WOPIMiddlewareTest.php
@@ -0,0 +1,93 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2022 Julius Härtl <[email protected]>
+ *
+ * @author Julius Härtl <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+namespace OCA\Richdocuments\Middleware;
+
+use OCA\Richdocuments\Db\WopiMapper;
+use OCP\IConfig;
+use OCP\IRequest;
+use Psr\Log\LoggerInterface;
+
+class WOPIMiddlewareTest extends \PHPUnit\Framework\TestCase {
+	/**
+	 * @var IConfig|(IConfig&\PHPUnit\Framework\MockObject\MockObject)|\PHPUnit\Framework\MockObject\MockObject
+	 */
+	private $config;
+	/**
+	 * @var IRequest|(IRequest&\PHPUnit\Framework\MockObject\MockObject)|\PHPUnit\Framework\MockObject\MockObject
+	 */
+	private $request;
+	/**
+	 * @var WopiMapper|(WopiMapper&\PHPUnit\Framework\MockObject\MockObject)|\PHPUnit\Framework\MockObject\MockObject
+	 */
+	private $wopiMapper;
+	/**
+	 * @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface|(LoggerInterface&\PHPUnit\Framework\MockObject\MockObject)
+	 */
+	private $logger;
+	private WOPIMiddleware $middleware;
+
+	public function setUp(): void {
+		parent::setUp();
+		$this->config = $this->createMock(IConfig::class);
+		$this->request = $this->createMock(IRequest::class);
+		$this->wopiMapper = $this->createMock(WopiMapper::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->middleware = new WOPIMiddleware(
+			$this->config,
+			$this->request,
+			$this->wopiMapper,
+			$this->logger,
+		);
+	}
+
+	/** @dataProvider dataAllow */
+	public function testAllow($ip, $allowList, $result) {
+		$this->request->expects($this->once())
+			->method('getRemoteAddress')
+			->willReturn($ip);
+		$this->config->expects(self::any())
+			->method('getAppValue')
+			->willReturn($allowList);
+		self::assertEquals($result, $this->middleware->isWOPIAllowed());
+	}
+
+	public function dataAllow() {
+		return [
+			['192.168.178.1', '192.168.178.1', true],
+			['192.168.178.1', '192.168.178.2', false],
+			['192.168.178.1', '192.168.178.1/24', true],
+			['192.168.178.230', '192.168.178.1/24', true],
+			['192.168.179.1', '192.168.178.1/24', false],
+			['10.0.0.10', '10.0.0.0/8', true],
+			['2001:0DB8:8280:97e8:6c18:0000:a53f:0001', '2001:0DB8:8280:97e8:6c18:0000:a53f:0001', true],
+			['2001:0DB8:8280:97e8:6c18:0000:a53f:0001', '2001:0DB8:8280:97e8:6c18:0000:a53f:0001/128', true],
+			['2001:0DB8:8280:97e8:6c18:0000:a53f:0001', '2001:0DB8:8280::/48', true],
+			['2001:0DB8:8180:97e8:6c18:0000:a53f:0001', '2001:0DB8:8280::/48', false],
+			['2001:0DB8:8180:97e8:6c18:0000:a53f:0001', '2001:0DB8::/32', true],
+		];
+	}
+}
diff --git a/tests/stub.phpstub b/tests/stub.phpstub
@@ -56,3 +56,9 @@ class OC_Helper {
 	public static function getFileTemplateManager() {
 	}
 }
+
+namespace Symfony\Component\HttpFoundation {
+	class IpUtils {
+		public static function checkIp(?string $requestIp, $ips) {}
+	}
+}