Check all remotes for local access
Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
parent
d7a74d0e35
commit
5e402f8aae
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
*
|
||||
|
|
|
@ -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',
|
||||
|
|
|
@ -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',
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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(),
|
||||
|
|
|
@ -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 {
|
||||
}
|
|
@ -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([]);
|
||||
|
|
Loading…
Reference in New Issue