From c2a072b27dab76a0e1611cad45cd25ab84a5b2fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 11 Mar 2021 11:32:29 +0100 Subject: [PATCH] Log and continue when failing to update encryption keys during for individual files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/Encryption/EncryptionWrapper.php | 2 ++ lib/private/Encryption/HookManager.php | 2 ++ lib/private/Encryption/Update.php | 34 +++++++++++++------- tests/lib/Encryption/UpdateTest.php | 30 +++++++++-------- 4 files changed, 43 insertions(+), 25 deletions(-) diff --git a/lib/private/Encryption/EncryptionWrapper.php b/lib/private/Encryption/EncryptionWrapper.php index edbdc692b4..09d8435f16 100644 --- a/lib/private/Encryption/EncryptionWrapper.php +++ b/lib/private/Encryption/EncryptionWrapper.php @@ -31,6 +31,7 @@ use OC\Memcache\ArrayCache; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage; use OCP\ILogger; +use Psr\Log\LoggerInterface; /** * Class EncryptionWrapper @@ -100,6 +101,7 @@ class EncryptionWrapper { Filesystem::getMountManager(), $this->manager, $fileHelper, + \OC::$server->get(LoggerInterface::class), $uid ); return new Encryption( diff --git a/lib/private/Encryption/HookManager.php b/lib/private/Encryption/HookManager.php index 8ddd506b69..16d45556b2 100644 --- a/lib/private/Encryption/HookManager.php +++ b/lib/private/Encryption/HookManager.php @@ -25,6 +25,7 @@ namespace OC\Encryption; use OC\Files\Filesystem; use OC\Files\View; +use Psr\Log\LoggerInterface; class HookManager { /** @@ -67,6 +68,7 @@ class HookManager { Filesystem::getMountManager(), \OC::$server->getEncryptionManager(), \OC::$server->getEncryptionFilesHelper(), + \OC::$server->get(LoggerInterface::class), $uid ); } diff --git a/lib/private/Encryption/Update.php b/lib/private/Encryption/Update.php index beb76a223b..f4e7f4d94b 100644 --- a/lib/private/Encryption/Update.php +++ b/lib/private/Encryption/Update.php @@ -26,40 +26,43 @@ namespace OC\Encryption; +use Exception; +use InvalidArgumentException; +use OC; use OC\Files\Filesystem; use OC\Files\Mount; use OC\Files\View; +use OCP\Encryption\Exceptions\GenericEncryptionException; +use OCP\ILogger; +use Psr\Log\LoggerInterface; /** * update encrypted files, e.g. because a file was shared */ class Update { - /** @var \OC\Files\View */ + /** @var View */ protected $view; - /** @var \OC\Encryption\Util */ + /** @var Util */ protected $util; /** @var \OC\Files\Mount\Manager */ protected $mountManager; - /** @var \OC\Encryption\Manager */ + /** @var Manager */ protected $encryptionManager; /** @var string */ protected $uid; - /** @var \OC\Encryption\File */ + /** @var File */ protected $file; + /** @var LoggerInterface */ + protected $logger; + /** - * - * @param \OC\Files\View $view - * @param \OC\Encryption\Util $util - * @param \OC\Files\Mount\Manager $mountManager - * @param \OC\Encryption\Manager $encryptionManager - * @param \OC\Encryption\File $file * @param string $uid */ public function __construct( @@ -68,6 +71,7 @@ class Update { Mount\Manager $mountManager, Manager $encryptionManager, File $file, + LoggerInterface $logger, $uid ) { $this->view = $view; @@ -75,6 +79,7 @@ class Update { $this->mountManager = $mountManager; $this->encryptionManager = $encryptionManager; $this->file = $file; + $this->logger = $logger; $this->uid = $uid; } @@ -155,7 +160,7 @@ class Update { $view = new View('/' . $owner . '/files'); $path = $view->getPath($info->getId()); if ($path === null) { - throw new \InvalidArgumentException('No file found for ' . $info->getId()); + throw new InvalidArgumentException('No file found for ' . $info->getId()); } return [$owner, $path]; @@ -187,7 +192,12 @@ class Update { foreach ($allFiles as $file) { $usersSharing = $this->file->getAccessList($file); - $encryptionModule->update($file, $this->uid, $usersSharing); + try { + $encryptionModule->update($file, $this->uid, $usersSharing); + } catch (GenericEncryptionException $e) { + // If the update of an individual file fails e.g. due to a corrupt key we should continue the operation and just log the failure + $this->logger->error('Failed to update encryption module for ' . $this->uid . ' ' . $file, [ 'exception' => $e ]); + } } } } diff --git a/tests/lib/Encryption/UpdateTest.php b/tests/lib/Encryption/UpdateTest.php index 08fb08bf79..ca1d549399 100644 --- a/tests/lib/Encryption/UpdateTest.php +++ b/tests/lib/Encryption/UpdateTest.php @@ -22,9 +22,13 @@ namespace Test\Encryption; use OC\Encryption\Update; +use OC\Encryption\Util; use OC\Files\Mount\Manager; use OC\Files\View; +use Psr\Log\LoggerInterface; use Test\TestCase; +use OC\Encryption\File; +use OCP\Encryption\IEncryptionModule; class UpdateTest extends TestCase { @@ -37,7 +41,7 @@ class UpdateTest extends TestCase { /** @var \OC\Files\View | \PHPUnit\Framework\MockObject\MockObject */ private $view; - /** @var \OC\Encryption\Util | \PHPUnit\Framework\MockObject\MockObject */ + /** @var Util | \PHPUnit\Framework\MockObject\MockObject */ private $util; /** @var \OC\Files\Mount\Manager | \PHPUnit\Framework\MockObject\MockObject */ @@ -52,21 +56,19 @@ class UpdateTest extends TestCase { /** @var \OC\Encryption\File | \PHPUnit\Framework\MockObject\MockObject */ private $fileHelper; + /** @var \PHPUnit\Framework\MockObject\MockObject|LoggerInterface */ + private $logger; + protected function setUp(): void { parent::setUp(); - $this->view = $this->getMockBuilder(View::class) - ->disableOriginalConstructor()->getMock(); - $this->util = $this->getMockBuilder('\OC\Encryption\Util') - ->disableOriginalConstructor()->getMock(); - $this->mountManager = $this->getMockBuilder(Manager::class) - ->disableOriginalConstructor()->getMock(); - $this->encryptionManager = $this->getMockBuilder('\OC\Encryption\Manager') - ->disableOriginalConstructor()->getMock(); - $this->fileHelper = $this->getMockBuilder('\OC\Encryption\File') - ->disableOriginalConstructor()->getMock(); - $this->encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule') - ->disableOriginalConstructor()->getMock(); + $this->view = $this->createMock(View::class); + $this->util = $this->createMock(Util::class); + $this->mountManager = $this->createMock(Manager::class); + $this->encryptionManager = $this->createMock(\OC\Encryption\Manager::class); + $this->fileHelper = $this->createMock(File::class); + $this->encryptionModule = $this->createMock(IEncryptionModule::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->uid = 'testUser1'; @@ -76,6 +78,7 @@ class UpdateTest extends TestCase { $this->mountManager, $this->encryptionManager, $this->fileHelper, + $this->logger, $this->uid); } @@ -223,6 +226,7 @@ class UpdateTest extends TestCase { $this->mountManager, $this->encryptionManager, $this->fileHelper, + $this->logger, $this->uid ] )->setMethods($methods)->getMock();