diff options
28 files changed, 890 insertions, 475 deletions
diff --git a/apps/dav/lib/CalDAV/Activity/Provider/Base.php b/apps/dav/lib/CalDAV/Activity/Provider/Base.php index 48ed7b8b107..672129a8311 100644 --- a/apps/dav/lib/CalDAV/Activity/Provider/Base.php +++ b/apps/dav/lib/CalDAV/Activity/Provider/Base.php @@ -38,9 +38,6 @@ abstract class Base implements IProvider { /** @var IUserManager */ protected $userManager; - /** @var string[] */ - protected $userDisplayNames = []; - /** @var IGroupManager */ protected $groupManager; diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index b00a4f9d6ed..a894b65d756 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -417,7 +417,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription [, $name] = Uri\split($row['principaluri']); $uri = $row['uri'] . '_shared_by_' . $name; - $row['displayname'] = $row['displayname'] . ' (' . $this->getUserDisplayName($name) . ')'; + $row['displayname'] = $row['displayname'] . ' (' . ($this->userManager->getDisplayName($name) ?? ($name ?? '')) . ')'; $components = []; if ($row['components']) { $components = explode(',',$row['components']); @@ -493,25 +493,6 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription return array_values($calendars); } - - /** - * @param $uid - * @return string - */ - private function getUserDisplayName($uid) { - if (!isset($this->userDisplayNames[$uid])) { - $user = $this->userManager->get($uid); - - if ($user instanceof IUser) { - $this->userDisplayNames[$uid] = $user->getDisplayName(); - } else { - $this->userDisplayNames[$uid] = $uid; - } - } - - return $this->userDisplayNames[$uid]; - } - /** * @return array */ diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php index 2c7b06a4396..515072fd227 100644 --- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php +++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php @@ -178,12 +178,7 @@ class IMipPlugin extends SabreIMipPlugin { $recipientName = $iTipMessage->recipientName ?: null; if ($senderName === null || empty(trim($senderName))) { - $user = $this->userManager->get($this->userId); - if ($user) { - // getDisplayName automatically uses the uid - // if no display-name is set - $senderName = $user->getDisplayName(); - } + $senderName = $this->userManager->getDisplayName($this->userId); } /** @var VEvent $vevent */ @@ -225,7 +220,7 @@ class IMipPlugin extends SabreIMipPlugin { ]; $fromEMail = Util::getDefaultEmailAddress('invitations-noreply'); - $fromName = $l10n->t('%1$s via %2$s', [$senderName, $this->defaults->getName()]); + $fromName = $l10n->t('%1$s via %2$s', [$senderName ?? $this->userId, $this->defaults->getName()]); $message = $this->mailer->createMessage() ->setFrom([$fromEMail => $fromName]) diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index 72499ad3037..03ddb93c084 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -207,7 +207,7 @@ class CardDavBackend implements BackendInterface, SyncSupport { [, $name] = \Sabre\Uri\split($row['principaluri']); $uri = $row['uri'] . '_shared_by_' . $name; - $displayName = $row['displayname'] . ' (' . $this->getUserDisplayName($name) . ')'; + $displayName = $row['displayname'] . ' (' . ($this->userManager->getDisplayName($name) ?? $name ?? '') . ')'; $addressBooks[$row['id']] = [ 'id' => $row['id'], @@ -256,20 +256,6 @@ class CardDavBackend implements BackendInterface, SyncSupport { return array_values($addressBooks); } - private function getUserDisplayName($uid) { - if (!isset($this->userDisplayNames[$uid])) { - $user = $this->userManager->get($uid); - - if ($user instanceof IUser) { - $this->userDisplayNames[$uid] = $user->getDisplayName(); - } else { - $this->userDisplayNames[$uid] = $uid; - } - } - - return $this->userDisplayNames[$uid]; - } - /** * @param int $addressBookId */ diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php index 0d8076f7aa4..0b64704a87e 100644 --- a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php +++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php @@ -183,13 +183,10 @@ class IMipPluginTest extends TestCase { $message = $this->_testMessage(); $message->senderName = null; - $user = $this->createMock(IUser::class); - $user->method('getDisplayName')->willReturn('Mr. Wizard'); - $this->userManager->expects($this->once()) - ->method('get') + ->method('getDisplayName') ->with('user123') - ->willReturn($user); + ->willReturn('Mr. Wizard'); $this->_expectSend(); $this->plugin->schedule($message); diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 7ce93ebff71..9e50ec46011 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2777,19 +2777,12 @@ <InvalidOperand occurrences="1"> <code>$result</code> </InvalidOperand> - <InvalidReturnStatement occurrences="3"> - <code>$helper->getFileSize($fullPath)</code> + <InvalidReturnStatement occurrences="1"> <code>$result</code> - <code>$space</code> </InvalidReturnStatement> - <InvalidReturnType occurrences="3"> - <code>filesize</code> - <code>free_space</code> + <InvalidReturnType occurrences="1"> <code>rename</code> </InvalidReturnType> - <NullableReturnStatement occurrences="1"> - <code>$helper->getFileSize($fullPath)</code> - </NullableReturnStatement> <TypeDoesNotContainNull occurrences="2"> <code>$space === false || is_null($space)</code> <code>is_null($space)</code> diff --git a/lib/base.php b/lib/base.php index 04c456a12d8..fa69a7d6076 100644 --- a/lib/base.php +++ b/lib/base.php @@ -662,9 +662,20 @@ class OC { $config = \OC::$server->get(\OCP\IConfig::class); if (!defined('PHPUNIT_RUN')) { - OC\Log\ErrorHandler::setLogger(\OC::$server->getLogger()); - $debug = $config->getSystemValue('debug', false); - OC\Log\ErrorHandler::register($debug); + $errorHandler = new OC\Log\ErrorHandler( + \OCP\Server::get(\Psr\Log\LoggerInterface::class), + ); + $exceptionHandler = [$errorHandler, 'onException']; + if ($config->getSystemValue('debug', false)) { + set_error_handler([$errorHandler, 'onAll'], E_ALL); + if (\OC::$CLI) { + $exceptionHandler = ['OC_Template', 'printExceptionErrorPage']; + } + } else { + set_error_handler([$errorHandler, 'onError']); + } + register_shutdown_function([$errorHandler, 'onShutdown']); + set_exception_handler($exceptionHandler); } /** @var \OC\AppFramework\Bootstrap\Coordinator $bootstrapCoordinator */ diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 2b5475f6efb..397f36e660b 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -535,6 +535,7 @@ return array( 'OCP\\Security\\ICredentialsManager' => $baseDir . '/lib/public/Security/ICredentialsManager.php', 'OCP\\Security\\ICrypto' => $baseDir . '/lib/public/Security/ICrypto.php', 'OCP\\Security\\IHasher' => $baseDir . '/lib/public/Security/IHasher.php', + 'OCP\\Security\\IRemoteHostValidator' => $baseDir . '/lib/public/Security/IRemoteHostValidator.php', 'OCP\\Security\\ISecureRandom' => $baseDir . '/lib/public/Security/ISecureRandom.php', 'OCP\\Security\\ITrustedDomainHelper' => $baseDir . '/lib/public/Security/ITrustedDomainHelper.php', 'OCP\\Security\\VerificationToken\\IVerificationToken' => $baseDir . '/lib/public/Security/VerificationToken/IVerificationToken.php', @@ -1288,7 +1289,6 @@ return array( 'OC\\Http\\Client\\Client' => $baseDir . '/lib/private/Http/Client/Client.php', 'OC\\Http\\Client\\ClientService' => $baseDir . '/lib/private/Http/Client/ClientService.php', 'OC\\Http\\Client\\DnsPinMiddleware' => $baseDir . '/lib/private/Http/Client/DnsPinMiddleware.php', - 'OC\\Http\\Client\\LocalAddressChecker' => $baseDir . '/lib/private/Http/Client/LocalAddressChecker.php', 'OC\\Http\\Client\\NegativeDnsCache' => $baseDir . '/lib/private/Http/Client/NegativeDnsCache.php', 'OC\\Http\\Client\\Response' => $baseDir . '/lib/private/Http/Client/Response.php', 'OC\\Http\\CookieHelper' => $baseDir . '/lib/private/Http/CookieHelper.php', @@ -1362,6 +1362,8 @@ return array( 'OC\\NaturalSort_DefaultCollator' => $baseDir . '/lib/private/NaturalSort_DefaultCollator.php', 'OC\\NavigationManager' => $baseDir . '/lib/private/NavigationManager.php', 'OC\\NeedsUpdateException' => $baseDir . '/lib/private/NeedsUpdateException.php', + 'OC\\Net\\HostnameClassifier' => $baseDir . '/lib/private/Net/HostnameClassifier.php', + 'OC\\Net\\IpAddressClassifier' => $baseDir . '/lib/private/Net/IpAddressClassifier.php', 'OC\\NotSquareException' => $baseDir . '/lib/private/NotSquareException.php', 'OC\\Notification\\Action' => $baseDir . '/lib/private/Notification/Action.php', 'OC\\Notification\\Manager' => $baseDir . '/lib/private/Notification/Manager.php', @@ -1517,6 +1519,7 @@ return array( 'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', 'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php', + 'OC\\Security\\RemoteHostValidator' => $baseDir . '/lib/private/Security/RemoteHostValidator.php', 'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php', 'OC\\Security\\TrustedDomainHelper' => $baseDir . '/lib/private/Security/TrustedDomainHelper.php', 'OC\\Security\\VerificationToken\\CleanUpJob' => $baseDir . '/lib/private/Security/VerificationToken/CleanUpJob.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index ad62c3585e6..4c430720ef1 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -568,6 +568,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Security\\ICredentialsManager' => __DIR__ . '/../../..' . '/lib/public/Security/ICredentialsManager.php', 'OCP\\Security\\ICrypto' => __DIR__ . '/../../..' . '/lib/public/Security/ICrypto.php', 'OCP\\Security\\IHasher' => __DIR__ . '/../../..' . '/lib/public/Security/IHasher.php', + 'OCP\\Security\\IRemoteHostValidator' => __DIR__ . '/../../..' . '/lib/public/Security/IRemoteHostValidator.php', 'OCP\\Security\\ISecureRandom' => __DIR__ . '/../../..' . '/lib/public/Security/ISecureRandom.php', 'OCP\\Security\\ITrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/public/Security/ITrustedDomainHelper.php', 'OCP\\Security\\VerificationToken\\IVerificationToken' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/IVerificationToken.php', @@ -1321,7 +1322,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Http\\Client\\Client' => __DIR__ . '/../../..' . '/lib/private/Http/Client/Client.php', 'OC\\Http\\Client\\ClientService' => __DIR__ . '/../../..' . '/lib/private/Http/Client/ClientService.php', 'OC\\Http\\Client\\DnsPinMiddleware' => __DIR__ . '/../../..' . '/lib/private/Http/Client/DnsPinMiddleware.php', - 'OC\\Http\\Client\\LocalAddressChecker' => __DIR__ . '/../../..' . '/lib/private/Http/Client/LocalAddressChecker.php', 'OC\\Http\\Client\\NegativeDnsCache' => __DIR__ . '/../../..' . '/lib/private/Http/Client/NegativeDnsCache.php', 'OC\\Http\\Client\\Response' => __DIR__ . '/../../..' . '/lib/private/Http/Client/Response.php', 'OC\\Http\\CookieHelper' => __DIR__ . '/../../..' . '/lib/private/Http/CookieHelper.php', @@ -1395,6 +1395,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\NaturalSort_DefaultCollator' => __DIR__ . '/../../..' . '/lib/private/NaturalSort_DefaultCollator.php', 'OC\\NavigationManager' => __DIR__ . '/../../..' . '/lib/private/NavigationManager.php', 'OC\\NeedsUpdateException' => __DIR__ . '/../../..' . '/lib/private/NeedsUpdateException.php', + 'OC\\Net\\HostnameClassifier' => __DIR__ . '/../../..' . '/lib/private/Net/HostnameClassifier.php', + 'OC\\Net\\IpAddressClassifier' => __DIR__ . '/../../..' . '/lib/private/Net/IpAddressClassifier.php', 'OC\\NotSquareException' => __DIR__ . '/../../..' . '/lib/private/NotSquareException.php', 'OC\\Notification\\Action' => __DIR__ . '/../../..' . '/lib/private/Notification/Action.php', 'OC\\Notification\\Manager' => __DIR__ . '/../../..' . '/lib/private/Notification/Manager.php', @@ -1550,6 +1552,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', 'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php', + 'OC\\Security\\RemoteHostValidator' => __DIR__ . '/../../..' . '/lib/private/Security/RemoteHostValidator.php', 'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php', 'OC\\Security\\TrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/private/Security/TrustedDomainHelper.php', 'OC\\Security\\VerificationToken\\CleanUpJob' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/CleanUpJob.php', diff --git a/lib/private/Http/Client/Client.php b/lib/private/Http/Client/Client.php index d4dba3e5a44..2e370395132 100644 --- a/lib/private/Http/Client/Client.php +++ b/lib/private/Http/Client/Client.php @@ -37,8 +37,11 @@ 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\Security\IRemoteHostValidator; +use function parse_url; /** * Class Client @@ -52,19 +55,18 @@ class Client implements IClient { private $config; /** @var ICertificateManager */ private $certificateManager; - /** @var LocalAddressChecker */ - private $localAddressChecker; + private IRemoteHostValidator $remoteHostValidator; public function __construct( IConfig $config, ICertificateManager $certificateManager, GuzzleClient $client, - LocalAddressChecker $localAddressChecker + IRemoteHostValidator $remoteHostValidator ) { $this->config = $config; $this->client = $client; $this->certificateManager = $certificateManager; - $this->localAddressChecker = $localAddressChecker; + $this->remoteHostValidator = $remoteHostValidator; } private function buildRequestOptions(array $options): array { @@ -181,7 +183,13 @@ class Client implements IClient { return; } - $this->localAddressChecker->throwIfLocalAddress($uri); + $host = parse_url($uri, PHP_URL_HOST); + if ($host === false || $host === null) { + throw new LocalServerException('Could not detect any host'); + } + if (!$this->remoteHostValidator->isValid($host)) { + throw new LocalServerException('Host violates local access rules'); + } } /** diff --git a/lib/private/Http/Client/ClientService.php b/lib/private/Http/Client/ClientService.php index e868d4af7a5..bbc2330176f 100644 --- a/lib/private/Http/Client/ClientService.php +++ b/lib/private/Http/Client/ClientService.php @@ -33,6 +33,7 @@ use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; use OCP\ICertificateManager; use OCP\IConfig; +use OCP\Security\IRemoteHostValidator; /** * Class ClientService @@ -46,17 +47,16 @@ class ClientService implements IClientService { private $certificateManager; /** @var DnsPinMiddleware */ private $dnsPinMiddleware; - /** @var LocalAddressChecker */ - private $localAddressChecker; + private IRemoteHostValidator $remoteHostValidator; public function __construct(IConfig $config, ICertificateManager $certificateManager, DnsPinMiddleware $dnsPinMiddleware, - LocalAddressChecker $localAddressChecker) { + IRemoteHostValidator $remoteHostValidator) { $this->config = $config; $this->certificateManager = $certificateManager; $this->dnsPinMiddleware = $dnsPinMiddleware; - $this->localAddressChecker = $localAddressChecker; + $this->remoteHostValidator = $remoteHostValidator; } /** @@ -73,7 +73,7 @@ class ClientService implements IClientService { $this->config, $this->certificateManager, $client, - $this->localAddressChecker + $this->remoteHostValidator, ); } } diff --git a/lib/private/Http/Client/DnsPinMiddleware.php b/lib/private/Http/Client/DnsPinMiddleware.php index 00bc209d7b1..294a23f9de1 100644 --- a/lib/private/Http/Client/DnsPinMiddleware.php +++ b/lib/private/Http/Client/DnsPinMiddleware.php @@ -25,20 +25,21 @@ declare(strict_types=1); */ namespace OC\Http\Client; +use OC\Net\IpAddressClassifier; +use OCP\Http\Client\LocalServerException; use Psr\Http\Message\RequestInterface; class DnsPinMiddleware { /** @var NegativeDnsCache */ private $negativeDnsCache; - /** @var LocalAddressChecker */ - private $localAddressChecker; + private IpAddressClassifier $ipAddressClassifier; public function __construct( NegativeDnsCache $negativeDnsCache, - LocalAddressChecker $localAddressChecker + IpAddressClassifier $ipAddressClassifier ) { $this->negativeDnsCache = $negativeDnsCache; - $this->localAddressChecker = $localAddressChecker; + $this->ipAddressClassifier = $ipAddressClassifier; } /** @@ -133,7 +134,10 @@ class DnsPinMiddleware { $curlResolves["$hostName:$port"] = []; foreach ($targetIps as $ip) { - $this->localAddressChecker->throwIfLocalIp($ip); + if (!$this->ipAddressClassifier->isLocalAddress($ip)) { + // TODO: continue with all non-local IPs? + throw new LocalServerException('Host violates local access rules'); + } $curlResolves["$hostName:$port"][] = $ip; } } diff --git a/lib/private/Http/Client/LocalAddressChecker.php b/lib/private/Http/Client/LocalAddressChecker.php deleted file mode 100644 index eb24f002d7d..00000000000 --- a/lib/private/Http/Client/LocalAddressChecker.php +++ /dev/null @@ -1,102 +0,0 @@ -<?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 IPLib\Address\IPv6; -use IPLib\Factory; -use IPLib\ParseStringFlag; -use OCP\Http\Client\LocalServerException; -use Psr\Log\LoggerInterface; -use Symfony\Component\HttpFoundation\IpUtils; - -class LocalAddressChecker { - private LoggerInterface $logger; - - public function __construct(LoggerInterface $logger) { - $this->logger = $logger; - } - - public function throwIfLocalIp(string $ip) : void { - $parsedIp = Factory::parseAddressString( - $ip, - ParseStringFlag::IPV4_MAYBE_NON_DECIMAL | ParseStringFlag::IPV4ADDRESS_MAYBE_NON_QUAD_DOTTED - ); - if ($parsedIp === null) { - /* Not an IP */ - return; - } - /* Replace by normalized form */ - if ($parsedIp instanceof IPv6) { - $ip = (string)($parsedIp->toIPv4() ?? $parsedIp); - } else { - $ip = (string)$parsedIp; - } - - $localRanges = [ - '100.64.0.0/10', // See RFC 6598 - '192.0.0.0/24', // See RFC 6890 - ]; - if ( - (bool)filter_var($ip, FILTER_VALIDATE_IP) && - ( - !filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE) || - IpUtils::checkIp($ip, $localRanges) - )) { - $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 = idn_to_utf8(strtolower(urldecode($host))); - // Remove brackets from IPv6 addresses - if (strpos($host, '[') === 0 && substr($host, -1) === ']') { - $host = substr($host, 1, -1); - } - - // Disallow local network top-level domains from RFC 6762 - $localTopLevelDomains = ['local','localhost','intranet','internal','private','corp','home','lan']; - $topLevelDomain = substr((strrchr($host, '.') ?: ''), 1); - if (in_array($topLevelDomain, $localTopLevelDomains)) { - $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/Log/ErrorHandler.php b/lib/private/Log/ErrorHandler.php index d56fecb1ecb..c4b9631e75a 100644 --- a/lib/private/Log/ErrorHandler.php +++ b/lib/private/Log/ErrorHandler.php @@ -1,4 +1,7 @@ <?php + +declare(strict_types=1); + /** * @copyright Copyright (c) 2016, ownCloud, Inc. * @@ -27,84 +30,75 @@ */ namespace OC\Log; +use Error; use OCP\ILogger; +use Psr\Log\LoggerInterface; +use Throwable; class ErrorHandler { - /** @var ILogger */ - private static $logger; + private LoggerInterface $logger; + + public function __construct(LoggerInterface $logger) { + $this->logger = $logger; + } /** - * remove password in URLs - * @param string $msg - * @return string + * Remove password in URLs */ - protected static function removePassword($msg) { + private static function removePassword(string $msg): string { return preg_replace('#//(.*):(.*)@#', '//xxx:xxx@', $msg); } - public static function register($debug = false) { - $handler = new ErrorHandler(); - - if ($debug) { - set_error_handler([$handler, 'onAll'], E_ALL); - if (\OC::$CLI) { - set_exception_handler(['OC_Template', 'printExceptionErrorPage']); - } - } else { - set_error_handler([$handler, 'onError']); - } - register_shutdown_function([$handler, 'onShutdown']); - set_exception_handler([$handler, 'onException']); - } - - public static function setLogger(ILogger $logger) { - self::$logger = $logger; - } - - //Fatal errors handler - public static function onShutdown() { + /** + * Fatal errors handler + */ + public function onShutdown(): void { $error = error_get_last(); - if ($error && self::$logger) { - //ob_end_clean(); + if ($error) { $msg = $error['message'] . ' at ' . $error['file'] . '#' . $error['line']; - self::$logger->critical(self::removePassword($msg), ['app' => 'PHP']); + $this->logger->critical(self::removePassword($msg), ['app' => 'PHP']); } } /** - * Uncaught exception handler - * - * @param \Exception $exception + * Uncaught exception handler */ - public static function onException($exception) { + public function onException(Throwable $exception): void { $class = get_class($exception); $msg = $exception->getMessage(); $msg = "$class: $msg at " . $exception->getFile() . '#' . $exception->getLine(); - self::$logger->critical(self::removePassword($msg), ['app' => 'PHP']); + $this->logger->critical(self::removePassword($msg), ['app' => 'PHP']); } - //Recoverable errors handler - public static function onError($number, $message, $file, $line) { + /** + * Recoverable errors handler + */ + public function onError(int $number, string $message, string $file, int $line): bool { if (!(error_reporting() & $number)) { - return; + return true; } $msg = $message . ' at ' . $file . '#' . $line; - $e = new \Error(self::removePassword($msg)); - self::$logger->logException($e, ['app' => 'PHP', 'level' => self::errnoToLogLevel($number)]); + $e = new Error(self::removePassword($msg)); + $this->logger->log(self::errnoToLogLevel($number), $e->getMessage(), ['app' => 'PHP']); + return true; } - //Recoverable handler which catch all errors, warnings and notices - public static function onAll($number, $message, $file, $line) { + /** + * Recoverable handler which catch all errors, warnings and notices + */ + public function onAll(int $number, string $message, string $file, int $line): bool { $msg = $message . ' at ' . $file . '#' . $line; - $e = new \Error(self::removePassword($msg)); - self::$logger->logException($e, ['app' => 'PHP', 'level' => self::errnoToLogLevel($number)]); + $e = new Error(self::removePassword($msg)); + $this->logger->log(self::errnoToLogLevel($number), $e->getMessage(), ['app' => 'PHP']); + return true; } - public static function errnoToLogLevel(int $errno): int { + private static function errnoToLogLevel(int $errno): int { switch ($errno) { case E_USER_WARNING: return ILogger::WARN; + case E_DEPRECATED: case E_USER_DEPRECATED: return ILogger::DEBUG; diff --git a/lib/private/Net/HostnameClassifier.php b/lib/private/Net/HostnameClassifier.php new file mode 100644 index 00000000000..626aa47083e --- /dev/null +++ b/lib/private/Net/HostnameClassifier.php @@ -0,0 +1,74 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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\Net; + +use function filter_var; +use function in_array; +use function strrchr; +use function substr; +use function substr_count; + +/** + * Classifier for network hostnames + * + * @internal + */ +class HostnameClassifier { + private const LOCAL_TOPLEVEL_DOMAINS = [ + 'local', + 'localhost', + 'intranet', + 'internal', + 'private', + 'corp', + 'home', + 'lan', + ]; + + /** + * Check host identifier for local hostname + * + * IP addresses are not considered local. Use the IpAddressClassifier for those. + * + * @param string $hostname + * + * @return bool + */ + public function isLocalHostname(string $hostname): bool { + // Disallow local network top-level domains from RFC 6762 + $topLevelDomain = substr((strrchr($hostname, '.') ?: ''), 1); + if (in_array($topLevelDomain, self::LOCAL_TOPLEVEL_DOMAINS)) { + return true; + } + + // Disallow hostname only + if (substr_count($hostname, '.') === 0 && !filter_var($hostname, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { + return true; + } + + return false; + } +} diff --git a/lib/private/Net/IpAddressClassifier.php b/lib/private/Net/IpAddressClassifier.php new file mode 100644 index 00000000000..d4698864ec8 --- /dev/null +++ b/lib/private/Net/IpAddressClassifier.php @@ -0,0 +1,81 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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\Net; + +use IPLib\Address\IPv6; +use IPLib\Factory; +use IPLib\ParseStringFlag; +use Symfony\Component\HttpFoundation\IpUtils; +use function filter_var; + +/** + * Classifier for IP addresses + * + * @internal + */ +class IpAddressClassifier { + private const LOCAL_ADDRESS_RANGES = [ + '100.64.0.0/10', // See RFC 6598 + '192.0.0.0/24', // See RFC 6890 + ]; + + /** + * Check host identifier for local IPv4 and IPv6 address ranges + * + * Hostnames are not considered local. Use the HostnameClassifier for those. + * + * @param string $ip + * + * @return bool + */ + public function isLocalAddress(string $ip): bool { + $parsedIp = Factory::parseAddressString( + $ip, + ParseStringFlag::IPV4_MAYBE_NON_DECIMAL | ParseStringFlag::IPV4ADDRESS_MAYBE_NON_QUAD_DOTTED + ); + if ($parsedIp === null) { + /* Not an IP */ + return false; + } + /* Replace by normalized form */ + if ($parsedIp instanceof IPv6) { + $ip = (string)($parsedIp->toIPv4() ?? $parsedIp); + } else { + $ip = (string)$parsedIp; + } + + if (!filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE)) { + /* Range address */ + return true; + } + if (IpUtils::checkIp($ip, self::LOCAL_ADDRESS_RANGES)) { + /* Within local range */ + return true; + } + + return false; + } +} diff --git a/lib/private/Security/RemoteHostValidator.php b/lib/private/Security/RemoteHostValidator.php new file mode 100644 index 00000000000..e48bd862472 --- /dev/null +++ b/lib/private/Security/RemoteHostValidator.php @@ -0,0 +1,76 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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\Security; + +use OC\Net\HostnameClassifier; +use OC\Net\IpAddressClassifier; +use OCP\IConfig; +use OCP\Security\IRemoteHostValidator; +use Psr\Log\LoggerInterface; +use function strpos; +use function strtolower; +use function substr; +use function urldecode; + +/** + * @internal + */ +final class RemoteHostValidator implements IRemoteHostValidator { + private IConfig $config; + private HostnameClassifier $hostnameClassifier; + private IpAddressClassifier $ipAddressClassifier; + private LoggerInterface $logger; + + public function __construct(IConfig $config, + HostnameClassifier $hostnameClassifier, + IpAddressClassifier $ipAddressClassifier, + LoggerInterface $logger) { + $this->config = $config; + $this->hostnameClassifier = $hostnameClassifier; + $this->ipAddressClassifier = $ipAddressClassifier; + $this->logger = $logger; + } + + public function isValid(string $host): bool { + if ($this->config->getSystemValueBool('allow_local_remote_servers', false)) { + return true; + } + + $host = idn_to_utf8(strtolower(urldecode($host))); + // Remove brackets from IPv6 addresses + if (strpos($host, '[') === 0 && substr($host, -1) === ']') { + $host = substr($host, 1, -1); + } + + if ($this->hostnameClassifier->isLocalHostname($host) + || $this->ipAddressClassifier->isLocalAddress($host)) { + $this->logger->warning("Host $host was not connected to because it violates local access rules"); + return false; + } + + return true; + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 1da1b614b5b..03aa75060df 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -105,8 +105,6 @@ 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; @@ -858,7 +856,7 @@ class Server extends ServerContainer implements IServerContainer { $this->registerAlias(\OCP\Security\ISecureRandom::class, SecureRandom::class); /** @deprecated 19.0.0 */ $this->registerDeprecatedAlias('SecureRandom', \OCP\Security\ISecureRandom::class); - + $this->registerAlias(\OCP\Security\IRemoteHostValidator::class, \OC\Security\RemoteHostValidator::class); $this->registerAlias(IVerificationToken::class, VerificationToken::class); $this->registerAlias(ICrypto::class, Crypto::class); @@ -890,22 +888,11 @@ 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(LoggerInterface::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) { return new EventLogger($c->get(SystemConfig::class), $c->get(LoggerInterface::class), $c->get(Log::class)); diff --git a/lib/public/Security/IRemoteHostValidator.php b/lib/public/Security/IRemoteHostValidator.php new file mode 100644 index 00000000000..99f149aee04 --- /dev/null +++ b/lib/public/Security/IRemoteHostValidator.php @@ -0,0 +1,51 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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\Security; + +/** + * Validator for remote hosts + * + * @since 26.0.0 + */ +interface IRemoteHostValidator { + + /** + * Validate if a host may be connected to + * + * By default, Nextcloud does not connect to any local servers. That is neither + * localhost nor any host in the local network. + * + * Admins can overwrite this behavior with the global `allow_local_remote_servers` + * settings flag. If the flag is set to `true`, local hosts will be considered + * valid. + * + * @param string $host hostname of the remote server, IPv4 or IPv6 address + * + * @return bool + * @since 26.0.0 + */ + public function isValid(string $host): bool; +} diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php index dfdd67fbb23..a038058069e 100644 --- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php @@ -863,6 +863,7 @@ class ContactsStoreTest extends TestCase { ['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'], ['core', 'shareapi_restrict_user_enumeration_full_match', 'yes', 'yes'], ['core', 'shareapi_exclude_groups', 'no', 'yes'], + ['core', 'shareapi_exclude_groups_list', '', ''], ['core', 'shareapi_only_share_with_group_members', 'no', 'no'], ]); diff --git a/tests/lib/ErrorHandlerTest.php b/tests/lib/ErrorHandlerTest.php index ea53e67005c..f6bf850f0b5 100644 --- a/tests/lib/ErrorHandlerTest.php +++ b/tests/lib/ErrorHandlerTest.php @@ -1,4 +1,7 @@ <?php + +declare(strict_types=1); + /** * ownCloud * @@ -22,7 +25,26 @@ namespace Test; -class ErrorHandlerTest extends \Test\TestCase { +use OC\Log\ErrorHandler; +use OCP\ILogger; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; + +class ErrorHandlerTest extends TestCase { + + /** @var MockObject */ + private LoggerInterface $logger; + + private ErrorHandler $errorHandler; + + protected function setUp(): void { + parent::setUp(); + + $this->logger = $this->createMock(LoggerInterface::class); + $this->errorHandler = new ErrorHandler( + $this->logger + ); + } /** * provide username, password combinations for testRemovePassword @@ -47,24 +69,19 @@ class ErrorHandlerTest extends \Test\TestCase { * @param string $username * @param string $password */ - public function testRemovePassword($username, $password) { + public function testRemovePasswordFromError($username, $password) { $url = 'http://'.$username.':'.$password.'@owncloud.org'; $expectedResult = 'http://xxx:xxx@owncloud.org'; - $result = TestableErrorHandler::testRemovePassword($url); + $this->logger->expects(self::once()) + ->method('log') + ->with( + ILogger::ERROR, + 'Could not reach ' . $expectedResult . ' at file#4', + ['app' => 'PHP'], + ); - $this->assertEquals($expectedResult, $result); - } -} + $result = $this->errorHandler->onError(E_USER_ERROR, 'Could not reach ' . $url, 'file', 4); -/** - * dummy class to access protected methods of \OC\Log\ErrorHandler - */ -class TestableErrorHandler extends \OC\Log\ErrorHandler { - - /** - * @param string $msg - */ - public static function testRemovePassword($msg) { - return self::removePassword($msg); + self::assertTrue($result); } } diff --git a/tests/lib/Http/Client/ClientServiceTest.php b/tests/lib/Http/Client/ClientServiceTest.php index 94f4d51ecee..ed1165236aa 100644 --- a/tests/lib/Http/Client/ClientServiceTest.php +++ b/tests/lib/Http/Client/ClientServiceTest.php @@ -1,4 +1,7 @@ <?php + +declare(strict_types=1); + /** * Copyright (c) 2015 Lukas Reschke <lukas@owncloud.com> * This file is licensed under the Affero General Public License version 3 or @@ -14,9 +17,9 @@ 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\Security\IRemoteHostValidator; /** * Class ClientServiceTest @@ -33,13 +36,13 @@ class ClientServiceTest extends \Test\TestCase { ->method('addDnsPinning') ->willReturn(function () { }); - $localAddressChecker = $this->createMock(LocalAddressChecker::class); + $remoteHostValidator = $this->createMock(IRemoteHostValidator::class); $clientService = new ClientService( $config, $certificateManager, $dnsPinMiddleware, - $localAddressChecker + $remoteHostValidator ); $handler = new CurlHandler(); @@ -52,7 +55,7 @@ class ClientServiceTest extends \Test\TestCase { $config, $certificateManager, $guzzleClient, - $localAddressChecker + $remoteHostValidator ), $clientService->newClient() ); diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php index fa2374aeb7e..93948a5daf3 100644 --- a/tests/lib/Http/Client/ClientTest.php +++ b/tests/lib/Http/Client/ClientTest.php @@ -1,4 +1,7 @@ <?php + +declare(strict_types=1); + /** * Copyright (c) 2015 Lukas Reschke <lukas@owncloud.com> * This file is licensed under the Affero General Public License version 3 or @@ -10,12 +13,13 @@ 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; use OCP\IConfig; +use OCP\Security\IRemoteHostValidator; use PHPUnit\Framework\MockObject\MockObject; +use function parse_url; /** * Class ClientTest @@ -29,8 +33,8 @@ class ClientTest extends \Test\TestCase { private $client; /** @var IConfig|MockObject */ private $config; - /** @var LocalAddressChecker|MockObject */ - private $localAddressChecker; + /** @var IRemoteHostValidator|MockObject */ + private IRemoteHostValidator $remoteHostValidator; /** @var array */ private $defaultRequestOptions; @@ -39,12 +43,12 @@ class ClientTest extends \Test\TestCase { $this->config = $this->createMock(IConfig::class); $this->guzzleClient = $this->createMock(\GuzzleHttp\Client::class); $this->certificateManager = $this->createMock(ICertificateManager::class); - $this->localAddressChecker = $this->createMock(LocalAddressChecker::class); + $this->remoteHostValidator = $this->createMock(IRemoteHostValidator::class); $this->client = new Client( $this->config, $this->certificateManager, $this->guzzleClient, - $this->localAddressChecker + $this->remoteHostValidator ); } @@ -146,22 +150,22 @@ class ClientTest extends \Test\TestCase { 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 - ['normal.host.com'], + ['https://localhost/foo.bar'], + ['https://localHost/foo.bar'], + ['https://random-host/foo.bar'], + ['https://[::1]/bla.blub'], + ['https://[::]/bla.blub'], + ['https://192.168.0.1'], + ['https://172.16.42.1'], + ['https://[fdf8:f53b:82e4::53]/secret.ics'], + ['https://[fe80::200:5aee:feaa:20a2]/secret.ics'], + ['https://[0:0:0:0:0:0:10.0.0.1]/secret.ics'], + ['https://[0:0:0:0:0:ffff:127.0.0.0]/secret.ics'], + ['https://10.0.0.1'], + ['https://another-host.local'], + ['https://service.localhost'], + ['!@#$', true], // test invalid url + ['https://normal.host.com'], ]; } @@ -175,9 +179,7 @@ class ClientTest extends \Test\TestCase { ->with('allow_local_remote_servers', false) ->willReturn(true); -// $this->expectException(LocalServerException::class); - - self::invokePrivate($this->client, 'preventLocalAddress', ['http://' . $uri, []]); + self::invokePrivate($this->client, 'preventLocalAddress', [$uri, []]); } /** @@ -188,9 +190,7 @@ class ClientTest extends \Test\TestCase { $this->config->expects($this->never()) ->method('getSystemValueBool'); -// $this->expectException(LocalServerException::class); - - self::invokePrivate($this->client, 'preventLocalAddress', ['http://' . $uri, [ + self::invokePrivate($this->client, 'preventLocalAddress', [$uri, [ 'nextcloud' => ['allow_local_address' => true], ]]); } @@ -200,14 +200,14 @@ class ClientTest extends \Test\TestCase { * @param string $uri */ public function testPreventLocalAddressOnGet(string $uri): void { + $host = parse_url($uri, PHP_URL_HOST); $this->expectException(LocalServerException::class); - $this->localAddressChecker - ->expects($this->once()) - ->method('throwIfLocalAddress') - ->with('http://' . $uri) - ->will($this->throwException(new LocalServerException())); + $this->remoteHostValidator + ->method('isValid') + ->with($host) + ->willReturn(false); - $this->client->get('http://' . $uri); + $this->client->get($uri); } /** @@ -215,14 +215,14 @@ class ClientTest extends \Test\TestCase { * @param string $uri */ public function testPreventLocalAddressOnHead(string $uri): void { + $host = parse_url($uri, PHP_URL_HOST); $this->expectException(LocalServerException::class); - $this->localAddressChecker - ->expects($this->once()) - ->method('throwIfLocalAddress') - ->with('http://' . $uri) - ->will($this->throwException(new LocalServerException())); + $this->remoteHostValidator + ->method('isValid') + ->with($host) + ->willReturn(false); - $this->client->head('http://' . $uri); + $this->client->head($uri); } /** @@ -230,14 +230,14 @@ class ClientTest extends \Test\TestCase { * @param string $uri */ public function testPreventLocalAddressOnPost(string $uri): void { + $host = parse_url($uri, PHP_URL_HOST); $this->expectException(LocalServerException::class); - $this->localAddressChecker - ->expects($this->once()) - ->method('throwIfLocalAddress') - ->with('http://' . $uri) - ->will($this->throwException(new LocalServerException())); + $this->remoteHostValidator + ->method('isValid') + ->with($host) + ->willReturn(false); - $this->client->post('http://' . $uri); + $this->client->post($uri); } /** @@ -245,14 +245,14 @@ class ClientTest extends \Test\TestCase { * @param string $uri */ public function testPreventLocalAddressOnPut(string $uri): void { + $host = parse_url($uri, PHP_URL_HOST); $this->expectException(LocalServerException::class); - $this->localAddressChecker - ->expects($this->once()) - ->method('throwIfLocalAddress') - ->with('http://' . $uri) - ->will($this->throwException(new LocalServerException())); + $this->remoteHostValidator + ->method('isValid') + ->with($host) + ->willReturn(false); - $this->client->put('http://' . $uri); + $this->client->put($uri); } /** @@ -260,14 +260,14 @@ class ClientTest extends \Test\TestCase { * @param string $uri */ public function testPreventLocalAddressOnDelete(string $uri): void { + $host = parse_url($uri, PHP_URL_HOST); $this->expectException(LocalServerException::class); - $this->localAddressChecker - ->expects($this->once()) - ->method('throwIfLocalAddress') - ->with('http://' . $uri) - ->will($this->throwException(new LocalServerException())); + $this->remoteHostValidator + ->method('isValid') + ->with($host) + ->willReturn(false); - $this->client->delete('http://' . $uri); + $this->client->delete($uri); } private function setUpDefaultRequestOptions(): void { diff --git a/tests/lib/Http/Client/LocalAddressCheckerTest.php b/tests/lib/Http/Client/LocalAddressCheckerTest.php deleted file mode 100644 index 024c52b3705..00000000000 --- a/tests/lib/Http/Client/LocalAddressCheckerTest.php +++ /dev/null @@ -1,158 +0,0 @@ -<?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\Http\Client\LocalServerException; -use OC\Http\Client\LocalAddressChecker; -use Psr\Log\LoggerInterface; - -class LocalAddressCheckerTest extends \Test\TestCase { - /** @var LocalAddressChecker */ - private $localAddressChecker; - - protected function setUp(): void { - parent::setUp(); - - $logger = $this->createMock(LoggerInterface::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:ffff:10.0.0.1'], - ['0:0:0:0:0:ffff:127.0.0.0'], - ['10.0.0.1'], - ['::'], - ['::1'], - ['100.100.100.200'], - ['192.0.0.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:ffff: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 - ['100.100.100.200'], - ['192.0.0.1'], - ['randomdomain.internal'], - ['0177.0.0.9'], - ['⑯⑨。②⑤④。⑯⑨｡②⑤④'], - ['127。②⑤④。⑯⑨.②⑤④'], - ['127.0.00000000000000000000000000000000001'], - ['127.1'], - ['127.000.001'], - ['0177.0.0.01'], - ['0x7f.0x0.0x0.0x1'], - ['0x7f000001'], - ['2130706433'], - ['00000000000000000000000000000000000000000000000000177.1'], - ['0x7f.1'], - ['127.0x1'], - ['[0000:0000:0000:0000:0000:0000:0000:0001]'], - ['[0:0:0:0:0:0:0:1]'], - ['[0:0:0:0::0:0:1]'], - ['%31%32%37%2E%30%2E%30%2E%31'], - ['%31%32%37%2E%30%2E%30.%31'], - ['[%3A%3A%31]'], - ]; - } - - 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/Net/HostnameClassifierTest.php b/tests/lib/Net/HostnameClassifierTest.php new file mode 100644 index 00000000000..f363a08bb8a --- /dev/null +++ b/tests/lib/Net/HostnameClassifierTest.php @@ -0,0 +1,78 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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 lib\Net; + +use OC\Net\HostnameClassifier; +use Test\TestCase; + +class HostnameClassifierTest extends TestCase { + private HostnameClassifier $classifier; + + protected function setUp(): void { + parent::setUp(); + + $this->classifier = new HostnameClassifier(); + } + + public function localHostnamesData():array { + return [ + ['localhost'], + ['localHost'], + ['random-host'], + ['another-host.local'], + ['service.localhost'], + ['randomdomain.internal'], + ]; + } + + /** + * @dataProvider localHostnamesData + */ + public function testLocalHostname(string $host): void { + $isLocal = $this->classifier->isLocalHostname($host); + + self::assertTrue($isLocal); + } + + public function publicHostnamesData(): array { + return [ + ['example.com'], + ['example.net'], + ['example.org'], + ['host.domain'], + ['cloud.domain.tld'], + ]; + } + + /** + * @dataProvider publicHostnamesData + */ + public function testPublicHostname(string $host): void { + $isLocal = $this->classifier->isLocalHostname($host); + + self::assertFalse($isLocal); + } +} diff --git a/tests/lib/Net/IpAddressClassifierTest.php b/tests/lib/Net/IpAddressClassifierTest.php new file mode 100644 index 00000000000..593abcd2b40 --- /dev/null +++ b/tests/lib/Net/IpAddressClassifierTest.php @@ -0,0 +1,80 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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 lib\Net; + +use OC\Net\IpAddressClassifier; +use Test\TestCase; + +class IpAddressClassifierTest extends TestCase { + private IpAddressClassifier $classifier; + + protected function setUp(): void { + parent::setUp(); + + $this->classifier = new IpAddressClassifier(); + } + + public function publicIpAddressData(): array { + return [ + ['8.8.8.8'], + ['8.8.4.4'], + ['2001:4860:4860::8888'], + ['2001:4860:4860::8844'], + ]; + } + + /** + * @dataProvider publicIpAddressData + */ + public function testPublicAddress(string $ip): void { + $isLocal = $this->classifier->isLocalAddress($ip); + + self::assertFalse($isLocal); + } + + public function localIpAddressData(): array { + return [ + ['192.168.0.1'], + ['fe80::200:5aee:feaa:20a2'], + ['0:0:0:0:0:ffff:10.0.0.1'], + ['0:0:0:0:0:ffff:127.0.0.0'], + ['10.0.0.1'], + ['::'], + ['::1'], + ['100.100.100.200'], + ['192.0.0.1'], + ]; + } + + /** + * @dataProvider localIpAddressData + */ + public function testLocalAddress(string $ip): void { + $isLocal = $this->classifier->isLocalAddress($ip); + + self::assertTrue($isLocal); + } +} diff --git a/tests/lib/Security/RemoteHostValidatorIntegrationTest.php b/tests/lib/Security/RemoteHostValidatorIntegrationTest.php new file mode 100644 index 00000000000..73cbbd7b0e8 --- /dev/null +++ b/tests/lib/Security/RemoteHostValidatorIntegrationTest.php @@ -0,0 +1,144 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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 lib\Security; + +use OC\Net\HostnameClassifier; +use OC\Net\IpAddressClassifier; +use OC\Security\RemoteHostValidator; +use OCP\IConfig; +use OCP\Server; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\NullLogger; +use Test\TestCase; + +class RemoteHostValidatorIntegrationTest extends TestCase { + + /** @var IConfig|IConfig&MockObject|MockObject */ + private IConfig $config; + private RemoteHostValidator $validator; + + protected function setUp(): void { + parent::setUp(); + + // Mock config to avoid any side effects + $this->config = $this->createMock(IConfig::class); + + $this->validator = new RemoteHostValidator( + $this->config, + Server::get(HostnameClassifier::class), + Server::get(IpAddressClassifier::class), + new NullLogger(), + ); + } + + public function localHostsData(): array { + return [ + ['[::1]'], + ['[::]'], + ['192.168.0.1'], + ['172.16.42.1'], + ['[fdf8:f53b:82e4::53]'], + ['[fe80::200:5aee:feaa:20a2]'], + ['[0:0:0:0:0:ffff:10.0.0.1]'], + ['[0:0:0:0:0:ffff:127.0.0.0]'], + ['10.0.0.1'], + ['!@#$'], // test invalid url + ['100.100.100.200'], + ['192.0.0.1'], + ['0177.0.0.9'], + ['⑯⑨。②⑤④。⑯⑨｡②⑤④'], + ['127。②⑤④。⑯⑨.②⑤④'], + ['127.0.00000000000000000000000000000000001'], + ['127.1'], + ['127.000.001'], + ['0177.0.0.01'], + ['0x7f.0x0.0x0.0x1'], + ['0x7f000001'], + ['2130706433'], + ['00000000000000000000000000000000000000000000000000177.1'], + ['0x7f.1'], + ['127.0x1'], + ['[0000:0000:0000:0000:0000:0000:0000:0001]'], + ['[0:0:0:0:0:0:0:1]'], + ['[0:0:0:0::0:0:1]'], + ['%31%32%37%2E%30%2E%30%2E%31'], + ['%31%32%37%2E%30%2E%30.%31'], + ['[%3A%3A%31]'], + ]; + } + + /** + * @dataProvider localHostsData + */ + public function testLocalHostsWhenNotAllowed(string $host): void { + $this->config + ->method('getSystemValueBool') + ->with('allow_local_remote_servers', false) + ->willReturn(false); + + $isValid = $this->validator->isValid($host); + + self::assertFalse($isValid); + } + + /** + * @dataProvider localHostsData + */ + public function testLocalHostsWhenAllowed(string $host): void { + $this->config + ->method('getSystemValueBool') + ->with('allow_local_remote_servers', false) + ->willReturn(true); + + $isValid = $this->validator->isValid($host); + + self::assertTrue($isValid); + } + + public function externalAddressesData():array { + return [ + ['8.8.8.8'], + ['8.8.4.4'], + ['8.8.8.8'], + ['8.8.4.4'], + ['[2001:4860:4860::8888]'], + ]; + } + + /** + * @dataProvider externalAddressesData + */ + public function testExternalHost(string $host): void { + $this->config + ->method('getSystemValueBool') + ->with('allow_local_remote_servers', false) + ->willReturn(false); + + $isValid = $this->validator->isValid($host); + + self::assertTrue($isValid); + } +} diff --git a/tests/lib/Security/RemoteHostValidatorTest.php b/tests/lib/Security/RemoteHostValidatorTest.php new file mode 100644 index 00000000000..acaa7a4be30 --- /dev/null +++ b/tests/lib/Security/RemoteHostValidatorTest.php @@ -0,0 +1,111 @@ +<?php + +declare(strict_types=1); + +/* + * @copyright 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @author 2022 Christoph Wurst <christoph@winzerhof-wurst.at> + * + * @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 lib\Security; + +use OC\Net\HostnameClassifier; +use OC\Net\IpAddressClassifier; +use OC\Security\RemoteHostValidator; +use OCP\IConfig; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; +use Test\TestCase; + +class RemoteHostValidatorTest extends TestCase { + + /** @var IConfig|IConfig&MockObject|MockObject */ + private IConfig $config; + /** @var HostnameClassifier|HostnameClassifier&MockObject|MockObject */ + private HostnameClassifier $hostnameClassifier; + /** @var IpAddressClassifier|IpAddressClassifier&MockObject|MockObject */ + private IpAddressClassifier $ipAddressClassifier; + /** @var MockObject|LoggerInterface|LoggerInterface&MockObject */ + private LoggerInterface $logger; + private RemoteHostValidator $validator; + + protected function setUp(): void { + parent::setUp(); + + $this->config = $this->createMock(IConfig::class); + $this->hostnameClassifier = $this->createMock(HostnameClassifier::class); + $this->ipAddressClassifier = $this->createMock(IpAddressClassifier::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->validator = new RemoteHostValidator( + $this->config, + $this->hostnameClassifier, + $this->ipAddressClassifier, + $this->logger, + ); + } + + public function testValid(): void { + $host = 'nextcloud.com'; + $this->hostnameClassifier + ->method('isLocalHostname') + ->with($host) + ->willReturn(false); + $this->ipAddressClassifier + ->method('isLocalAddress') + ->with($host) + ->willReturn(false); + + $valid = $this->validator->isValid($host); + + self::assertTrue($valid); + } + + public function testLocalHostname(): void { + $host = 'localhost'; + $this->hostnameClassifier + ->method('isLocalHostname') + ->with($host) + ->willReturn(true); + $this->ipAddressClassifier + ->method('isLocalAddress') + ->with($host) + ->willReturn(false); + + $valid = $this->validator->isValid($host); + + self::assertFalse($valid); + } + + public function testLocalAddress(): void { + $host = '10.0.0.10'; + $this->hostnameClassifier + ->method('isLocalHostname') + ->with($host) + ->willReturn(false); + $this->ipAddressClassifier + ->method('isLocalAddress') + ->with($host) + ->willReturn(true); + + $valid = $this->validator->isValid($host); + + self::assertFalse($valid); + } +} |