Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/nextcloud/server.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLukas Reschke <lukas@statuscode.ch>2021-03-23 19:41:31 +0300
committerGitHub <noreply@github.com>2021-04-06 14:37:47 +0300
commit5f3abffe6f37b4f8639fde8bcaf35d873a17636c (patch)
tree3498450ac8351f5a292dacc7cb17de9b27e4535b
parent2056b76c5fb29fa9273c50e17e54c5cf43f8a5fc (diff)
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);
+ }
+}