From 8aea25b5b92dac105f7e862470ee0dcf0e876615 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 27 Oct 2022 14:33:31 +0200 Subject: [PATCH] Add remote host validation API Signed-off-by: Christoph Wurst --- lib/composer/composer/autoload_classmap.php | 5 +- lib/composer/composer/autoload_static.php | 5 +- lib/private/Http/Client/Client.php | 18 +- lib/private/Http/Client/ClientService.php | 10 +- lib/private/Http/Client/DnsPinMiddleware.php | 14 +- .../Http/Client/LocalAddressChecker.php | 102 ----------- lib/private/Net/HostnameClassifier.php | 74 ++++++++ lib/private/Net/IpAddressClassifier.php | 81 +++++++++ lib/private/Security/RemoteHostValidator.php | 76 +++++++++ lib/private/Server.php | 15 +- lib/public/Security/IRemoteHostValidator.php | 51 ++++++ tests/lib/Http/Client/ClientServiceTest.php | 11 +- tests/lib/Http/Client/ClientTest.php | 114 ++++++------- .../Http/Client/LocalAddressCheckerTest.php | 158 ------------------ tests/lib/Net/HostnameClassifierTest.php | 78 +++++++++ tests/lib/Net/IpAddressClassifierTest.php | 80 +++++++++ .../RemoteHostValidatorIntegrationTest.php | 144 ++++++++++++++++ .../lib/Security/RemoteHostValidatorTest.php | 111 ++++++++++++ 18 files changed, 795 insertions(+), 352 deletions(-) delete mode 100644 lib/private/Http/Client/LocalAddressChecker.php create mode 100644 lib/private/Net/HostnameClassifier.php create mode 100644 lib/private/Net/IpAddressClassifier.php create mode 100644 lib/private/Security/RemoteHostValidator.php create mode 100644 lib/public/Security/IRemoteHostValidator.php delete mode 100644 tests/lib/Http/Client/LocalAddressCheckerTest.php create mode 100644 tests/lib/Net/HostnameClassifierTest.php create mode 100644 tests/lib/Net/IpAddressClassifierTest.php create mode 100644 tests/lib/Security/RemoteHostValidatorIntegrationTest.php create mode 100644 tests/lib/Security/RemoteHostValidatorTest.php 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 @@ - - * - * @author Lukas Reschke - * - * @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 . - * - */ -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/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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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/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 @@ * 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 @@ * 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 @@ - - * - * @author Lukas Reschke - * - * @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 . - * - */ - -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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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 @@ + + * + * @author 2022 Christoph Wurst + * + * @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 . + */ + +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); + } +}