From 5fafd55108d1ecd13befaa589902a84a23276af8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 11 Jun 2013 12:03:50 +0200 Subject: [PATCH 1/6] make sure that only one process can enter the migration mode --- apps/files_encryption/hooks/hooks.php | 6 +- apps/files_encryption/lib/util.php | 83 ++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index c4b247da1e..9f36393d59 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -67,10 +67,10 @@ class Hooks { $session->setPrivateKey($privateKey, $params['uid']); // Check if first-run file migration has already been performed - $migrationCompleted = $util->getMigrationStatus(); + $ready = $util->beginMigration(); // If migration not yet done - if (!$migrationCompleted) { + if ($ready) { $userView = new \OC_FilesystemView('/' . $params['uid']); @@ -102,7 +102,7 @@ class Hooks { } // Register successful migration in DB - $util->setMigrationStatus(1); + $util->finishMigration(); } diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index a6711880c2..82b6ca2f32 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -1058,7 +1058,7 @@ class Util { * @param $status * @return bool */ - public function setMigrationStatus($status) { + private function setMigrationStatus($status) { $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ?'; @@ -1074,7 +1074,7 @@ class Util { return true; } else { - + \OCP\Util::writeLog('Encryption library', "Could not set migration status for " . $this->userId, \OCP\Util::ERROR); return false; } @@ -1082,12 +1082,80 @@ class Util { } /** - * @brief Check whether pwd recovery is enabled for a given user - * @return bool 1 = yes, 0 = no, false = no record + * @brief start migration mode to initially encrypt users data + * @return boolean + */ + public function beginMigration() { + + $return = false; + + $transaction = \OC_DB::beginTransaction(); + + if ($transaction === false) { + \OCP\Util::writeLog('Encryption library', "Your database migration doesn't support transactions", \OCP\Util::WARN); + } + + $migrationStatus = $this->getMigrationStatus(); + + if ($migrationStatus === '0') { + + $return = $this->setMigrationStatus(-1); + + if ($return === true) { + \OCP\Util::writeLog('Encryption library', "Enter migration mode for initial encryption for user " . $this->userId, \OCP\Util::INFO); + } else { + \OCP\Util::writeLog('Encryption library', "Could not activate migration mode for " . $this->userId . ", encryption aborted", \OCP\Util::ERROR); + } + } else { + \OCP\Util::writeLog('Encryption library', "Another process already performs the migration for user " . $this->userId, \OCP\Util::INFO); + } + + \OC_DB::commit(); + + return $return; + } + + /** + * @brief close migration mode after users data has been encrypted successfully + * @return boolean + */ + public function finishMigration() { + + $return = false; + + $transaction = \OC_DB::beginTransaction(); + + if ($transaction === false) { + \OCP\Util::writeLog('Encryption library', "Your database migration doesn't support transactions", \OCP\Util::WARN); + } + + $migrationStatus = $this->getMigrationStatus(); + + if ($migrationStatus === '-1') { + + $return = $this->setMigrationStatus(1); + + if ($return === true) { + \OCP\Util::writeLog('Encryption library', "Leave migration mode for: " . $this->userId . " successfully.", \OCP\Util::INFO); + } else { + \OCP\Util::writeLog('Encryption library', "Could not deactivate migration mode for " . $this->userId, \OCP\Util::ERROR); + } + } else { + \OCP\Util::writeLog('Encryption library', "Someone else finished the migration mode to early for user " . $this->userId, \OCP\Util::ERROR); + } + + \OC_DB::commit(); + + return $return; + } + + /** + * @brief check if files are already migrated to the encryption system + * @return '1' = yes, '0' = no, '-1' = migration in progress, false = no record * @note If records are not being returned, check for a hidden space * at the start of the uid in db */ - public function getMigrationStatus() { + private function getMigrationStatus() { $sql = 'SELECT `migration_status` FROM `*PREFIX*encryption` WHERE `uid` = ?'; @@ -1112,14 +1180,11 @@ class Util { // If no record is found if (empty($migrationStatus)) { - + \OCP\Util::writeLog('Encryption library', "Could not get migration status for " . $this->userId . ", no record found", \OCP\Util::ERROR); return false; - // If a record is found } else { - return $migrationStatus[0]; - } } From 3ec6b19cdf6b1774d359c10f6cd7f74b5f376d18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 11 Jun 2013 13:07:39 +0200 Subject: [PATCH 2/6] use constants for different migration status --- apps/files_encryption/lib/util.php | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index 82b6ca2f32..a5aa121f93 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -96,10 +96,13 @@ class Util { //// DONE: test new encryption with sharing //// TODO: test new encryption with proxies + const MIGRATION_COMPLETED = 1; // migration to new encryption completed + const MIGRATION_IN_PROGRESS = -1; // migration is running + const MIGRATION_OPEN = 0; // user still needs to be migrated + private $view; // OC_FilesystemView object for filesystem operations private $userId; // ID of the currently logged-in user - private $pwd; // User Password private $client; // Client side encryption mode flag private $publicKeyDir; // Dir containing all public user keys private $encryptionDir; // Dir containing user's files_encryption @@ -1097,9 +1100,9 @@ class Util { $migrationStatus = $this->getMigrationStatus(); - if ($migrationStatus === '0') { + if ($migrationStatus === self::MIGRATION_OPEN) { - $return = $this->setMigrationStatus(-1); + $return = $this->setMigrationStatus(self::MIGRATION_IN_PROGRESS); if ($return === true) { \OCP\Util::writeLog('Encryption library', "Enter migration mode for initial encryption for user " . $this->userId, \OCP\Util::INFO); @@ -1107,7 +1110,7 @@ class Util { \OCP\Util::writeLog('Encryption library', "Could not activate migration mode for " . $this->userId . ", encryption aborted", \OCP\Util::ERROR); } } else { - \OCP\Util::writeLog('Encryption library', "Another process already performs the migration for user " . $this->userId, \OCP\Util::INFO); + \OCP\Util::writeLog('Encryption library', "Another process already performs the migration for user " . $this->userId, \OCP\Util::WARN); } \OC_DB::commit(); @@ -1131,9 +1134,9 @@ class Util { $migrationStatus = $this->getMigrationStatus(); - if ($migrationStatus === '-1') { + if ($migrationStatus === self::MIGRATION_IN_PROGRESS) { - $return = $this->setMigrationStatus(1); + $return = $this->setMigrationStatus(self::MIGRATION_COMPLETED); if ($return === true) { \OCP\Util::writeLog('Encryption library', "Leave migration mode for: " . $this->userId . " successfully.", \OCP\Util::INFO); @@ -1151,7 +1154,7 @@ class Util { /** * @brief check if files are already migrated to the encryption system - * @return '1' = yes, '0' = no, '-1' = migration in progress, false = no record + * @return migration status, false = in case of no record * @note If records are not being returned, check for a hidden space * at the start of the uid in db */ @@ -1184,7 +1187,7 @@ class Util { return false; // If a record is found } else { - return $migrationStatus[0]; + return (int)$migrationStatus[0]; } } From c78a90fd54c790a21c9ba4d8dcf86a68ebef0edd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Wed, 12 Jun 2013 12:21:11 +0200 Subject: [PATCH 3/6] use number of manipulated rows as idicator if it was possible to enter the migration mode --- apps/files_encryption/hooks/hooks.php | 5 +- apps/files_encryption/lib/util.php | 88 ++++++--------------------- 2 files changed, 24 insertions(+), 69 deletions(-) diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index 9f36393d59..7e68f476a7 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -67,7 +67,10 @@ class Hooks { $session->setPrivateKey($privateKey, $params['uid']); // Check if first-run file migration has already been performed - $ready = $util->beginMigration(); + $ready = false; + if ($util->getMigrationStatus() === Util::MIGRATION_OPEN) { + $ready = $util->beginMigration(); + } // If migration not yet done if ($ready) { diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index a5aa121f93..f6da417c6f 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -1056,64 +1056,26 @@ class Util { } - /** - * @brief Set file migration status for user - * @param $status - * @return bool - */ - private function setMigrationStatus($status) { - - $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ?'; - - $args = array( - $status, - $this->userId - ); - - $query = \OCP\DB::prepare($sql); - - if ($query->execute($args)) { - - return true; - - } else { - \OCP\Util::writeLog('Encryption library', "Could not set migration status for " . $this->userId, \OCP\Util::ERROR); - return false; - - } - - } - /** * @brief start migration mode to initially encrypt users data * @return boolean */ public function beginMigration() { - + $return = false; - $transaction = \OC_DB::beginTransaction(); + $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ? and `migration_status` = ?'; + $args = array(self::MIGRATION_IN_PROGRESS, $this->userId, self::MIGRATION_OPEN); + $query = \OCP\DB::prepare($sql); + $result = $query->execute($args); + $manipulatedRows = $result->numRows(); - if ($transaction === false) { - \OCP\Util::writeLog('Encryption library', "Your database migration doesn't support transactions", \OCP\Util::WARN); - } - - $migrationStatus = $this->getMigrationStatus(); - - if ($migrationStatus === self::MIGRATION_OPEN) { - - $return = $this->setMigrationStatus(self::MIGRATION_IN_PROGRESS); - - if ($return === true) { - \OCP\Util::writeLog('Encryption library', "Enter migration mode for initial encryption for user " . $this->userId, \OCP\Util::INFO); - } else { - \OCP\Util::writeLog('Encryption library', "Could not activate migration mode for " . $this->userId . ", encryption aborted", \OCP\Util::ERROR); - } + 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', "Another process already performs the migration for user " . $this->userId, \OCP\Util::WARN); + \OCP\Util::writeLog('Encryption library', "Could not activate migration mode for " . $this->userId . ". Probably another process already started the initial encryption", \OCP\Util::WARN); } - - \OC_DB::commit(); return $return; } @@ -1126,29 +1088,19 @@ class Util { $return = false; - $transaction = \OC_DB::beginTransaction(); + $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); + $result = $query->execute($args); + $manipulatedRows = $result->numRows(); - if ($transaction === false) { - \OCP\Util::writeLog('Encryption library', "Your database migration doesn't support transactions", \OCP\Util::WARN); - } - - $migrationStatus = $this->getMigrationStatus(); - - if ($migrationStatus === self::MIGRATION_IN_PROGRESS) { - - $return = $this->setMigrationStatus(self::MIGRATION_COMPLETED); - - if ($return === true) { - \OCP\Util::writeLog('Encryption library', "Leave migration mode for: " . $this->userId . " successfully.", \OCP\Util::INFO); - } else { - \OCP\Util::writeLog('Encryption library', "Could not deactivate migration mode for " . $this->userId, \OCP\Util::ERROR); - } + if ($manipulatedRows === 1) { + $result = true; + \OCP\Util::writeLog('Encryption library', "Finish migration successfully for " . $this->userId, \OCP\Util::INFO); } else { - \OCP\Util::writeLog('Encryption library', "Someone else finished the migration mode to early for user " . $this->userId, \OCP\Util::ERROR); + \OCP\Util::writeLog('Encryption library', "Could not deactivate migration mode for " . $this->userId, \OCP\Util::WARN); } - \OC_DB::commit(); - return $return; } @@ -1158,7 +1110,7 @@ class Util { * @note If records are not being returned, check for a hidden space * at the start of the uid in db */ - private function getMigrationStatus() { + public function getMigrationStatus() { $sql = 'SELECT `migration_status` FROM `*PREFIX*encryption` WHERE `uid` = ?'; From df78085171b8c059bfa12f4b60b9f9433529512a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Wed, 12 Jun 2013 14:00:53 +0200 Subject: [PATCH 4/6] adapt test to the code changes --- apps/files_encryption/tests/util.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php index 0dc452a41c..93b3d111e4 100755 --- a/apps/files_encryption/tests/util.php +++ b/apps/files_encryption/tests/util.php @@ -178,8 +178,7 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { $params['uid'] = \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_LEGACY_USER; $params['password'] = \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_LEGACY_USER; - $util = new Encryption\Util($this->view, \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_LEGACY_USER); - $util->setMigrationStatus(0); + $this->setMigrationStatus(0, \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_LEGACY_USER); $this->assertTrue(OCA\Encryption\Hooks::login($params)); @@ -269,7 +268,7 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { $params['password'] = \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_LEGACY_USER; $util = new Encryption\Util($this->view, \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_LEGACY_USER); - $util->setMigrationStatus(0); + $this->setMigrationStatus(0, \Test_Encryption_Util::TEST_ENCRYPTION_UTIL_LEGACY_USER); $this->assertTrue(OCA\Encryption\Hooks::login($params)); @@ -314,4 +313,20 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { $params['password'] = $password; OCA\Encryption\Hooks::login($params); } + + private function setMigrationStatus($status, $user) { + $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ?'; + $args = array( + $status, + $user + ); + + $query = \OCP\DB::prepare($sql); + if ($query->execute($args)) { + return true; + } else { + return false; + } + } + } From 77944cf7b861fbe580c119f5471e3783e9a0cea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Thu, 13 Jun 2013 10:11:23 +0200 Subject: [PATCH 5/6] fix typo in var name --- apps/files_encryption/lib/util.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index f6da417c6f..b6e3543bca 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -1095,7 +1095,7 @@ class Util { $manipulatedRows = $result->numRows(); if ($manipulatedRows === 1) { - $result = true; + $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); From 6394fedb3309dc163ed37081809f80f90157e096 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Thu, 13 Jun 2013 10:35:30 +0200 Subject: [PATCH 6/6] add comment to explain the helper function --- apps/files_encryption/tests/util.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php index 93b3d111e4..fb10284c58 100755 --- a/apps/files_encryption/tests/util.php +++ b/apps/files_encryption/tests/util.php @@ -314,6 +314,14 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { OCA\Encryption\Hooks::login($params); } + /** + * helper function to set migration status to the right value + * to be able to test the migration path + * + * @param $status needed migration status for test + * @param $user for which user the status should be set + * @return boolean + */ private function setMigrationStatus($status, $user) { $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ?'; $args = array(