From a2e2005e6796959ee58cdf2bf96fd711174797eb Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 9 Jul 2015 18:04:35 +0200 Subject: [PATCH 1/2] make sure that we always detect legacy files correctly --- lib/private/encryption/util.php | 29 ----- .../files/storage/wrapper/encryption.php | 109 +++++++++++++--- tests/lib/encryption/utiltest.php | 13 -- .../lib/files/storage/wrapper/encryption.php | 120 +++++++++++++++++- 4 files changed, 209 insertions(+), 62 deletions(-) diff --git a/lib/private/encryption/util.php b/lib/private/encryption/util.php index 8bff65428d..d0733941a3 100644 --- a/lib/private/encryption/util.php +++ b/lib/private/encryption/util.php @@ -127,35 +127,6 @@ class Util { return $id; } - /** - * read header into array - * - * @param string $header - * @return array - */ - public function readHeader($header) { - - $result = array(); - - if (substr($header, 0, strlen(self::HEADER_START)) === self::HEADER_START) { - $endAt = strpos($header, self::HEADER_END); - if ($endAt !== false) { - $header = substr($header, 0, $endAt + strlen(self::HEADER_END)); - - // +1 to not start with an ':' which would result in empty element at the beginning - $exploded = explode(':', substr($header, strlen(self::HEADER_START)+1)); - - $element = array_shift($exploded); - while ($element !== self::HEADER_END) { - $result[$element] = array_shift($exploded); - $element = array_shift($exploded); - } - } - } - - return $result; - } - /** * create header for encrypted file * diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index 8818b822fa..ebd6062b1d 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -31,6 +31,7 @@ use OC\Encryption\Util; use OC\Files\Filesystem; use OC\Files\Mount\Manager; use OC\Files\Storage\LocalTempFileTrait; +use OCP\Encryption\Exceptions\GenericEncryptionException; use OCP\Encryption\IFile; use OCP\Encryption\IManager; use OCP\Encryption\Keys\IStorage; @@ -174,9 +175,8 @@ class Encryption extends Wrapper { public function file_get_contents($path) { $encryptionModule = $this->getEncryptionModule($path); - $info = $this->getCache()->get($path); - if ($encryptionModule || $info['encrypted'] === true) { + if ($encryptionModule) { $handle = $this->fopen($path, "r"); if (!$handle) { return false; @@ -338,14 +338,15 @@ class Encryption extends Wrapper { * @param string $path * @param string $mode * @return resource + * @throws GenericEncryptionException + * @throws ModuleDoesNotExistsException */ public function fopen($path, $mode) { $encryptionEnabled = $this->encryptionManager->isEnabled(); $shouldEncrypt = false; $encryptionModule = null; - $rawHeader = $this->getHeader($path); - $header = $this->util->readHeader($rawHeader); + $header = $this->getHeader($path); $fullPath = $this->getFullPath($path); $encryptionModuleId = $this->util->getEncryptionModuleId($header); @@ -380,6 +381,10 @@ class Encryption extends Wrapper { || $mode === 'wb' || $mode === 'wb+' ) { + // don't overwrite encrypted files if encyption is not enabled + if ($targetIsEncrypted && $encryptionEnabled === false) { + throw new GenericEncryptionException('Tried to access encrypted file but encryption is not enabled'); + } if ($encryptionEnabled) { // if $encryptionModuleId is empty, the default module will be used $encryptionModule = $this->encryptionManager->getEncryptionModule($encryptionModuleId); @@ -416,7 +421,7 @@ class Encryption extends Wrapper { $source = $this->storage->fopen($path, $mode); $handle = \OC\Files\Stream\Encryption::wrap($source, $path, $fullPath, $header, $this->uid, $encryptionModule, $this->storage, $this, $this->util, $this->fileHelper, $mode, - $size, $unencryptedSize, strlen($rawHeader)); + $size, $unencryptedSize, $this->getHeaderSize($path)); return $handle; } @@ -605,6 +610,72 @@ class Encryption extends Wrapper { return Filesystem::normalizePath($this->mountPoint . '/' . $path); } + /** + * read first block of encrypted file, typically this will contain the + * encryption header + * + * @param string $path + * @return string + */ + protected function readFirstBlock($path) { + $firstBlock = ''; + if ($this->storage->file_exists($path)) { + $handle = $this->storage->fopen($path, 'r'); + $firstBlock = fread($handle, $this->util->getHeaderSize()); + fclose($handle); + } + return $firstBlock; + } + + /** + * return header size of given file + * + * @param string $path + * @return int + */ + protected function getHeaderSize($path) { + $headerSize = 0; + $realFile = $this->util->stripPartialFileExtension($path); + if ($this->storage->file_exists($realFile)) { + $path = $realFile; + } + $firstBlock = $this->readFirstBlock($path); + + if (substr($firstBlock, 0, strlen(Util::HEADER_START)) === Util::HEADER_START) { + $headerSize = strlen($firstBlock); + } + + return $headerSize; + } + + /** + * parse raw header to array + * + * @param string $rawHeader + * @return array + */ + protected function parseRawHeader($rawHeader) { + $result = array(); + if (substr($rawHeader, 0, strlen(Util::HEADER_START)) === Util::HEADER_START) { + $header = $rawHeader; + $endAt = strpos($header, Util::HEADER_END); + if ($endAt !== false) { + $header = substr($header, 0, $endAt + strlen(Util::HEADER_END)); + + // +1 to not start with an ':' which would result in empty element at the beginning + $exploded = explode(':', substr($header, strlen(Util::HEADER_START)+1)); + + $element = array_shift($exploded); + while ($element !== Util::HEADER_END) { + $result[$element] = array_shift($exploded); + $element = array_shift($exploded); + } + } + } + + return $result; + } + /** * read header from file * @@ -612,21 +683,29 @@ class Encryption extends Wrapper { * @return array */ protected function getHeader($path) { - $header = ''; $realFile = $this->util->stripPartialFileExtension($path); if ($this->storage->file_exists($realFile)) { $path = $realFile; } - if ($this->storage->file_exists($path)) { - $handle = $this->storage->fopen($path, 'r'); - $firstBlock = fread($handle, $this->util->getHeaderSize()); - fclose($handle); - if (substr($firstBlock, 0, strlen(Util::HEADER_START)) === Util::HEADER_START) { - $header = $firstBlock; + $firstBlock = $this->readFirstBlock($path); + $result = $this->parseRawHeader($firstBlock); + + // if the header doesn't contain a encryption module we check if it is a + // legacy file. If true, we add the default encryption module + if (!isset($result[Util::HEADER_ENCRYPTION_MODULE_KEY])) { + if (!empty($result)) { + $result[Util::HEADER_ENCRYPTION_MODULE_KEY] = 'OC_DEFAULT_MODULE'; + } else { + // if the header was empty we have to check first if it is a encrypted file at all + $info = $this->getCache()->get($path); + if (isset($info['encrypted']) && $info['encrypted'] === true) { + $result[Util::HEADER_ENCRYPTION_MODULE_KEY] = 'OC_DEFAULT_MODULE'; + } } } - return $header; + + return $result; } /** @@ -639,8 +718,7 @@ class Encryption extends Wrapper { */ protected function getEncryptionModule($path) { $encryptionModule = null; - $rawHeader = $this->getHeader($path); - $header = $this->util->readHeader($rawHeader); + $header = $this->getHeader($path); $encryptionModuleId = $this->util->getEncryptionModuleId($header); if (!empty($encryptionModuleId)) { try { @@ -675,4 +753,5 @@ class Encryption extends Wrapper { return false; } + } diff --git a/tests/lib/encryption/utiltest.php b/tests/lib/encryption/utiltest.php index d5f5ce4c2e..5aadb4e857 100644 --- a/tests/lib/encryption/utiltest.php +++ b/tests/lib/encryption/utiltest.php @@ -72,19 +72,6 @@ class UtilTest extends TestCase { ]; } - /** - * @dataProvider providesHeaders - */ - public function testReadHeader($header, $expected, $moduleId) { - $expected['oc_encryption_module'] = $moduleId; - $result = $this->util->readHeader($header); - $this->assertSameSize($expected, $result); - foreach ($expected as $key => $value) { - $this->assertArrayHasKey($key, $result); - $this->assertSame($value, $result[$key]); - } - } - /** * @dataProvider providesHeaders */ diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index a10e95a3f8..677bbffc3d 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/tests/lib/files/storage/wrapper/encryption.php @@ -2,11 +2,19 @@ namespace Test\Files\Storage\Wrapper; +use OC\Encryption\Util; use OC\Files\Storage\Temporary; use OC\Files\View; class Encryption extends \Test\Files\Storage\Storage { + /** + * block size will always be 8192 for a PHP stream + * @see https://bugs.php.net/bug.php?id=21641 + * @var integer + */ + protected $headerSize = 8192; + /** * @var Temporary */ @@ -407,18 +415,26 @@ class Encryption extends \Test\Files\Storage\Storage { $this->encryptionManager, $util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager ] ) + ->setMethods(['readFirstBlock', 'parseRawHeader']) ->getMock(); + $instance->expects($this->once())->method(('parseRawHeader')) + ->willReturn([Util::HEADER_ENCRYPTION_MODULE_KEY => 'OC_DEFAULT_MODULE']); + + if ($strippedPathExists) { + $instance->expects($this->once())->method('readFirstBlock') + ->with($strippedPath)->willReturn(''); + } else { + $instance->expects($this->once())->method('readFirstBlock') + ->with($path)->willReturn(''); + } + $util->expects($this->once())->method('stripPartialFileExtension') ->with($path)->willReturn($strippedPath); - $sourceStorage->expects($this->at(0)) + $sourceStorage->expects($this->once()) ->method('file_exists') ->with($strippedPath) ->willReturn($strippedPathExists); - $sourceStorage->expects($this->at(1)) - ->method('file_exists') - ->with($strippedPathExists ? $strippedPath : $path) - ->willReturn(false); $this->invokePrivate($instance, 'getHeader', [$path]); } @@ -432,4 +448,98 @@ class Encryption extends \Test\Files\Storage\Storage { array('/foo/bar.txt.ocTransferId7437493.part', true, '/foo/bar.txt'), ); } + + /** + * test if getHeader adds the default module correctly to the header for + * legacy files + * + * @dataProvider dataTestGetHeaderAddLegacyModule + */ + public function testGetHeaderAddLegacyModule($header, $isEncrypted, $expected) { + + $sourceStorage = $this->getMockBuilder('\OC\Files\Storage\Storage') + ->disableOriginalConstructor()->getMock(); + + $util = $this->getMockBuilder('\OC\Encryption\Util') + ->setConstructorArgs([new View(), new \OC\User\Manager(), $this->groupManager, $this->config]) + ->getMock(); + + $cache = $this->getMockBuilder('\OC\Files\Cache\Cache') + ->disableOriginalConstructor()->getMock(); + $cache->expects($this->any()) + ->method('get') + ->willReturnCallback(function($path) use ($isEncrypted) {return ['encrypted' => $isEncrypted, 'path' => $path];}); + + $instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + ->setConstructorArgs( + [ + [ + 'storage' => $sourceStorage, + 'root' => 'foo', + 'mountPoint' => '/', + 'mount' => $this->mount + ], + $this->encryptionManager, $util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager + ] + ) + ->setMethods(['readFirstBlock', 'parseRawHeader', 'getCache']) + ->getMock(); + + $instance->expects($this->once())->method(('parseRawHeader'))->willReturn($header); + $instance->expects($this->any())->method('getCache')->willReturn($cache); + + $result = $this->invokePrivate($instance, 'getHeader', ['test.txt']); + $this->assertSameSize($expected, $result); + foreach ($result as $key => $value) { + $this->assertArrayHasKey($key, $expected); + $this->assertSame($expected[$key], $value); + } + } + + public function dataTestGetHeaderAddLegacyModule() { + return [ + [['cipher' => 'AES-128'], true, ['cipher' => 'AES-128', Util::HEADER_ENCRYPTION_MODULE_KEY => 'OC_DEFAULT_MODULE']], + [[], true, [Util::HEADER_ENCRYPTION_MODULE_KEY => 'OC_DEFAULT_MODULE']], + [[], false, []], + ]; + } + + /** + * @dataProvider dataTestParseRawHeader + */ + public function testParseRawHeader($rawHeader, $expected) { + $instance = new \OC\Files\Storage\Wrapper\Encryption( + [ + 'storage' => $this->sourceStorage, + 'root' => 'foo', + 'mountPoint' => '/', + 'mount' => $this->mount + ], + $this->encryptionManager, $this->util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager + + ); + + $result = $this->invokePrivate($instance, 'parseRawHeader', [$rawHeader]); + $this->assertSameSize($expected, $result); + foreach ($result as $key => $value) { + $this->assertArrayHasKey($key, $expected); + $this->assertSame($expected[$key], $value); + } + } + + public function dataTestParseRawHeader() { + return [ + [str_pad('HBEGIN:oc_encryption_module:0:HEND', $this->headerSize, '-', STR_PAD_RIGHT) + , [Util::HEADER_ENCRYPTION_MODULE_KEY => '0']], + [str_pad('HBEGIN:oc_encryption_module:0:custom_header:foo:HEND', $this->headerSize, '-', STR_PAD_RIGHT) + , ['custom_header' => 'foo', Util::HEADER_ENCRYPTION_MODULE_KEY => '0']], + [str_pad('HelloWorld', $this->headerSize, '-', STR_PAD_RIGHT), []], + ['', []], + [str_pad('HBEGIN:oc_encryption_module:0', $this->headerSize, '-', STR_PAD_RIGHT) + , []], + [str_pad('oc_encryption_module:0:HEND', $this->headerSize, '-', STR_PAD_RIGHT) + , []], + ]; + } + } From 16d8014cdd9caf17ec6887d9c6538c2ea7c8b6a6 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 10 Jul 2015 13:14:07 +0200 Subject: [PATCH 2/2] set targetIsEncrypted to true if file cache indicates that we try to read a encrypted file --- lib/private/files/storage/wrapper/encryption.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index ebd6062b1d..61290791fa 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -403,6 +403,7 @@ class Encryption extends Wrapper { // OC_DEFAULT_MODULE to read the file $encryptionModule = $this->encryptionManager->getEncryptionModule('OC_DEFAULT_MODULE'); $shouldEncrypt = true; + $targetIsEncrypted = true; } } } catch (ModuleDoesNotExistsException $e) {