From 0e889da4a870c6160563382ad432497346e288c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 15 Jan 2024 15:40:21 +0100 Subject: [PATCH] Remove obsolete check of curl SSL version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Controller/CheckSetupController.php | 74 ------- .../Controller/CheckSetupControllerTest.php | 198 ------------------ core/js/setupchecks.js | 6 - 3 files changed, 278 deletions(-) diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 00883a0d51eed..8897548a97713 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -45,7 +45,6 @@ */ namespace OCA\Settings\Controller; -use GuzzleHttp\Exception\ClientException; use OC\AppFramework\Http; use OC\IntegrityCheck\Checker; use OCP\AppFramework\Controller; @@ -53,7 +52,6 @@ use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -67,8 +65,6 @@ class CheckSetupController extends Controller { /** @var IConfig */ private $config; - /** @var IClientService */ - private $clientService; /** @var IURLGenerator */ private $urlGenerator; /** @var IL10N */ @@ -86,7 +82,6 @@ class CheckSetupController extends Controller { public function __construct($AppName, IRequest $request, IConfig $config, - IClientService $clientService, IURLGenerator $urlGenerator, IL10N $l10n, Checker $checker, @@ -97,7 +92,6 @@ public function __construct($AppName, ) { parent::__construct($AppName, $request); $this->config = $config; - $this->clientService = $clientService; $this->urlGenerator = $urlGenerator; $this->l10n = $l10n; $this->checker = $checker; @@ -129,73 +123,6 @@ private function isFairUseOfFreePushService(): bool { return $this->manager->isFairUseOfFreePushService(); } - /** - * Public for the sake of unit-testing - * - * @return array - */ - protected function getCurlVersion() { - return curl_version(); - } - - /** - * Check if the used SSL lib is outdated. Older OpenSSL and NSS versions do - * have multiple bugs which likely lead to problems in combination with - * functionality required by ownCloud such as SNI. - * - * @link https://github.com/owncloud/core/issues/17446#issuecomment-122877546 - * @link https://bugzilla.redhat.com/show_bug.cgi?id=1241172 - * @return string - */ - private function isUsedTlsLibOutdated() { - // Don't run check when: - // 1. Server has `has_internet_connection` set to false - // 2. AppStore AND S2S is disabled - if (!$this->config->getSystemValue('has_internet_connection', true)) { - return ''; - } - if (!$this->config->getSystemValue('appstoreenabled', true) - && $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes') === 'no' - && $this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes') === 'no') { - return ''; - } - - $versionString = $this->getCurlVersion(); - if (isset($versionString['ssl_version'])) { - $versionString = $versionString['ssl_version']; - } else { - return ''; - } - - $features = $this->l10n->t('installing and updating apps via the App Store or Federated Cloud Sharing'); - if (!$this->config->getSystemValue('appstoreenabled', true)) { - $features = $this->l10n->t('Federated Cloud Sharing'); - } - - // Check if NSS and perform heuristic check - if (str_starts_with($versionString, 'NSS/')) { - try { - $firstClient = $this->clientService->newClient(); - $firstClient->get('https://nextcloud.com/'); - - $secondClient = $this->clientService->newClient(); - $secondClient->get('https://nextcloud.com/'); - } catch (ClientException $e) { - if ($e->getResponse()->getStatusCode() === 400) { - return $this->l10n->t('cURL is using an outdated %1$s version (%2$s). Please update your operating system or features such as %3$s will not work reliably.', ['NSS', $versionString, $features]); - } - } catch (\Exception $e) { - $this->logger->warning('error checking curl', [ - 'app' => 'settings', - 'exception' => $e, - ]); - return $this->l10n->t('Could not determine if TLS version of cURL is outdated or not because an error happened during the HTTPS request against https://nextcloud.com. Please check the Nextcloud log file for more details.'); - } - } - - return ''; - } - /** * Checks if the correct memcache module for PHP is installed. Only * fails if memcached is configured and the working module is not installed. @@ -365,7 +292,6 @@ public function check() { return new DataResponse( [ 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), - 'isUsedTlsLibOutdated' => $this->isUsedTlsLibOutdated(), 'reverseProxyDocs' => $this->urlGenerator->linkToDocs('admin-reverse-proxy'), 'isCorrectMemcachedPHPModuleInstalled' => $this->isCorrectMemcachedPHPModuleInstalled(), 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 1b778da661f79..3bec435bd6acb 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -40,7 +40,6 @@ use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\RedirectResponse; -use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; @@ -49,7 +48,6 @@ use OCP\Notification\IManager; use OCP\SetupCheck\ISetupCheckManager; use PHPUnit\Framework\MockObject\MockObject; -use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -66,8 +64,6 @@ class CheckSetupControllerTest extends TestCase { private $request; /** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */ private $config; - /** @var IClientService | \PHPUnit\Framework\MockObject\MockObject*/ - private $clientService; /** @var IURLGenerator | \PHPUnit\Framework\MockObject\MockObject */ private $urlGenerator; /** @var IL10N | \PHPUnit\Framework\MockObject\MockObject */ @@ -90,8 +86,6 @@ protected function setUp(): void { ->disableOriginalConstructor()->getMock(); $this->config = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor()->getMock(); - $this->clientService = $this->getMockBuilder(IClientService::class) - ->disableOriginalConstructor()->getMock(); $this->urlGenerator = $this->getMockBuilder(IURLGenerator::class) ->disableOriginalConstructor()->getMock(); $this->l10n = $this->getMockBuilder(IL10N::class) @@ -112,7 +106,6 @@ protected function setUp(): void { 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, @@ -149,8 +142,6 @@ public function testCheck() { $this->request->expects($this->never()) ->method('getHeader'); - $this->clientService->expects($this->never()) - ->method('newClient'); $this->checkSetupController ->expects($this->once()) @@ -200,7 +191,6 @@ public function testCheck() { $expected = new DataResponse( [ - 'isUsedTlsLibOutdated' => '', 'reverseProxyDocs' => 'reverse-proxy-doc-link', 'isCorrectMemcachedPHPModuleInstalled' => true, 'isSettimelimitAvailable' => true, @@ -216,192 +206,6 @@ public function testCheck() { $this->assertEquals($expected, $this->checkSetupController->check()); } - public function testGetCurlVersion() { - $checkSetupController = $this->getMockBuilder(CheckSetupController::class) - ->setConstructorArgs([ - 'settings', - $this->request, - $this->config, - $this->clientService, - $this->urlGenerator, - $this->l10n, - $this->checker, - $this->logger, - $this->tempManager, - $this->notificationManager, - $this->setupCheckManager, - ]) - ->setMethods(null)->getMock(); - - $this->assertArrayHasKey('ssl_version', $this->invokePrivate($checkSetupController, 'getCurlVersion')); - } - - public function testIsUsedTlsLibOutdatedWithAnotherLibrary() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'SSLlib']); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithMisbehavingCurl() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn([]); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'OpenSSL/1.0.1d']); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithMatchingOpenSslVersion1() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'OpenSSL/1.0.2b']); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsBuggyNss400() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'NSS/1.0.2b']); - $client = $this->getMockBuilder('\OCP\Http\Client\IClient') - ->disableOriginalConstructor()->getMock(); - $exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException') - ->disableOriginalConstructor()->getMock(); - $response = $this->getMockBuilder(ResponseInterface::class) - ->disableOriginalConstructor()->getMock(); - $response->expects($this->once()) - ->method('getStatusCode') - ->willReturn(400); - $exception->expects($this->once()) - ->method('getResponse') - ->willReturn($response); - - $client->expects($this->once()) - ->method('get') - ->with('https://nextcloud.com/', []) - ->will($this->throwException($exception)); - - $this->clientService->expects($this->once()) - ->method('newClient') - ->willReturn($client); - - $this->assertSame('cURL is using an outdated NSS version (NSS/1.0.2b). Please update your operating system or features such as installing and updating apps via the App Store or Federated Cloud Sharing will not work reliably.', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - - public function testIsBuggyNss200() { - $this->config->expects($this->any()) - ->method('getSystemValue') - ->willReturn(true); - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn(['ssl_version' => 'NSS/1.0.2b']); - $client = $this->getMockBuilder('\OCP\Http\Client\IClient') - ->disableOriginalConstructor()->getMock(); - $exception = $this->getMockBuilder('\GuzzleHttp\Exception\ClientException') - ->disableOriginalConstructor()->getMock(); - $response = $this->getMockBuilder(ResponseInterface::class) - ->disableOriginalConstructor()->getMock(); - $response->expects($this->once()) - ->method('getStatusCode') - ->willReturn(200); - $exception->expects($this->once()) - ->method('getResponse') - ->willReturn($response); - - $client->expects($this->once()) - ->method('get') - ->with('https://nextcloud.com/', []) - ->will($this->throwException($exception)); - - $this->clientService->expects($this->once()) - ->method('newClient') - ->willReturn($client); - - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithInternetDisabled() { - $this->config - ->expects($this->once()) - ->method('getSystemValue') - ->with('has_internet_connection', true) - ->willReturn(false); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithAppstoreDisabledAndServerToServerSharingEnabled() { - $this->config - ->expects($this->exactly(2)) - ->method('getSystemValue') - ->willReturnMap([ - ['has_internet_connection', true, true], - ['appstoreenabled', true, false], - ]); - $this->config - ->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['files_sharing', 'outgoing_server2server_share_enabled', 'yes', 'no'], - ['files_sharing', 'incoming_server2server_share_enabled', 'yes', 'yes'], - ]); - - $this->checkSetupController - ->expects($this->once()) - ->method('getCurlVersion') - ->willReturn([]); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - - public function testIsUsedTlsLibOutdatedWithAppstoreDisabledAndServerToServerSharingDisabled() { - $this->config - ->expects($this->exactly(2)) - ->method('getSystemValue') - ->willReturnMap([ - ['has_internet_connection', true, true], - ['appstoreenabled', true, false], - ]); - $this->config - ->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['files_sharing', 'outgoing_server2server_share_enabled', 'yes', 'no'], - ['files_sharing', 'incoming_server2server_share_enabled', 'yes', 'no'], - ]); - - $this->checkSetupController - ->expects($this->never()) - ->method('getCurlVersion') - ->willReturn([]); - $this->assertSame('', $this->invokePrivate($this->checkSetupController, 'isUsedTlsLibOutdated')); - } - public function testRescanFailedIntegrityCheck() { $this->checker ->expects($this->once()) @@ -890,7 +694,6 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, @@ -935,7 +738,6 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m 'settings', $this->request, $this->config, - $this->clientService, $this->urlGenerator, $this->l10n, $this->checker, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 653036ba1bcb5..67ebabe1ae65f 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -188,12 +188,6 @@ type: OC.SetupChecks.MESSAGE_TYPE_ERROR }); } - if(data.isUsedTlsLibOutdated) { - messages.push({ - msg: data.isUsedTlsLibOutdated, - type: OC.SetupChecks.MESSAGE_TYPE_WARNING - }); - } if(!data.isCorrectMemcachedPHPModuleInstalled) { messages.push({ msg: t('core', 'Memcached is configured as distributed cache, but the wrong PHP module "memcache" is installed. \\OC\\Memcache\\Memcached only supports "memcached" and not "memcache". See the {linkstart}memcached wiki about both modules ↗{linkend}.')