From 81e3ae4753774cf33e6f5bcaf1dc479e1dc5831c Mon Sep 17 00:00:00 2001 From: Jasper Knockaert Date: Tue, 5 Jan 2021 11:14:49 +0100 Subject: [PATCH 1/7] avoid fread on directories and unencrypted files Reworking the logic in order to first check the filecache and only then reading the fileheader. This in order to solve #21578. --- .../Files/Storage/Wrapper/Encryption.php | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index ef44be5cefb67..b37fcdb2bd0ed 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -929,19 +929,22 @@ protected function getHeader($path) { $path = $realFile; } - $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'; - } elseif ($exists) { - // if the header was empty we have to check first if it is a encrypted file at all - // We would do query to filecache only if we know that entry in filecache exists - $info = $this->getCache()->get($path); - if (isset($info['encrypted']) && $info['encrypted'] === true) { + $result = []; + + // first check if it is an encrypted file at all + // We would do query to filecache only if we know that entry in filecache exists + + $info = $this->getCache()->get($path); + if (isset($info['encrypted']) && $info['encrypted'] === true) { + $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'; + } elseif ($exists) { $result[Util::HEADER_ENCRYPTION_MODULE_KEY] = 'OC_DEFAULT_MODULE'; } } From 87c640193bd9d2a6a4a29163ad0cb539f263cacc Mon Sep 17 00:00:00 2001 From: Jasper Knockaert Date: Tue, 5 Jan 2021 19:09:16 +0100 Subject: [PATCH 2/7] attemtp to fix test --- tests/lib/Files/Storage/Wrapper/EncryptionTest.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index eccec7c28d5b0..ef524a55fdafc 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -560,6 +560,12 @@ public function dataTestCopyKeys() { * @param string $strippedPath */ public function testGetHeader($path, $strippedPathExists, $strippedPath) { + $cache = $this->getMockBuilder('\OC\Files\Cache\Cache') + ->disableOriginalConstructor()->getMock(); + $cache->expects($this->any()) + ->method('get') + ->willReturn(['encrypted' => true]); + $sourceStorage = $this->getMockBuilder('\OC\Files\Storage\Storage') ->disableOriginalConstructor()->getMock(); @@ -586,7 +592,7 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath) { $this->encryptionManager, $util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager, $this->arrayCache ] ) - ->setMethods(['readFirstBlock', 'parseRawHeader']) + ->setMethods(['getCache','readFirstBlock', 'parseRawHeader']) ->getMock(); $instance->expects($this->once())->method(('parseRawHeader')) From 664e74a82ec957c44ea5ec782e2718546c534f87 Mon Sep 17 00:00:00 2001 From: Jasper Knockaert Date: Tue, 5 Jan 2021 20:41:02 +0100 Subject: [PATCH 3/7] 2nd attempt to fix tests --- .../Files/Storage/Wrapper/EncryptionTest.php | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index ef524a55fdafc..6bdacda8c7668 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -560,12 +560,6 @@ public function dataTestCopyKeys() { * @param string $strippedPath */ public function testGetHeader($path, $strippedPathExists, $strippedPath) { - $cache = $this->getMockBuilder('\OC\Files\Cache\Cache') - ->disableOriginalConstructor()->getMock(); - $cache->expects($this->any()) - ->method('get') - ->willReturn(['encrypted' => true]); - $sourceStorage = $this->getMockBuilder('\OC\Files\Storage\Storage') ->disableOriginalConstructor()->getMock(); @@ -579,6 +573,14 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath) { $this->arrayCache ] )->getMock(); + + $cache = $this->getMockBuilder('\OC\Files\Cache\Cache') + ->disableOriginalConstructor()->getMock(); + $cache->expects($this->any()) + ->method('get') + ->willReturnCallback(function ($path) { + return ['encrypted' => true, 'path' => $path]; + }); $instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') ->setConstructorArgs( @@ -594,7 +596,9 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath) { ) ->setMethods(['getCache','readFirstBlock', 'parseRawHeader']) ->getMock(); - + + $instance->expects($this->any())->method('getCache')->willReturn($cache); + $instance->expects($this->once())->method(('parseRawHeader')) ->willReturn([Util::HEADER_ENCRYPTION_MODULE_KEY => 'OC_DEFAULT_MODULE']); From 261966ace5f539f0ab676a063506f6e63469dce5 Mon Sep 17 00:00:00 2001 From: Jasper Knockaert Date: Tue, 5 Jan 2021 21:45:50 +0100 Subject: [PATCH 4/7] Update EncryptionTest.php --- tests/lib/Files/Storage/Wrapper/EncryptionTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index 6bdacda8c7668..d152a51ed858a 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -597,7 +597,7 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath) { ->setMethods(['getCache','readFirstBlock', 'parseRawHeader']) ->getMock(); - $instance->expects($this->any())->method('getCache')->willReturn($cache); + $instance->expects($this->once())->method('getCache')->willReturn($cache); $instance->expects($this->once())->method(('parseRawHeader')) ->willReturn([Util::HEADER_ENCRYPTION_MODULE_KEY => 'OC_DEFAULT_MODULE']); @@ -671,8 +671,8 @@ public function testGetHeaderAddLegacyModule($header, $isEncrypted, $exists, $ex ->setMethods(['readFirstBlock', 'parseRawHeader', 'getCache']) ->getMock(); - $instance->expects($this->once())->method(('parseRawHeader'))->willReturn($header); - $instance->expects($this->any())->method('getCache')->willReturn($cache); + $instance->expects($this->any())->method(('parseRawHeader'))->willReturn($header); + $instance->expects($this->once())->method('getCache')->willReturn($cache); $result = $this->invokePrivate($instance, 'getHeader', ['test.txt']); $this->assertSameSize($expected, $result); From f9a0380eb1da63a9d70c039d458897aa2366b2ca Mon Sep 17 00:00:00 2001 From: Jasper Knockaert Date: Sat, 16 Jan 2021 14:33:44 +0100 Subject: [PATCH 5/7] consolidation of boolean expression --- lib/private/Files/Storage/Wrapper/Encryption.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index b37fcdb2bd0ed..c4953eddf2323 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -941,11 +941,8 @@ protected function getHeader($path) { // 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'; - } elseif ($exists) { - $result[Util::HEADER_ENCRYPTION_MODULE_KEY] = 'OC_DEFAULT_MODULE'; + if (!isset($result[Util::HEADER_ENCRYPTION_MODULE_KEY] && (!empty($result) || $exists)) { + $result[Util::HEADER_ENCRYPTION_MODULE_KEY] = 'OC_DEFAULT_MODULE'; } } } From 252d1ae512eef3c093cfe05fe63783ffca2b3bc5 Mon Sep 17 00:00:00 2001 From: Jasper Knockaert Date: Sat, 16 Jan 2021 14:38:19 +0100 Subject: [PATCH 6/7] fix brakcets --- lib/private/Files/Storage/Wrapper/Encryption.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index c4953eddf2323..5505a61ead2e4 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -943,7 +943,6 @@ protected function getHeader($path) { // legacy file. If true, we add the default encryption module if (!isset($result[Util::HEADER_ENCRYPTION_MODULE_KEY] && (!empty($result) || $exists)) { $result[Util::HEADER_ENCRYPTION_MODULE_KEY] = 'OC_DEFAULT_MODULE'; - } } } From 40fde94b4d019f5c1914225d5be6854241abeb9c Mon Sep 17 00:00:00 2001 From: Jasper Knockaert Date: Sat, 16 Jan 2021 14:49:53 +0100 Subject: [PATCH 7/7] fix even more brackets Signed-off-by: Jasper Knockaert jasper@knockaert.nl --- 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 5505a61ead2e4..4ac10923968e8 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -941,7 +941,7 @@ protected function getHeader($path) { // 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] && (!empty($result) || $exists)) { + if (!isset($result[Util::HEADER_ENCRYPTION_MODULE_KEY]) && (!empty($result) || $exists)) { $result[Util::HEADER_ENCRYPTION_MODULE_KEY] = 'OC_DEFAULT_MODULE'; } }