Merge pull request #21693 from nextcloud/fix/noid/import-certificates-only-by-system

Improve CertificateManager to not be user context dependent

10 files changed, 125 insertions, 106 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index aa2b5100b79..f9479cc6d21 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -769,6 +769,7 @@ return array(
     'OC\\Contacts\\ContactsMenu\\Providers\\EMailProvider' => $baseDir . '/lib/private/Contacts/ContactsMenu/Providers/EMailProvider.php',
     'OC\\Core\\Application' => $baseDir . '/core/Application.php',
     'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => $baseDir . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php',
+    'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => $baseDir . '/core/BackgroundJobs/CheckForUserCertificates.php',
     'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => $baseDir . '/core/BackgroundJobs/CleanupLoginFlowV2.php',
     'OC\\Core\\Command\\App\\CheckCode' => $baseDir . '/core/Command/App/CheckCode.php',
     'OC\\Core\\Command\\App\\Disable' => $baseDir . '/core/Command/App/Disable.php',
@@ -1254,6 +1255,7 @@ return array(
     'OC\\Repair\\NC20\\EncryptionLegacyCipher' => $baseDir . '/lib/private/Repair/NC20/EncryptionLegacyCipher.php',
     'OC\\Repair\\NC20\\EncryptionMigration' => $baseDir . '/lib/private/Repair/NC20/EncryptionMigration.php',
     'OC\\Repair\\NC20\\ShippedDashboardEnable' => $baseDir . '/lib/private/Repair/NC20/ShippedDashboardEnable.php',
+    'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => $baseDir . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php',
     'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php',
     'OC\\Repair\\Owncloud\\DropAccountTermsTable' => $baseDir . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
     'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 2eef38f5d89..a0336fc6bd1 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -798,6 +798,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Contacts\\ContactsMenu\\Providers\\EMailProvider' => __DIR__ . '/../../..' . '/lib/private/Contacts/ContactsMenu/Providers/EMailProvider.php',
         'OC\\Core\\Application' => __DIR__ . '/../../..' . '/core/Application.php',
         'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php',
+        'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CheckForUserCertificates.php',
         'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CleanupLoginFlowV2.php',
         'OC\\Core\\Command\\App\\CheckCode' => __DIR__ . '/../../..' . '/core/Command/App/CheckCode.php',
         'OC\\Core\\Command\\App\\Disable' => __DIR__ . '/../../..' . '/core/Command/App/Disable.php',
@@ -1283,6 +1284,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Repair\\NC20\\EncryptionLegacyCipher' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionLegacyCipher.php',
         'OC\\Repair\\NC20\\EncryptionMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionMigration.php',
         'OC\\Repair\\NC20\\ShippedDashboardEnable' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/ShippedDashboardEnable.php',
+        'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php',
         'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php',
         'OC\\Repair\\Owncloud\\DropAccountTermsTable' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
         'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',
diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php
index a6cfd77d98a..974feee8995 100644
--- a/lib/private/Files/Storage/DAV.php
+++ b/lib/private/Files/Storage/DAV.php
@@ -122,9 +122,6 @@ class DAV extends Common {
 			if ($this->secure === true) {
 				// inject mock for testing
 				$this->certManager = \OC::$server->getCertificateManager();
-				if (is_null($this->certManager)) { //no user
-					$this->certManager = \OC::$server->getCertificateManager(null);
-				}
 			}
 			$this->root = $params['root'] ?? '/';
 			$this->root = '/' . ltrim($this->root, '/');
diff --git a/lib/private/Http/Client/Client.php b/lib/private/Http/Client/Client.php
index 35171810a68..5ac29afe31d 100644
--- a/lib/private/Http/Client/Client.php
+++ b/lib/private/Http/Client/Client.php
@@ -105,10 +105,6 @@ class Client implements IClient {
 			return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';
 		}
 
-		if ($this->certificateManager->listCertificates() === []) {
-			return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';
-		}
-
 		return $this->certificateManager->getAbsoluteBundlePath();
 	}
 
diff --git a/lib/private/Repair.php b/lib/private/Repair.php
index eb66ed9dd55..2b9b14b58b6 100644
--- a/lib/private/Repair.php
+++ b/lib/private/Repair.php
@@ -52,6 +52,7 @@ use OC\Repair\NC18\ResetGeneratedAvatarFlag;
 use OC\Repair\NC20\EncryptionLegacyCipher;
 use OC\Repair\NC20\EncryptionMigration;
 use OC\Repair\NC20\ShippedDashboardEnable;
+use OC\Repair\NC21\AddCheckForUserCertificatesJob;
 use OC\Repair\OldGroupMembershipShares;
 use OC\Repair\Owncloud\DropAccountTermsTable;
 use OC\Repair\Owncloud\SaveAccountsTableData;
@@ -164,6 +165,7 @@ class Repair implements IOutput {
 			\OC::$server->query(EncryptionMigration::class),
 			\OC::$server->get(ShippedDashboardEnable::class),
 			\OC::$server->get(AddBruteForceCleanupJob::class),
+			\OC::$server->get(AddCheckForUserCertificatesJob::class),
 		];
 	}
 
diff --git a/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php b/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php
new file mode 100644
index 00000000000..df6637e3948
--- /dev/null
+++ b/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php
@@ -0,0 +1,61 @@
+<?php
+/**
+ * @copyright Copyright (c) 2020 Morris Jobke <hey@morrisjobke.de>
+ *
+ * @author Morris Jobke <hey@morrisjobke.de>
+ *
+ * @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\Repair\NC21;
+
+use OC\Core\BackgroundJobs\CheckForUserCertificates;
+use OCP\BackgroundJob\IJobList;
+use OCP\IConfig;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class AddCheckForUserCertificatesJob implements IRepairStep {
+
+	/** @var IJobList */
+	protected $jobList;
+	/** @var IConfig */
+	private $config;
+
+	public function __construct(IConfig $config, IJobList $jobList) {
+		$this->jobList = $jobList;
+		$this->config = $config;
+	}
+
+	public function getName() {
+		return 'Queue a one-time job to check for user uploaded certificates';
+	}
+
+	private function shouldRun() {
+		$versionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0.0');
+
+		// was added to 21.0.0.2
+		return version_compare($versionFromBeforeUpdate, '21.0.0.2', '<');
+	}
+
+	public function run(IOutput $output) {
+		if ($this->shouldRun()) {
+			$this->config->setAppValue('files_external', 'user_certificate_scan', 'not-run-yet');
+			$this->jobList->add(CheckForUserCertificates::class);
+		}
+	}
+}
diff --git a/lib/private/Security/CertificateManager.php b/lib/private/Security/CertificateManager.php
index e69132ff4df..ef0c6563320 100644
--- a/lib/private/Security/CertificateManager.php
+++ b/lib/private/Security/CertificateManager.php
@@ -40,11 +40,6 @@ use OCP\Security\ISecureRandom;
  */
 class CertificateManager implements ICertificateManager {
 	/**
-	 * @var string
-	 */
-	protected $uid;
-
-	/**
 	 * @var \OC\Files\View
 	 */
 	protected $view;
@@ -63,18 +58,15 @@ class CertificateManager implements ICertificateManager {
 	protected $random;
 
 	/**
-	 * @param string $uid
 	 * @param \OC\Files\View $view relative to data/
 	 * @param IConfig $config
 	 * @param ILogger $logger
 	 * @param ISecureRandom $random
 	 */
-	public function __construct($uid,
-								\OC\Files\View $view,
+	public function __construct(\OC\Files\View $view,
 								IConfig $config,
 								ILogger $logger,
 								ISecureRandom $random) {
-		$this->uid = $uid;
 		$this->view = $view;
 		$this->config = $config;
 		$this->logger = $logger;
@@ -112,6 +104,29 @@ class CertificateManager implements ICertificateManager {
 		return $result;
 	}
 
+	private function hasCertificates(): bool {
+		if (!$this->config->getSystemValue('installed', false)) {
+			return false;
+		}
+
+		$path = $this->getPathToCertificates() . 'uploads/';
+		if (!$this->view->is_dir($path)) {
+			return false;
+		}
+		$result = [];
+		$handle = $this->view->opendir($path);
+		if (!is_resource($handle)) {
+			return false;
+		}
+		while (false !== ($file = readdir($handle))) {
+			if ($file !== '.' && $file !== '..') {
+				return true;
+			}
+		}
+		closedir($handle);
+		return false;
+	}
+
 	/**
 	 * create the certificate bundle of all trusted certificated
 	 */
@@ -148,7 +163,7 @@ class CertificateManager implements ICertificateManager {
 		fwrite($fhCerts, $defaultCertificates);
 
 		// Append the system certificate bundle
-		$systemBundle = $this->getCertificateBundle(null);
+		$systemBundle = $this->getCertificateBundle();
 		if ($systemBundle !== $certPath && $this->view->file_exists($systemBundle)) {
 			$systemCertificates = $this->view->file_get_contents($systemBundle);
 			fwrite($fhCerts, $systemCertificates);
@@ -207,73 +222,50 @@ class CertificateManager implements ICertificateManager {
 	}
 
 	/**
-	 * Get the path to the certificate bundle for this user
+	 * Get the path to the certificate bundle
 	 *
-	 * @param string|null $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle
 	 * @return string
 	 */
-	public function getCertificateBundle($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
-		}
-		return $this->getPathToCertificates($uid) . 'rootcerts.crt';
+	public function getCertificateBundle() {
+		return $this->getPathToCertificates() . 'rootcerts.crt';
 	}
 
 	/**
-	 * Get the full local path to the certificate bundle for this user
+	 * Get the full local path to the certificate bundle
 	 *
-	 * @param string $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle
 	 * @return string
 	 */
-	public function getAbsoluteBundlePath($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
+	public function getAbsoluteBundlePath() {
+		if (!$this->hasCertificates()) {
+			return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';
 		}
-		if ($this->needsRebundling($uid)) {
-			if (is_null($uid)) {
-				$manager = new CertificateManager(null, $this->view, $this->config, $this->logger, $this->random);
-				$manager->createCertificateBundle();
-			} else {
-				$this->createCertificateBundle();
-			}
+
+		if ($this->needsRebundling()) {
+			$this->createCertificateBundle();
 		}
-		return $this->view->getLocalFile($this->getCertificateBundle($uid));
+
+		return $this->view->getLocalFile($this->getCertificateBundle());
 	}
 
 	/**
-	 * @param string|null $uid (optional) user to get the certificate path for, use `null` to get the system path
 	 * @return string
 	 */
-	private function getPathToCertificates($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
-		}
-		return is_null($uid) ? '/files_external/' : '/' . $uid . '/files_external/';
+	private function getPathToCertificates() {
+		return '/files_external/';
 	}
 
 	/**
 	 * Check if we need to re-bundle the certificates because one of the sources has updated
 	 *
-	 * @param string $uid (optional) user to get the certificate path for, use `null` to get the system path
 	 * @return bool
 	 */
-	private function needsRebundling($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
-		}
-		$sourceMTimes = [$this->getFilemtimeOfCaBundle()];
-		$targetBundle = $this->getCertificateBundle($uid);
+	private function needsRebundling() {
+		$targetBundle = $this->getCertificateBundle();
 		if (!$this->view->file_exists($targetBundle)) {
 			return true;
 		}
 
-		if (!is_null($uid)) { // also depend on the system bundle
-			$sourceMTimes[] = $this->view->filemtime($this->getCertificateBundle(null));
-		}
-
-		$sourceMTime = array_reduce($sourceMTimes, function ($max, $mtime) {
-			return max($max, $mtime);
-		}, 0);
+		$sourceMTime = $this->getFilemtimeOfCaBundle();
 		return $sourceMTime > $this->view->filemtime($targetBundle);
 	}
 
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 5de7808523e..8e2452b2f30 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -178,6 +178,7 @@ use OCP\IAppConfig;
 use OCP\IAvatarManager;
 use OCP\ICache;
 use OCP\ICacheFactory;
+use OCP\ICertificateManager;
 use OCP\IDateTimeFormatter;
 use OCP\IDateTimeZone;
 use OCP\IDBConnection;
@@ -823,23 +824,8 @@ class Server extends ServerContainer implements IServerContainer {
 		/** @deprecated 19.0.0 */
 		$this->registerDeprecatedAlias('DatabaseConnection', IDBConnection::class);
 
-
-		$this->registerService(IClientService::class, function (ContainerInterface $c) {
-			$user = \OC_User::getUser();
-			$uid = $user ? $user : null;
-			return new ClientService(
-				$c->get(\OCP\IConfig::class),
-				$c->get(ILogger::class),
-				new \OC\Security\CertificateManager(
-					$uid,
-					new View(),
-					$c->get(\OCP\IConfig::class),
-					$c->get(ILogger::class),
-					$c->get(ISecureRandom::class)
-				)
-			);
-		});
-		/** @deprecated 19.0.0 */
+		$this->registerAlias(ICertificateManager::class, CertificateManager::class);
+		$this->registerAlias(IClientService::class, ClientService::class);
 		$this->registerDeprecatedAlias('HttpClientService', IClientService::class);
 		$this->registerService(IEventLogger::class, function (ContainerInterface $c) {
 			$eventLogger = new EventLogger();
@@ -1840,28 +1826,12 @@ class Server extends ServerContainer implements IServerContainer {
 	}
 
 	/**
-	 * Get the certificate manager for the user
+	 * Get the certificate manager
 	 *
-	 * @param string $userId (optional) if not specified the current loggedin user is used, use null to get the system certificate manager
-	 * @return \OCP\ICertificateManager | null if $uid is null and no user is logged in
-	 * @deprecated 20.0.0
+	 * @return \OCP\ICertificateManager
 	 */
-	public function getCertificateManager($userId = '') {
-		if ($userId === '') {
-			$userSession = $this->get(IUserSession::class);
-			$user = $userSession->getUser();
-			if (is_null($user)) {
-				return null;
-			}
-			$userId = $user->getUID();
-		}
-		return new CertificateManager(
-			$userId,
-			new View(),
-			$this->get(\OCP\IConfig::class),
-			$this->get(ILogger::class),
-			$this->get(ISecureRandom::class)
-		);
+	public function getCertificateManager() {
+		return $this->get(ICertificateManager::class);
 	}
 
 	/**
diff --git a/lib/public/ICertificateManager.php b/lib/public/ICertificateManager.php
index 1c7949a89cf..da97dc105d0 100644
--- a/lib/public/ICertificateManager.php
+++ b/lib/public/ICertificateManager.php
@@ -25,12 +25,12 @@
 namespace OCP;
 
 /**
- * Manage trusted certificates for users
+ * Manage trusted certificates
  * @since 8.0.0
  */
 interface ICertificateManager {
 	/**
-	 * Returns all certificates trusted by the user
+	 * Returns all certificates trusted by the system
 	 *
 	 * @return \OCP\ICertificate[]
 	 * @since 8.0.0
@@ -53,20 +53,18 @@ interface ICertificateManager {
 	public function removeCertificate($name);
 
 	/**
-	 * Get the path to the certificate bundle for this user
+	 * Get the path to the certificate bundle
 	 *
-	 * @param string $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle (since 9.0.0)
 	 * @return string
 	 * @since 8.0.0
 	 */
-	public function getCertificateBundle($uid = '');
+	public function getCertificateBundle();
 
 	/**
-	 * Get the full local path to the certificate bundle for this user
+	 * Get the full local path to the certificate bundle
 	 *
-	 * @param string $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle
 	 * @return string
 	 * @since 9.0.0
 	 */
-	public function getAbsoluteBundlePath($uid = '');
+	public function getAbsoluteBundlePath();
 }
diff --git a/lib/public/IServerContainer.php b/lib/public/IServerContainer.php
index 215cfab84fd..ed08b66b994 100644
--- a/lib/public/IServerContainer.php
+++ b/lib/public/IServerContainer.php
@@ -387,14 +387,13 @@ interface IServerContainer extends ContainerInterface, IContainer {
 	public function getSearch();
 
 	/**
-	 * Get the certificate manager for the user
+	 * Get the certificate manager
 	 *
-	 * @param string $userId (optional) if not specified the current loggedin user is used, use null to get the system certificate manager
-	 * @return \OCP\ICertificateManager | null if $userId is null and no user is logged in
+	 * @return \OCP\ICertificateManager
 	 * @since 8.0.0
 	 * @deprecated 20.0.0 have it injected or fetch it through \Psr\Container\ContainerInterface::get
 	 */
-	public function getCertificateManager($userId = null);
+	public function getCertificateManager();
 
 	/**
 	 * Create a new event source