Improve networking checks

Whilst we currently state that SSRF is generally outside of our threat model, this is something where we should invest to improve this.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

5 files changed, 316 insertions, 42 deletions

diff --git a/lib/private/Http/Client/Client.php b/lib/private/Http/Client/Client.php
index 12d1efb45a2..c1426d141d1 100644
--- a/lib/private/Http/Client/Client.php
+++ b/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;
-		}
-
-		$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);
+			return true;
 		}
 
-		// 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');
-		}
+		return false;
+	}
 
-		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');
+	protected function preventLocalAddress(string $uri, array $options): void {
+		if ($this->isLocalAddressAllowed($options)) {
+			return;
 		}
 
-		// 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);
 	}
 
 	/**
diff --git a/lib/private/Http/Client/ClientService.php b/lib/private/Http/Client/ClientService.php
index 3858032308a..231436004ba 100644
--- a/lib/private/Http/Client/ClientService.php
+++ b/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
+		);
 	}
 }
diff --git a/lib/private/Http/Client/DnsPinMiddleware.php b/lib/private/Http/Client/DnsPinMiddleware.php
new file mode 100644
index 00000000000..5a87ff705a6
--- /dev/null
+++ b/lib/private/Http/Client/DnsPinMiddleware.php
@@ -0,0 +1,128 @@
+<?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);
+			};
+		};
+	}
+}
diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php
new file mode 100644
index 00000000000..e9c92da4a5c
--- /dev/null
+++ b/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);
+	}
+}
diff --git a/lib/private/Http/Client/NegativeDnsCache.php b/lib/private/Http/Client/NegativeDnsCache.php
new file mode 100644
index 00000000000..550d75a9c08
--- /dev/null
+++ b/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));
+	}
+}