[stable31] feat(encryption): Add integration tests for occ commands and fix them

apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php

@@ -5,6 +5,7 @@
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OCA\DAV\Tests\unit\Connector\Sabre\RequestTest;
 
 use OCP\AppFramework\Http;

apps/encryption/lib/Command/DropLegacyFileKey.php

@@ -85,7 +85,7 @@ private function scanFolder(OutputInterface $output, string $folder): bool {
 					$output->writeln('<error>' . $path . ' does not have a proper header</error>');
 				} else {
 					try {
-						$legacyFileKey = $this->keyManager->getFileKey($path, null, true);
+						$legacyFileKey = $this->keyManager->getFileKey($path, true);
 						if ($legacyFileKey === '') {
 							$output->writeln('Got an empty legacy filekey for ' . $path . ', continuing', OutputInterface::VERBOSITY_VERBOSE);
 							continue;

apps/encryption/lib/Crypto/Crypt.php

@@ -342,9 +342,8 @@ public function encryptPrivateKey($privateKey, $password, $uid = '') {
 	 * @param string $privateKey
 	 * @param string $password
 	 * @param string $uid for regular users, empty for system keys
-	 * @return false|string
 	 */
-	public function decryptPrivateKey($privateKey, $password = '', $uid = '') {
+	public function decryptPrivateKey($privateKey, $password = '', $uid = '') : string|false {
 		$header = $this->parseHeader($privateKey);
 
 		if (isset($header['cipher'])) {

apps/encryption/lib/Crypto/DecryptAll.php

@@ -1,10 +1,13 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2019-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OCA\Encryption\Crypto;
 
 use OCA\Encryption\Exceptions\PrivateKeyMissingException;
@@ -18,14 +21,6 @@
 use Symfony\Component\Console\Question\Question;
 
 class DecryptAll {
-
-	/**
-	 * @param Util $util
-	 * @param KeyManager $keyManager
-	 * @param Crypt $crypt
-	 * @param Session $session
-	 * @param QuestionHelper $questionHelper
-	 */
 	public function __construct(
 		protected Util $util,
 		protected KeyManager $keyManager,
@@ -37,13 +32,8 @@ public function __construct(
 
 	/**
 	 * prepare encryption module to decrypt all files
-	 *
-	 * @param InputInterface $input
-	 * @param OutputInterface $output
-	 * @param $user
-	 * @return bool
 	 */
-	public function prepare(InputInterface $input, OutputInterface $output, $user) {
+	public function prepare(InputInterface $input, OutputInterface $output, ?string $user): bool {
 		$question = new Question('Please enter the recovery key password: ');
 
 		if ($this->util->isMasterKeyEnabled()) {
@@ -52,7 +42,7 @@ public function prepare(InputInterface $input, OutputInterface $output, $user) {
 			$password = $this->keyManager->getMasterKeyPassword();
 		} else {
 			$recoveryKeyId = $this->keyManager->getRecoveryKeyId();
-			if (!empty($user)) {
+			if ($user !== null && $user !== '') {
 				$output->writeln('You can only decrypt the users files if you know');
 				$output->writeln('the users password or if they activated the recovery key.');
 				$output->writeln('');
@@ -96,12 +86,9 @@ public function prepare(InputInterface $input, OutputInterface $output, $user) {
 	/**
 	 * get the private key which will be used to decrypt all files
 	 *
-	 * @param string $user
-	 * @param string $password
-	 * @return bool|string
 	 * @throws PrivateKeyMissingException
 	 */
-	protected function getPrivateKey($user, $password) {
+	protected function getPrivateKey(string $user, string $password): string|false {
 		$recoveryKeyId = $this->keyManager->getRecoveryKeyId();
 		$masterKeyId = $this->keyManager->getMasterKeyId();
 		if ($user === $recoveryKeyId) {
@@ -118,7 +105,7 @@ protected function getPrivateKey($user, $password) {
 		return $privateKey;
 	}
 
-	protected function updateSession($user, $privateKey) {
+	protected function updateSession(string $user, string $privateKey): void {
 		$this->session->prepareDecryptAll($user, $privateKey);
 	}
 }

apps/encryption/lib/Crypto/EncryptAll.php

@@ -1,10 +1,13 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2017-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OCA\Encryption\Crypto;
 
 use OC\Encryption\Exceptions\DecryptionFailedException;
@@ -60,11 +63,8 @@ public function __construct(
 
 	/**
 	 * start to encrypt all files
-	 *
-	 * @param InputInterface $input
-	 * @param OutputInterface $output
 	 */
-	public function encryptAll(InputInterface $input, OutputInterface $output) {
+	public function encryptAll(InputInterface $input, OutputInterface $output): void {
 		$this->input = $input;
 		$this->output = $output;
 
@@ -111,7 +111,7 @@ public function encryptAll(InputInterface $input, OutputInterface $output) {
 	/**
 	 * create key-pair for every user
 	 */
-	protected function createKeyPairs() {
+	protected function createKeyPairs(): void {
 		$this->output->writeln("\n");
 		$progress = new ProgressBar($this->output);
 		$progress->setFormat(" %message% \n [%bar%]");
@@ -146,7 +146,7 @@ protected function createKeyPairs() {
 	/**
 	 * iterate over all user and encrypt their files
 	 */
-	protected function encryptAllUsersFiles() {
+	protected function encryptAllUsersFiles(): void {
 		$this->output->writeln("\n");
 		$progress = new ProgressBar($this->output);
 		$progress->setFormat(" %message% \n [%bar%]");
@@ -168,10 +168,8 @@ protected function encryptAllUsersFiles() {
 
 	/**
 	 * encrypt all user files with the master key
-	 *
-	 * @param ProgressBar $progress
 	 */
-	protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress) {
+	protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress): void {
 		$userNo = 1;
 		foreach ($this->userManager->getBackends() as $backend) {
 			$limit = 500;
@@ -190,12 +188,8 @@ protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress) {
 
 	/**
 	 * encrypt files from the given user
-	 *
-	 * @param string $uid
-	 * @param ProgressBar $progress
-	 * @param string $userCount
 	 */
-	protected function encryptUsersFiles($uid, ProgressBar $progress, $userCount) {
+	protected function encryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void {
 		$this->setupUserFS($uid);
 		$directories = [];
 		$directories[] = '/' . $uid . '/files';
@@ -268,7 +262,7 @@ protected function encryptFile(FileInfo $fileInfo, string $path): bool {
 	/**
 	 * output one-time encryption passwords
 	 */
-	protected function outputPasswords() {
+	protected function outputPasswords(): void {
 		$table = new Table($this->output);
 		$table->setHeaders(['Username', 'Private key password']);
 
@@ -309,10 +303,8 @@ protected function outputPasswords() {
 
 	/**
 	 * write one-time encryption passwords to a csv file
-	 *
-	 * @param array $passwords
 	 */
-	protected function writePasswordsToFile(array $passwords) {
+	protected function writePasswordsToFile(array $passwords): void {
 		$fp = $this->rootView->fopen('oneTimeEncryptionPasswords.csv', 'w');
 		foreach ($passwords as $pwd) {
 			fputcsv($fp, $pwd);
@@ -330,21 +322,18 @@ protected function writePasswordsToFile(array $passwords) {
 
 	/**
 	 * setup user file system
-	 *
-	 * @param string $uid
 	 */
-	protected function setupUserFS($uid) {
+	protected function setupUserFS(string $uid): void {
 		\OC_Util::tearDownFS();
 		\OC_Util::setupFS($uid);
 	}
 
 	/**
 	 * generate one time password for the user and store it in a array
 	 *
-	 * @param string $uid
 	 * @return string password
 	 */
-	protected function generateOneTimePassword($uid) {
+	protected function generateOneTimePassword(string $uid): string {
 		$password = $this->secureRandom->generate(16, ISecureRandom::CHAR_HUMAN_READABLE);
 		$this->userPasswords[$uid] = $password;
 		return $password;
@@ -353,7 +342,7 @@ protected function generateOneTimePassword($uid) {
 	/**
 	 * send encryption key passwords to the users by mail
 	 */
-	protected function sendPasswordsByMail() {
+	protected function sendPasswordsByMail(): void {
 		$noMail = [];
 
 		$this->output->writeln('');

apps/encryption/lib/Crypto/Encryption.php

@@ -438,7 +438,7 @@ public function getUnencryptedBlockSize($signed = false) {
 	 * @throws DecryptionFailedException
 	 */
 	public function isReadable($path, $uid) {
-		$fileKey = $this->keyManager->getFileKey($path, $uid, null);
+		$fileKey = $this->keyManager->getFileKey($path, null);
 		if (empty($fileKey)) {
 			$owner = $this->util->getOwner($path);
 			if ($owner !== $uid) {

apps/encryption/lib/KeyManager.php

@@ -136,7 +136,11 @@ public function validateMasterKey() {
 		if (!$this->session->isPrivateKeySet()) {
 			$masterKey = $this->getSystemPrivateKey($this->masterKeyId);
 			$decryptedMasterKey = $this->crypt->decryptPrivateKey($masterKey, $this->getMasterKeyPassword(), $this->masterKeyId);
-			$this->session->setPrivateKey($decryptedMasterKey);
+			if ($decryptedMasterKey === false) {
+				$this->logger->error('A public master key is available but decrypting it failed. This should never happen.');
+			} else {
+				$this->session->setPrivateKey($decryptedMasterKey);
+			}
 		}
 
 		// after the encryption key is available we are ready to go

apps/encryption/lib/Recovery.php

@@ -67,20 +67,16 @@ public function enableAdminRecovery($password) {
 
 	/**
 	 * change recovery key id
-	 *
-	 * @param string $newPassword
-	 * @param string $oldPassword
-	 * @return bool
 	 */
-	public function changeRecoveryKeyPassword($newPassword, $oldPassword) {
+	public function changeRecoveryKeyPassword(string $newPassword, string $oldPassword): bool {
 		$recoveryKey = $this->keyManager->getSystemPrivateKey($this->keyManager->getRecoveryKeyId());
 		$decryptedRecoveryKey = $this->crypt->decryptPrivateKey($recoveryKey, $oldPassword);
 		if ($decryptedRecoveryKey === false) {
 			return false;
 		}
 		$encryptedRecoveryKey = $this->crypt->encryptPrivateKey($decryptedRecoveryKey, $newPassword);
 		$header = $this->crypt->generateHeader();
-		if ($encryptedRecoveryKey) {
+		if ($encryptedRecoveryKey !== false) {
 			$this->keyManager->setSystemPrivateKey($this->keyManager->getRecoveryKeyId(), $header . $encryptedRecoveryKey);
 			return true;
 		}
@@ -163,7 +159,7 @@ private function addRecoveryKeys(string $path): void {
 			if ($item['type'] === 'dir') {
 				$this->addRecoveryKeys($filePath . '/');
 			} else {
-				$fileKey = $this->keyManager->getFileKey($filePath, $this->user->getUID(), null);
+				$fileKey = $this->keyManager->getFileKey($filePath, null);
 				if (!empty($fileKey)) {
 					$accessList = $this->file->getAccessList($filePath);
 					$publicKeys = [];