extend fix-key-location to handle cases from broken cross-storage moves

Signed-off-by: Robin Appelman <robin@icewind.nl>

apps/encryption/lib/Command/FixKeyLocation.php

@@ -23,8 +23,11 @@ declare(strict_types=1);
 
 namespace OCA\Encryption\Command;
 
+use OC\Encryption\Manager;
 use OC\Encryption\Util;
+use OC\Files\Storage\Wrapper\Encryption;
 use OC\Files\View;
+use OCP\Encryption\IManager;
 use OCP\Files\Config\ICachedMountInfo;
 use OCP\Files\Config\IUserMountCache;
 use OCP\Files\Folder;
@@ -46,14 +49,25 @@ class FixKeyLocation extends Command {
 	private IRootFolder $rootFolder;
 	private string $keyRootDirectory;
 	private View $rootView;
+	private Manager $encryptionManager;
 
-	public function __construct(IUserManager $userManager, IUserMountCache $userMountCache, Util $encryptionUtil, IRootFolder $rootFolder) {
+	public function __construct(
+		IUserManager $userManager,
+		IUserMountCache $userMountCache,
+		Util $encryptionUtil,
+		IRootFolder $rootFolder,
+		IManager $encryptionManager
+	) {
 		$this->userManager = $userManager;
 		$this->userMountCache = $userMountCache;
 		$this->encryptionUtil = $encryptionUtil;
 		$this->rootFolder = $rootFolder;
 		$this->keyRootDirectory = rtrim($this->encryptionUtil->getKeyStorageRoot(), '/');
 		$this->rootView = new View();
+		if (!$encryptionManager instanceof Manager) {
+			throw new \Exception("Wrong encryption manager");
+		}
+		$this->encryptionManager = $encryptionManager;
 
 		parent::__construct();
 	}
@@ -88,18 +102,71 @@ class FixKeyLocation extends Command {
 				continue;
 			}
 
-			$files = $this->getAllFiles($mountRootFolder);
+			$files = $this->getAllEncryptedFiles($mountRootFolder);
 			foreach ($files as $file) {
-				if ($this->isKeyStoredForUser($user, $file)) {
-					if ($dryRun) {
-						$output->writeln("<info>" . $file->getPath() . "</info> needs migration");
-					} else {
-						$output->write("Migrating key for <info>" . $file->getPath() . "</info> ");
-						if ($this->copyKeyAndValidate($user, $file)) {
-							$output->writeln("<info>✓</info>");
+				/** @var File $file */
+				$hasSystemKey = $this->hasSystemKey($file);
+				$hasUserKey = $this->hasUserKey($user, $file);
+				if (!$hasSystemKey) {
+					if ($hasUserKey) {
+						// key was stored incorrectly as user key, migrate
+
+						if ($dryRun) {
+							$output->writeln("<info>" . $file->getPath() . "</info> needs migration");
 						} else {
-							$output->writeln("<fg=red>❌</>");
-							$output->writeln("  Failed to validate key for <error>" . $file->getPath() . "</error>, key will not be migrated");
+							$output->write("Migrating key for <info>" . $file->getPath() . "</info> ");
+							if ($this->copyUserKeyToSystemAndValidate($user, $file)) {
+								$output->writeln("<info>✓</info>");
+							} else {
+								$output->writeln("<fg=red>❌</>");
+								$output->writeln("  Failed to validate key for <error>" . $file->getPath() . "</error>, key will not be migrated");
+							}
+						}
+					} else {
+						// no matching key, probably from a broken cross-storage move
+
+						$shouldBeEncrypted = $file->getStorage()->instanceOfStorage(Encryption::class);
+						$isActuallyEncrypted = $this->isDataEncrypted($file);
+						if ($isActuallyEncrypted) {
+							if ($dryRun) {
+								if ($shouldBeEncrypted) {
+									$output->write("<info>" . $file->getPath() . "</info> needs migration");
+								} else {
+									$output->write("<info>" . $file->getPath() . "</info> needs decryption");
+								}
+								$foundKey = $this->findUserKeyForSystemFile($user, $file);
+								if ($foundKey) {
+									$output->writeln(", valid key found at <info>" . $foundKey . "</info>");
+								} else {
+									$output->writeln(" <error>❌ No key found</error>");
+								}
+							} else {
+								if ($shouldBeEncrypted) {
+									$output->write("<info>Migrating key for " . $file->getPath() . "</info>");
+								} else {
+									$output->write("<info>Decrypting " . $file->getPath() . "</info>");
+								}
+								$foundKey = $this->findUserKeyForSystemFile($user, $file);
+								if ($foundKey) {
+									if ($shouldBeEncrypted) {
+										$systemKeyPath = $this->getSystemKeyPath($file);
+										$this->rootView->copy($foundKey, $systemKeyPath);
+										$output->writeln("  Migrated key from <info>" . $foundKey . "</info>");
+									} else {
+										$this->decryptWithSystemKey($file, $foundKey);
+										$output->writeln("  Decrypted with key from <info>" . $foundKey . "</info>");
+									}
+								} else {
+									$output->writeln(" <error>❌ No key found</error>");
+								}
+							}
+						} else {
+							if ($dryRun) {
+								$output->writeln("<info>" . $file->getPath() . " needs to be marked as not encrypted</info>");
+							} else {
+								$this->markAsUnEncrypted($file);
+								$output->writeln("<info>" . $file->getPath() . " marked as not encrypted</info>");
+							}
 						}
 					}
 				}
@@ -109,47 +176,68 @@ class FixKeyLocation extends Command {
 		return 0;
 	}
 
+	private function getUserRelativePath(string $path): string {
+		$parts = explode('/', $path, 3);
+		if (count($parts) >= 3) {
+			return '/' . $parts[2];
+		} else {
+			return '';
+		}
+	}
+
 	/**
 	 * @param IUser $user
 	 * @return ICachedMountInfo[]
 	 */
 	private function getSystemMountsForUser(IUser $user): array {
-		return array_filter($this->userMountCache->getMountsForUser($user), function(ICachedMountInfo $mount) use ($user) {
+		return array_filter($this->userMountCache->getMountsForUser($user), function (ICachedMountInfo $mount) use (
+			$user
+		) {
 			$mountPoint = substr($mount->getMountPoint(), strlen($user->getUID() . '/'));
 			return $this->encryptionUtil->isSystemWideMountPoint($mountPoint, $user->getUID());
 		});
 	}
 
 	/**
+	 * Get all files in a folder which are marked as encrypted
+	 *
 	 * @param Folder $folder
 	 * @return \Generator<File>
 	 */
-	private function getAllFiles(Folder $folder) {
+	private function getAllEncryptedFiles(Folder $folder) {
 		foreach ($folder->getDirectoryListing() as $child) {
 			if ($child instanceof Folder) {
-				yield from $this->getAllFiles($child);
+				yield from $this->getAllEncryptedFiles($child);
 			} else {
-				yield $child;
+				if (substr($child->getName(), -4) !== '.bak' && $child->isEncrypted()) {
+					yield $child;
+				}
 			}
 		}
 	}
 
-	/**
-	 * Check if the key for a file is stored in the user's keystore and not the system one
-	 *
-	 * @param IUser $user
-	 * @param Node $node
-	 * @return bool
-	 */
-	private function isKeyStoredForUser(IUser $user, Node $node): bool {
-		$path = trim(substr($node->getPath(), strlen($user->getUID()) + 1), '/');
-		$systemKeyPath = $this->keyRootDirectory . '/files_encryption/keys/' . $path . '/';
-		$userKeyPath = $this->keyRootDirectory . '/' . $user->getUID() . '/files_encryption/keys/' . $path . '/';
+	private function getSystemKeyPath(Node $node): string {
+		$path = $this->getUserRelativePath($node->getPath());
+		return $this->keyRootDirectory . '/files_encryption/keys/' . $path . '/';
+	}
 
+	private function getUserBaseKeyPath(IUser $user): string {
+		return $this->keyRootDirectory . '/' . $user->getUID() . '/files_encryption/keys';
+	}
+
+	private function getUserKeyPath(IUser $user, Node $node): string {
+		$path = $this->getUserRelativePath($node->getPath());
+		return $this->getUserBaseKeyPath($user) . '/' . $path . '/';
+	}
+
+	private function hasSystemKey(Node $node): bool {
 		// this uses View instead of the RootFolder because the keys might not be in the cache
-		$systemKeyExists = $this->rootView->file_exists($systemKeyPath);
-		$userKeyExists = $this->rootView->file_exists($userKeyPath);
-		return $userKeyExists && !$systemKeyExists;
+		return $this->rootView->file_exists($this->getSystemKeyPath($node));
+	}
+
+	private function hasUserKey(IUser $user, Node $node): bool {
+		// this uses View instead of the RootFolder because the keys might not be in the cache
+		return $this->rootView->file_exists($this->getUserKeyPath($user, $node));
 	}
 
 	/**
@@ -159,28 +247,201 @@ class FixKeyLocation extends Command {
 	 * @param File $node
 	 * @return bool
 	 */
-	private function copyKeyAndValidate(IUser $user, File $node): bool {
+	private function copyUserKeyToSystemAndValidate(IUser $user, File $node): bool {
 		$path = trim(substr($node->getPath(), strlen($user->getUID()) + 1), '/');
 		$systemKeyPath = $this->keyRootDirectory . '/files_encryption/keys/' . $path . '/';
 		$userKeyPath = $this->keyRootDirectory . '/' . $user->getUID() . '/files_encryption/keys/' . $path . '/';
 
 		$this->rootView->copy($userKeyPath, $systemKeyPath);
-		try {
-			// check that the copied key is valid
-			$fh = $node->fopen('r');
-			// read a single chunk
-			$data = fread($fh, 8192);
-			if ($data === false) {
-				throw new \Exception("Read failed");
-			}
-
+		if ($this->tryReadFile($node)) {
 			// cleanup wrong key location
 			$this->rootView->rmdir($userKeyPath);
 			return true;
-		} catch (\Exception $e) {
+		} else {
 			// remove the copied key if we know it's invalid
 			$this->rootView->rmdir($systemKeyPath);
 			return false;
 		}
 	}
+
+	private function tryReadFile(File $node): bool {
+		try {
+			$fh = $node->fopen('r');
+			// read a single chunk
+			$data = fread($fh, 8192);
+			if ($data === false) {
+				return false;
+			} else {
+				return true;
+			}
+		} catch (\Exception $e) {
+			return false;
+		}
+	}
+
+	/**
+	 * Get the contents of a file without decrypting it
+	 *
+	 * @param File $node
+	 * @return resource
+	 */
+	private function openWithoutDecryption(File $node, string $mode) {
+		$storage = $node->getStorage();
+		$internalPath = $node->getInternalPath();
+		if ($storage->instanceOfStorage(Encryption::class)) {
+			/** @var Encryption $storage */
+			try {
+				$storage->setEnabled(false);
+				$handle = $storage->fopen($internalPath, 'r');
+				$storage->setEnabled(true);
+			} catch (\Exception $e) {
+				$storage->setEnabled(true);
+				throw $e;
+			}
+		} else {
+			$handle = $storage->fopen($internalPath, $mode);
+		}
+		/** @var resource|false $handle */
+		if ($handle === false) {
+			throw new \Exception("Failed to open " . $node->getPath());
+		}
+		return $handle;
+	}
+
+	/**
+	 * Check if the data stored for a file is encrypted, regardless of it's metadata
+	 *
+	 * @param File $node
+	 * @return bool
+	 */
+	private function isDataEncrypted(File $node): bool {
+		$handle = $this->openWithoutDecryption($node, 'r');
+		$firstBlock = fread($handle, $this->encryptionUtil->getHeaderSize());
+		fclose($handle);
+
+		$header = $this->encryptionUtil->parseRawHeader($firstBlock);
+		return isset($header['oc_encryption_module']);
+	}
+
+	/**
+	 * Attempt to find a key (stored for user) for a file (that needs a system key) even when it's not stored in the expected location
+	 *
+	 * @param File $node
+	 * @return string
+	 */
+	private function findUserKeyForSystemFile(IUser $user, File $node): ?string {
+		$userKeyPath = $this->getUserBaseKeyPath($user);
+		$possibleKeys = $this->findKeysByFileName($userKeyPath, $node->getName());
+		foreach ($possibleKeys as $possibleKey) {
+			if ($this->testSystemKey($user, $possibleKey, $node)) {
+				return $possibleKey;
+			}
+		}
+		return null;
+	}
+
+	/**
+	 * Attempt to find a key for a file even when it's not stored in the expected location
+	 *
+	 * @param string $basePath
+	 * @param string $name
+	 * @return \Generator<string>
+	 */
+	private function findKeysByFileName(string $basePath, string $name) {
+		if ($this->rootView->is_dir($basePath . '/' . $name . '/OC_DEFAULT_MODULE')) {
+			yield $basePath . '/' . $name;
+		} else {
+			/** @var false|resource $dh */
+			$dh = $this->rootView->opendir($basePath);
+			if (!$dh) {
+				throw new \Exception("Invalid base path " . $basePath);
+			}
+			while ($child = readdir($dh)) {
+				if ($child != '..' && $child != '.') {
+					$childPath = $basePath . '/' . $child;
+
+					// recurse if the child is not a key folder
+					if ($this->rootView->is_dir($childPath) && !is_dir($childPath . '/OC_DEFAULT_MODULE')) {
+						yield from $this->findKeysByFileName($childPath, $name);
+					}
+				}
+			}
+		}
+	}
+
+	/**
+	 * Test if the provided key is valid as a system key for the file
+	 *
+	 * @param IUser $user
+	 * @param string $key
+	 * @param File $node
+	 * @return bool
+	 */
+	private function testSystemKey(IUser $user, string $key, File $node): bool {
+		$systemKeyPath = $this->getSystemKeyPath($node);
+
+		if ($this->rootView->file_exists($systemKeyPath)) {
+			// already has a key, reject new key
+			return false;
+		}
+
+		$this->rootView->copy($key, $systemKeyPath);
+		$isValid = $this->tryReadFile($node);
+		$this->rootView->rmdir($systemKeyPath);
+		return $isValid;
+	}
+
+	/**
+	 * Decrypt a file with the specified system key and mark the key as not-encrypted
+	 *
+	 * @param File $node
+	 * @param string $key
+	 * @return void
+	 */
+	private function decryptWithSystemKey(File $node, string $key): void {
+		$storage = $node->getStorage();
+		$name = $node->getName();
+
+		$node->move($node->getPath() . '.bak');
+		$systemKeyPath = $this->getSystemKeyPath($node);
+		$this->rootView->copy($key, $systemKeyPath);
+
+		try {
+			if (!$storage->instanceOfStorage(Encryption::class)) {
+				$storage = $this->encryptionManager->forceWrapStorage($node->getMountPoint(), $storage);
+			}
+			/** @var false|resource $source */
+			$source = $storage->fopen($node->getInternalPath(), 'r');
+			if (!$source) {
+				throw new \Exception("Failed to open " . $node->getPath() . " with " . $key);
+			}
+			$decryptedNode = $node->getParent()->newFile($name);
+
+			$target = $this->openWithoutDecryption($decryptedNode, 'w');
+			stream_copy_to_stream($source, $target);
+			fclose($target);
+			fclose($source);
+
+			$decryptedNode->getStorage()->getScanner()->scan($decryptedNode->getInternalPath());
+		} catch (\Exception $e) {
+			$this->rootView->rmdir($systemKeyPath);
+
+			// remove the .bak
+			$node->move(substr($node->getPath(), 0, -4));
+
+			throw $e;
+		}
+
+		if ($this->isDataEncrypted($decryptedNode)) {
+			throw new \Exception($node->getPath() . " still encrypted after attempting to decrypt with " . $key);
+		}
+
+		$this->markAsUnEncrypted($decryptedNode);
+
+		$this->rootView->rmdir($systemKeyPath);
+	}
+
+	private function markAsUnEncrypted(Node $node): void {
+		$node->getStorage()->getCache()->update($node->getId(), ['encrypted' => 0]);
+	}
 }

lib/private/Encryption/EncryptionWrapper.php

@@ -29,7 +29,8 @@ use OC\Files\Storage\Wrapper\Encryption;
 use OC\Files\View;
 use OC\Memcache\ArrayCache;
 use OCP\Files\Mount\IMountPoint;
-use OCP\Files\Storage;
+use OCP\Files\Storage\IDisableEncryptionStorage;
+use OCP\Files\Storage\IStorage;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -64,18 +65,19 @@ class EncryptionWrapper {
 	 * Wraps the given storage when it is not a shared storage
 	 *
 	 * @param string $mountPoint
-	 * @param Storage $storage
+	 * @param IStorage $storage
 	 * @param IMountPoint $mount
-	 * @return Encryption|Storage
+	 * @param bool $force apply the wrapper even if the storage normally has encryption disabled, helpful for repair steps
+	 * @return Encryption|IStorage
 	 */
-	public function wrapStorage($mountPoint, Storage $storage, IMountPoint $mount) {
+	public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $mount, bool $force = false) {
 		$parameters = [
 			'storage' => $storage,
 			'mountPoint' => $mountPoint,
 			'mount' => $mount
 		];
 
-		if (!$storage->instanceOfStorage(Storage\IDisableEncryptionStorage::class) && $mountPoint !== '/') {
+		if ($force || (!$storage->instanceOfStorage(IDisableEncryptionStorage::class) && $mountPoint !== '/')) {
 			$user = \OC::$server->getUserSession()->getUser();
 			$mountManager = Filesystem::getMountManager();
 			$uid = $user ? $user->getUID() : null;

lib/private/Encryption/Manager.php

@@ -32,6 +32,8 @@ use OC\Memcache\ArrayCache;
 use OC\ServiceUnavailableException;
 use OCP\Encryption\IEncryptionModule;
 use OCP\Encryption\IManager;
+use OCP\Files\Mount\IMountPoint;
+use OCP\Files\Storage\IStorage;
 use OCP\IConfig;
 use OCP\IL10N;
 use Psr\Log\LoggerInterface;
@@ -234,6 +236,11 @@ class Manager implements IManager {
 		}
 	}
 
+	public function forceWrapStorage(IMountPoint $mountPoint, IStorage $storage) {
+		$encryptionWrapper = new EncryptionWrapper($this->arrayCache, $this, $this->logger);
+		return $encryptionWrapper->wrapStorage($mountPoint->getMountPoint(), $storage, $mountPoint, true);
+	}
+
 
 	/**
 	 * check if key storage is ready

lib/private/Encryption/Util.php

@@ -357,4 +357,32 @@ class Util {
 	public function getKeyStorageRoot(): string {
 		return $this->config->getAppValue('core', 'encryption_key_storage_root', '');
 	}
+
+	/**
+	 * parse raw header to array
+	 *
+	 * @param string $rawHeader
+	 * @return array
+	 */
+	public function parseRawHeader(string $rawHeader) {
+		$result = [];
+		if (str_starts_with($rawHeader, Util::HEADER_START)) {
+			$header = $rawHeader;
+			$endAt = strpos($header, Util::HEADER_END);
+			if ($endAt !== false) {
+				$header = substr($header, 0, $endAt + strlen(Util::HEADER_END));
+
+				// +1 to not start with an ':' which would result in empty element at the beginning
+				$exploded = explode(':', substr($header, strlen(Util::HEADER_START) + 1));
+
+				$element = array_shift($exploded);
+				while ($element !== Util::HEADER_END && $element !== null) {
+					$result[$element] = array_shift($exploded);
+					$element = array_shift($exploded);
+				}
+			}
+		}
+
+		return $result;
+	}
 }

lib/private/Files/Storage/Wrapper/Encryption.php

@@ -100,6 +100,8 @@ class Encryption extends Wrapper {
 	/** @var CappedMemoryCache<bool> */
 	private CappedMemoryCache $encryptedPaths;
 
+	private $enabled = true;
+
 	/**
 	 * @param array $parameters
 	 */
@@ -392,6 +394,10 @@ class Encryption extends Wrapper {
 			return $this->storage->fopen($path, $mode);
 		}
 
+		if (!$this->enabled) {
+			return $this->storage->fopen($path, $mode);
+		}
+
 		$encryptionEnabled = $this->encryptionManager->isEnabled();
 		$shouldEncrypt = false;
 		$encryptionModule = null;
@@ -937,34 +943,6 @@ class Encryption extends Wrapper {
 		return $headerSize;
 	}
 
-	/**
-	 * parse raw header to array
-	 *
-	 * @param string $rawHeader
-	 * @return array
-	 */
-	protected function parseRawHeader($rawHeader) {
-		$result = [];
-		if (str_starts_with($rawHeader, Util::HEADER_START)) {
-			$header = $rawHeader;
-			$endAt = strpos($header, Util::HEADER_END);
-			if ($endAt !== false) {
-				$header = substr($header, 0, $endAt + strlen(Util::HEADER_END));
-
-				// +1 to not start with an ':' which would result in empty element at the beginning
-				$exploded = explode(':', substr($header, strlen(Util::HEADER_START) + 1));
-
-				$element = array_shift($exploded);
-				while ($element !== Util::HEADER_END) {
-					$result[$element] = array_shift($exploded);
-					$element = array_shift($exploded);
-				}
-			}
-		}
-
-		return $result;
-	}
-
 	/**
 	 * read header from file
 	 *
@@ -988,7 +966,7 @@ class Encryption extends Wrapper {
 
 		if ($isEncrypted) {
 			$firstBlock = $this->readFirstBlock($path);
-			$result = $this->parseRawHeader($firstBlock);
+			$result = $this->util->parseRawHeader($firstBlock);
 
 			// if the header doesn't contain a encryption module we check if it is a
 			// legacy file. If true, we add the default encryption module
@@ -1103,4 +1081,14 @@ class Encryption extends Wrapper {
 	public function clearIsEncryptedCache(): void {
 		$this->encryptedPaths->clear();
 	}
+
+	/**
+	 * Allow temporarily disabling the wrapper
+	 *
+	 * @param bool $enabled
+	 * @return void
+	 */
+	public function setEnabled(bool $enabled): void {
+		$this->enabled = $enabled;
+	}
 }