From 9cee8ff9f8729c22030c9c5b59e13dfd5796c956 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 23 Jul 2015 15:18:59 +0200 Subject: [PATCH 1/4] Only set is encrypted when encryption is enabled --- lib/private/files/storage/wrapper/encryption.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index 61290791fa..af3d14082c 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -521,7 +521,7 @@ class Encryption extends Wrapper { if ($preserveMtime) { $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath)); } - $isEncrypted = $this->mount->getOption('encrypt', true) ? 1 : 0; + $isEncrypted = $this->encryptionManager->isEnabled() && $this->mount->getOption('encrypt', true) ? 1 : 0; // in case of a rename we need to manipulate the source cache because // this information will be kept for the new target From 42baeb36dce9999f91763ef9d24103b4c52ff03b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 23 Jul 2015 17:01:44 +0200 Subject: [PATCH 2/4] Add a unit test for the disabled encryption case --- .../lib/files/storage/wrapper/encryption.php | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index 677bbffc3d..612cf82797 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/tests/lib/files/storage/wrapper/encryption.php @@ -139,7 +139,15 @@ class Encryption extends \Test\Files\Storage\Storage { ->disableOriginalConstructor() ->setMethods(['getOption']) ->getMock(); - $this->mount->expects($this->any())->method('getOption')->willReturn(true); + $this->mount->expects($this->any())->method('getOption')->willReturnCallback(function ($option, $default) { + if ($option === 'encrypt' && $default === true) { + global $mockedMountPointEncryptionEnabled; + if ($mockedMountPointEncryptionEnabled !== null) { + return $mockedMountPointEncryptionEnabled; + } + } + return true; + }); $this->cache = $this->getMockBuilder('\OC\Files\Cache\Cache') ->disableOriginalConstructor()->getMock(); @@ -542,4 +550,55 @@ class Encryption extends \Test\Files\Storage\Storage { ]; } + public function dataCopyBetweenStorage() { + return [ + [true, true, true], + [true, false, false], + [false, true, false], + [false, false, false], + ]; + } + + /** + * @dataProvider dataCopyBetweenStorage + * + * @param bool $encryptionEnabled + * @param bool $mountPointEncryptionEnabled + * @param bool $expectedEncrypted + */ + public function testCopyBetweenStorage($encryptionEnabled, $mountPointEncryptionEnabled, $expectedEncrypted) { + $storage2 = $this->getMockBuilder('OCP\Files\Storage') + ->disableOriginalConstructor() + ->getMock(); + + $sourceInternalPath = $targetInternalPath = 'file.txt'; + $preserveMtime = $isRename = false; + + $storage2->expects($this->any()) + ->method('fopen') + ->willReturnCallback(function($path, $mode) { + $temp = \OC::$server->getTempManager(); + return fopen($temp->getTemporaryFile(), $mode); + }); + + $this->encryptionManager->expects($this->any()) + ->method('isEnabled') + ->willReturn($encryptionEnabled); + + // FIXME can not overwrite the return after definition +// $this->mount->expects($this->at(0)) +// ->method('getOption') +// ->with('encrypt', true) +// ->willReturn($mountPointEncryptionEnabled); + global $mockedMountPointEncryptionEnabled; + $mockedMountPointEncryptionEnabled = $mountPointEncryptionEnabled; + + $this->cache->expects($this->once()) + ->method('put') + ->with($sourceInternalPath, ['encrypted' => $expectedEncrypted]); + + $this->invokePrivate($this->instance, 'copyBetweenStorage', [$storage2, $sourceInternalPath, $targetInternalPath, $preserveMtime, $isRename]); + + $this->assertFalse(false); + } } From 20cf8ec205fb796ec919071f193b7f6f1ab2ae76 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 23 Jul 2015 18:27:05 +0200 Subject: [PATCH 3/4] Add an update script to reset the value In case encryption was not enabled, we accidently set encrypted = 1 for files inside mount points, since 8.1.0. This breaks opening the files in 8.1.1 because we fixed the code that checks if a file is encrypted. In order to fix the file, we need to reset the flag of the file. However, the flag might be set because the file is in fact encrypted because it was uploaded at a time where encryption was enabled. So we can only do this when: - Current version of ownCloud before the update is 8.1.0 or 8.2.0.(0-2) - Encryption is disabled - files_encryption is not known in the app config If the first two are not the case, we are save. However, if files_encryption values exist in the config, we might have a false negative here. Now if there is no file with unencrypted size greater 0, that means there are no files that are still encrypted with "files_encryption" encryption. So we can also safely reset the flag here. If this is not the case, we go with "better save then sorry" and don't change the flag but write a message to the ownCloud log file. --- apps/files/appinfo/update.php | 96 +++++++++++++++++++++++++++++++++++ apps/files/appinfo/version | 2 +- version.php | 2 +- 3 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 apps/files/appinfo/update.php diff --git a/apps/files/appinfo/update.php b/apps/files/appinfo/update.php new file mode 100644 index 0000000000..d11a6237ad --- /dev/null +++ b/apps/files/appinfo/update.php @@ -0,0 +1,96 @@ + + * @author Joas Schilling + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ +$installedVersion = \OC::$server->getConfig()->getAppValue('files', 'installed_version'); +$ocVersion = explode('.', \OC::$server->getSystemConfig()->getValue('version')); + +/** + * In case encryption was not enabled, we accidently set encrypted = 1 for + * files inside mount points, since 8.1.0. This breaks opening the files in + * 8.1.1 because we fixed the code that checks if a file is encrypted. + * In order to fix the file, we need to reset the flag of the file. However, + * the flag might be set because the file is in fact encrypted because it was + * uploaded at a time where encryption was enabled. + * + * So we can only do this when: + * - Current version of ownCloud before the update is 8.1.0 or 8.2.0.(0-2) + * - Encryption is disabled + * - files_encryption is not known in the app config + * + * If the first two are not the case, we are save. However, if files_encryption + * values exist in the config, we might have a false negative here. + * Now if there is no file with unencrypted size greater 0, that means there are + * no files that are still encrypted with "files_encryption" encryption. So we + * can also safely reset the flag here. + * + * If this is not the case, we go with "better save then sorry" and don't change + * the flag but write a message to the ownCloud log file. + */ + +/** + * @param \OCP\IDBConnection $conn + */ +function owncloud_reset_encrypted_flag(\OCP\IDBConnection $conn) { + $conn->executeUpdate('UPDATE `*PREFIX*filecache` SET `encrypted` = 0'); +} + +// Current version of ownCloud before the update is 8.1.0 or 8.2.0.(0-2) +if ($installedVersion === '1.1.9' && ( + // 8.1.0.x + (((int) $ocVersion[0]) === 8 && ((int) $ocVersion[1]) === 1 && ((int) $ocVersion[2]) === 0) + || + // < 8.2.0.3 + (((int) $ocVersion[0]) === 8 && ((int) $ocVersion[1]) === 2 && ((int) $ocVersion[2]) === 0 && ((int) $ocVersion[3]) < 3) + )) { + + // Encryption is not enabled + if (!\OC::$server->getEncryptionManager()->isEnabled()) { + $conn = \OC::$server->getDatabaseConnection(); + + // Old encryption is not known in app config + $oldEncryption = \OC::$server->getConfig()->getAppKeys('files_encryption'); + if (empty($oldEncryption)) { + owncloud_reset_encrypted_flag($conn); + } else { + $query = $conn->prepare('SELECT * FROM `*PREFIX*filecache` WHERE `encrypted` = 1 AND `unencrypted_size` > 0', 1); + $query->execute(); + $empty = $query->fetch(); + + if (empty($empty)) { + owncloud_reset_encrypted_flag($conn); + } else { + /** + * Sorry in case you are a false positive, but we are not 100% that + * you don't have any encrypted files anymore, so we can not reset + * the value safely + */ + \OC::$server->getLogger()->warning( + 'If you have a problem with files not being accessible and ' + . 'you are not using encryption, please have a look at the following' + . 'issue: {issue}', + [ + 'issue' => 'https://github.com/owncloud/core/issues/17846', + ] + ); + } + } + } +} diff --git a/apps/files/appinfo/version b/apps/files/appinfo/version index 512a1faa68..5ed5faa5f1 100644 --- a/apps/files/appinfo/version +++ b/apps/files/appinfo/version @@ -1 +1 @@ -1.1.9 +1.1.10 diff --git a/version.php b/version.php index 03593c4716..7ccd2e6b54 100644 --- a/version.php +++ b/version.php @@ -22,7 +22,7 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version=array(8, 2, 0, 2); +$OC_Version=array(8, 2, 0, 3); // The human readable string $OC_VersionString='8.2 pre alpha'; From 5923270004ac0b74ab464dd8c058c59628c960b0 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Mon, 27 Jul 2015 12:08:13 +0200 Subject: [PATCH 4/4] add condition to update query --- apps/files/appinfo/update.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files/appinfo/update.php b/apps/files/appinfo/update.php index d11a6237ad..2691c05c34 100644 --- a/apps/files/appinfo/update.php +++ b/apps/files/appinfo/update.php @@ -49,7 +49,7 @@ $ocVersion = explode('.', \OC::$server->getSystemConfig()->getValue('version')); * @param \OCP\IDBConnection $conn */ function owncloud_reset_encrypted_flag(\OCP\IDBConnection $conn) { - $conn->executeUpdate('UPDATE `*PREFIX*filecache` SET `encrypted` = 0'); + $conn->executeUpdate('UPDATE `*PREFIX*filecache` SET `encrypted` = 0 WHERE `encrypted` = 1'); } // Current version of ownCloud before the update is 8.1.0 or 8.2.0.(0-2)