From dd54f35f29ec8274e52f6714cacbab22ce87017e Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Sun, 14 Jan 2024 19:52:51 +0100 Subject: [PATCH] fix(certificate manager): migrate certificate bundles from files_external to data CertificateManager doesn't work propertly if the files_external app is disabled, so let's store directly in /data/certificate_manager the bundled certificates. This always has to be done on local disk as curl currently requires a path to the cert bundle. When we require PHP 8.1 we will be able to simply store the certificate bundle in database/memory/cache and pass it through the CURLOPT_SSLCERT_BLOB option. Signed-off-by: Thomas Citharel --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Repair.php | 2 + .../Repair/NC29/MoveCertificateBundles.php | 66 +++++++++++++++++ lib/private/Security/CertificateManager.php | 5 +- .../NC29/MoveCertificateBundlesTest.php | 70 +++++++++++++++++++ 6 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 lib/private/Repair/NC29/MoveCertificateBundles.php create mode 100644 tests/lib/Repair/NC29/MoveCertificateBundlesTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 2fffd0c2f2db3..1f64909c143c6 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1655,6 +1655,7 @@ 'OC\\Repair\\NC22\\LookupServerSendCheck' => $baseDir . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => $baseDir . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => $baseDir . '/lib/private/Repair/NC25/AddMissingSecretJob.php', + 'OC\\Repair\\NC29\\MoveCertificateBundles' => $baseDir . '/lib/private/Repair/NC29/MoveCertificateBundles.php', 'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviews.php', 'OC\\Repair\\Owncloud\\CleanPreviewsBackgroundJob' => $baseDir . '/lib/private/Repair/Owncloud/CleanPreviewsBackgroundJob.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index d38db8050cc68..4723a4520a7ba 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1688,6 +1688,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\NC22\\LookupServerSendCheck' => __DIR__ . '/../../..' . '/lib/private/Repair/NC22/LookupServerSendCheck.php', 'OC\\Repair\\NC24\\AddTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC24/AddTokenCleanupJob.php', 'OC\\Repair\\NC25\\AddMissingSecretJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC25/AddMissingSecretJob.php', + 'OC\\Repair\\NC29\\MoveCertificateBundles' => __DIR__ . '/../../..' . '/lib/private/Repair/NC29/MoveCertificateBundles.php', 'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\CleanPreviews' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviews.php', 'OC\\Repair\\Owncloud\\CleanPreviewsBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/CleanPreviewsBackgroundJob.php', diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 1af9b0d1b22c7..8f16e943c9f6a 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -68,6 +68,7 @@ use OC\Repair\NC22\LookupServerSendCheck; use OC\Repair\NC24\AddTokenCleanupJob; use OC\Repair\NC25\AddMissingSecretJob; +use OC\Repair\NC29\MoveCertificateBundles; use OC\Repair\OldGroupMembershipShares; use OC\Repair\Owncloud\CleanPreviews; use OC\Repair\Owncloud\DropAccountTermsTable; @@ -209,6 +210,7 @@ public static function getRepairSteps(): array { \OCP\Server::get(AddMissingSecretJob::class), \OCP\Server::get(AddRemoveOldTasksBackgroundJob::class), \OCP\Server::get(AddMetadataGenerationJob::class), + \OCP\Server::get(MoveCertificateBundles::class), ]; } diff --git a/lib/private/Repair/NC29/MoveCertificateBundles.php b/lib/private/Repair/NC29/MoveCertificateBundles.php new file mode 100644 index 0000000000000..05a9f651b6b87 --- /dev/null +++ b/lib/private/Repair/NC29/MoveCertificateBundles.php @@ -0,0 +1,66 @@ + + * + * @author Thomas Citharel + * + * @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\Repair\NC29; + +use OC\Files\View; +use OCP\IConfig; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + + +class MoveCertificateBundles implements IRepairStep { + + const OLD_PATH = '/files_external'; + const ROOT_CERTS_FILENAME = '/rootcerts.crt'; + const CERTS_UPLOAD_PATH = '/uploads'; + + protected string $newRootPath; + + public function __construct(protected View $view, protected IConfig $config) { + $this->newRootPath = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/data/certificate_manager'; + } + + public function getName(): string { + return 'Move the certificate bundles from data/files_external/ to data/certificate_manager/'; + } + + + public function run(IOutput $output): void { + if (!$this->shouldRun()) { + return; + } + + $oldCertificateBundlePath = self::OLD_PATH . self::ROOT_CERTS_FILENAME; + $oldUploadsPath = self::OLD_PATH . self::CERTS_UPLOAD_PATH; + + $this->view->copy($oldUploadsPath, $this->newRootPath . self::CERTS_UPLOAD_PATH, true); + $this->view->copy($oldCertificateBundlePath, $this->newRootPath . self::ROOT_CERTS_FILENAME, true); + + $this->view->unlink($oldCertificateBundlePath); + $this->view->unlink($oldUploadsPath); + } + + protected function shouldRun(): bool { + return $this->view->file_exists($this->newRootPath . self::ROOT_CERTS_FILENAME); + } +} diff --git a/lib/private/Security/CertificateManager.php b/lib/private/Security/CertificateManager.php index cf5f0f41d56a6..fc0fd715f9e7a 100644 --- a/lib/private/Security/CertificateManager.php +++ b/lib/private/Security/CertificateManager.php @@ -34,6 +34,7 @@ use OC\Files\Filesystem; use OC\Files\View; +use OCP\App\IAppManager; use OCP\ICertificate; use OCP\ICertificateManager; use OCP\IConfig; @@ -45,12 +46,14 @@ */ class CertificateManager implements ICertificateManager { private ?string $bundlePath = null; + private ?string $certificatesPath = null; public function __construct( protected View $view, protected IConfig $config, protected LoggerInterface $logger, protected ISecureRandom $random, + protected IAppManager $appManager ) { } @@ -249,7 +252,7 @@ public function getAbsoluteBundlePath(): string { } private function getPathToCertificates(): string { - return '/files_external/'; + return $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/data/certificate_manager/'; } /** diff --git a/tests/lib/Repair/NC29/MoveCertificateBundlesTest.php b/tests/lib/Repair/NC29/MoveCertificateBundlesTest.php new file mode 100644 index 0000000000000..5af576797beb9 --- /dev/null +++ b/tests/lib/Repair/NC29/MoveCertificateBundlesTest.php @@ -0,0 +1,70 @@ + + * + * @author Thomas Citharel + * + * @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\Repair\NC29; + +use OC\Files\View; +use OC\Repair\NC29\MoveCertificateBundles; +use OCP\IConfig; +use OCP\Migration\IOutput; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +/** + * Class FixMountStoragesTest + * + * @package Test\Repair\NC11 + * @group DB + */ +class MoveCertificateBundlesTest extends TestCase { + private View|MockObject $view; + private IConfig|MockObject $config; + private IOutput $output; + + private MoveCertificateBundles $repair; + + protected function setUp(): void { + parent::setUp(); + + $this->output = $this->createMock(IOutput::class); + + $this->view = $this->createMock(View::class); + $this->config = $this->createMock(IConfig::class); + $this->config->expects($this->once())->method('getSystemValue')->with('datadirectory', \OC::$SERVERROOT . '/data-autotest')->willReturn(\OC::$SERVERROOT . '/data-autotest'); + + $this->repair = new MoveCertificateBundles( + $this->view, + $this->config + ); + } + + public function testGetName() { + $this->assertSame('Move the certificate bundles from data/files_external/ to data/certificate_manager/', $this->repair->getName()); + } + + public function testSkipRepairStep() { + $this->view->expects($this->once())->method('file_exists')->with('/data-autotest/certificate_manager/rootcerts.crt')->willReturn(true); + $this->view->expects($this->never())->method('copy'); + $this->repair->run($this->output); + } +}