Merge pull request #42807 from nextcloud/enh/remove-curl-obsolete-ssl…

…-check
nextcloud · Jan 17, 2024 · 5de3028 · 5de3028
2 parents 5a493b1 + 0e889da
commit 5de3028
Show file tree

Hide file tree

Showing 3 changed files with 0 additions and 278 deletions.
diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php
@@ -45,15 +45,13 @@
  */
 namespace OCA\Settings\Controller;
 
-use GuzzleHttp\Exception\ClientException;
 use OC\AppFramework\Http;
 use OC\IntegrityCheck\Checker;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI;
 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
@@ -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,