Merge pull request #7155 from owncloud/enc_improved_error_handling

[encryption] improved error handling

apps/files_encryption/ajax/getMigrationStatus.php

@@ -13,16 +13,14 @@ use OCA\Encryption\Util;
 $loginname = isset($_POST['user']) ? $_POST['user'] : '';
 $password = isset($_POST['password']) ? $_POST['password'] : '';
 
-$migrationCompleted = true;
+$migrationStatus = Util::MIGRATION_COMPLETED;
 
 if ($loginname !== '' && $password !== '') {
 	$username = \OCP\User::checkPassword($loginname, $password);
 	if ($username) {
 		$util = new Util(new \OC_FilesystemView('/'), $username);
-		if ($util->getMigrationStatus() !== Util::MIGRATION_COMPLETED) {
+		$migrationStatus = $util->getMigrationStatus();
-			$migrationCompleted = false;
-		}
 	}
 }
 
-\OCP\JSON::success(array('data' => array('migrationCompleted' => $migrationCompleted)));
+\OCP\JSON::success(array('data' => array('migrationStatus' => $migrationStatus)));

apps/files_encryption/appinfo/app.php

@@ -10,6 +10,7 @@ OC::$CLASSPATH['OCA\Encryption\Session'] = 'files_encryption/lib/session.php';
 OC::$CLASSPATH['OCA\Encryption\Capabilities'] = 'files_encryption/lib/capabilities.php';
 OC::$CLASSPATH['OCA\Encryption\Helper'] = 'files_encryption/lib/helper.php';
 
+\OCP\Util::addscript('files_encryption', 'encryption');
 \OCP\Util::addscript('files_encryption', 'detect-migration');
 
 if (!OC_Config::getValue('maintenance', false)) {

apps/files_encryption/hooks/hooks.php

@@ -85,10 +85,9 @@ class Hooks {
 			$ready = $util->beginMigration();
 		} elseif ($migrationStatus === Util::MIGRATION_IN_PROGRESS) {
 			// refuse login as long as the initial encryption is running
-			while ($migrationStatus === Util::MIGRATION_IN_PROGRESS) {
+			sleep(5);
-				sleep(60);
+			\OCP\User::logout();
-				$migrationStatus = $util->getMigrationStatus();
+			return false;
-			}
 		}
 
 		// If migration not yet done
@@ -109,21 +108,27 @@ class Hooks {
 
 			}
 
-			// Encrypt existing user files:
+			// Encrypt existing user files
-			if (
+			try {
-				$util->encryptAll('/' . $params['uid'] . '/' . 'files', $session->getLegacyKey(), $params['password'])
+				$result = $util->encryptAll('/' . $params['uid'] . '/' . 'files', $session->getLegacyKey(), $params['password']);
-			) {
+			} catch (\Exception $ex) {
+				\OCP\Util::writeLog('Encryption library', 'Initial encryption failed! Error: ' . $ex->getMessage(), \OCP\Util::FATAL);
+				$util->resetMigrationStatus();
+				\OCP\User::logout();
+				$result = false;
+			}
+
+			if ($result) {
 
 				\OC_Log::write(
 					'Encryption library', 'Encryption of existing files belonging to "' . $params['uid'] . '" completed'
 					, \OC_Log::INFO
 				);
 
+				// Register successful migration in DB
+				$util->finishMigration();
+
 			}
-
-			// Register successful migration in DB
-			$util->finishMigration();
-
 		}
 
 		return true;

apps/files_encryption/js/detect-migration.js

@@ -17,10 +17,14 @@ $(document).ready(function(){
 			data: {user: user, password: password},
 			async: false,
 			success: function(response) {
-				if (response.data.migrationCompleted === false) {
+				if (response.data.migrationStatus === OC.Encryption.MIGRATION_OPEN) {
 					var message = t('files_encryption', 'Initial encryption started... This can take some time. Please wait.');
 					$('#messageText').text(message);
 					$('#message').removeClass('hidden').addClass('update');
+				} else if (response.data.migrationStatus === OC.Encryption.MIGRATION_IN_PROGRESS) {
+					var message = t('files_encryption', 'Initial encryption running... Please try again later.');
+					$('#messageText').text(message);
+					$('#message').removeClass('hidden').addClass('update');
 				}
 			}
 		});

apps/files_encryption/js/encryption.js

@@ -0,0 +1,12 @@
+/**
+ * Copyright (c) 2014
+ *  Bjoern Schiessle <schiessle@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or later.
+ * See the COPYING-README file.
+ */
+
+OC.Encryption={
+	MIGRATION_OPEN:0,
+	MIGRATION_COMPLETED:1,
+	MIGRATION_IN_PROGRESS:-1,
+};

apps/files_encryption/lib/util.php

@@ -316,7 +316,8 @@ class Util {
 			$found = array(
 				'plain' => array(),
 				'encrypted' => array(),
-				'legacy' => array()
+				'legacy' => array(),
+				'broken' => array(),
 			);
 		}
 
@@ -327,10 +328,7 @@ class Util {
 			if(is_resource($handle)) {
 				while (false !== ($file = readdir($handle))) {
 
-					if (
+					if ($file !== "." && $file !== "..") {
-						$file !== "."
-						&& $file !== ".."
-					) {
 
 						$filePath = $directory . '/' . $this->view->getRelativePath('/' . $file);
 						$relPath = \OCA\Encryption\Helper::stripUserFilesPath($filePath);
@@ -357,15 +355,23 @@ class Util {
 							// NOTE: This is inefficient;
 							// scanning every file like this
 							// will eat server resources :(
-							if (
+							if ($isEncryptedPath) {
-								Keymanager::getFileKey($this->view, $this, $relPath)
-								&& $isEncryptedPath
-							) {
 
-								$found['encrypted'][] = array(
+								$fileKey = Keymanager::getFileKey($this->view, $this, $relPath);
-									'name' => $file,
+								$shareKey = Keymanager::getShareKey($this->view, $this->userId, $this, $relPath);
-									'path' => $filePath
+								// if file is encrypted but now file key is available, throw exception
-								);
+								if ($fileKey === false || $shareKey === false) {
+									\OCP\Util::writeLog('encryption library', 'No keys available to decrypt the file: ' . $filePath, \OCP\Util::ERROR);
+									$found['broken'][] = array(
+										'name' => $file,
+										'path' => $filePath,
+									);
+								} else {
+									$found['encrypted'][] = array(
+										'name' => $file,
+										'path' => $filePath,
+									);
+								}
 
 								// If the file uses old
 								// encryption system
@@ -771,6 +777,12 @@ class Util {
 				$successful = false;
 			}
 
+			// if there are broken encrypted files than the complete decryption
+			// was not successful
+			if (!empty($found['broken'])) {
+				$successful = false;
+			}
+
 			if ($successful) {
 				$this->view->deleteAll($this->keyfilesPath);
 				$this->view->deleteAll($this->shareKeysPath);
@@ -1185,27 +1197,49 @@ class Util {
 		return $result;
 	}
 
+	/**
+	 * @brief set migration status
+	 * @param int $status
+	 * @return boolean
+	 */
+	private function setMigrationStatus($status) {
+
+		$sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ?';
+		$args = array($status, $this->userId);
+		$query = \OCP\DB::prepare($sql);
+		$manipulatedRows = $query->execute($args);
+
+		if ($manipulatedRows === 1) {
+			$result = true;
+			\OCP\Util::writeLog('Encryption library', "Migration status set to " . self::MIGRATION_OPEN, \OCP\Util::INFO);
+		} else {
+			$result = false;
+			\OCP\Util::writeLog('Encryption library', "Could not set migration status to " . self::MIGRATION_OPEN, \OCP\Util::WARN);
+		}
+
+		return $result;
+	}
+
 	/**
 	 * @brief start migration mode to initially encrypt users data
 	 * @return boolean
 	 */
 	public function beginMigration() {
 
-		$return = false;
+		$result = $this->setMigrationStatus(self::MIGRATION_IN_PROGRESS);
 
-		$sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ? and `migration_status` = ?';
+		if ($result) {
-		$args = array(self::MIGRATION_IN_PROGRESS, $this->userId, self::MIGRATION_OPEN);
-		$query = \OCP\DB::prepare($sql);
-		$manipulatedRows = $query->execute($args);
-
-		if ($manipulatedRows === 1) {
-			$return = true;
 			\OCP\Util::writeLog('Encryption library', "Start migration to encryption mode for " . $this->userId, \OCP\Util::INFO);
 		} else {
 			\OCP\Util::writeLog('Encryption library', "Could not activate migration mode for " . $this->userId . ". Probably another process already started the initial encryption", \OCP\Util::WARN);
 		}
 
-		return $return;
+		return $result;
+	}
+
+	public function resetMigrationStatus() {
+		return $this->setMigrationStatus(self::MIGRATION_OPEN);
+
 	}
 
 	/**
@@ -1213,22 +1247,15 @@ class Util {
 	 * @return boolean
 	 */
 	public function finishMigration() {
+		$result = $this->setMigrationStatus(self::MIGRATION_COMPLETED);
 
-		$return = false;
+		if ($result) {
-
-		$sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ? and `migration_status` = ?';
-		$args = array(self::MIGRATION_COMPLETED, $this->userId, self::MIGRATION_IN_PROGRESS);
-		$query = \OCP\DB::prepare($sql);
-		$manipulatedRows = $query->execute($args);
-
-		if ($manipulatedRows === 1) {
-			$return = true;
 			\OCP\Util::writeLog('Encryption library', "Finish migration successfully for " . $this->userId, \OCP\Util::INFO);
 		} else {
 			\OCP\Util::writeLog('Encryption library', "Could not deactivate migration mode for " . $this->userId, \OCP\Util::WARN);
 		}
 
-		return $return;
+		return $result;
 	}
 
 	/**

apps/files_encryption/tests/util.php

@@ -64,6 +64,8 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase {
 
 
 	function setUp() {
+		// login user
+		\Test_Encryption_Util::loginHelper(\Test_Encryption_Util::TEST_ENCRYPTION_UTIL_USER1);
 		\OC_User::setUserId(\Test_Encryption_Util::TEST_ENCRYPTION_UTIL_USER1);
 		$this->userId = \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_USER1;
 		$this->pass = \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_USER1;
@@ -358,9 +360,12 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase {
 		$fileInfoEncrypted = $this->view->getFileInfo($this->userId . '/files/' . $filename);
 
 		$this->assertTrue($fileInfoEncrypted instanceof \OC\Files\FileInfo);
+		$this->assertEquals($fileInfoEncrypted['encrypted'], 1);
 
-		// encrypt all unencrypted files
+		// decrypt all encrypted files
-		$util->decryptAll('/' . $this->userId . '/' . 'files');
+		$result = $util->decryptAll('/' . $this->userId . '/' . 'files');
+
+		$this->assertTrue($result);
 
 		$fileInfoUnencrypted = $this->view->getFileInfo($this->userId . '/files/' . $filename);
 
@@ -369,11 +374,83 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase {
 		// check if mtime and etags unchanged
 		$this->assertEquals($fileInfoEncrypted['mtime'], $fileInfoUnencrypted['mtime']);
 		$this->assertEquals($fileInfoEncrypted['etag'], $fileInfoUnencrypted['etag']);
+		// file should no longer be encrypted
+		$this->assertEquals(0, $fileInfoUnencrypted['encrypted']);
 
 		$this->view->unlink($this->userId . '/files/' . $filename);
 
 	}
 
+	function testDescryptAllWithBrokenFiles() {
+
+		$file1 = "/decryptAll1" . uniqid() . ".txt";
+		$file2 = "/decryptAll2" . uniqid() . ".txt";
+
+		$util = new Encryption\Util($this->view, $this->userId);
+
+		$this->view->file_put_contents($this->userId . '/files/' . $file1, $this->dataShort);
+		$this->view->file_put_contents($this->userId . '/files/' . $file2, $this->dataShort);
+
+		$fileInfoEncrypted1 = $this->view->getFileInfo($this->userId . '/files/' . $file1);
+		$fileInfoEncrypted2 = $this->view->getFileInfo($this->userId . '/files/' . $file2);
+
+		$this->assertTrue($fileInfoEncrypted1 instanceof \OC\Files\FileInfo);
+		$this->assertTrue($fileInfoEncrypted2 instanceof \OC\Files\FileInfo);
+		$this->assertEquals($fileInfoEncrypted1['encrypted'], 1);
+		$this->assertEquals($fileInfoEncrypted2['encrypted'], 1);
+
+		// rename keyfile for file1 so that the decryption for file1 fails
+		// Expected behaviour: decryptAll() returns false, file2 gets decrypted anyway
+		$this->view->rename($this->userId . '/files_encryption/keyfiles/' . $file1 . '.key',
+				$this->userId . '/files_encryption/keyfiles/' . $file1 . '.key.moved');
+
+		// decrypt all encrypted files
+		$result = $util->decryptAll('/' . $this->userId . '/' . 'files');
+
+		$this->assertFalse($result);
+
+		$fileInfoUnencrypted1 = $this->view->getFileInfo($this->userId . '/files/' . $file1);
+		$fileInfoUnencrypted2 = $this->view->getFileInfo($this->userId . '/files/' . $file2);
+
+		$this->assertTrue($fileInfoUnencrypted1 instanceof \OC\Files\FileInfo);
+		$this->assertTrue($fileInfoUnencrypted2 instanceof \OC\Files\FileInfo);
+
+		// file1 should be still encrypted; file2 should be decrypted
+		$this->assertEquals(1, $fileInfoUnencrypted1['encrypted']);
+		$this->assertEquals(0, $fileInfoUnencrypted2['encrypted']);
+
+		// keyfiles and share keys should still exist
+		$this->assertTrue($this->view->is_dir($this->userId . '/files_encryption/keyfiles/'));
+		$this->assertTrue($this->view->is_dir($this->userId . '/files_encryption/share-keys/'));
+
+		// rename the keyfile for file1 back
+		$this->view->rename($this->userId . '/files_encryption/keyfiles/' . $file1 . '.key.moved',
+				$this->userId . '/files_encryption/keyfiles/' . $file1 . '.key');
+
+		// try again to decrypt all encrypted files
+		$result = $util->decryptAll('/' . $this->userId . '/' . 'files');
+
+		$this->assertTrue($result);
+
+		$fileInfoUnencrypted1 = $this->view->getFileInfo($this->userId . '/files/' . $file1);
+		$fileInfoUnencrypted2 = $this->view->getFileInfo($this->userId . '/files/' . $file2);
+
+		$this->assertTrue($fileInfoUnencrypted1 instanceof \OC\Files\FileInfo);
+		$this->assertTrue($fileInfoUnencrypted2 instanceof \OC\Files\FileInfo);
+
+		// now both files should be decrypted
+		$this->assertEquals(0, $fileInfoUnencrypted1['encrypted']);
+		$this->assertEquals(0, $fileInfoUnencrypted2['encrypted']);
+
+		// keyfiles and share keys should be deleted
+		$this->assertFalse($this->view->is_dir($this->userId . '/files_encryption/keyfiles/'));
+		$this->assertFalse($this->view->is_dir($this->userId . '/files_encryption/share-keys/'));
+
+		$this->view->unlink($this->userId . '/files/' . $file1);
+		$this->view->unlink($this->userId . '/files/' . $file2);
+
+	}
+
 	/**
 	 * @large
 	 */

settings/ajax/decryptall.php

@@ -16,7 +16,14 @@ $util = new \OCA\Encryption\Util($view, \OCP\User::getUser());
 $result = $util->initEncryption($params);
 
 if ($result !== false) {
-	$successful = $util->decryptAll();
+
+	try {
+		$successful = $util->decryptAll();
+	} catch (\Exception $ex) {
+		\OCP\Util::writeLog('encryption library', "Decryption finished unexpected: " . $ex->getMessage(), \OCP\Util::ERROR);
+		$successful = false;
+	}
+
 	if ($successful === true) {
 		\OCP\JSON::success(array('data' => array('message' => 'Files decrypted successfully')));
 	} else {