diff --git a/lib/private/Encryption/Keys/Storage.php b/lib/private/Encryption/Keys/Storage.php index 4299615d8f880..42a9d587650bf 100644 --- a/lib/private/Encryption/Keys/Storage.php +++ b/lib/private/Encryption/Keys/Storage.php @@ -12,6 +12,7 @@ use OC\Files\Filesystem; use OC\Files\View; use OC\ServerNotAvailableException; +use OCP\Cache\CappedMemoryCache; use OCP\Encryption\Keys\IStorage; use OCP\IConfig; use OCP\Security\ICrypto; @@ -26,7 +27,8 @@ class Storage implements IStorage { private string $root_dir; private string $encryption_base_dir; private string $backup_base_dir; - private array $keyCache = []; + /** @var CappedMemoryCache */ + private CappedMemoryCache $keyCache; public function __construct( private readonly View $view, @@ -34,6 +36,7 @@ public function __construct( private readonly ICrypto $crypto, private readonly IConfig $config, ) { + $this->keyCache = new CappedMemoryCache(); $this->encryption_base_dir = '/files_encryption'; $this->keys_base_dir = $this->encryption_base_dir . '/keys'; $this->backup_base_dir = $this->encryption_base_dir . '/backup'; @@ -120,6 +123,7 @@ public function setSystemUserKey($keyId, $key, $encryptionModuleId) { public function deleteUserKey($uid, $keyId, $encryptionModuleId) { try { $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, $uid); + $this->keyCache->remove($path); return !$this->view->file_exists($path) || $this->view->unlink($path); } catch (UserNotFoundException $e) { // this exception can come from initMountPoints() from setupUserMounts() @@ -141,6 +145,7 @@ public function deleteUserKey($uid, $keyId, $encryptionModuleId) { #[\Override] public function deleteFileKey($path, $keyId, $encryptionModuleId) { $keyDir = $this->util->getFileKeyDir($encryptionModuleId, $path); + $this->keyCache->remove($keyDir . $keyId); return !$this->view->file_exists($keyDir . $keyId) || $this->view->unlink($keyDir . $keyId); } @@ -150,6 +155,7 @@ public function deleteFileKey($path, $keyId, $encryptionModuleId) { #[\Override] public function deleteAllFileKeys($path) { $keyDir = $this->util->getFileKeyDir('', $path); + $this->clearCachedKeysBelow($keyDir); return !$this->view->file_exists($keyDir) || $this->view->deleteAll($keyDir); } @@ -159,9 +165,24 @@ public function deleteAllFileKeys($path) { #[\Override] public function deleteSystemUserKey($keyId, $encryptionModuleId) { $path = $this->constructUserKeyPath($encryptionModuleId, $keyId, null); + $this->keyCache->remove($path); return !$this->view->file_exists($path) || $this->view->unlink($path); } + /** + * Drop all cached keys stored inside the given key directory + * + * @param string $keyDir path to a key directory, with or without trailing slash + */ + private function clearCachedKeysBelow(string $keyDir): void { + $prefix = rtrim($keyDir, '/') . '/'; + foreach (array_keys($this->keyCache->getData()) as $cachedPath) { + if (str_starts_with((string)$cachedPath, $prefix)) { + $this->keyCache->remove($cachedPath); + } + } + } + /** * construct path to users key * @@ -231,8 +252,9 @@ private function getKey($path): array { ]; if ($this->view->file_exists($path)) { - if (isset($this->keyCache[$path])) { - $key = $this->keyCache[$path]; + $cachedKey = $this->keyCache->get($path); + if ($cachedKey !== null) { + $key = $cachedKey; } else { $data = $this->view->file_get_contents($path); @@ -282,7 +304,7 @@ private function getKey($path): array { } } - $this->keyCache[$path] = $key; + $this->keyCache->set($path, $key); } } @@ -313,7 +335,7 @@ private function setKey($path, $key) { $result = $this->view->file_put_contents($path, $data); if (is_int($result) && $result > 0) { - $this->keyCache[$path] = $key; + $this->keyCache->set($path, $key); return true; } @@ -334,6 +356,8 @@ public function renameKeys($source, $target) { if ($this->view->file_exists($sourcePath)) { $this->keySetPreparation(dirname($targetPath)); + $this->clearCachedKeysBelow($sourcePath); + $this->clearCachedKeysBelow($targetPath); $this->view->rename($sourcePath, $targetPath); return true; @@ -356,6 +380,7 @@ public function copyKeys($source, $target) { if ($this->view->file_exists($sourcePath)) { $this->keySetPreparation(dirname($targetPath)); + $this->clearCachedKeysBelow($targetPath); $this->view->copy($sourcePath, $targetPath); return true; } diff --git a/tests/lib/Encryption/Keys/StorageTest.php b/tests/lib/Encryption/Keys/StorageTest.php index 8b3ede87408e5..621e519f6363e 100644 --- a/tests/lib/Encryption/Keys/StorageTest.php +++ b/tests/lib/Encryption/Keys/StorageTest.php @@ -11,6 +11,7 @@ use OC\Encryption\Keys\Storage; use OC\Encryption\Util; use OC\Files\View; +use OCP\Cache\CappedMemoryCache; use OCP\IConfig; use OCP\Security\ICrypto; use PHPUnit\Framework\MockObject\MockObject; @@ -400,6 +401,172 @@ public function testDeleteFileKey(): void { ); } + /** + * Set up the mocks needed to read file keys for arbitrary paths + */ + private function mockFileKeyEnvironment(): void { + $this->config->method('getSystemValueString') + ->with('version') + ->willReturn('20.0.0.2'); + $this->config->method('getSystemValueBool') + ->willReturn(true); + $this->util->method('getUidAndFilename') + ->willReturnCallback([$this, 'getUidAndFilenameCallback']); + $this->util->method('stripPartialFileExtension') + ->willReturnArgument(0); + $this->util->method('isSystemWideMountPoint') + ->willReturn(false); + $this->view->method('file_exists') + ->willReturn(true); + $this->view->method('is_dir') + ->willReturn(true); + } + + /** + * Make the view return a dummy key for every path and collect the read paths + * + * @param string[] $reads + */ + private function trackKeyReads(array &$reads, ?string $uid = null): void { + $this->view->method('file_get_contents') + ->willReturnCallback(function (string $path) use (&$reads, $uid): string { + $reads[] = $path; + return json_encode(['key' => base64_encode('key'), 'uid' => $uid]); + }); + } + + public function testGetFileKeyIsCached(): void { + $this->mockFileKeyEnvironment(); + $reads = []; + $this->trackKeyReads($reads); + + $this->storage->getFileKey('/user1/files/foo.txt', 'fileKey', 'encModule'); + $this->storage->getFileKey('/user1/files/foo.txt', 'fileKey', 'encModule'); + + $this->assertSame(['/user1/files_encryption/keys/files/foo.txt/encModule/fileKey'], $reads); + } + + public function testKeyCacheIsCapped(): void { + $this->mockFileKeyEnvironment(); + $reads = []; + $this->trackKeyReads($reads); + + for ($i = 0; $i < 600; $i++) { + $this->storage->getFileKey('/user1/files/foo' . $i . '.txt', 'fileKey', 'encModule'); + } + + /** @var CappedMemoryCache $keyCache */ + $keyCache = self::invokePrivate($this->storage, 'keyCache'); + $this->assertCount(512, $keyCache->getData()); + } + + public function testDeleteFileKeyInvalidatesCache(): void { + $this->mockFileKeyEnvironment(); + $this->view->method('unlink')->willReturn(true); + $reads = []; + $this->trackKeyReads($reads); + + $this->storage->getFileKey('/user1/files/foo.txt', 'fileKey', 'encModule'); + $this->assertTrue($this->storage->deleteFileKey('/user1/files/foo.txt', 'fileKey', 'encModule')); + $reads = []; + + $this->storage->getFileKey('/user1/files/foo.txt', 'fileKey', 'encModule'); + + $this->assertSame(['/user1/files_encryption/keys/files/foo.txt/encModule/fileKey'], $reads); + } + + public function testDeleteAllFileKeysInvalidatesCache(): void { + $this->mockFileKeyEnvironment(); + $this->view->method('deleteAll')->willReturn(true); + $reads = []; + $this->trackKeyReads($reads); + + $this->storage->getFileKey('/user1/files/foo.txt', 'fileKey', 'encModule'); + $this->storage->getFileKey('/user1/files/foo.txt', 'otherKey', 'encModule'); + // a sibling sharing the name prefix must stay cached + $this->storage->getFileKey('/user1/files/foobar.txt', 'fileKey', 'encModule'); + $this->assertTrue($this->storage->deleteAllFileKeys('/user1/files/foo.txt')); + $reads = []; + + $this->storage->getFileKey('/user1/files/foo.txt', 'fileKey', 'encModule'); + $this->storage->getFileKey('/user1/files/foo.txt', 'otherKey', 'encModule'); + $this->storage->getFileKey('/user1/files/foobar.txt', 'fileKey', 'encModule'); + + $this->assertSame([ + '/user1/files_encryption/keys/files/foo.txt/encModule/fileKey', + '/user1/files_encryption/keys/files/foo.txt/encModule/otherKey', + ], $reads); + } + + public function testRenameKeysInvalidatesCache(): void { + $this->mockFileKeyEnvironment(); + $this->view->method('rename')->willReturn(true); + $reads = []; + $this->trackKeyReads($reads); + + $this->storage->getFileKey('/user1/files/source.txt', 'fileKey', 'encModule'); + $this->storage->getFileKey('/user1/files/target.txt', 'fileKey', 'encModule'); + $this->assertTrue($this->storage->renameKeys('/user1/files/source.txt', '/user1/files/target.txt')); + $reads = []; + + $this->storage->getFileKey('/user1/files/source.txt', 'fileKey', 'encModule'); + $this->storage->getFileKey('/user1/files/target.txt', 'fileKey', 'encModule'); + + $this->assertSame([ + '/user1/files_encryption/keys/files/source.txt/encModule/fileKey', + '/user1/files_encryption/keys/files/target.txt/encModule/fileKey', + ], $reads); + } + + public function testCopyKeysInvalidatesTargetCache(): void { + $this->mockFileKeyEnvironment(); + $this->view->method('copy')->willReturn(true); + $reads = []; + $this->trackKeyReads($reads); + + $this->storage->getFileKey('/user1/files/source.txt', 'fileKey', 'encModule'); + $this->storage->getFileKey('/user1/files/target.txt', 'fileKey', 'encModule'); + $this->assertTrue($this->storage->copyKeys('/user1/files/source.txt', '/user1/files/target.txt')); + $reads = []; + + $this->storage->getFileKey('/user1/files/source.txt', 'fileKey', 'encModule'); + $this->storage->getFileKey('/user1/files/target.txt', 'fileKey', 'encModule'); + + $this->assertSame([ + '/user1/files_encryption/keys/files/target.txt/encModule/fileKey', + ], $reads); + } + + public function testDeleteUserKeyInvalidatesCache(): void { + $this->mockFileKeyEnvironment(); + $this->view->method('unlink')->willReturn(true); + $reads = []; + $this->trackKeyReads($reads, 'user1'); + + $this->storage->getUserKey('user1', 'publicKey', 'encModule'); + $this->assertTrue($this->storage->deleteUserKey('user1', 'publicKey', 'encModule')); + $reads = []; + + $this->storage->getUserKey('user1', 'publicKey', 'encModule'); + + $this->assertSame(['/user1/files_encryption/encModule/user1.publicKey'], $reads); + } + + public function testDeleteSystemUserKeyInvalidatesCache(): void { + $this->mockFileKeyEnvironment(); + $this->view->method('unlink')->willReturn(true); + $reads = []; + $this->trackKeyReads($reads); + + $this->storage->getSystemUserKey('shareKey_56884', 'encModule'); + $this->assertTrue($this->storage->deleteSystemUserKey('shareKey_56884', 'encModule')); + $reads = []; + + $this->storage->getSystemUserKey('shareKey_56884', 'encModule'); + + $this->assertSame(['/files_encryption/encModule/shareKey_56884'], $reads); + } + #[\PHPUnit\Framework\Attributes\DataProvider('dataProviderCopyRename')] public function testRenameKeys($source, $target, $systemWideMountSource, $systemWideMountTarget, $expectedSource, $expectedTarget): void { $this->view->expects($this->any())