Merge pull request #25153 from nextcloud/bugfix/noid/force-signature-verification-on-occ

Force signature verification of apps on occ

core/Command/Integrity/CheckApp.php

@@ -70,7 +70,7 @@ class CheckApp extends Base {
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$appid = $input->getArgument('appid');
 		$path = (string)$input->getOption('path');
-		$result = $this->checker->verifyAppSignature($appid, $path);
+		$result = $this->checker->verifyAppSignature($appid, $path, true);
 		$this->writeArrayInOutputFormat($input, $output, $result);
 		if (count($result) > 0) {
 			return 1;

core/Command/Integrity/CheckCore.php

@@ -61,6 +61,11 @@ class CheckCore extends Base {
 	 * {@inheritdoc }
 	 */
 	protected function execute(InputInterface $input, OutputInterface $output): int {
+		if (!$this->checker->isCodeCheckEnforced()) {
+			$output->writeln('<comment>integrity:check-core can not be used on git checkouts</comment>');
+			return 2;
+		}
+
 		$result = $this->checker->verifyCoreSignature();
 		$this->writeArrayInOutputFormat($input, $output, $result);
 		if (count($result) > 0) {

core/Command/Upgrade.php

@@ -263,7 +263,7 @@ class Upgrade extends Command {
 			return self::ERROR_SUCCESS;
 		} elseif ($this->config->getSystemValueBool('maintenance')) {
 			//Possible scenario: Nextcloud core is updated but an app failed
-			$output->writeln('<warning>Nextcloud is in maintenance mode</warning>');
+			$output->writeln('<comment>Nextcloud is in maintenance mode</comment>');
 			$output->write('<comment>Maybe an upgrade is already in process. Please check the '
 				. 'logfile (data/nextcloud.log). If you want to re-run the '
 				. 'upgrade procedure, remove the "maintenance mode" from '

lib/private/IntegrityCheck/Checker.php

@@ -44,7 +44,6 @@ use OCP\Files\IMimeTypeDetector;
 use OCP\ICache;
 use OCP\ICacheFactory;
 use OCP\IConfig;
-use OCP\ITempManager;
 use phpseclib\Crypt\RSA;
 use phpseclib\File\X509;
 
@@ -66,14 +65,12 @@ class Checker {
 	private $appLocator;
 	/** @var FileAccessHelper */
 	private $fileAccessHelper;
-	/** @var IConfig */
+	/** @var IConfig|null */
 	private $config;
 	/** @var ICache */
 	private $cache;
-	/** @var IAppManager */
+	/** @var IAppManager|null */
 	private $appManager;
-	/** @var ITempManager */
-	private $tempManager;
 	/** @var IMimeTypeDetector */
 	private $mimeTypeDetector;
 
@@ -81,19 +78,17 @@ class Checker {
 	 * @param EnvironmentHelper $environmentHelper
 	 * @param FileAccessHelper $fileAccessHelper
 	 * @param AppLocator $appLocator
-	 * @param IConfig $config
+	 * @param IConfig|null $config
 	 * @param ICacheFactory $cacheFactory
-	 * @param IAppManager $appManager
-	 * @param ITempManager $tempManager
+	 * @param IAppManager|null $appManager
 	 * @param IMimeTypeDetector $mimeTypeDetector
 	 */
 	public function __construct(EnvironmentHelper $environmentHelper,
 								FileAccessHelper $fileAccessHelper,
 								AppLocator $appLocator,
-								IConfig $config = null,
+								?IConfig $config,
 								ICacheFactory $cacheFactory,
-								IAppManager $appManager = null,
-								ITempManager $tempManager,
+								?IAppManager $appManager,
 								IMimeTypeDetector $mimeTypeDetector) {
 		$this->environmentHelper = $environmentHelper;
 		$this->fileAccessHelper = $fileAccessHelper;
@@ -101,7 +96,6 @@ class Checker {
 		$this->config = $config;
 		$this->cache = $cacheFactory->createDistributed(self::CACHE_KEY);
 		$this->appManager = $appManager;
-		$this->tempManager = $tempManager;
 		$this->mimeTypeDetector = $mimeTypeDetector;
 	}
 
@@ -311,12 +305,13 @@ class Checker {
 	 * @param string $signaturePath
 	 * @param string $basePath
 	 * @param string $certificateCN
+	 * @param bool $forceVerify
 	 * @return array
 	 * @throws InvalidSignatureException
 	 * @throws \Exception
 	 */
-	private function verify(string $signaturePath, string $basePath, string $certificateCN): array {
-		if (!$this->isCodeCheckEnforced()) {
+	private function verify(string $signaturePath, string $basePath, string $certificateCN, bool $forceVerify = false): array {
+		if (!$forceVerify && !$this->isCodeCheckEnforced()) {
 			return [];
 		}
 
@@ -495,9 +490,10 @@ class Checker {
 	 *
 	 * @param string $appId
 	 * @param string $path Optional path. If none is given it will be guessed.
+	 * @param bool $forceVerify
 	 * @return array
 	 */
-	public function verifyAppSignature(string $appId, string $path = ''): array {
+	public function verifyAppSignature(string $appId, string $path = '', bool $forceVerify = false): array {
 		try {
 			if ($path === '') {
 				$path = $this->appLocator->getAppPath($appId);
@@ -505,7 +501,8 @@ class Checker {
 			$result = $this->verify(
 					$path . '/appinfo/signature.json',
 					$path,
-					$appId
+					$appId,
+					$forceVerify
 			);
 		} catch (\Exception $e) {
 			$result = [

lib/private/Server.php

@@ -942,7 +942,6 @@ class Server extends ServerContainer implements IServerContainer {
 				$config,
 				$c->get(ICacheFactory::class),
 				$appManager,
-				$c->get(ITempManager::class),
 				$c->get(IMimeTypeDetector::class)
 			);
 		});

tests/lib/IntegrityCheck/CheckerTest.php

@@ -77,7 +77,6 @@ class CheckerTest extends TestCase {
 			$this->config,
 			$this->cacheFactory,
 			$this->appManager,
-			\OC::$server->getTempManager(),
 			$this->mimeTypeDetector
 		);
 	}
@@ -1279,7 +1278,6 @@ class CheckerTest extends TestCase {
 				$this->config,
 				$this->cacheFactory,
 				$this->appManager,
-				\OC::$server->getTempManager(),
 				$this->mimeTypeDetector,
 			])
 			->setMethods([