catch errors during decryption

3 files changed, 113 insertions, 17 deletions

diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php
index ced4b823cf0..f3f69997f2c 100644
--- a/apps/files_encryption/lib/util.php
+++ b/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 (
-						$file !== "."
-						&& $file !== ".."
-					) {
+					if ($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 (
-								Keymanager::getFileKey($this->view, $this, $relPath)
-								&& $isEncryptedPath
-							) {
-
-								$found['encrypted'][] = array(
-									'name' => $file,
-									'path' => $filePath
-								);
+							if ($isEncryptedPath) {
+
+								$fileKey = Keymanager::getFileKey($this->view, $this, $relPath);
+								$shareKey = Keymanager::getShareKey($this->view, $this->userId, $this, $relPath);
+								// 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);
diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php
index 228f7df1b90..321bdc76f8b 100755
--- a/apps/files_encryption/tests/util.php
+++ b/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
-		$util->decryptAll('/' . $this->userId . '/' . 'files');
+		// decrypt all encrypted 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
 	 */
diff --git a/settings/ajax/decryptall.php b/settings/ajax/decryptall.php
index ebc17aacfaa..d7c104ab151 100644
--- a/settings/ajax/decryptall.php
+++ b/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 {