aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLukas Reschke <lukas@statuscode.ch>2021-03-23 16:41:31 +0000
committerGitHub <noreply@github.com>2021-04-06 11:37:47 +0000
commit5f3abffe6f37b4f8639fde8bcaf35d873a17636c (patch)
tree3498450ac8351f5a292dacc7cb17de9b27e4535b
parent2056b76c5fb29fa9273c50e17e54c5cf43f8a5fc (diff)
downloadnextcloud-server-5f3abffe6f37b4f8639fde8bcaf35d873a17636c.tar.gz
nextcloud-server-5f3abffe6f37b4f8639fde8bcaf35d873a17636c.zip
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>
-rw-r--r--lib/private/Http/Client/Client.php68
-rw-r--r--lib/private/Http/Client/ClientService.php26
-rw-r--r--lib/private/Http/Client/DnsPinMiddleware.php128
-rw-r--r--lib/private/Http/Client/LocalAddressChecker.php85
-rw-r--r--lib/private/Http/Client/NegativeDnsCache.php51
-rw-r--r--lib/private/Server.php19
-rw-r--r--tests/lib/Http/Client/ClientServiceTest.php33
-rw-r--r--tests/lib/Http/Client/ClientTest.php131
-rw-r--r--tests/lib/Http/Client/LocalAddressCheckerTest.php134
-rw-r--r--tests/lib/Http/Client/NegativeDnsCacheTest.php65
10 files changed, 656 insertions, 84 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));
+ }
+}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 6a1550f83e0..c09ec0a8e18 100644
--- a/lib/private/Server.php
+++ b/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();
diff --git a/tests/lib/Http/Client/ClientServiceTest.php b/tests/lib/Http/Client/ClientServiceTest.php
index b1bc5a188ce..f529e25966e 100644
--- a/tests/lib/Http/Client/ClientServiceTest.php
+++ b/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()
);
}
diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php
index 78054725e8c..395e82b8a4c 100644
--- a/tests/lib/Http/Client/ClientTest.php
+++ b/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'
@@ -178,15 +177,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
- */
public function testPreventLocalAddressDisabledByGlobalConfig(string $uri): void {
$this->config->expects($this->once())
->method('getSystemValueBool')
@@ -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())
- ->method('getSystemValueBool')
- ->with('allow_local_remote_servers', false)
- ->willReturn(true);
- $this->config
- ->expects($this->at(1))
- ->method('getSystemValue')
- ->with('proxy', null)
- ->willReturn('foo');
+ $map = [
+ ['proxy', '', 'foo'],
+ ['proxyuserpwd', '', null],
+ ['proxyexclude', [], []],
+ ];
+
$this->config
- ->expects($this->at(2))
->method('getSystemValue')
- ->with('proxyuserpwd', null)
- ->willReturn(null);
+ ->will($this->returnValueMap($map));
$this->config
- ->expects($this->at(3))
- ->method('getSystemValue')
- ->with('proxyexclude', [])
- ->willReturn([]);
+ ->method('getSystemValueBool')
+ ->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', [[]]));
}
}
diff --git a/tests/lib/Http/Client/LocalAddressCheckerTest.php b/tests/lib/Http/Client/LocalAddressCheckerTest.php
new file mode 100644
index 00000000000..b2e09c0700b
--- /dev/null
+++ b/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'],
+ ];
+ }
+}
diff --git a/tests/lib/Http/Client/NegativeDnsCacheTest.php b/tests/lib/Http/Client/NegativeDnsCacheTest.php
new file mode 100644
index 00000000000..237e30865d0
--- /dev/null
+++ b/tests/lib/Http/Client/NegativeDnsCacheTest.php
@@ -0,0 +1,65 @@
+<?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\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->expects($this->at(0))
+ ->method('createLocal')
+ ->with('NegativeDnsCache')
+ ->willReturn($this->cache);
+
+ $this->negativeDnsCache = new NegativeDnsCache($this->cacheFactory);
+ }
+
+ public function testSetNegativeCacheForDnsType() : void {
+ $this->cache->set($this->createCacheKey($domain, $type), "true", $ttl);
+ }
+
+ public function testIsNegativeCached() {
+ $this->cache->expects($this->at(0))
+ ->method('createDistributed')
+ ->with('hasKey', 'www.example.com-0')
+ ->willReturn($imagePathCache);
+
+ $this->negativeDnsCache->hasKey('www.example.com', DNS_A);
+ }
+}