Merge pull request #16243 from owncloud/error_if_download_failed

redirect to error page if download failed

apps/encryption/appinfo/application.php

@@ -209,7 +209,8 @@ class Application extends \OCP\AppFramework\App {
 					$c->query('Crypt'),
 					$server->getLogger(),
 					$server->getUserSession(),
-					$server->getConfig());
+					$server->getConfig(),
+					$server->getUserManager());
 			});
 
 	}

apps/encryption/lib/crypto/encryption.php

@@ -270,6 +270,15 @@ class Encryption implements IEncryptionModule {
 	 * @return mixed decrypted data
 	 */
 	public function decrypt($data) {
+		if (empty($this->fileKey)) {
+			$msg = $this->l->t('Can not decrypt this file, probably this is a shared file. Please ask the file owner to reshare the file with you.');
+			$this->logger->error('Can not decrypt this file,
+			probably this is a shared file.
+			Please ask the file owner to reshare the file with you.');
+
+			throw new DecryptionFailedException($msg);
+		}
+
 		$result = '';
 		if (!empty($data)) {
 			$result = $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher);
@@ -345,6 +354,36 @@ class Encryption implements IEncryptionModule {
 		return 6126;
 	}
 
+	/**
+	 * check if the encryption module is able to read the file,
+	 * e.g. if all encryption keys exists
+	 *
+	 * @param string $path
+	 * @param string $uid user for whom we want to check if he can read the file
+	 * @return bool
+	 * @throws DecryptionFailedException
+	 */
+	public function isReadable($path, $uid) {
+		$fileKey = $this->keyManager->getFileKey($path, $uid);
+		if (empty($fileKey)) {
+			$owner = $this->util->getOwner($path);
+			if ($owner !== $uid) {
+				// if it is a shared file we throw a exception with a useful
+				// error message because in this case it means that the file was
+				// shared with the user at a point where the user didn't had a
+				// valid private/public key
+				$msg = 'Encryption module "' . $this->getDisplayName() .
+					'" is not able to read ' . $path;
+				$hint = $this->l->t('Can not read this file, probably this is a shared file. Please ask the file owner to reshare the file with you.');
+				$this->logger->warning($msg);
+				throw new DecryptionFailedException($msg, 0, null, $hint);
+			}
+			return false;
+		}
+
+		return true;
+	}
+
 	/**
 	 * @param string $path
 	 * @return string
@@ -360,4 +399,5 @@ class Encryption implements IEncryptionModule {
 
 		return $realPath;
 	}
+
 }

apps/encryption/lib/util.php

@@ -29,6 +29,7 @@ use OCA\Encryption\Crypto\Crypt;
 use OCP\IConfig;
 use OCP\ILogger;
 use OCP\IUser;
+use OCP\IUserManager;
 use OCP\IUserSession;
 use OCP\PreConditionNotMetException;
 
@@ -53,6 +54,10 @@ class Util {
 	 * @var IConfig
 	 */
 	private $config;
+	/**
+	 * @var IUserManager
+	 */
+	private $userManager;
 
 	/**
 	 * Util constructor.
@@ -62,18 +67,21 @@ class Util {
 	 * @param ILogger $logger
 	 * @param IUserSession $userSession
 	 * @param IConfig $config
+	 * @param IUserManager $userManager
 	 */
 	public function __construct(View $files,
 								Crypt $crypt,
 								ILogger $logger,
 								IUserSession $userSession,
-								IConfig $config
+								IConfig $config,
+								IUserManager $userManager
 	) {
 		$this->files = $files;
 		$this->crypt = $crypt;
 		$this->logger = $logger;
 		$this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser() : false;
 		$this->config = $config;
+		$this->userManager = $userManager;
 	}
 
 	/**
@@ -117,5 +125,26 @@ class Util {
 		return $this->files->file_exists($uid . '/files');
 	}
 
+	/**
+	 * get owner from give path, path relative to data/ expected
+	 *
+	 * @param string $path relative to data/
+	 * @return string
+	 * @throws \BadMethodCallException
+	 */
+	public function getOwner($path) {
+		$owner = '';
+		$parts = explode('/', $path, 3);
+		if (count($parts) > 1) {
+			$owner = $parts[1];
+			if ($this->userManager->userExists($owner) === false) {
+				throw new \BadMethodCallException('Unknown user: ' .
+				'method expects path to a user folder relative to the data folder');
+			}
+
+		}
+
+		return $owner;
+	}
 
 }

apps/encryption/settings/settings-personal.php

@@ -35,7 +35,8 @@ $util = new \OCA\Encryption\Util(
 	$crypt,
 	\OC::$server->getLogger(),
 	$userSession,
-	\OC::$server->getConfig());
+	\OC::$server->getConfig(),
+	\OC::$server->getUserManager());
 
 $keyManager = new \OCA\Encryption\KeyManager(
 	\OC::$server->getEncryptionKeyStorage(),

apps/encryption/tests/lib/UtilTest.php

@@ -28,11 +28,17 @@ use Test\TestCase;
 
 class UtilTest extends TestCase {
 	private static $tempStorage = [];
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
 	private $configMock;
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
 	private $filesMock;
-	/**
-	 * @var Util
-	 */
+
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	private $userManagerMock;
+
+	/** @var Util */
 	private $instance;
 
 	public function testSetRecoveryForUser() {
@@ -40,9 +46,6 @@ class UtilTest extends TestCase {
 		$this->assertArrayHasKey('recoveryEnabled', self::$tempStorage);
 	}
 
-	/**
-	 *
-	 */
 	public function testIsRecoveryEnabledForUser() {
 		$this->assertTrue($this->instance->isRecoveryEnabledForUser('admin'));
 
@@ -62,6 +65,7 @@ class UtilTest extends TestCase {
 	protected function setUp() {
 		parent::setUp();
 		$this->filesMock = $this->getMock('OC\Files\View');
+		$this->userManagerMock = $this->getMock('\OCP\IUserManager');
 
 		$cryptMock = $this->getMockBuilder('OCA\Encryption\Crypto\Crypt')
 			->disableOriginalConstructor()
@@ -98,7 +102,7 @@ class UtilTest extends TestCase {
 			->method('setUserValue')
 			->will($this->returnCallback([$this, 'setValueTester']));
 
-		$this->instance = new Util($this->filesMock, $cryptMock, $loggerMock, $userSessionMock, $configMock);
+		$this->instance = new Util($this->filesMock, $cryptMock, $loggerMock, $userSessionMock, $configMock, $this->userManagerMock);
 	}
 
 	/**

apps/files_sharing/lib/sharedstorage.php

@@ -217,7 +217,13 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
 	}
 
 	public function isReadable($path) {
-		return $this->file_exists($path);
+		$isReadable = false;
+		if ($source = $this->getSourcePath($path)) {
+			list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source);
+			$isReadable = $storage->isReadable($internalPath);
+		}
+
+		return $isReadable && $this->file_exists($path);
 	}
 
 	public function isUpdatable($path) {

lib/private/encryption/exceptions/decryptionfailedexception.php

@@ -27,4 +27,15 @@ use OCP\Encryption\Exceptions\GenericEncryptionException;
 
 class DecryptionFailedException extends GenericEncryptionException {
 
+	/**
+	 * @param string $message
+	 * @param int $code
+	 * @param \Exception $previous
+	 * @param string $hint
+	 */
+	public function __construct($message = '', $code = 0, \Exception $previous = null, $hint = '') {
+		parent::__construct($message, $code, $previous, $hint);
+
+}
+
 }

lib/private/files.php

@@ -129,52 +129,60 @@ class OC_Files {
 			$zip = new ZipStreamer(false);
 		}
 		OC_Util::obEnd();
-		if ($zip or \OC\Files\Filesystem::isReadable($filename)) {
-			self::sendHeaders($filename, $name, $zip);
-		} elseif (!\OC\Files\Filesystem::file_exists($filename)) {
-			header("HTTP/1.0 404 Not Found");
-			$tmpl = new OC_Template('', '404', 'guest');
-			$tmpl->printPage();
-		} else {
-			header("HTTP/1.0 403 Forbidden");
-			die('403 Forbidden');
-		}
-		if($only_header) {
-			return ;
-		}
-		if ($zip) {
-			$executionTime = intval(ini_get('max_execution_time'));
-			set_time_limit(0);
-			if ($get_type === self::ZIP_FILES) {
-				foreach ($files as $file) {
-					$file = $dir . '/' . $file;
-					if (\OC\Files\Filesystem::is_file($file)) {
-						$fh = \OC\Files\Filesystem::fopen($file, 'r');
-						$zip->addFileFromStream($fh, basename($file));
-						fclose($fh);
-					} elseif (\OC\Files\Filesystem::is_dir($file)) {
-						self::zipAddDir($file, $zip);
-					}
-				}
-			} elseif ($get_type === self::ZIP_DIR) {
-				$file = $dir . '/' . $files;
-				self::zipAddDir($file, $zip);
+
+		try {
+
+			if ($zip or \OC\Files\Filesystem::isReadable($filename)) {
+				self::sendHeaders($filename, $name, $zip);
+			} elseif (!\OC\Files\Filesystem::file_exists($filename)) {
+				header("HTTP/1.0 404 Not Found");
+				$tmpl = new OC_Template('', '404', 'guest');
+				$tmpl->printPage();
+			} else {
+				header("HTTP/1.0 403 Forbidden");
+				die('403 Forbidden');
 			}
-			$zip->finalize();
-			set_time_limit($executionTime);
-		} else {
-			if ($xsendfile) {
-				$view = \OC\Files\Filesystem::getView();
-				/** @var $storage \OC\Files\Storage\Storage */
-				list($storage) = $view->resolvePath($filename);
-				if ($storage->isLocal()) {
-					self::addSendfileHeader($filename);
+			if ($only_header) {
+				return;
+			}
+			if ($zip) {
+				$executionTime = intval(ini_get('max_execution_time'));
+				set_time_limit(0);
+				if ($get_type === self::ZIP_FILES) {
+					foreach ($files as $file) {
+						$file = $dir . '/' . $file;
+						if (\OC\Files\Filesystem::is_file($file)) {
+							$fh = \OC\Files\Filesystem::fopen($file, 'r');
+							$zip->addFileFromStream($fh, basename($file));
+							fclose($fh);
+						} elseif (\OC\Files\Filesystem::is_dir($file)) {
+							self::zipAddDir($file, $zip);
+						}
+					}
+				} elseif ($get_type === self::ZIP_DIR) {
+					$file = $dir . '/' . $files;
+					self::zipAddDir($file, $zip);
+				}
+				$zip->finalize();
+				set_time_limit($executionTime);
+			} else {
+				if ($xsendfile) {
+					$view = \OC\Files\Filesystem::getView();
+					/** @var $storage \OC\Files\Storage\Storage */
+					list($storage) = $view->resolvePath($filename);
+					if ($storage->isLocal()) {
+						self::addSendfileHeader($filename);
+					} else {
+						\OC\Files\Filesystem::readfile($filename);
+					}
 				} else {
 					\OC\Files\Filesystem::readfile($filename);
 				}
-			} else {
-				\OC\Files\Filesystem::readfile($filename);
 			}
+		} catch (\Exception $ex) {
+			$l = \OC::$server->getL10N('core');
+			$hint = method_exists($ex, 'getHint') ? $ex->getHint() : '';
+			\OC_Template::printErrorPage($l->t('Can\'t read file'), $hint);
 		}
 	}
 

lib/private/files/storage/wrapper/encryption.php

@@ -252,6 +252,30 @@ class Encryption extends Wrapper {
 		return $result;
 	}
 
+	/**
+	 * check if a file can be read
+	 *
+	 * @param string $path
+	 * @return bool
+	 */
+	public function isReadable($path) {
+
+		$isReadable = true;
+
+		$metaData = $this->getMetaData($path);
+		if (
+			!$this->is_dir($path) &&
+			isset($metaData['encrypted']) &&
+			$metaData['encrypted'] === true
+		) {
+			$fullPath = $this->getFullPath($path);
+			$module = $this->getEncryptionModule($path);
+			$isReadable = $module->isReadable($fullPath, $this->uid);
+		}
+
+		return $this->storage->isReadable($path) && $isReadable;
+	}
+
 	/**
 	 * see http://php.net/manual/en/function.copy.php
 	 *

lib/public/encryption/exceptions/genericencryptionexception.php

@@ -30,17 +30,26 @@ namespace OCP\Encryption\Exceptions;
  */
 class GenericEncryptionException extends \Exception {
 
+	/** @var string */
+	protected $hint;
+
 	/**
 	 * @param string $message
 	 * @param int $code
 	 * @param \Exception $previous
 	 * @since 8.1.0
 	 */
-	public function __construct($message = '', $code = 0, \Exception $previous = null) {
+	public function __construct($message = '', $code = 0, \Exception $previous = null, $hint = '') {
 		if (empty($message)) {
 			$message = 'Unspecified encryption exception';
 		}
 		parent::__construct($message, $code, $previous);
+
+		$this->hint = $hint;
+	}
+
+	public function getHint() {
+		return $this->hint;
 	}
 
 }

lib/public/encryption/iencryptionmodule.php

@@ -119,4 +119,16 @@ interface IEncryptionModule {
 	 * @since 8.1.0
 	 */
 	public function getUnencryptedBlockSize();
+
+	/**
+	 * check if the encryption module is able to read the file,
+	 * e.g. if all encryption keys exists
+	 *
+	 * @param string $path
+	 * @param string $uid user for whom we want to check if he can read the file
+	 * @return boolean
+	 * @since 8.1.0
+	 */
+	public function isReadable($path, $uid);
+
 }

settings/changepassword/controller.php

@@ -89,7 +89,8 @@ class Controller {
 				$crypt,
 				\OC::$server->getLogger(),
 				\OC::$server->getUserSession(),
-				\OC::$server->getConfig());
+				\OC::$server->getConfig(),
+				\OC::$server->getUserManager());
 			$keyManager = new \OCA\Encryption\KeyManager(
 				$keyStorage,
 				$crypt,

tests/lib/files/storage/wrapper/encryption.php

@@ -117,7 +117,7 @@ class Encryption extends \Test\Files\Storage\Storage {
 					$this->encryptionManager, $this->util, $logger, $file, null, $this->keyStore, $this->update
 				]
 			)
-			->setMethods(['getMetaData', 'getCache'])
+			->setMethods(['getMetaData', 'getCache', 'getEncryptionModule'])
 			->getMock();
 
 		$this->instance->expects($this->any())
@@ -127,6 +127,10 @@ class Encryption extends \Test\Files\Storage\Storage {
 		$this->instance->expects($this->any())
 			->method('getCache')
 			->willReturn($this->cache);
+
+		$this->instance->expects($this->any())
+			->method('getEncryptionModule')
+			->willReturn($mockModule);
 	}
 
 	/**
@@ -135,7 +139,7 @@ class Encryption extends \Test\Files\Storage\Storage {
 	protected function buildMockModule() {
 		$this->encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
 			->disableOriginalConstructor()
-			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize'])
+			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable'])
 			->getMock();
 
 		$this->encryptionModule->expects($this->any())->method('getId')->willReturn('UNIT_TEST_MODULE');
@@ -147,6 +151,7 @@ class Encryption extends \Test\Files\Storage\Storage {
 		$this->encryptionModule->expects($this->any())->method('update')->willReturn(true);
 		$this->encryptionModule->expects($this->any())->method('shouldEncrypt')->willReturn(true);
 		$this->encryptionModule->expects($this->any())->method('getUnencryptedBlockSize')->willReturn(8192);
+		$this->encryptionModule->expects($this->any())->method('isReadable')->willReturn(true);
 		return $this->encryptionModule;
 	}
 

tests/lib/files/stream/encryption.php

@@ -253,13 +253,14 @@ class Encryption extends \Test\TestCase {
 	protected function buildMockModule() {
 		$encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
 			->disableOriginalConstructor()
-			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize'])
+			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable'])
 			->getMock();
 
 		$encryptionModule->expects($this->any())->method('getId')->willReturn('UNIT_TEST_MODULE');
 		$encryptionModule->expects($this->any())->method('getDisplayName')->willReturn('Unit test module');
 		$encryptionModule->expects($this->any())->method('begin')->willReturn([]);
 		$encryptionModule->expects($this->any())->method('end')->willReturn('');
+		$encryptionModule->expects($this->any())->method('isReadable')->willReturn(true);
 		$encryptionModule->expects($this->any())->method('encrypt')->willReturnCallback(function($data) {
 			// simulate different block size by adding some padding to the data
 			if (isset($data[6125])) {