Merge pull request #17840 from owncloud/fix-enc-wrapper-without-encryption

Only set is encrypted when encryption is enabled

apps/files/appinfo/update.php

@@ -0,0 +1,96 @@
+<?php
+/**
+ * @author Björn Schießle <schiessle@owncloud.com>
+ * @author Joas Schilling <nickvergessen@owncloud.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+$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 WHERE `encrypted` = 1');
+}
+
+// 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',
+					]
+				);
+			}
+		}
+	}
+}

apps/files/appinfo/version

@@ -1 +1 @@
-1.1.9
+1.1.10

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

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);
+	}
 }

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';