Adapt code to new encryption system

fileKey gets deleted upon save as it’s stored in shareKeys instead now.
We use presence of a fileKey to detect if a file is using the legacy
 system or the new one, because we do not always have access to header
 data.

Signed-off-by: Côme Chilliet <[email protected]>

apps/encryption/lib/Crypto/Crypt.php

@@ -185,14 +185,9 @@ private function getOpenSSLConfig() {
 	}
 
 	/**
-	 * @param string $plainContent
-	 * @param string $passPhrase
-	 * @param int $version
-	 * @param int $position
-	 * @return false|string
 	 * @throws EncryptionFailedException
 	 */
-	public function symmetricEncryptFileContent($plainContent, $passPhrase, $version, $position) {
+	public function symmetricEncryptFileContent(string $plainContent, string $passPhrase, int $version, string $position): string|false {
 		if (!$plainContent) {
 			$this->logger->error('Encryption Library, symmetrical encryption failed no content given',
 				['app' => 'encryption']);
@@ -409,7 +404,7 @@ public function encryptPrivateKey($privateKey, $password, $uid = '') {
 			$privateKey,
 			$hash,
 			0,
-			0
+			'0'
 		);
 
 		return $encryptedKey;
@@ -537,12 +532,8 @@ private function checkSignature(string $data, string $passPhrase, string $expect
 
 	/**
 	 * create signature
-	 *
-	 * @param string $data
-	 * @param string $passPhrase
-	 * @return string
 	 */
-	private function createSignature($data, $passPhrase) {
+	private function createSignature(string $data, string $passPhrase): string {
 		$passPhrase = hash('sha512', $passPhrase . 'a', true);
 		return hash_hmac('sha256', $data, $passPhrase);
 	}

apps/encryption/lib/Crypto/Encryption.php

@@ -266,14 +266,14 @@ public function begin($path, $user, $mode, array $header, array $accessList) {
 	 * buffer.
 	 *
 	 * @param string $path to the file
-	 * @param int $position
+	 * @param string $position
 	 * @return string remained data which should be written to the file in case
 	 *                of a write operation
 	 * @throws PublicKeyMissingException
 	 * @throws \Exception
 	 * @throws \OCA\Encryption\Exceptions\MultiKeyEncryptException
 	 */
-	public function end($path, $position = 0) {
+	public function end($path, $position = '0') {
 		$result = '';
 		if ($this->isWriteOperation) {
 			// in case of a part file we remember the new signature versions
@@ -308,11 +308,13 @@ public function end($path, $position = 0) {
 			}
 
 			$publicKeys = $this->keyManager->addSystemKeys($this->accessList, $publicKeys, $this->getOwner($path));
-			//TODO adapt this to new return of the method, same for other calls of multiKeyEncrypt
-			$encryptedKeyfiles = $this->crypt->multiKeyEncrypt($this->fileKey, $publicKeys);
-			$this->keyManager->setAllFileKeys($this->path, $encryptedKeyfiles);
+			$shareKeys = $this->crypt->multiKeyEncrypt($this->fileKey, $publicKeys);
+			$this->keyManager->deleteLegacyFileKey($this->path);
+			foreach ($shareKeys as $uid => $keyFile) {
+				$this->keyManager->setShareKey($this->path, $uid, $keyFile);
+			}
 		}
-		return $result;
+		return $result ?: '';
 	}
 
 
@@ -362,7 +364,7 @@ public function encrypt($data, $position = 0) {
 				// Read the chunk from the start of $data
 				$chunk = substr($data, 0, $this->getUnencryptedBlockSize(true));
 
-				$encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version + 1, $position);
+				$encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version + 1, (string)$position);
 
 				// Remove the chunk we just processed from
 				// $data, leaving only unprocessed data in $data
@@ -400,18 +402,18 @@ public function decrypt($data, $position = 0) {
 	 * @param string $path path to the file which should be updated
 	 * @param string $uid of the user who performs the operation
 	 * @param array $accessList who has access to the file contains the key 'users' and 'public'
-	 * @return boolean
+	 * @return bool
 	 */
 	public function update($path, $uid, array $accessList) {
 		if (empty($accessList)) {
 			if (isset(self::$rememberVersion[$path])) {
 				$this->keyManager->setVersion($path, self::$rememberVersion[$path], new View());
 				unset(self::$rememberVersion[$path]);
 			}
-			return;
+			return false;
 		}
 
-		$fileKey = $this->keyManager->getFileKey($path, $uid);
+		$fileKey = $this->keyManager->getFileKey($path, $uid, null);
 
 		if (!empty($fileKey)) {
 			$publicKeys = [];
@@ -429,11 +431,13 @@ public function update($path, $uid, array $accessList) {
 
 			$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $this->getOwner($path));
 
-			$encryptedFileKey = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
+			$shareKeys = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
 
 			$this->keyManager->deleteAllFileKeys($path);
 
-			$this->keyManager->setAllFileKeys($path, $encryptedFileKey);
+			foreach ($shareKeys as $uid => $keyFile) {
+				$this->keyManager->setShareKey($this->path, $uid, $keyFile);
+			}
 		} else {
 			$this->logger->debug('no file key found, we assume that the file "{file}" is not encrypted',
 				['file' => $path, 'app' => 'encryption']);

apps/encryption/lib/KeyManager.php

@@ -440,18 +440,19 @@ public function getPrivateKey($userId) {
 	/**
 	 * @param string $path
 	 * @param $uid
+	 * @param ?bool $useLegacyFileKey null means try both
 	 * @return string
 	 */
-	public function getFileKey(string $path, ?string $uid, bool $useLegacyFileKey): string {
+	public function getFileKey(string $path, ?string $uid, ?bool $useLegacyFileKey): string {
 		if ($uid === '') {
 			$uid = null;
 		}
 		$publicAccess = is_null($uid);
-
-		if ($useLegacyFileKey) {
+		$encryptedFileKey = '';
+		if ($useLegacyFileKey ?? true) {
 			$encryptedFileKey = $this->keyStorage->getFileKey($path, $this->fileKeyId, Encryption::ID);
 
-			if (empty($encryptedFileKey)) {
+			if (empty($encryptedFileKey) && $useLegacyFileKey) {
 				return '';
 			}
 		}
@@ -477,13 +478,14 @@ public function getFileKey(string $path, ?string $uid, bool $useLegacyFileKey):
 			$privateKey = $this->session->getPrivateKey();
 		}
 
-		if ($useLegacyFileKey) {
+		if ($useLegacyFileKey ?? true) {
 			if ($encryptedFileKey && $shareKey && $privateKey) {
 				return $this->crypt->multiKeyDecryptLegacy($encryptedFileKey,
 					$shareKey,
 					$privateKey);
 			}
-		} else {
+		}
+		if ($useLegacyFileKey ?? false) {
 			if ($shareKey && $privateKey) {
 				return $this->crypt->multiKeyDecrypt($shareKey, $privateKey);
 			}
@@ -664,6 +666,10 @@ public function deleteAllFileKeys($path) {
 		return $this->keyStorage->deleteAllFileKeys($path);
 	}
 
+	public function deleteLegacyFileKey(string $path): bool {
+		return $this->keyStorage->deleteFileKey($path, $this->fileKeyId, Encryption::ID);
+	}
+
 	/**
 	 * @param array $userIds
 	 * @return array

apps/encryption/lib/Recovery.php

@@ -35,8 +35,6 @@
 use OCP\PreConditionNotMetException;
 
 class Recovery {
-
-
 	/**
 	 * @var null|IUser
 	 */
@@ -102,7 +100,7 @@ public function enableAdminRecovery($password) {
 		}
 
 		if ($keyManager->checkRecoveryPassword($password)) {
-			$appConfig->setAppValue('encryption', 'recoveryAdminEnabled', 1);
+			$appConfig->setAppValue('encryption', 'recoveryAdminEnabled', '1');
 			return true;
 		}
 
@@ -140,7 +138,7 @@ public function disableAdminRecovery($recoveryPassword) {
 
 		if ($keyManager->checkRecoveryPassword($recoveryPassword)) {
 			// Set recoveryAdmin as disabled
-			$this->config->setAppValue('encryption', 'recoveryAdminEnabled', 0);
+			$this->config->setAppValue('encryption', 'recoveryAdminEnabled', '0');
 			return true;
 		}
 		return false;
@@ -169,7 +167,7 @@ public function isRecoveryEnabledForUser($user = '') {
 	 * @return bool
 	 */
 	public function isRecoveryKeyEnabled() {
-		$enabled = $this->config->getAppValue('encryption', 'recoveryAdminEnabled', 0);
+		$enabled = $this->config->getAppValue('encryption', 'recoveryAdminEnabled', '0');
 
 		return ($enabled === '1');
 	}
@@ -199,16 +197,15 @@ public function setRecoveryForUser($value) {
 
 	/**
 	 * add recovery key to all encrypted files
-	 * @param string $path
 	 */
-	private function addRecoveryKeys($path) {
+	private function addRecoveryKeys(string $path): void {
 		$dirContent = $this->view->getDirectoryContent($path);
 		foreach ($dirContent as $item) {
 			$filePath = $item->getPath();
 			if ($item['type'] === 'dir') {
 				$this->addRecoveryKeys($filePath . '/');
 			} else {
-				$fileKey = $this->keyManager->getFileKey($filePath, $this->user->getUID());
+				$fileKey = $this->keyManager->getFileKey($filePath, $this->user->getUID(), null);
 				if (!empty($fileKey)) {
 					$accessList = $this->file->getAccessList($filePath);
 					$publicKeys = [];
@@ -218,18 +215,20 @@ private function addRecoveryKeys($path) {
 
 					$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $this->user->getUID());
 
-					$encryptedKeyfiles = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
-					$this->keyManager->setAllFileKeys($filePath, $encryptedKeyfiles);
+					$shareKeys = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
+					$this->keyManager->deleteLegacyFileKey($filePath);
+					foreach ($shareKeys as $uid => $keyFile) {
+						$this->keyManager->setShareKey($filePath, $uid, $keyFile);
+					}
 				}
 			}
 		}
 	}
 
 	/**
 	 * remove recovery key to all encrypted files
-	 * @param string $path
 	 */
-	private function removeRecoveryKeys($path) {
+	private function removeRecoveryKeys(string $path): void {
 		$dirContent = $this->view->getDirectoryContent($path);
 		foreach ($dirContent as $item) {
 			$filePath = $item->getPath();
@@ -243,11 +242,8 @@ private function removeRecoveryKeys($path) {
 
 	/**
 	 * recover users files with the recovery key
-	 *
-	 * @param string $recoveryPassword
-	 * @param string $user
 	 */
-	public function recoverUsersFiles($recoveryPassword, $user) {
+	public function recoverUsersFiles(string $recoveryPassword, string $user): void {
 		$encryptedKey = $this->keyManager->getSystemPrivateKey($this->keyManager->getRecoveryKeyId());
 
 		$privateKey = $this->crypt->decryptPrivateKey($encryptedKey, $recoveryPassword);
@@ -258,12 +254,8 @@ public function recoverUsersFiles($recoveryPassword, $user) {
 
 	/**
 	 * recover users files
-	 *
-	 * @param string $path
-	 * @param string $privateKey
-	 * @param string $uid
 	 */
-	private function recoverAllFiles($path, $privateKey, $uid) {
+	private function recoverAllFiles(string $path, string $privateKey, string $uid): void {
 		$dirContent = $this->view->getDirectoryContent($path);
 
 		foreach ($dirContent as $item) {
@@ -279,19 +271,17 @@ private function recoverAllFiles($path, $privateKey, $uid) {
 
 	/**
 	 * recover file
-	 *
-	 * @param string $path
-	 * @param string $privateKey
-	 * @param string $uid
 	 */
-	private function recoverFile($path, $privateKey, $uid) {
+	private function recoverFile(string $path, string $privateKey, string $uid): void {
 		$encryptedFileKey = $this->keyManager->getEncryptedFileKey($path);
 		$shareKey = $this->keyManager->getShareKey($path, $this->keyManager->getRecoveryKeyId());
 
 		if ($encryptedFileKey && $shareKey && $privateKey) {
-			$fileKey = $this->crypt->multiKeyDecrypt($encryptedFileKey,
+			$fileKey = $this->crypt->multiKeyDecryptLegacy($encryptedFileKey,
 				$shareKey,
 				$privateKey);
+		} elseif ($shareKey && $privateKey) {
+			$fileKey = $this->crypt->multiKeyDecrypt($shareKey, $privateKey);
 		}
 
 		if (!empty($fileKey)) {
@@ -303,8 +293,11 @@ private function recoverFile($path, $privateKey, $uid) {
 
 			$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $uid);
 
-			$encryptedKeyfiles = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
-			$this->keyManager->setAllFileKeys($path, $encryptedKeyfiles);
+			$shareKeys = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
+			$this->keyManager->deleteLegacyFileKey($path);
+			foreach ($shareKeys as $uid => $keyFile) {
+				$this->keyManager->setShareKey($path, $uid, $keyFile);
+			}
 		}
 	}
 }

apps/encryption/tests/Crypto/EncryptionTest.php

@@ -42,7 +42,6 @@
 use Test\TestCase;
 
 class EncryptionTest extends TestCase {
-
 	/** @var Encryption */
 	private $instance;
 
@@ -156,7 +155,7 @@ public function endTest() {
 			->willReturnCallback([$this, 'addSystemKeysCallback']);
 		$this->cryptMock->expects($this->any())
 			->method('multiKeyEncrypt')
-			->willReturn(true);
+			->willReturn([]);
 
 		$this->instance->end('/foo/bar');
 	}
@@ -276,7 +275,7 @@ public function testBeginDecryptAll() {
 			->with($path, $recoveryKeyId)
 			->willReturn($recoveryShareKey);
 		$this->cryptMock->expects($this->once())
-			->method('multiKeyDecrypt')
+			->method('multiKeyDecryptLegacy')
 			->with('encryptedFileKey', $recoveryShareKey, $decryptAllKey)
 			->willReturn($fileKey);
 
@@ -378,6 +377,7 @@ function ($user) {
 				function ($fileKey, $publicKeys) {
 					$this->assertEmpty($publicKeys);
 					$this->assertSame('fileKey', $fileKey);
+					return [];
 				}
 			);
 

lib/public/Encryption/IEncryptionModule.php

@@ -60,7 +60,7 @@ public function getDisplayName();
 	 * @param array $header contains the header data read from the file
 	 * @param array $accessList who has access to the file contains the key 'users' and 'public'
 	 *
-	 * $return array $header contain data as key-value pairs which should be
+	 * @return array $header contain data as key-value pairs which should be
 	 *                       written to the header, in case of a write operation
 	 *                       or if no additional data is needed return a empty array
 	 * @since 8.1.0