From 1e631754d78e98d74ba0d3fb477d5eb815e9dfb3 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 17 Sep 2014 18:50:29 +0200 Subject: [PATCH] Fix share key finding algorithm in various cases Instead of inaccurate pattern matching, use the list of users who we know have access to the file to build the list of share keys. This covers the following cases: - Move/copy files into a subfolder within a share - Unsharing from a user - Deleting files directlry / moving share keys to trashbin --- apps/files_encryption/hooks/hooks.php | 8 +- apps/files_encryption/lib/helper.php | 46 ++++++--- apps/files_encryption/lib/keymanager.php | 54 ++++++----- apps/files_encryption/tests/helper.php | 56 +++++++++++ apps/files_encryption/tests/hooks.php | 105 ++++++++++++++------- apps/files_encryption/tests/keymanager.php | 60 +++++++++--- apps/files_encryption/tests/share.php | 57 +++++++++++ apps/files_encryption/tests/trashbin.php | 89 +++++++++++++---- apps/files_encryption/tests/util.php | 16 ++-- apps/files_trashbin/lib/trashbin.php | 6 +- 10 files changed, 384 insertions(+), 113 deletions(-) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index b1e7e8c52a5..db4e1fa9428 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -520,7 +520,13 @@ class Hooks { $newKeyfilePath .= '.key'; // handle share-keys - $matches = Helper::findShareKeys($oldShareKeyPath, $view); + $matches = Helper::findShareKeys($pathOld, $oldShareKeyPath, $view); + if (count($matches) === 0) { + \OC_Log::write( + 'Encryption library', 'No share keys found for "' . $pathOld . '"', + \OC_Log::WARN + ); + } foreach ($matches as $src) { $dst = \OC\Files\Filesystem::normalizePath(str_replace($pathOld, $pathNew, $src)); $view->$operation($src, $dst); diff --git a/apps/files_encryption/lib/helper.php b/apps/files_encryption/lib/helper.php index d427c51732f..ab19938d633 100755 --- a/apps/files_encryption/lib/helper.php +++ b/apps/files_encryption/lib/helper.php @@ -431,24 +431,40 @@ class Helper { /** * find all share keys for a given file - * @param string $path to the file - * @param \OC\Files\View $view view, relative to data/ - * @return array list of files, path relative to data/ + * + * @param string $filePath path to the file name relative to the user's files dir + * for example "subdir/filename.txt" + * @param string $shareKeyPath share key prefix path relative to the user's data dir + * for example "user1/files_encryption/share-keys/subdir/filename.txt" + * @param \OC\Files\View $rootView root view, relative to data/ + * @return array list of share key files, path relative to data/$user */ - public static function findShareKeys($path, $view) { + public static function findShareKeys($filePath, $shareKeyPath, $rootView) { $result = array(); - $pathinfo = pathinfo($path); - $dirContent = $view->opendir($pathinfo['dirname']); - - if (is_resource($dirContent)) { - while (($file = readdir($dirContent)) !== false) { - if (!\OC\Files\Filesystem::isIgnoredDir($file)) { - if (preg_match("/" . $pathinfo['filename'] . ".(.*).shareKey/", $file)) { - $result[] = $pathinfo['dirname'] . '/' . $file; - } - } + + $user = \OCP\User::getUser(); + $util = new Util($rootView, $user); + // get current sharing state + $sharingEnabled = \OCP\Share::isEnabled(); + + // get users sharing this file + $usersSharing = $util->getSharingUsersArray($sharingEnabled, $filePath); + + $pathinfo = pathinfo($shareKeyPath); + + $baseDir = $pathinfo['dirname'] . '/'; + $fileName = $pathinfo['basename']; + foreach ($usersSharing as $user) { + $keyName = $fileName . '.' . $user . '.shareKey'; + if ($rootView->file_exists($baseDir . $keyName)) { + $result[] = $baseDir . $keyName; + } else { + \OC_Log::write( + 'Encryption library', + 'No share key found for user "' . $user . '" for file "' . $pathOld . '"', + \OC_Log::WARN + ); } - closedir($dirContent); } return $result; diff --git a/apps/files_encryption/lib/keymanager.php b/apps/files_encryption/lib/keymanager.php index 931469f4b74..9560126ef33 100755 --- a/apps/files_encryption/lib/keymanager.php +++ b/apps/files_encryption/lib/keymanager.php @@ -459,13 +459,17 @@ class Keymanager { \OCP\Util::writeLog('files_encryption', 'delAllShareKeys: delete share keys: ' . $baseDir . $filePath, \OCP\Util::DEBUG); $result = $view->unlink($baseDir . $filePath); } else { - $parentDir = dirname($baseDir . $filePath); - $filename = pathinfo($filePath, PATHINFO_BASENAME); - foreach($view->getDirectoryContent($parentDir) as $content) { - $path = $content['path']; - if (self::getFilenameFromShareKey($content['name']) === $filename) { - \OCP\Util::writeLog('files_encryption', 'dellAllShareKeys: delete share keys: ' . '/' . $userId . '/' . $path, \OCP\Util::DEBUG); - $result &= $view->unlink('/' . $userId . '/' . $path); + $sharingEnabled = \OCP\Share::isEnabled(); + $users = $util->getSharingUsersArray($sharingEnabled, $filePath); + foreach($users as $user) { + $keyName = $baseDir . $filePath . '.' . $user . '.shareKey'; + if ($view->file_exists($keyName)) { + \OCP\Util::writeLog( + 'files_encryption', + 'dellAllShareKeys: delete share keys: "' . $keyName . '"', + \OCP\Util::DEBUG + ); + $result &= $view->unlink($keyName); } } } @@ -539,17 +543,20 @@ class Keymanager { if ($view->is_dir($dir . '/' . $file)) { self::recursiveDelShareKeys($dir . '/' . $file, $userIds, $owner, $view); } else { - $realFile = $realFileDir . self::getFilenameFromShareKey($file); foreach ($userIds as $userId) { - if (preg_match("/(.*)." . $userId . ".shareKey/", $file)) { - if ($userId === $owner && - $view->file_exists($realFile)) { - \OCP\Util::writeLog('files_encryption', 'original file still exists, keep owners share key!', \OCP\Util::ERROR); - continue; - } - \OCP\Util::writeLog('files_encryption', 'recursiveDelShareKey: delete share key: ' . $file, \OCP\Util::DEBUG); - $view->unlink($dir . '/' . $file); + $fileNameFromShareKey = self::getFilenameFromShareKey($file, $userId); + if (!$fileNameFromShareKey) { + continue; } + $realFile = $realFileDir . $fileNameFromShareKey; + + if ($userId === $owner && + $view->file_exists($realFile)) { + \OCP\Util::writeLog('files_encryption', 'original file still exists, keep owners share key!', \OCP\Util::ERROR); + continue; + } + \OCP\Util::writeLog('files_encryption', 'recursiveDelShareKey: delete share key: ' . $file, \OCP\Util::DEBUG); + $view->unlink($dir . '/' . $file); } } } @@ -591,16 +598,19 @@ class Keymanager { /** * extract filename from share key name * @param string $shareKey (filename.userid.sharekey) + * @param string $userId * @return string|false filename or false */ - protected static function getFilenameFromShareKey($shareKey) { - $parts = explode('.', $shareKey); + protected static function getFilenameFromShareKey($shareKey, $userId) { + $expectedSuffix = '.' . $userId . '.' . 'shareKey'; + $suffixLen = strlen($expectedSuffix); - $filename = false; - if(count($parts) > 2) { - $filename = implode('.', array_slice($parts, 0, count($parts)-2)); + $suffix = substr($shareKey, -$suffixLen); + + if ($suffix !== $expectedSuffix) { + return false; } - return $filename; + return substr($shareKey, 0, -$suffixLen); } } diff --git a/apps/files_encryption/tests/helper.php b/apps/files_encryption/tests/helper.php index 582d8149a8a..b94fdeb4d6c 100644 --- a/apps/files_encryption/tests/helper.php +++ b/apps/files_encryption/tests/helper.php @@ -108,4 +108,60 @@ class Test_Encryption_Helper extends \PHPUnit_Framework_TestCase { \Test_Encryption_Util::loginHelper(\Test_Encryption_Helper::TEST_ENCRYPTION_HELPER_USER1); } + function userNamesProvider() { + return array( + array('testuser' . uniqid()), + array('user.name.with.dots'), + ); + } + + /** + * Tests whether share keys can be found + * + * @dataProvider userNamesProvider + */ + function testFindShareKeys($userName) { + // note: not using dataProvider as we want to make + // sure that the correct keys are match and not any + // other ones that might happen to have similar names + \Test_Encryption_Util::setupHooks(); + \Test_Encryption_Util::loginHelper($userName, true); + $testDir = 'testFindShareKeys' . uniqid() . '/'; + $baseDir = $userName . '/files/' . $testDir; + $fileList = array( + 't est.txt', + 't est_.txt', + 't est.doc.txt', + 't est(.*).txt', // make sure the regexp is escaped + 'multiple.dots.can.happen.too.txt', + 't est.' . $userName . '.txt', + 't est_.' . $userName . '.shareKey.txt', + 'who would upload their.shareKey', + 'user ones file.txt', + 'user ones file.txt.backup', + '.t est.txt' + ); + + $rootView = new \OC\Files\View('/'); + $rootView->mkdir($baseDir); + foreach ($fileList as $fileName) { + $rootView->file_put_contents($baseDir . $fileName, 'dummy'); + } + + $shareKeysDir = $userName . '/files_encryption/share-keys/' . $testDir; + foreach ($fileList as $fileName) { + // make sure that every file only gets its correct respective keys + $result = Encryption\Helper::findShareKeys($baseDir . $fileName, $shareKeysDir . $fileName, $rootView); + $this->assertEquals( + array($shareKeysDir . $fileName . '.' . $userName . '.shareKey'), + $result + ); + } + + // clean up + $rootView->unlink($baseDir); + \Test_Encryption_Util::logoutHelper(); + \OC_User::deleteUser($userName); + } + } diff --git a/apps/files_encryption/tests/hooks.php b/apps/files_encryption/tests/hooks.php index cc5b6d5b6f6..14d44fe5bb3 100644 --- a/apps/files_encryption/tests/hooks.php +++ b/apps/files_encryption/tests/hooks.php @@ -36,8 +36,8 @@ use OCA\Encryption; */ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { - const TEST_ENCRYPTION_HOOKS_USER1 = "test-encryption-hooks-user1"; - const TEST_ENCRYPTION_HOOKS_USER2 = "test-encryption-hooks-user2"; + const TEST_ENCRYPTION_HOOKS_USER1 = "test-encryption-hooks-user1.dot"; + const TEST_ENCRYPTION_HOOKS_USER2 = "test-encryption-hooks-user2.dot"; /** * @var \OC\Files\View @@ -49,7 +49,26 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { public $filename; public $folder; + private static $testFiles; + public static function setUpBeforeClass() { + // note: not using a data provider because these + // files all need to coexist to make sure the + // share keys are found properly (pattern matching) + self::$testFiles = array( + 't est.txt', + 't est_.txt', + 't est.doc.txt', + 't est(.*).txt', // make sure the regexp is escaped + 'multiple.dots.can.happen.too.txt', + 't est.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.txt', + 't est_.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey.txt', + 'who would upload their.shareKey', + 'user ones file.txt', + 'user ones file.txt.backup', + '.t est.txt' + ); + // reset backend \OC_User::clearBackends(); \OC_User::useBackend('database'); @@ -281,25 +300,33 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { } } - /** - * test rename operation - */ function testRenameHook() { + // create all files to make sure all keys can coexist properly + foreach (self::$testFiles as $file) { + // save file with content + $cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $file, $this->data); - // save file with content - $cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename, $this->data); + // test that data was successfully written + $this->assertTrue(is_int($cryptedFile)); + } - // test that data was successfully written - $this->assertTrue(is_int($cryptedFile)); + foreach (self::$testFiles as $file) { + $this->doTestRenameHook($file); + } + } + /** + * test rename operation + */ + function doTestRenameHook($filename) { // check if keys exists $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' - . $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); + . $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' - . $this->filename . '.key')); + . $filename . '.key')); // make subfolder and sub-subfolder $this->rootView->mkdir('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder); @@ -310,50 +337,58 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { // move the file to the sub-subfolder $root = $this->rootView->getRoot(); $this->rootView->chroot('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/'); - $this->rootView->rename($this->filename, '/' . $this->folder . '/' . $this->folder . '/' . $this->filename); + $this->rootView->rename($filename, '/' . $this->folder . '/' . $this->folder . '/' . $filename); $this->rootView->chroot($root); - $this->assertFalse($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename)); - $this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $this->filename)); + $this->assertFalse($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $filename)); + $this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $filename)); // keys should be renamed too $this->assertFalse($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' - . $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); + . $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); $this->assertFalse($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' - . $this->filename . '.key')); + . $filename . '.key')); $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' . $this->folder . '/' . $this->folder . '/' - . $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); + . $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' . $this->folder . '/' . $this->folder . '/' - . $this->filename . '.key')); + . $filename . '.key')); // cleanup $this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder); } - /** - * test rename operation - */ function testCopyHook() { + // create all files to make sure all keys can coexist properly + foreach (self::$testFiles as $file) { + // save file with content + $cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $file, $this->data); - // save file with content - $cryptedFile = file_put_contents('crypt:///' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename, $this->data); + // test that data was successfully written + $this->assertTrue(is_int($cryptedFile)); + } - // test that data was successfully written - $this->assertTrue(is_int($cryptedFile)); + foreach (self::$testFiles as $file) { + $this->doTestCopyHook($file); + } + } + /** + * test rename operation + */ + function doTestCopyHook($filename) { // check if keys exists $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' - . $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); + . $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' - . $this->filename . '.key')); + . $filename . '.key')); // make subfolder and sub-subfolder $this->rootView->mkdir('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder); @@ -362,29 +397,29 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { $this->assertTrue($this->rootView->is_dir('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder)); // copy the file to the sub-subfolder - \OC\Files\Filesystem::copy($this->filename, '/' . $this->folder . '/' . $this->folder . '/' . $this->filename); + \OC\Files\Filesystem::copy($filename, '/' . $this->folder . '/' . $this->folder . '/' . $filename); - $this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename)); - $this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $this->filename)); + $this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $filename)); + $this->assertTrue($this->rootView->file_exists('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder . '/' . $this->folder . '/' . $filename)); // keys should be copied too $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' - . $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); + . $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' - . $this->filename . '.key')); + . $filename . '.key')); $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/share-keys/' . $this->folder . '/' . $this->folder . '/' - . $this->filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); + . $filename . '.' . self::TEST_ENCRYPTION_HOOKS_USER1 . '.shareKey')); $this->assertTrue($this->rootView->file_exists( '/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files_encryption/keyfiles/' . $this->folder . '/' . $this->folder . '/' - . $this->filename . '.key')); + . $filename . '.key')); // cleanup $this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->folder); - $this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $this->filename); + $this->rootView->unlink('/' . self::TEST_ENCRYPTION_HOOKS_USER1 . '/files/' . $filename); } /** diff --git a/apps/files_encryption/tests/keymanager.php b/apps/files_encryption/tests/keymanager.php index f90832280a2..e8a9d7dda53 100644 --- a/apps/files_encryption/tests/keymanager.php +++ b/apps/files_encryption/tests/keymanager.php @@ -23,7 +23,7 @@ use OCA\Encryption; */ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { - const TEST_USER = "test-keymanager-user"; + const TEST_USER = "test-keymanager-user.dot"; public $userId; public $pass; @@ -135,15 +135,25 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { $this->assertArrayHasKey('key', $sslInfo); } + function fileNameFromShareKeyProvider() { + return array( + array('file.user.shareKey', 'user', 'file'), + array('file.name.with.dots.user.shareKey', 'user', 'file.name.with.dots'), + array('file.name.user.with.dots.shareKey', 'user.with.dots', 'file.name'), + array('file.txt', 'user', false), + array('user.shareKey', 'user', false), + ); + } + /** * @small + * + * @dataProvider fileNameFromShareKeyProvider */ - function testGetFilenameFromShareKey() { - $this->assertEquals("file", - \TestProtectedKeymanagerMethods::testGetFilenameFromShareKey("file.user.shareKey")); - $this->assertEquals("file.name.with.dots", - \TestProtectedKeymanagerMethods::testGetFilenameFromShareKey("file.name.with.dots.user.shareKey")); - $this->assertFalse(\TestProtectedKeymanagerMethods::testGetFilenameFromShareKey("file.txt")); + function testGetFilenameFromShareKey($fileName, $user, $expectedFileName) { + $this->assertEquals($expectedFileName, + \TestProtectedKeymanagerMethods::testGetFilenameFromShareKey($fileName, $user) + ); } /** @@ -249,6 +259,12 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.user1.shareKey', 'data'); $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey', 'data'); $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.test.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.test-keymanager-userxdot.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.userx.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.userx.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey', 'data'); + $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.user1.shareKey', 'data'); $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file2.user2.shareKey', 'data'); $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file2.user3.shareKey', 'data'); $this->view->file_put_contents('/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/file2.user3.shareKey', 'data'); @@ -279,6 +295,23 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { $this->assertTrue($this->view->file_exists( '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/subfolder/file2.user3.shareKey')); + // check if share keys for user or file with similar name + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.test.shareKey')); + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.test-keymanager-userxdot.shareKey')); + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.userx.shareKey')); + // FIXME: this case currently cannot be distinguished, needs further fixing + /* + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.userx.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey')); + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.user1.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey')); + $this->assertTrue($this->view->file_exists( + '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/file1.' . Test_Encryption_Keymanager::TEST_USER . '.user1.shareKey')); + */ + // owner key from existing file should still exists because the file is still there $this->assertTrue($this->view->file_exists( '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey')); @@ -432,18 +465,19 @@ class Test_Encryption_Keymanager extends \PHPUnit_Framework_TestCase { $this->assertTrue($this->view->file_exists( '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/existingFile.txt.user3.shareKey')); - // try to del all share keys froma file, should fail because the file still exists + // try to del all share keys from file, should succeed because the does not exist any more $result2 = Encryption\Keymanager::delAllShareKeys($this->view, Test_Encryption_Keymanager::TEST_USER, 'folder1/nonexistingFile.txt'); $this->assertTrue($result2); // check if share keys are really gone $this->assertFalse($this->view->file_exists( '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.' . Test_Encryption_Keymanager::TEST_USER . '.shareKey')); - $this->assertFalse($this->view->file_exists( + // check that it only deleted keys or users who had access, others remain + $this->assertTrue($this->view->file_exists( '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.user1.shareKey')); - $this->assertFalse($this->view->file_exists( + $this->assertTrue($this->view->file_exists( '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.user2.shareKey')); - $this->assertFalse($this->view->file_exists( + $this->assertTrue($this->view->file_exists( '/'.Test_Encryption_Keymanager::TEST_USER.'/files_encryption/share-keys/folder1/nonexistingFile.txt.user3.shareKey')); // cleanup @@ -479,8 +513,8 @@ class TestProtectedKeymanagerMethods extends \OCA\Encryption\Keymanager { /** * @param string $sharekey */ - public static function testGetFilenameFromShareKey($sharekey) { - return self::getFilenameFromShareKey($sharekey); + public static function testGetFilenameFromShareKey($sharekey, $user) { + return self::getFilenameFromShareKey($sharekey, $user); } /** diff --git a/apps/files_encryption/tests/share.php b/apps/files_encryption/tests/share.php index 1f1304bb527..1cd7cfc738b 100755 --- a/apps/files_encryption/tests/share.php +++ b/apps/files_encryption/tests/share.php @@ -1058,9 +1058,66 @@ class Test_Encryption_Share extends \PHPUnit_Framework_TestCase { // check if additional share key for user2 exists $this->assertTrue($view->file_exists('files_encryption/share-keys' . $newFolder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey')); + // check that old keys were removed/moved properly + $this->assertFalse($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey')); + // tear down \OC\Files\Filesystem::unlink($newFolder); \OC\Files\Filesystem::unlink('/newfolder'); } + function testMoveFileToFolder() { + + $view = new \OC\Files\View('/' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1); + + $filename = '/tmp-' . uniqid(); + $folder = '/folder' . uniqid(); + + \OC\Files\Filesystem::mkdir($folder); + + // Save long data as encrypted file using stream wrapper + $cryptedFile = \OC\Files\Filesystem::file_put_contents($folder . $filename, $this->dataShort); + + // Test that data was successfully written + $this->assertTrue(is_int($cryptedFile)); + + // Get file decrypted contents + $decrypt = \OC\Files\Filesystem::file_get_contents($folder . $filename); + + $this->assertEquals($this->dataShort, $decrypt); + + $subFolder = $folder . '/subfolder' . uniqid(); + \OC\Files\Filesystem::mkdir($subFolder); + + // get the file info from previous created file + $fileInfo = \OC\Files\Filesystem::getFileInfo($folder); + $this->assertTrue($fileInfo instanceof \OC\Files\FileInfo); + + // share the folder + \OCP\Share::shareItem('folder', $fileInfo['fileid'], \OCP\Share::SHARE_TYPE_USER, \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2, OCP\PERMISSION_ALL); + + // check that the share keys exist + $this->assertTrue($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey')); + $this->assertTrue($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey')); + + // move the file into the subfolder + \OC\Files\Filesystem::rename($folder . $filename, $subFolder . $filename); + + // Get file decrypted contents + $newDecrypt = \OC\Files\Filesystem::file_get_contents($subFolder . $filename); + $this->assertEquals($this->dataShort, $newDecrypt); + + // check if additional share key for user2 exists + $this->assertTrue($view->file_exists('files_encryption/share-keys' . $subFolder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey')); + $this->assertTrue($view->file_exists('files_encryption/share-keys' . $subFolder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey')); + + // check that old keys were removed/moved properly + $this->assertFalse($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER1 . '.shareKey')); + $this->assertFalse($view->file_exists('files_encryption/share-keys' . $folder . '/' . $filename . '.' . \Test_Encryption_Share::TEST_ENCRYPTION_SHARE_USER2 . '.shareKey')); + + // tear down + \OC\Files\Filesystem::unlink($subFolder); + \OC\Files\Filesystem::unlink($folder); + } + } diff --git a/apps/files_encryption/tests/trashbin.php b/apps/files_encryption/tests/trashbin.php index a5479de1b8d..ae0974431be 100755 --- a/apps/files_encryption/tests/trashbin.php +++ b/apps/files_encryption/tests/trashbin.php @@ -120,24 +120,33 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase { // generate filename $filename = 'tmp-' . uniqid() . '.txt'; + $filename2 = $filename . '.backup'; // a second file with similar name // save file with content $cryptedFile = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename, $this->dataShort); + $cryptedFile2 = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename2, $this->dataShort); // test that data was successfully written $this->assertTrue(is_int($cryptedFile)); + $this->assertTrue(is_int($cryptedFile2)); // check if key for admin exists $this->assertTrue($this->view->file_exists( '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' . $filename . '.key')); + $this->assertTrue($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' . $filename2 + . '.key')); // check if share key for admin exists $this->assertTrue($this->view->file_exists( '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/' . $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); + $this->assertTrue($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/' + . $filename2 . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); - // delete file + // delete first file \OC\FIles\Filesystem::unlink($filename); // check if file not exists @@ -154,6 +163,20 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase { '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/' . $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); + // check that second file still exists + $this->assertTrue($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $filename2)); + + // check that key for second file still exists + $this->assertTrue($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' . $filename2 + . '.key')); + + // check that share key for second file still exists + $this->assertTrue($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/' + . $filename2 . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); + // get files $trashFiles = $this->view->getDirectoryContent( '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_trashbin/files/'); @@ -179,41 +202,75 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase { $this->assertTrue($this->view->file_exists( '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_trashbin/share-keys/' . $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey.' . $trashFileSuffix)); - - // return filename for next test - return $filename . '.' . $trashFileSuffix; } /** * @medium * test restore file - * - * @depends testDeleteFile */ - function testRestoreFile($filename) { + function testRestoreFile() { + // generate filename + $filename = 'tmp-' . uniqid() . '.txt'; + $filename2 = $filename . '.backup'; // a second file with similar name + + // save file with content + $cryptedFile = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename, $this->dataShort); + $cryptedFile2 = file_put_contents('crypt:///' .\Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1. '/files/'. $filename2, $this->dataShort); + + // delete both files + \OC\Files\Filesystem::unlink($filename); + \OC\Files\Filesystem::unlink($filename2); + + $trashFiles = $this->view->getDirectoryContent( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_trashbin/files/'); + + $trashFileSuffix = null; + $trashFileSuffix2 = null; + // find created file with timestamp + foreach ($trashFiles as $file) { + if (strncmp($file['path'], $filename, strlen($filename))) { + $path_parts = pathinfo($file['name']); + $trashFileSuffix = $path_parts['extension']; + } + if (strncmp($file['path'], $filename2, strlen($filename2))) { + $path_parts = pathinfo($file['name']); + $trashFileSuffix2 = $path_parts['extension']; + } + } // prepare file information - $path_parts = pathinfo($filename); - $trashFileSuffix = $path_parts['extension']; $timestamp = str_replace('d', '', $trashFileSuffix); - $fileNameWithoutSuffix = str_replace('.' . $trashFileSuffix, '', $filename); - // restore file - $this->assertTrue(\OCA\Files_Trashbin\Trashbin::restore($filename, $fileNameWithoutSuffix, $timestamp)); + // restore first file + $this->assertTrue(\OCA\Files_Trashbin\Trashbin::restore($filename . '.' . $trashFileSuffix, $filename, $timestamp)); // check if file exists $this->assertTrue($this->view->file_exists( - '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $fileNameWithoutSuffix)); + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $filename)); // check if key for admin exists $this->assertTrue($this->view->file_exists( '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' - . $fileNameWithoutSuffix . '.key')); + . $filename . '.key')); // check if share key for admin exists $this->assertTrue($this->view->file_exists( '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/' - . $fileNameWithoutSuffix . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); + . $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); + + // check that second file was NOT restored + $this->assertFalse($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files/' . $filename2)); + + // check if key for admin exists + $this->assertFalse($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/keyfiles/' + . $filename2 . '.key')); + + // check if share key for admin exists + $this->assertFalse($this->view->file_exists( + '/' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '/files_encryption/share-keys/' + . $filename2 . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); } /** @@ -242,7 +299,7 @@ class Test_Encryption_Trashbin extends \PHPUnit_Framework_TestCase { . $filename . '.' . \Test_Encryption_Trashbin::TEST_ENCRYPTION_TRASHBIN_USER1 . '.shareKey')); // delete file - \OC\FIles\Filesystem::unlink($filename); + \OC\Files\Filesystem::unlink($filename); // check if file not exists $this->assertFalse($this->view->file_exists( diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php index f337eb46355..245d1c38222 100755 --- a/apps/files_encryption/tests/util.php +++ b/apps/files_encryption/tests/util.php @@ -53,12 +53,7 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { \OC_User::clearBackends(); \OC_User::useBackend('database'); - // Filesystem related hooks - \OCA\Encryption\Helper::registerFilesystemHooks(); - - // clear and register hooks - \OC_FileProxy::clearProxies(); - \OC_FileProxy::register(new OCA\Encryption\Proxy()); + self::setupHooks(); // create test user \Test_Encryption_Util::loginHelper(\Test_Encryption_Util::TEST_ENCRYPTION_UTIL_USER1, true); @@ -134,6 +129,15 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { \OC_Group::deleteGroup(self::TEST_ENCRYPTION_UTIL_GROUP2); } + public static function setupHooks() { + // Filesystem related hooks + \OCA\Encryption\Helper::registerFilesystemHooks(); + + // clear and register hooks + \OC_FileProxy::clearProxies(); + \OC_FileProxy::register(new OCA\Encryption\Proxy()); + } + /** * @medium * test that paths set during User construction are correct diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php index 5f1226d89d8..6be635ef0d0 100644 --- a/apps/files_trashbin/lib/trashbin.php +++ b/apps/files_trashbin/lib/trashbin.php @@ -303,12 +303,8 @@ class Trashbin { } $rootView->rename($sharekeys, $user . '/files_trashbin/share-keys/' . $filename . '.d' . $timestamp); } else { - // get local path to share-keys - $localShareKeysPath = $rootView->getLocalFile($sharekeys); - $escapedLocalShareKeysPath = preg_replace('/(\*|\?|\[)/', '[$1]', $localShareKeysPath); - // handle share-keys - $matches = glob($escapedLocalShareKeysPath . '*.shareKey'); + $matches = \OCA\Encryption\Helper::findShareKeys($ownerPath, $sharekeys, $rootView); foreach ($matches as $src) { // get source file parts $pathinfo = pathinfo($src); -- 2.39.5