From dc479aae2d055dafddb250a382eb801a68d42afb Mon Sep 17 00:00:00 2001
From: Morris Jobke <hey@morrisjobke.de>
Date: Sun, 5 Jul 2020 14:31:19 +0200
Subject: Improve CertificateManager to not be user context dependent

* removes the ability for users to import their own certificates (for external storage)
* reliably returns the same certificate bundles system wide (and not depending on the user context and available sessions)

The user specific certificates were broken in some cases anyways, as they are only loaded if the specific user is logged in and thus causing unexpected behavior for background jobs and other non-user triggered code paths.

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
---
 lib/private/Security/CertificateManager.php | 66 +++++++----------------------
 1 file changed, 15 insertions(+), 51 deletions(-)

(limited to 'lib/private/Security')

diff --git a/lib/private/Security/CertificateManager.php b/lib/private/Security/CertificateManager.php
index e69132ff4df..ed873527d3c 100644
--- a/lib/private/Security/CertificateManager.php
+++ b/lib/private/Security/CertificateManager.php
@@ -39,11 +39,6 @@ use OCP\Security\ISecureRandom;
  * Manage trusted certificates for users
  */
 class CertificateManager implements ICertificateManager {
-	/**
-	 * @var string
-	 */
-	protected $uid;
-
 	/**
 	 * @var \OC\Files\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;
@@ -148,7 +140,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 +199,45 @@ 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;
-		}
-		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();
-			}
+	public function getAbsoluteBundlePath() {
+		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);
 	}
 
-- 
cgit v1.2.3