]> source.dussan.org Git - nextcloud-server.git/commitdiff
Check all remotes for local access
authorJoas Schilling <coding@schilljs.com>
Tue, 24 Mar 2020 13:19:57 +0000 (14:19 +0100)
committerJoas Schilling <coding@schilljs.com>
Tue, 14 Apr 2020 16:56:06 +0000 (18:56 +0200)
Signed-off-by: Joas Schilling <coding@schilljs.com>
apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php
config/config.sample.php
lib/composer/composer/autoload_classmap.php
lib/composer/composer/autoload_static.php
lib/private/Http/Client/Client.php
lib/private/Http/Client/ClientService.php
lib/private/Server.php
lib/public/Http/Client/LocalServerException.php [new file with mode: 0644]
tests/lib/Http/Client/ClientTest.php

index fadf61fd7de7701dacb4769c334a51d7eedd6c30..8883a5d353cf709647f4a762d285d8e78c62788e 100644 (file)
@@ -32,6 +32,7 @@ use GuzzleHttp\HandlerStack;
 use GuzzleHttp\Middleware;
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCP\Http\Client\IClientService;
+use OCP\Http\Client\LocalServerException;
 use OCP\IConfig;
 use OCP\ILogger;
 use Psr\Http\Message\RequestInterface;
@@ -215,48 +216,15 @@ class RefreshWebcalService {
                        return null;
                }
 
-               if ($allowLocalAccess !== 'yes') {
-                       $host = strtolower(parse_url($url, PHP_URL_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("Subscription $subscriptionId was not refreshed because it violates local access rules");
-                               return null;
-                       }
-
-                       // Disallow hostname only
-                       if (substr_count($host, '.') === 0) {
-                               $this->logger->warning("Subscription $subscriptionId was not refreshed because it violates local access rules");
-                               return null;
-                       }
-
-                       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("Subscription $subscriptionId was not refreshed because it violates local access rules");
-                               return null;
-                       }
-
-                       // 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("Subscription $subscriptionId was not refreshed because it violates local access rules");
-                                       return null;
-                               }
-                       }
-               }
-
                try {
                        $params = [
                                'allow_redirects' => [
                                        'redirects' => 10
                                ],
                                'handler' => $handlerStack,
+                               'nextcloud' => [
+                                       'allow_local_address' => $allowLocalAccess === 'yes',
+                               ]
                        ];
 
                        $user = parse_url($subscription['source'], PHP_URL_USER);
@@ -306,9 +274,18 @@ class RefreshWebcalService {
                                        }
                                        return $vCalendar->serialize();
                        }
+               } catch (LocalServerException $ex) {
+                       $this->logger->logException($ex, [
+                               'message' => "Subscription $subscriptionId was not refreshed because it violates local access rules",
+                               'level' => ILogger::WARN,
+                       ]);
+
+                       return null;
                } catch (Exception $ex) {
-                       $this->logger->logException($ex);
-                       $this->logger->warning("Subscription $subscriptionId could not be refreshed due to a network error");
+                       $this->logger->logException($ex, [
+                               'message' => "Subscription $subscriptionId could not be refreshed due to a network error",
+                               'level' => ILogger::WARN,
+                       ]);
 
                        return null;
                }
index 9e5f7b6baf6d48b0c93df5832abd895bf5d646eb..00e3a6779fd477634bebcbaa607f1c7c5910bc2a 100644 (file)
@@ -558,6 +558,13 @@ $CONFIG = [
  */
 'proxyexclude' => [],
 
+/**
+ * Allow remote servers with local addresses e.g. in federated shares, webcal services and more
+ *
+ * Defaults to false
+ */
+'allow_local_remote_servers' => true,
+
 /**
  * Deleted Items (trash bin)
  *
index c600c15068ddd928927c96d5e56a1a3af3e2b98e..f20b42f79e9d284b1c1a6962da336b5ad77496af 100644 (file)
@@ -323,6 +323,7 @@ return array(
     'OCP\\Http\\Client\\IClient' => $baseDir . '/lib/public/Http/Client/IClient.php',
     'OCP\\Http\\Client\\IClientService' => $baseDir . '/lib/public/Http/Client/IClientService.php',
     'OCP\\Http\\Client\\IResponse' => $baseDir . '/lib/public/Http/Client/IResponse.php',
+    'OCP\\Http\\Client\\LocalServerException' => $baseDir . '/lib/public/Http/Client/LocalServerException.php',
     'OCP\\IAddressBook' => $baseDir . '/lib/public/IAddressBook.php',
     'OCP\\IAppConfig' => $baseDir . '/lib/public/IAppConfig.php',
     'OCP\\IAvatar' => $baseDir . '/lib/public/IAvatar.php',
index 87a9460f77b0020441fa3a91944782584584e1af..07f3e6a969f0f85284b21683cb24b0dceed26ce3 100644 (file)
@@ -352,6 +352,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\Http\\Client\\IClient' => __DIR__ . '/../../..' . '/lib/public/Http/Client/IClient.php',
         'OCP\\Http\\Client\\IClientService' => __DIR__ . '/../../..' . '/lib/public/Http/Client/IClientService.php',
         'OCP\\Http\\Client\\IResponse' => __DIR__ . '/../../..' . '/lib/public/Http/Client/IResponse.php',
+        'OCP\\Http\\Client\\LocalServerException' => __DIR__ . '/../../..' . '/lib/public/Http/Client/LocalServerException.php',
         'OCP\\IAddressBook' => __DIR__ . '/../../..' . '/lib/public/IAddressBook.php',
         'OCP\\IAppConfig' => __DIR__ . '/../../..' . '/lib/public/IAppConfig.php',
         'OCP\\IAvatar' => __DIR__ . '/../../..' . '/lib/public/IAvatar.php',
index 19d5877f9fbe24853d6bb5adf691358e89043757..d21ce413f1aaea0cd83f587a4421ce061112f672 100644 (file)
@@ -34,8 +34,10 @@ 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;
 
 /**
  * Class Client
@@ -47,20 +49,19 @@ class Client implements IClient {
        private $client;
        /** @var IConfig */
        private $config;
+       /** @var ILogger */
+       private $logger;
        /** @var ICertificateManager */
        private $certificateManager;
 
-       /**
-        * @param IConfig $config
-        * @param ICertificateManager $certificateManager
-        * @param GuzzleClient $client
-        */
        public function __construct(
                IConfig $config,
+               ILogger $logger,
                ICertificateManager $certificateManager,
                GuzzleClient $client
        ) {
                $this->config = $config;
+               $this->logger = $logger;
                $this->client = $client;
                $this->certificateManager = $certificateManager;
        }
@@ -144,6 +145,53 @@ class Client implements IClient {
                return $proxy;
        }
 
+       protected function preventLocalAddress(string $uri, array $options): void {
+               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) {
+                       $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');
+                       }
+               }
+       }
+
        /**
         * Sends a GET request
         *
@@ -174,6 +222,7 @@ class Client implements IClient {
         * @throws \Exception If the request could not get completed
         */
        public function get(string $uri, array $options = []): IResponse {
+               $this->preventLocalAddress($uri, $options);
                $response = $this->client->request('get', $uri, $this->buildRequestOptions($options));
                $isStream = isset($options['stream']) && $options['stream'];
                return new Response($response, $isStream);
@@ -204,6 +253,7 @@ class Client implements IClient {
         * @throws \Exception If the request could not get completed
         */
        public function head(string $uri, array $options = []): IResponse {
+               $this->preventLocalAddress($uri, $options);
                $response = $this->client->request('head', $uri, $this->buildRequestOptions($options));
                return new Response($response);
        }
@@ -238,6 +288,8 @@ class Client implements IClient {
         * @throws \Exception If the request could not get completed
         */
        public function post(string $uri, array $options = []): IResponse {
+               $this->preventLocalAddress($uri, $options);
+
                if (isset($options['body']) && is_array($options['body'])) {
                        $options['form_params'] = $options['body'];
                        unset($options['body']);
@@ -276,6 +328,7 @@ class Client implements IClient {
         * @throws \Exception If the request could not get completed
         */
        public function put(string $uri, array $options = []): IResponse {
+               $this->preventLocalAddress($uri, $options);
                $response = $this->client->request('put', $uri, $this->buildRequestOptions($options));
                return new Response($response);
        }
@@ -310,6 +363,7 @@ class Client implements IClient {
         * @throws \Exception If the request could not get completed
         */
        public function delete(string $uri, array $options = []): IResponse {
+               $this->preventLocalAddress($uri, $options);
                $response = $this->client->request('delete', $uri, $this->buildRequestOptions($options));
                return new Response($response);
        }
@@ -344,6 +398,7 @@ class Client implements IClient {
         * @throws \Exception If the request could not get completed
         */
        public function options(string $uri, array $options = []): IResponse {
+               $this->preventLocalAddress($uri, $options);
                $response = $this->client->request('options', $uri, $this->buildRequestOptions($options));
                return new Response($response);
        }
index 2b18daaf7378c4029d840cb3c610222c02def8f2..55f03f30399347a42938263d457f109c32a56e16 100644 (file)
@@ -32,6 +32,7 @@ use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\ICertificateManager;
 use OCP\IConfig;
+use OCP\ILogger;
 
 /**
  * Class ClientService
@@ -41,16 +42,16 @@ use OCP\IConfig;
 class ClientService implements IClientService {
        /** @var IConfig */
        private $config;
+       /** @var ILogger */
+       private $logger;
        /** @var ICertificateManager */
        private $certificateManager;
 
-       /**
-        * @param IConfig $config
-        * @param ICertificateManager $certificateManager
-        */
        public function __construct(IConfig $config,
+                                                               ILogger $logger,
                                                                ICertificateManager $certificateManager) {
                $this->config = $config;
+               $this->logger = $logger;
                $this->certificateManager = $certificateManager;
        }
 
@@ -58,6 +59,6 @@ class ClientService implements IClientService {
         * @return Client
         */
        public function newClient(): IClient {
-               return new Client($this->config, $this->certificateManager, new GuzzleClient());
+               return new Client($this->config, $this->logger, $this->certificateManager, new GuzzleClient());
        }
 }
index 1a3eabc852e1b111736213106e9d2fdbcd05c320..a7432342a2798a3e4c739160b20e9cb4665aa9f3 100644 (file)
@@ -804,6 +804,7 @@ class Server extends ServerContainer implements IServerContainer {
                        $uid = $user ? $user : null;
                        return new ClientService(
                                $c->getConfig(),
+                               $c->getLogger(),
                                new \OC\Security\CertificateManager(
                                        $uid,
                                        new View(),
diff --git a/lib/public/Http/Client/LocalServerException.php b/lib/public/Http/Client/LocalServerException.php
new file mode 100644 (file)
index 0000000..22e533b
--- /dev/null
@@ -0,0 +1,29 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020 Joas Schilling <coding@schilljs.com>
+ *
+ * @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 OCP\Http\Client;
+
+/**
+ * @since 19.0.0
+ */
+class LocalServerException extends \RuntimeException {
+}
index 2f9e70a8bb9ea901a43e4075a60b4ed07bb36dce..b136a0ca30087deff4420728c478d8655bcf9228 100644 (file)
@@ -11,33 +11,38 @@ namespace Test\Http\Client;
 use GuzzleHttp\Psr7\Response;
 use OC\Http\Client\Client;
 use OC\Security\CertificateManager;
+use OCP\Http\Client\LocalServerException;
 use OCP\ICertificateManager;
 use OCP\IConfig;
+use OCP\ILogger;
+use PHPUnit\Framework\MockObject\MockObject;
 
 /**
  * Class ClientTest
  */
 class ClientTest extends \Test\TestCase {
-       /** @var \GuzzleHttp\Client|\PHPUnit_Framework_MockObject_MockObject */
+       /** @var \GuzzleHttp\Client|MockObject */
        private $guzzleClient;
-       /** @var CertificateManager|\PHPUnit_Framework_MockObject_MockObject */
+       /** @var CertificateManager|MockObject */
        private $certificateManager;
        /** @var Client */
        private $client;
-       /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+       /** @var IConfig|MockObject */
        private $config;
+       /** @var ILogger|MockObject */
+       private $logger;
        /** @var array */
        private $defaultRequestOptions;
 
        protected function setUp(): void {
                parent::setUp();
                $this->config = $this->createMock(IConfig::class);
-               $this->guzzleClient = $this->getMockBuilder(\GuzzleHttp\Client::class)
-                       ->disableOriginalConstructor()
-                       ->getMock();
+               $this->logger = $this->createMock(ILogger::class);
+               $this->guzzleClient = $this->createMock(\GuzzleHttp\Client::class);
                $this->certificateManager = $this->createMock(ICertificateManager::class);
                $this->client = new Client(
                        $this->config,
+                       $this->logger,
                        $this->certificateManager,
                        $this->guzzleClient
                );
@@ -149,19 +154,127 @@ class ClientTest extends \Test\TestCase {
                ], self::invokePrivate($this->client, 'getProxyUri'));
        }
 
+       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
+               ];
+       }
+
+       /**
+        * @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')
+                       ->with('allow_local_remote_servers', false)
+                       ->willReturn(true);
+
+//             $this->expectException(LocalServerException::class);
+
+               self::invokePrivate($this->client, 'preventLocalAddress', ['http://' . $uri, []]);
+       }
+
+       /**
+        * @dataProvider dataPreventLocalAddress
+        * @param string $uri
+        */
+       public function testPreventLocalAddressDisabledByOption(string $uri): void {
+               $this->config->expects($this->never())
+                       ->method('getSystemValueBool');
+
+//             $this->expectException(LocalServerException::class);
+
+               self::invokePrivate($this->client, 'preventLocalAddress', ['http://' . $uri, [
+                       'nextcloud' => ['allow_local_address' => true],
+               ]]);
+       }
+
+       /**
+        * @dataProvider dataPreventLocalAddress
+        * @param string $uri
+        */
+       public function testPreventLocalAddressOnGet(string $uri): void {
+               $this->expectException(LocalServerException::class);
+               $this->client->get('http://' . $uri);
+       }
+
+       /**
+        * @dataProvider dataPreventLocalAddress
+        * @param string $uri
+        */
+       public function testPreventLocalAddressOnHead(string $uri): void {
+               $this->expectException(LocalServerException::class);
+               $this->client->head('http://' . $uri);
+       }
+
+       /**
+        * @dataProvider dataPreventLocalAddress
+        * @param string $uri
+        */
+       public function testPreventLocalAddressOnPost(string $uri): void {
+               $this->expectException(LocalServerException::class);
+               $this->client->post('http://' . $uri);
+       }
+
+       /**
+        * @dataProvider dataPreventLocalAddress
+        * @param string $uri
+        */
+       public function testPreventLocalAddressOnPut(string $uri): void {
+               $this->expectException(LocalServerException::class);
+               $this->client->put('http://' . $uri);
+       }
+
+       /**
+        * @dataProvider dataPreventLocalAddress
+        * @param string $uri
+        */
+       public function testPreventLocalAddressOnDelete(string $uri): void {
+               $this->expectException(LocalServerException::class);
+               $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(0))
+                       ->expects($this->at(1))
                        ->method('getSystemValue')
                        ->with('proxy', null)
                        ->willReturn('foo');
                $this->config
-                       ->expects($this->at(1))
+                       ->expects($this->at(2))
                        ->method('getSystemValue')
                        ->with('proxyuserpwd', null)
                        ->willReturn(null);
                $this->config
-                       ->expects($this->at(2))
+                       ->expects($this->at(3))
                        ->method('getSystemValue')
                        ->with('proxyexclude', [])
                        ->willReturn([]);