Merge pull request #26266 from nextcloud/future-proof-networking

Improve networking checks

lib/composer/composer/autoload_classmap.php

@@ -1157,6 +1157,9 @@ return array(
     'OC\\Hooks\\PublicEmitter' => $baseDir . '/lib/private/Hooks/PublicEmitter.php',
     'OC\\Http\\Client\\Client' => $baseDir . '/lib/private/Http/Client/Client.php',
     'OC\\Http\\Client\\ClientService' => $baseDir . '/lib/private/Http/Client/ClientService.php',
+    'OC\\Http\\Client\\DnsPinMiddleware' => $baseDir . '/lib/private/Http/Client/DnsPinMiddleware.php',
+    'OC\\Http\\Client\\LocalAddressChecker' => $baseDir . '/lib/private/Http/Client/LocalAddressChecker.php',
+    'OC\\Http\\Client\\NegativeDnsCache' => $baseDir . '/lib/private/Http/Client/NegativeDnsCache.php',
     'OC\\Http\\Client\\Response' => $baseDir . '/lib/private/Http/Client/Response.php',
     'OC\\Http\\CookieHelper' => $baseDir . '/lib/private/Http/CookieHelper.php',
     'OC\\Http\\WellKnown\\RequestManager' => $baseDir . '/lib/private/Http/WellKnown/RequestManager.php',

lib/composer/composer/autoload_static.php

@@ -1186,6 +1186,9 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Hooks\\PublicEmitter' => __DIR__ . '/../../..' . '/lib/private/Hooks/PublicEmitter.php',
         'OC\\Http\\Client\\Client' => __DIR__ . '/../../..' . '/lib/private/Http/Client/Client.php',
         'OC\\Http\\Client\\ClientService' => __DIR__ . '/../../..' . '/lib/private/Http/Client/ClientService.php',
+        'OC\\Http\\Client\\DnsPinMiddleware' => __DIR__ . '/../../..' . '/lib/private/Http/Client/DnsPinMiddleware.php',
+        'OC\\Http\\Client\\LocalAddressChecker' => __DIR__ . '/../../..' . '/lib/private/Http/Client/LocalAddressChecker.php',
+        'OC\\Http\\Client\\NegativeDnsCache' => __DIR__ . '/../../..' . '/lib/private/Http/Client/NegativeDnsCache.php',
         'OC\\Http\\Client\\Response' => __DIR__ . '/../../..' . '/lib/private/Http/Client/Response.php',
         'OC\\Http\\CookieHelper' => __DIR__ . '/../../..' . '/lib/private/Http/CookieHelper.php',
         'OC\\Http\\WellKnown\\RequestManager' => __DIR__ . '/../../..' . '/lib/private/Http/WellKnown/RequestManager.php',

lib/private/Http/Client/Client.php

@@ -38,7 +38,6 @@ use GuzzleHttp\Client as GuzzleClient;
 use GuzzleHttp\RequestOptions;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IResponse;
-use OCP\Http\Client\LocalServerException;
 use OCP\ICertificateManager;
 use OCP\IConfig;
 use OCP\ILogger;
@@ -57,17 +56,21 @@ class Client implements IClient {
 	private $logger;
 	/** @var ICertificateManager */
 	private $certificateManager;
+	/** @var LocalAddressChecker */
+	private $localAddressChecker;
 
 	public function __construct(
 		IConfig $config,
 		ILogger $logger,
 		ICertificateManager $certificateManager,
-		GuzzleClient $client
+		GuzzleClient $client,
+		LocalAddressChecker $localAddressChecker
 	) {
 		$this->config = $config;
 		$this->logger = $logger;
 		$this->client = $client;
 		$this->certificateManager = $certificateManager;
+		$this->localAddressChecker = $localAddressChecker;
 	}
 
 	private function buildRequestOptions(array $options): array {
@@ -78,6 +81,21 @@ class Client implements IClient {
 			RequestOptions::TIMEOUT => 30,
 		];
 
+		$options['nextcloud']['allow_local_address'] = $this->isLocalAddressAllowed($options);
+		if ($options['nextcloud']['allow_local_address'] === false) {
+			$onRedirectFunction = function (
+				\Psr\Http\Message\RequestInterface $request,
+				\Psr\Http\Message\ResponseInterface $response,
+				\Psr\Http\Message\UriInterface $uri
+			) use ($options) {
+				$this->preventLocalAddress($uri->__toString(), $options);
+			};
+
+			$defaults[RequestOptions::ALLOW_REDIRECTS] = [
+				'on_redirect' => $onRedirectFunction
+			];
+		}
+
 		// Only add RequestOptions::PROXY if Nextcloud is explicitly
 		// configured to use a proxy. This is needed in order not to override
 		// Guzzle default values.
@@ -155,51 +173,21 @@ class Client implements IClient {
 		return $proxy;
 	}
 
-	protected function preventLocalAddress(string $uri, array $options): void {
+	private function isLocalAddressAllowed(array $options) : bool {
 		if (($options['nextcloud']['allow_local_address'] ?? false) ||
 			$this->config->getSystemValueBool('allow_local_remote_servers', false)) {
+			return true;
+		}
+
+		return false;
+	}
+
+	protected function preventLocalAddress(string $uri, array $options): void {
+		if ($this->isLocalAddressAllowed($options)) {
 			return;
 		}
 
-		$host = parse_url($uri, PHP_URL_HOST);
-		if ($host === false || $host === null) {
-			$this->logger->warning("Could not detect any host in $uri");
-			throw new LocalServerException('Could not detect any host');
-		}
-
-		$host = strtolower($host);
-		// Remove brackets from IPv6 addresses
-		if (strpos($host, '[') === 0 && substr($host, -1) === ']') {
-			$host = substr($host, 1, -1);
-		}
-
-		// Disallow localhost and local network
-		if ($host === 'localhost' || substr($host, -6) === '.local' || substr($host, -10) === '.localhost') {
-			$this->logger->warning("Host $host was not connected to because it violates local access rules");
-			throw new LocalServerException('Host violates local access rules');
-		}
-
-		// Disallow hostname only
-		if (substr_count($host, '.') === 0) {
-			$this->logger->warning("Host $host was not connected to because it violates local access rules");
-			throw new LocalServerException('Host violates local access rules');
-		}
-
-		if ((bool)filter_var($host, FILTER_VALIDATE_IP) && !filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) {
-			$this->logger->warning("Host $host was not connected to because it violates local access rules");
-			throw new LocalServerException('Host violates local access rules');
-		}
-
-		// Also check for IPv6 IPv4 nesting, because that's not covered by filter_var
-		if ((bool)filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) && substr_count($host, '.') > 0) {
-			$delimiter = strrpos($host, ':'); // Get last colon
-			$ipv4Address = substr($host, $delimiter + 1);
-
-			if (!filter_var($ipv4Address, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) {
-				$this->logger->warning("Host $host was not connected to because it violates local access rules");
-				throw new LocalServerException('Host violates local access rules');
-			}
-		}
+		$this->localAddressChecker->ThrowIfLocalAddress($uri);
 	}
 
 	/**

lib/private/Http/Client/ClientService.php

@@ -28,6 +28,8 @@ declare(strict_types=1);
 namespace OC\Http\Client;
 
 use GuzzleHttp\Client as GuzzleClient;
+use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Handler\CurlHandler;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\ICertificateManager;
@@ -46,19 +48,39 @@ class ClientService implements IClientService {
 	private $logger;
 	/** @var ICertificateManager */
 	private $certificateManager;
+	/** @var DnsPinMiddleware */
+	private $dnsPinMiddleware;
+	/** @var LocalAddressChecker */
+	private $localAddressChecker;
 
 	public function __construct(IConfig $config,
 								ILogger $logger,
-								ICertificateManager $certificateManager) {
+								ICertificateManager $certificateManager,
+								DnsPinMiddleware $dnsPinMiddleware,
+								LocalAddressChecker $localAddressChecker) {
 		$this->config = $config;
 		$this->logger = $logger;
 		$this->certificateManager = $certificateManager;
+		$this->dnsPinMiddleware = $dnsPinMiddleware;
+		$this->localAddressChecker = $localAddressChecker;
 	}
 
 	/**
 	 * @return Client
 	 */
 	public function newClient(): IClient {
-		return new Client($this->config, $this->logger, $this->certificateManager, new GuzzleClient());
+		$handler = new CurlHandler();
+		$stack = HandlerStack::create($handler);
+		$stack->push($this->dnsPinMiddleware->addDnsPinning());
+
+		$client = new GuzzleClient(['handler' => $stack]);
+
+		return new Client(
+			$this->config,
+			$this->logger,
+			$this->certificateManager,
+			$client,
+			$this->localAddressChecker
+		);
 	}
 }

lib/private/Http/Client/DnsPinMiddleware.php

@@ -0,0 +1,129 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @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 OC\Http\Client;
+
+use Psr\Http\Message\RequestInterface;
+
+class DnsPinMiddleware {
+	/** @var NegativeDnsCache */
+	private $negativeDnsCache;
+	/** @var LocalAddressChecker */
+	private $localAddressChecker;
+
+	public function __construct(
+		NegativeDnsCache $negativeDnsCache,
+		LocalAddressChecker $localAddressChecker
+	) {
+		$this->negativeDnsCache = $negativeDnsCache;
+		$this->localAddressChecker = $localAddressChecker;
+	}
+
+	private function dnsResolve(string $target, int $recursionCount) : array {
+		if ($recursionCount >= 10) {
+			return [];
+		}
+
+		$recursionCount = $recursionCount++;
+		$targetIps = [];
+
+		$soaDnsEntry = dns_get_record($target, DNS_SOA);
+		if (isset($soaDnsEntry[0]) && isset($soaDnsEntry[0]['minimum-ttl'])) {
+			$dnsNegativeTtl = $soaDnsEntry[0]['minimum-ttl'];
+		} else {
+			$dnsNegativeTtl = null;
+		}
+
+		$dnsTypes = [DNS_A, DNS_AAAA, DNS_CNAME];
+		foreach ($dnsTypes as $key => $dnsType) {
+			if ($this->negativeDnsCache->isNegativeCached($target, $dnsType)) {
+				unset($dnsTypes[$key]);
+				continue;
+			}
+
+			$dnsResponses = dns_get_record($target, $dnsType);
+			$canHaveCnameRecord = true;
+			if (count($dnsResponses) > 0) {
+				foreach ($dnsResponses as $key => $dnsResponse) {
+					if (isset($dnsResponse['ip'])) {
+						$targetIps[] = $dnsResponse['ip'];
+						$canHaveCnameRecord = false;
+					} elseif (isset($dnsResponse['ipv6'])) {
+						$targetIps[] = $dnsResponse['ipv6'];
+						$canHaveCnameRecord = false;
+					} elseif (isset($dnsResponse['target']) && $canHaveCnameRecord) {
+						$targetIps = array_merge($targetIps, $this->dnsResolve($dnsResponse['target'], $recursionCount));
+						$canHaveCnameRecord = true;
+					}
+				}
+			} else {
+				if ($dnsNegativeTtl !== null) {
+					$this->negativeDnsCache->setNegativeCacheForDnsType($target, $dnsType, $dnsNegativeTtl);
+				}
+			}
+		}
+
+		return $targetIps;
+	}
+
+	public function addDnsPinning() {
+		return function (callable $handler) {
+			return function (
+				RequestInterface $request,
+				array $options
+			) use ($handler) {
+				if ($options['nextcloud']['allow_local_address'] === true) {
+					return $handler($request, $options);
+				}
+
+				$hostName = (string)$request->getUri()->getHost();
+				$port = $request->getUri()->getPort();
+
+				$ports = [
+					'80',
+					'443',
+				];
+
+				if ($port !== null) {
+					$ports[] = (string)$port;
+				}
+
+				$targetIps = $this->dnsResolve($hostName, 0);
+
+				foreach ($targetIps as $ip) {
+					$this->localAddressChecker->ThrowIfLocalIp($ip);
+
+					foreach ($ports as $port) {
+						$curlEntry = $hostName . ':' . $port . ':' . $ip;
+						$options['curl'][CURLOPT_RESOLVE][] = $curlEntry;
+					}
+				}
+
+				return $handler($request, $options);
+			};
+		};
+	}
+}

lib/private/Http/Client/LocalAddressChecker.php

@@ -0,0 +1,85 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @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 OC\Http\Client;
+
+use OCP\ILogger;
+use OCP\Http\Client\LocalServerException;
+
+class LocalAddressChecker {
+	/** @var ILogger */
+	private $logger;
+
+	public function __construct(ILogger $logger) {
+		$this->logger = $logger;
+	}
+
+	public function ThrowIfLocalIp(string $ip) : void {
+		if ((bool)filter_var($ip, FILTER_VALIDATE_IP) && !filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) {
+			$this->logger->warning("Host $ip was not connected to because it violates local access rules");
+			throw new LocalServerException('Host violates local access rules');
+		}
+
+		// Also check for IPv6 IPv4 nesting, because that's not covered by filter_var
+		if ((bool)filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) && substr_count($ip, '.') > 0) {
+			$delimiter = strrpos($ip, ':'); // Get last colon
+			$ipv4Address = substr($ip, $delimiter + 1);
+
+			if (!filter_var($ipv4Address, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) {
+				$this->logger->warning("Host $ip was not connected to because it violates local access rules");
+				throw new LocalServerException('Host violates local access rules');
+			}
+		}
+	}
+
+	public function ThrowIfLocalAddress(string $uri) : void {
+		$host = parse_url($uri, PHP_URL_HOST);
+		if ($host === false || $host === null) {
+			$this->logger->warning("Could not detect any host in $uri");
+			throw new LocalServerException('Could not detect any host');
+		}
+
+		$host = strtolower($host);
+		// Remove brackets from IPv6 addresses
+		if (strpos($host, '[') === 0 && substr($host, -1) === ']') {
+			$host = substr($host, 1, -1);
+		}
+
+		// Disallow localhost and local network
+		if ($host === 'localhost' || substr($host, -6) === '.local' || substr($host, -10) === '.localhost') {
+			$this->logger->warning("Host $host was not connected to because it violates local access rules");
+			throw new LocalServerException('Host violates local access rules');
+		}
+
+		// Disallow hostname only
+		if (substr_count($host, '.') === 0 && !(bool)filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
+			$this->logger->warning("Host $host was not connected to because it violates local access rules");
+			throw new LocalServerException('Host violates local access rules');
+		}
+
+		$this->ThrowIfLocalIp($host);
+	}
+}

lib/private/Http/Client/NegativeDnsCache.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @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 OC\Http\Client;
+
+use OCP\ICache;
+use OCP\ICacheFactory;
+
+class NegativeDnsCache {
+	/** @var ICache */
+	private $cache;
+
+	public function __construct(ICacheFactory $memcache) {
+		$this->cache = $memcache->createLocal('NegativeDnsCache');
+	}
+
+	private function createCacheKey(string $domain, int $type) : string {
+		return $domain . "-" . (string)$type;
+	}
+
+	public function setNegativeCacheForDnsType(string $domain, int $type, int $ttl) : void {
+		$this->cache->set($this->createCacheKey($domain, $type), "true", $ttl);
+	}
+
+	public function isNegativeCached(string $domain, int $type) : bool {
+		return $this->cache->hasKey($this->createCacheKey($domain, $type));
+	}
+}

lib/private/Server.php

@@ -100,6 +100,9 @@ use OC\Files\Type\Loader;
 use OC\Files\View;
 use OC\FullTextSearch\FullTextSearchManager;
 use OC\Http\Client\ClientService;
+use OC\Http\Client\DnsPinMiddleware;
+use OC\Http\Client\LocalAddressChecker;
+use OC\Http\Client\NegativeDnsCache;
 use OC\IntegrityCheck\Checker;
 use OC\IntegrityCheck\Helpers\AppLocator;
 use OC\IntegrityCheck\Helpers\EnvironmentHelper;
@@ -822,6 +825,22 @@ class Server extends ServerContainer implements IServerContainer {
 
 		$this->registerAlias(ICertificateManager::class, CertificateManager::class);
 		$this->registerAlias(IClientService::class, ClientService::class);
+		$this->registerService(LocalAddressChecker::class, function (ContainerInterface $c) {
+			return new LocalAddressChecker(
+				$c->get(ILogger::class),
+			);
+		});
+		$this->registerService(NegativeDnsCache::class, function (ContainerInterface $c) {
+			return new NegativeDnsCache(
+				$c->get(ICacheFactory::class),
+			);
+		});
+		$this->registerService(DnsPinMiddleware::class, function (ContainerInterface $c) {
+			return new DnsPinMiddleware(
+				$c->get(NegativeDnsCache::class),
+				$c->get(LocalAddressChecker::class)
+			);
+		});
 		$this->registerDeprecatedAlias('HttpClientService', IClientService::class);
 		$this->registerService(IEventLogger::class, function (ContainerInterface $c) {
 			$eventLogger = new EventLogger();

tests/lib/Http/Client/ClientServiceTest.php

@@ -9,8 +9,12 @@
 namespace Test\Http\Client;
 
 use GuzzleHttp\Client as GuzzleClient;
+use GuzzleHttp\HandlerStack;
+use GuzzleHttp\Handler\CurlHandler;
 use OC\Http\Client\Client;
 use OC\Http\Client\ClientService;
+use OC\Http\Client\DnsPinMiddleware;
+use OC\Http\Client\LocalAddressChecker;
 use OCP\ICertificateManager;
 use OCP\IConfig;
 use OCP\ILogger;
@@ -25,10 +29,35 @@ class ClientServiceTest extends \Test\TestCase {
 		/** @var ICertificateManager $certificateManager */
 		$certificateManager = $this->createMock(ICertificateManager::class);
 		$logger = $this->createMock(ILogger::class);
+		$dnsPinMiddleware = $this->createMock(DnsPinMiddleware::class);
+		$dnsPinMiddleware
+			->expects($this->atLeastOnce())
+			->method('addDnsPinning')
+			->willReturn(function () {
+			});
+		$localAddressChecker = $this->createMock(LocalAddressChecker::class);
+
+		$clientService = new ClientService(
+			$config,
+			$logger,
+			$certificateManager,
+			$dnsPinMiddleware,
+			$localAddressChecker
+		);
+
+		$handler = new CurlHandler();
+		$stack = HandlerStack::create($handler);
+		$stack->push($dnsPinMiddleware->addDnsPinning());
+		$guzzleClient = new GuzzleClient(['handler' => $stack]);
 
-		$clientService = new ClientService($config, $logger, $certificateManager);
 		$this->assertEquals(
-			new Client($config, $logger, $certificateManager, new GuzzleClient()),
+			new Client(
+				$config,
+				$logger,
+				$certificateManager,
+				$guzzleClient,
+				$localAddressChecker
+			),
 			$clientService->newClient()
 		);
 	}

tests/lib/Http/Client/ClientTest.php

@@ -10,6 +10,7 @@ namespace Test\Http\Client;
 
 use GuzzleHttp\Psr7\Response;
 use OC\Http\Client\Client;
+use OC\Http\Client\LocalAddressChecker;
 use OC\Security\CertificateManager;
 use OCP\Http\Client\LocalServerException;
 use OCP\ICertificateManager;
@@ -31,6 +32,8 @@ class ClientTest extends \Test\TestCase {
 	private $config;
 	/** @var ILogger|MockObject */
 	private $logger;
+	/** @var LocalAddressChecker|MockObject */
+	private $localAddressChecker;
 	/** @var array */
 	private $defaultRequestOptions;
 
@@ -40,17 +43,18 @@ class ClientTest extends \Test\TestCase {
 		$this->logger = $this->createMock(ILogger::class);
 		$this->guzzleClient = $this->createMock(\GuzzleHttp\Client::class);
 		$this->certificateManager = $this->createMock(ICertificateManager::class);
+		$this->localAddressChecker = $this->createMock(LocalAddressChecker::class);
 		$this->client = new Client(
 			$this->config,
 			$this->logger,
 			$this->certificateManager,
-			$this->guzzleClient
+			$this->guzzleClient,
+			$this->localAddressChecker
 		);
 	}
 
 	public function testGetProxyUri(): void {
 		$this->config
-			->expects($this->at(0))
 			->method('getSystemValue')
 			->with('proxy', null)
 			->willReturn(null);
@@ -58,21 +62,16 @@ class ClientTest extends \Test\TestCase {
 	}
 
 	public function testGetProxyUriProxyHostEmptyPassword(): void {
+		$map = [
+			['proxy', '', 'foo'],
+			['proxyuserpwd', '', null],
+			['proxyexclude', [], []],
+		];
+
 		$this->config
-			->expects($this->at(0))
 			->method('getSystemValue')
-			->with('proxy', null)
-			->willReturn('foo');
-		$this->config
-			->expects($this->at(1))
-			->method('getSystemValue')
-			->with('proxyuserpwd', null)
-			->willReturn(null);
-		$this->config
-			->expects($this->at(2))
-			->method('getSystemValue')
-			->with('proxyexclude', [])
-			->willReturn([]);
+			->will($this->returnValueMap($map));
+
 		$this->assertEquals([
 			'http' => 'foo',
 			'https' => 'foo'
@@ -174,15 +173,6 @@ class ClientTest extends \Test\TestCase {
 		];
 	}
 
-	/**
-	 * @dataProvider dataPreventLocalAddress
-	 * @param string $uri
-	 */
-	public function testPreventLocalAddress(string $uri): void {
-		$this->expectException(LocalServerException::class);
-		self::invokePrivate($this->client, 'preventLocalAddress', ['http://' . $uri, []]);
-	}
-
 	/**
 	 * @dataProvider dataPreventLocalAddress
 	 * @param string $uri
@@ -219,6 +209,12 @@ class ClientTest extends \Test\TestCase {
 	 */
 	public function testPreventLocalAddressOnGet(string $uri): void {
 		$this->expectException(LocalServerException::class);
+		$this->localAddressChecker
+			->expects($this->once())
+			->method('ThrowIfLocalAddress')
+			->with('http://' . $uri)
+			->will($this->throwException(new LocalServerException()));
+
 		$this->client->get('http://' . $uri);
 	}
 
@@ -228,6 +224,12 @@ class ClientTest extends \Test\TestCase {
 	 */
 	public function testPreventLocalAddressOnHead(string $uri): void {
 		$this->expectException(LocalServerException::class);
+		$this->localAddressChecker
+			->expects($this->once())
+			->method('ThrowIfLocalAddress')
+			->with('http://' . $uri)
+			->will($this->throwException(new LocalServerException()));
+
 		$this->client->head('http://' . $uri);
 	}
 
@@ -237,6 +239,12 @@ class ClientTest extends \Test\TestCase {
 	 */
 	public function testPreventLocalAddressOnPost(string $uri): void {
 		$this->expectException(LocalServerException::class);
+		$this->localAddressChecker
+		->expects($this->once())
+		->method('ThrowIfLocalAddress')
+		->with('http://' . $uri)
+		->will($this->throwException(new LocalServerException()));
+	
 		$this->client->post('http://' . $uri);
 	}
 
@@ -246,6 +254,12 @@ class ClientTest extends \Test\TestCase {
 	 */
 	public function testPreventLocalAddressOnPut(string $uri): void {
 		$this->expectException(LocalServerException::class);
+		$this->localAddressChecker
+			->expects($this->once())
+			->method('ThrowIfLocalAddress')
+			->with('http://' . $uri)
+			->will($this->throwException(new LocalServerException()));
+		
 		$this->client->put('http://' . $uri);
 	}
 
@@ -255,29 +269,30 @@ class ClientTest extends \Test\TestCase {
 	 */
 	public function testPreventLocalAddressOnDelete(string $uri): void {
 		$this->expectException(LocalServerException::class);
+		$this->localAddressChecker
+			->expects($this->once())
+			->method('ThrowIfLocalAddress')
+			->with('http://' . $uri)
+			->will($this->throwException(new LocalServerException()));
+
 		$this->client->delete('http://' . $uri);
 	}
 
 	private function setUpDefaultRequestOptions(): void {
-		$this->config->expects($this->once())
+		$map = [
+			['proxy', '', 'foo'],
+			['proxyuserpwd', '', null],
+			['proxyexclude', [], []],
+		];
+
+		$this->config
+			->method('getSystemValue')
+			->will($this->returnValueMap($map));
+		$this->config
 			->method('getSystemValueBool')
-			->with('allow_local_remote_servers', false)
-			->willReturn(true);
-		$this->config
-			->expects($this->at(1))
-			->method('getSystemValue')
-			->with('proxy', null)
-			->willReturn('foo');
-		$this->config
-			->expects($this->at(2))
-			->method('getSystemValue')
-			->with('proxyuserpwd', null)
-			->willReturn(null);
-		$this->config
-			->expects($this->at(3))
-			->method('getSystemValue')
-			->with('proxyexclude', [])
-			->willReturn([]);
+		 ->with('allow_local_remote_servers', false)
+		 ->willReturn(true);
+	
 		$this->certificateManager
 			->expects($this->once())
 			->method('getAbsoluteBundlePath')
@@ -295,6 +310,9 @@ class ClientTest extends \Test\TestCase {
 				'Accept-Encoding' => 'gzip',
 			],
 			'timeout' => 30,
+			'nextcloud' => [
+				'allow_local_address' => true,
+			],
 		];
 	}
 
@@ -471,6 +489,17 @@ class ClientTest extends \Test\TestCase {
 				'Accept-Encoding' => 'gzip',
 			],
 			'timeout' => 30,
+			'nextcloud' => [
+				'allow_local_address' => false,
+			],
+			'allow_redirects' => [
+				'on_redirect' => function (
+					\Psr\Http\Message\RequestInterface $request,
+					\Psr\Http\Message\ResponseInterface $response,
+					\Psr\Http\Message\UriInterface $uri
+				) {
+				},
+			],
 		], self::invokePrivate($this->client, 'buildRequestOptions', [[]]));
 	}
 
@@ -507,6 +536,17 @@ class ClientTest extends \Test\TestCase {
 				'Accept-Encoding' => 'gzip',
 			],
 			'timeout' => 30,
+			'nextcloud' => [
+				'allow_local_address' => false,
+			],
+			'allow_redirects' => [
+				'on_redirect' => function (
+					\Psr\Http\Message\RequestInterface $request,
+					\Psr\Http\Message\ResponseInterface $response,
+					\Psr\Http\Message\UriInterface $uri
+				) {
+				},
+			],
 		], self::invokePrivate($this->client, 'buildRequestOptions', [[]]));
 	}
 
@@ -544,6 +584,17 @@ class ClientTest extends \Test\TestCase {
 				'Accept-Encoding' => 'gzip',
 			],
 			'timeout' => 30,
+			'nextcloud' => [
+				'allow_local_address' => false,
+			],
+			'allow_redirects' => [
+				'on_redirect' => function (
+					\Psr\Http\Message\RequestInterface $request,
+					\Psr\Http\Message\ResponseInterface $response,
+					\Psr\Http\Message\UriInterface $uri
+				) {
+				},
+			],
 		], self::invokePrivate($this->client, 'buildRequestOptions', [[]]));
 	}
 }

tests/lib/Http/Client/LocalAddressCheckerTest.php

@@ -0,0 +1,134 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @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 Test\Http\Client;
+
+use OCP\ILogger;
+use OCP\Http\Client\LocalServerException;
+use OC\Http\Client\LocalAddressChecker;
+
+class LocalAddressCheckerTest extends \Test\TestCase {
+	/** @var LocalAddressChecker */
+	private $localAddressChecker;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$logger = $this->createMock(ILogger::class);
+		$this->localAddressChecker = new LocalAddressChecker($logger);
+	}
+
+	/**
+	 * @dataProvider dataPreventLocalAddress
+	 * @param string $uri
+	 */
+	public function testThrowIfLocalAddress($uri) : void {
+		$this->expectException(LocalServerException::class);
+		$this->localAddressChecker->ThrowIfLocalAddress('http://' . $uri);
+	}
+
+	/**
+	 * @dataProvider dataAllowLocalAddress
+	 * @param string $uri
+	 */
+	public function testThrowIfLocalAddressGood($uri) : void {
+		$this->localAddressChecker->ThrowIfLocalAddress('http://' . $uri);
+		$this->assertTrue(true);
+	}
+
+
+	/**
+	 * @dataProvider dataInternalIPs
+	 * @param string $ip
+	 */
+	public function testThrowIfLocalIpBad($ip) : void {
+		$this->expectException(LocalServerException::class);
+		$this->localAddressChecker->ThrowIfLocalIp($ip);
+	}
+
+	/**
+	 * @dataProvider dataPublicIPs
+	 * @param string $ip
+	 */
+	public function testThrowIfLocalIpGood($ip) : void {
+		$this->localAddressChecker->ThrowIfLocalIp($ip);
+		$this->assertTrue(true);
+	}
+
+	public function dataPublicIPs() : array {
+		return [
+			['8.8.8.8'],
+			['8.8.4.4'],
+			['2001:4860:4860::8888'],
+			['2001:4860:4860::8844'],
+		];
+	}
+
+	public function dataInternalIPs() : array {
+		return [
+			['192.168.0.1'],
+			['fe80::200:5aee:feaa:20a2'],
+			['0:0:0:0:0:0:10.0.0.1'],
+			['0:0:0:0:0:ffff:127.0.0.0'],
+			['10.0.0.1'],
+			['::'],
+			['::1'],
+		];
+	}
+
+	public function dataPreventLocalAddress():array {
+		return [
+			['localhost/foo.bar'],
+			['localHost/foo.bar'],
+			['random-host/foo.bar'],
+			['[::1]/bla.blub'],
+			['[::]/bla.blub'],
+			['192.168.0.1'],
+			['172.16.42.1'],
+			['[fdf8:f53b:82e4::53]/secret.ics'],
+			['[fe80::200:5aee:feaa:20a2]/secret.ics'],
+			['[0:0:0:0:0:0:10.0.0.1]/secret.ics'],
+			['[0:0:0:0:0:ffff:127.0.0.0]/secret.ics'],
+			['10.0.0.1'],
+			['another-host.local'],
+			['service.localhost'],
+			['!@#$'], // test invalid url
+		];
+	}
+
+	public function dataAllowLocalAddress():array {
+		return [
+			['example.com/foo.bar'],
+			['example.net/foo.bar'],
+			['example.org/foo.bar'],
+			['8.8.8.8/bla.blub'],
+			['8.8.4.4/bla.blub'],
+			['8.8.8.8'],
+			['8.8.4.4'],
+			['[2001:4860:4860::8888]/secret.ics'],
+		];
+	}
+}

tests/lib/Http/Client/NegativeDnsCacheTest.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2021, Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @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 Test\Http\Client;
+
+use OC\Http\Client\NegativeDnsCache;
+use OCP\ICache;
+use OCP\ICacheFactory;
+
+class NegativeDnsCacheTest extends \Test\TestCase {
+	/** @var ICache */
+	private $cache;
+	/** @var ICacheFactory */
+	private $cacheFactory;
+	/** @var NegativeDnsCache */
+	private $negativeDnsCache;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->cache = $this->createMock(ICache::class);
+		$this->cacheFactory = $this->createMock(ICacheFactory::class);
+		$this->cacheFactory
+			->method('createLocal')
+			->with('NegativeDnsCache')
+			->willReturn($this->cache);
+		
+		$this->negativeDnsCache = new NegativeDnsCache($this->cacheFactory);
+	}
+
+	public function testSetNegativeCacheForDnsType() : void {
+		$this->cache
+			->expects($this->once())
+			->method('set')
+			->with('www.example.com-1', 'true', 3600);
+
+		$this->negativeDnsCache->setNegativeCacheForDnsType("www.example.com", DNS_A, 3600);
+	}
+
+	public function testIsNegativeCached() {
+		$this->cache
+			->expects($this->once())
+			->method('hasKey')
+			->with('www.example.com-1')
+			->willReturn(true);
+
+		$this->assertTrue($this->negativeDnsCache->isNegativeCached("www.example.com", DNS_A));
+	}
+}