[stable30] 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\KeyManager;
@@ -56,13 +59,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()) {
@@ -71,7 +69,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 he activated the recovery key.');
 				$output->writeln('');
@@ -120,7 +118,7 @@ public function prepare(InputInterface $input, OutputInterface $output, $user) {
 	 * @return bool|string
 	 * @throws \OCA\Encryption\Exceptions\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) {
@@ -137,7 +135,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;
@@ -104,11 +107,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;
 
@@ -155,7 +155,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%]");
@@ -190,7 +190,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%]");
@@ -212,10 +212,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;
@@ -234,12 +232,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';
@@ -312,7 +306,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']);
 
@@ -353,10 +347,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);
@@ -374,21 +366,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;
@@ -397,7 +386,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

@@ -437,7 +437,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

@@ -90,20 +90,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;
 		}
@@ -186,7 +182,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 = [];

apps/encryption/lib/Session.php

@@ -1,10 +1,13 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
+
 namespace OCA\Encryption;
 
 use OCA\Encryption\Exceptions\PrivateKeyMissingException;
@@ -31,7 +34,7 @@ public function __construct(ISession $session) {
 	 *
 	 * @param string $status INIT_SUCCESSFUL, INIT_EXECUTED, NOT_INITIALIZED
 	 */
-	public function setStatus($status) {
+	public function setStatus(string $status): void {
 		$this->session->set('encryptionInitialized', $status);
 	}
 
@@ -40,7 +43,7 @@ public function setStatus($status) {
 	 *
 	 * @return string init status INIT_SUCCESSFUL, INIT_EXECUTED, NOT_INITIALIZED
 	 */
-	public function getStatus() {
+	public function getStatus(): string {
 		$status = $this->session->get('encryptionInitialized');
 		if (is_null($status)) {
 			$status = self::NOT_INITIALIZED;
@@ -51,10 +54,8 @@ public function getStatus() {
 
 	/**
 	 * check if encryption was initialized successfully
-	 *
-	 * @return bool
 	 */
-	public function isReady() {
+	public function isReady(): bool {
 		$status = $this->getStatus();
 		return $status === self::INIT_SUCCESSFUL;
 	}
@@ -65,7 +66,7 @@ public function isReady() {
 	 * @return string $privateKey The user's plaintext private key
 	 * @throws Exceptions\PrivateKeyMissingException
 	 */
-	public function getPrivateKey() {
+	public function getPrivateKey(): string {
 		$key = $this->session->get('privateKey');
 		if (is_null($key)) {
 			throw new Exceptions\PrivateKeyMissingException('please try to log-out and log-in again', 0);
@@ -75,10 +76,8 @@ public function getPrivateKey() {
 
 	/**
 	 * check if private key is set
-	 *
-	 * @return boolean
 	 */
-	public function isPrivateKeySet() {
+	public function isPrivateKeySet(): bool {
 		$key = $this->session->get('privateKey');
 		if (is_null($key)) {
 			return false;
@@ -94,39 +93,33 @@ public function isPrivateKeySet() {
 	 *
 	 * @note this should only be set on login
 	 */
-	public function setPrivateKey($key) {
+	public function setPrivateKey(string $key): void {
 		$this->session->set('privateKey', $key);
 	}
 
 	/**
 	 * store data needed for the decrypt all operation in the session
-	 *
-	 * @param string $user
-	 * @param string $key
 	 */
-	public function prepareDecryptAll($user, $key) {
+	public function prepareDecryptAll(string $user, string $key): void {
 		$this->session->set('decryptAll', true);
 		$this->session->set('decryptAllKey', $key);
 		$this->session->set('decryptAllUid', $user);
 	}
 
 	/**
 	 * check if we are in decrypt all mode
-	 *
-	 * @return bool
 	 */
-	public function decryptAllModeActivated() {
+	public function decryptAllModeActivated(): bool {
 		$decryptAll = $this->session->get('decryptAll');
 		return ($decryptAll === true);
 	}
 
 	/**
 	 * get uid used for decrypt all operation
 	 *
-	 * @return string
 	 * @throws \Exception
 	 */
-	public function getDecryptAllUid() {
+	public function getDecryptAllUid(): string {
 		$uid = $this->session->get('decryptAllUid');
 		if (is_null($uid) && $this->decryptAllModeActivated()) {
 			throw new \Exception('No uid found while in decrypt all mode');
@@ -140,10 +133,9 @@ public function getDecryptAllUid() {
 	/**
 	 * get private key for decrypt all operation
 	 *
-	 * @return string
 	 * @throws PrivateKeyMissingException
 	 */
-	public function getDecryptAllKey() {
+	public function getDecryptAllKey(): string {
 		$privateKey = $this->session->get('decryptAllKey');
 		if (is_null($privateKey) && $this->decryptAllModeActivated()) {
 			throw new PrivateKeyMissingException('No private key found while in decrypt all mode');
@@ -157,7 +149,7 @@ public function getDecryptAllKey() {
 	/**
 	 * remove keys from session
 	 */
-	public function clear() {
+	public function clear(): void {
 		$this->session->remove('publicSharePrivateKey');
 		$this->session->remove('privateKey');
 		$this->session->remove('encryptionInitialized');