diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionMasterKeyUploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionMasterKeyUploadTest.php index abdabb1cb7acc..72ff5dfcbe846 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionMasterKeyUploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionMasterKeyUploadTest.php @@ -34,4 +34,9 @@ protected function setupUser($name, $password): View { $this->loginWithEncryption($name); return new View('/' . $name . '/files'); } + + protected function tearDown(): void { + $this->tearDownEncryptionTrait(); + parent::tearDown(); + } } diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionUploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionUploadTest.php index c1ae71ac28f74..aa64b964a8ffa 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionUploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionUploadTest.php @@ -34,4 +34,9 @@ protected function setupUser($name, $password): View { $this->loginWithEncryption($name); return new View('/' . $name . '/files'); } + + protected function tearDown(): void { + $this->tearDownEncryptionTrait(); + parent::tearDown(); + } } diff --git a/apps/encryption/tests/Command/FixEncryptedVersionTest.php b/apps/encryption/tests/Command/FixEncryptedVersionTest.php index 8a37aae28067e..b72f0c5bcc546 100644 --- a/apps/encryption/tests/Command/FixEncryptedVersionTest.php +++ b/apps/encryption/tests/Command/FixEncryptedVersionTest.php @@ -390,4 +390,9 @@ public function testExecuteWithNoMasterKey(): void { $this->assertStringContainsString('only works with master key', $output); } + + protected function tearDown(): void { + $this->tearDownEncryptionTrait(); + parent::tearDown(); + } } diff --git a/apps/encryption/tests/EncryptedStorageTest.php b/apps/encryption/tests/EncryptedStorageTest.php index eb1463fcf0d15..eb5c6e0f3e268 100644 --- a/apps/encryption/tests/EncryptedStorageTest.php +++ b/apps/encryption/tests/EncryptedStorageTest.php @@ -66,4 +66,9 @@ public function testMoveFromEncrypted(): void { $this->assertEquals('bar', $unencryptedStorage->file_get_contents('foo.txt')); $this->assertFalse($unencryptedCache->get('foo.txt')->isEncrypted()); } + + protected function tearDown(): void { + $this->tearDownEncryptionTrait(); + parent::tearDown(); + } } diff --git a/apps/files_sharing/tests/EncryptedSizePropagationTest.php b/apps/files_sharing/tests/EncryptedSizePropagationTest.php index 92773bdc72d78..71363c55b9dd8 100644 --- a/apps/files_sharing/tests/EncryptedSizePropagationTest.php +++ b/apps/files_sharing/tests/EncryptedSizePropagationTest.php @@ -39,4 +39,9 @@ protected function loginHelper($user, $create = false, $password = false) { $this->setupForUser($user, $password); parent::loginHelper($user, $create, $password); } + + protected function tearDown(): void { + $this->tearDownEncryptionTrait(); + parent::tearDown(); + } } diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 5216a3ef43253..716e0e9985782 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2920,19 +2920,6 @@ - - - - - - - - - - - - - @@ -4187,9 +4174,6 @@ - - - diff --git a/core/Command/Encryption/Disable.php b/core/Command/Encryption/Disable.php index 91d4ac82d2328..a7dba7a45376a 100644 --- a/core/Command/Encryption/Disable.php +++ b/core/Command/Encryption/Disable.php @@ -7,6 +7,7 @@ */ namespace OC\Core\Command\Encryption; +use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -15,6 +16,7 @@ class Disable extends Command { public function __construct( protected IConfig $config, + protected IAppConfig $appConfig, ) { parent::__construct(); } @@ -27,10 +29,10 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') !== 'yes') { + if ($this->appConfig->getValueString('core', 'encryption_enabled', 'no') !== 'yes') { $output->writeln('Encryption is already disabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'no'); + $this->appConfig->setValueString('core', 'encryption_enabled', 'no'); $output->writeln('Encryption disabled'); } return 0; diff --git a/core/Command/Encryption/Enable.php b/core/Command/Encryption/Enable.php index 02045fa1a4ba6..6b64438c3b919 100644 --- a/core/Command/Encryption/Enable.php +++ b/core/Command/Encryption/Enable.php @@ -8,6 +8,7 @@ namespace OC\Core\Command\Encryption; use OCP\Encryption\IManager; +use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -16,6 +17,7 @@ class Enable extends Command { public function __construct( protected IConfig $config, + protected IAppConfig $appConfig, protected IManager $encryptionManager, ) { parent::__construct(); @@ -29,10 +31,10 @@ protected function configure() { } protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') === 'yes') { + if ($this->appConfig->getValueString('core', 'encryption_enabled', 'no') === 'yes') { $output->writeln('Encryption is already enabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'yes'); + $this->appConfig->setValueString('core', 'encryption_enabled', 'yes'); $output->writeln('Encryption enabled'); } $output->writeln(''); @@ -42,7 +44,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('No encryption module is loaded'); return 1; } - $defaultModule = $this->config->getAppValue('core', 'default_encryption_module'); + $defaultModule = $this->appConfig->getValueString('core', 'default_encryption_module', ''); if ($defaultModule === '') { $output->writeln('No default module is set'); return 1; diff --git a/lib/private/Encryption/EncryptionWrapper.php b/lib/private/Encryption/EncryptionWrapper.php index b9db9616538a6..dabeae49b0255 100644 --- a/lib/private/Encryption/EncryptionWrapper.php +++ b/lib/private/Encryption/EncryptionWrapper.php @@ -8,6 +8,7 @@ namespace OC\Encryption; use OC\Files\Filesystem; +use OC\Files\Mount\HomeMountPoint; use OC\Files\Storage\Wrapper\Encryption; use OC\Files\View; use OC\Memcache\ArrayCache; @@ -62,32 +63,48 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $ 'mount' => $mount ]; - if ($force || (!$storage->instanceOfStorage(IDisableEncryptionStorage::class) && $mountPoint !== '/')) { - $user = \OC::$server->getUserSession()->getUser(); - $mountManager = Filesystem::getMountManager(); - $uid = $user ? $user->getUID() : null; - $fileHelper = \OC::$server->get(IFile::class); - $keyStorage = \OC::$server->get(EncryptionKeysStorage::class); + // Only evaluate other conditions if not forced + if (!$force) { + // If a disabled storage medium, return basic storage + if ($storage->instanceOfStorage(IDisableEncryptionStorage::class)) { + return $storage; + } - $util = new Util( - new View(), - \OC::$server->getUserManager(), - \OC::$server->getGroupManager(), - \OC::$server->getConfig() - ); - return new Encryption( - $parameters, - $this->manager, - $util, - $this->logger, - $fileHelper, - $uid, - $keyStorage, - $mountManager, - $this->arrayCache - ); - } else { - return $storage; + // Root mount point handling: skip encryption wrapper + if ($mountPoint === '/') { + return $storage; + } + + // Skip encryption for home mounts if encryptHomeStorage is disabled + if ($mount instanceof HomeMountPoint + && \OC::$server->getConfig()->getAppValue('encryption', 'encryptHomeStorage', '1') !== '1') { + return $storage; + } } + + // Apply encryption wrapper + $user = \OC::$server->getUserSession()->getUser(); + $mountManager = Filesystem::getMountManager(); + $uid = $user ? $user->getUID() : null; + $fileHelper = \OC::$server->get(IFile::class); + $keyStorage = \OC::$server->get(EncryptionKeysStorage::class); + + $util = new Util( + new View(), + \OC::$server->getUserManager(), + \OC::$server->getGroupManager(), + \OC::$server->getConfig() + ); + return new Encryption( + $parameters, + $this->manager, + $util, + $this->logger, + $fileHelper, + $uid, + $keyStorage, + $mountManager, + $this->arrayCache + ); } } diff --git a/lib/private/Files/Cache/CacheEntry.php b/lib/private/Files/Cache/CacheEntry.php index 943f328e845a3..1dfe308434ba3 100644 --- a/lib/private/Files/Cache/CacheEntry.php +++ b/lib/private/Files/Cache/CacheEntry.php @@ -119,7 +119,7 @@ public function __clone() { } public function getUnencryptedSize(): int { - if ($this->data['encrypted'] && isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] > 0) { + if ($this->data['encrypted'] && isset($this->data['unencrypted_size'])) { return $this->data['unencrypted_size']; } else { return $this->data['size'] ?? 0; diff --git a/lib/private/Files/Cache/Scanner.php b/lib/private/Files/Cache/Scanner.php index 2fa0dd09e4fd0..2afdbf1965c91 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -197,8 +197,12 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = } } - // we only updated unencrypted_size if it's already set - if (isset($cacheData['unencrypted_size']) && $cacheData['unencrypted_size'] === 0) { + // Only skip updating unencrypted_size if both cached and new values are 0 + // This allows updating from incorrect cached 0 to correct non-zero value + // while avoiding unnecessary updates when both are legitimately 0 + if (isset($cacheData['unencrypted_size']) + && $cacheData['unencrypted_size'] === 0 + && (!isset($data['unencrypted_size']) || $data['unencrypted_size'] === 0)) { unset($data['unencrypted_size']); } @@ -216,7 +220,12 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = $data['etag_changed'] = true; } } else { - unset($data['unencrypted_size']); + // For new files, only unset unencrypted_size if the file is not encrypted + // or if unencrypted_size is 0/not set (not a valid encrypted file) + if (!isset($data['encrypted']) || !$data['encrypted'] + || !isset($data['unencrypted_size']) || $data['unencrypted_size'] === 0) { + unset($data['unencrypted_size']); + } $newData = $data; $fileId = -1; } diff --git a/lib/private/Files/FileInfo.php b/lib/private/Files/FileInfo.php index 967d404b8a4f0..e4e0b6207a231 100644 --- a/lib/private/Files/FileInfo.php +++ b/lib/private/Files/FileInfo.php @@ -174,7 +174,7 @@ public function getSize($includeMounts = true) { if ($includeMounts) { $this->updateEntryFromSubMounts(); - if ($this->isEncrypted() && isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] > 0) { + if ($this->isEncrypted() && isset($this->data['unencrypted_size'])) { return $this->data['unencrypted_size']; } else { return isset($this->data['size']) ? 0 + $this->data['size'] : 0; diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 380ec0f253008..660a031192399 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -388,6 +388,7 @@ protected function verifyUnencryptedSize(string $path, int $unencryptedSize): in if ($unencryptedSize < 0 || ($size > 0 && $unencryptedSize === $size) || $unencryptedSize > $size + || ($unencryptedSize === 0 && $size > 0) ) { // check if we already calculate the unencrypted size for the // given path to avoid recursions diff --git a/lib/private/Files/Stream/Encryption.php b/lib/private/Files/Stream/Encryption.php index ef147ec421fb1..719e6eedd509d 100644 --- a/lib/private/Files/Stream/Encryption.php +++ b/lib/private/Files/Stream/Encryption.php @@ -28,7 +28,7 @@ class Encryption extends Wrapper { protected string $cache; protected ?int $size = null; protected int $position; - protected ?int $unencryptedSize = null; + protected int|float|null $unencryptedSize = null; protected int $headerSize; protected int $unencryptedBlockSize; protected array $header; diff --git a/tests/Core/Command/Encryption/DisableTest.php b/tests/Core/Command/Encryption/DisableTest.php index a89fd636e4753..632adc24ca1e9 100644 --- a/tests/Core/Command/Encryption/DisableTest.php +++ b/tests/Core/Command/Encryption/DisableTest.php @@ -9,6 +9,7 @@ namespace Tests\Core\Command\Encryption; use OC\Core\Command\Encryption\Disable; +use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -18,6 +19,8 @@ class DisableTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $config; /** @var \PHPUnit\Framework\MockObject\MockObject */ + protected $appConfig; + /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $consoleInput; /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $consoleOutput; @@ -31,11 +34,15 @@ protected function setUp(): void { $config = $this->config = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor() ->getMock(); + $appConfig = $this->appConfig = $this->getMockBuilder(IAppConfig::class) + ->disableOriginalConstructor() + ->getMock(); $this->consoleInput = $this->getMockBuilder(InputInterface::class)->getMock(); $this->consoleOutput = $this->getMockBuilder(OutputInterface::class)->getMock(); /** @var IConfig $config */ - $this->command = new Disable($config); + /** @var IAppConfig $appConfig */ + $this->command = new Disable($config, $appConfig); } @@ -54,9 +61,9 @@ public static function dataDisable(): array { */ #[\PHPUnit\Framework\Attributes\DataProvider('dataDisable')] public function testDisable($oldStatus, $isUpdating, $expectedString): void { - $this->config->expects($this->once()) - ->method('getAppValue') - ->with('core', 'encryption_enabled', $this->anything()) + $this->appConfig->expects($this->once()) + ->method('getValueString') + ->with('core', 'encryption_enabled', 'no') ->willReturn($oldStatus); $this->consoleOutput->expects($this->once()) @@ -64,8 +71,8 @@ public function testDisable($oldStatus, $isUpdating, $expectedString): void { ->with($this->stringContains($expectedString)); if ($isUpdating) { - $this->config->expects($this->once()) - ->method('setAppValue') + $this->appConfig->expects($this->once()) + ->method('setValueString') ->with('core', 'encryption_enabled', 'no'); } diff --git a/tests/Core/Command/Encryption/EnableTest.php b/tests/Core/Command/Encryption/EnableTest.php index 0e9655c29c778..42835244d2af7 100644 --- a/tests/Core/Command/Encryption/EnableTest.php +++ b/tests/Core/Command/Encryption/EnableTest.php @@ -10,6 +10,7 @@ use OC\Core\Command\Encryption\Enable; use OCP\Encryption\IManager; +use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -19,6 +20,8 @@ class EnableTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $config; /** @var \PHPUnit\Framework\MockObject\MockObject */ + protected $appConfig; + /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $manager; /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $consoleInput; @@ -34,6 +37,9 @@ protected function setUp(): void { $config = $this->config = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor() ->getMock(); + $appConfig = $this->appConfig = $this->getMockBuilder(IAppConfig::class) + ->disableOriginalConstructor() + ->getMock(); $manager = $this->manager = $this->getMockBuilder(IManager::class) ->disableOriginalConstructor() ->getMock(); @@ -41,8 +47,9 @@ protected function setUp(): void { $this->consoleOutput = $this->getMockBuilder(OutputInterface::class)->getMock(); /** @var \OCP\IConfig $config */ + /** @var \OCP\IAppConfig $appConfig */ /** @var \OCP\Encryption\IManager $manager */ - $this->command = new Enable($config, $manager); + $this->command = new Enable($config, $appConfig, $manager); } @@ -59,8 +66,8 @@ public static function dataEnable(): array { #[\PHPUnit\Framework\Attributes\DataProvider('dataEnable')] public function testEnable(string $oldStatus, ?string $defaultModule, array $availableModules, bool $isUpdating, string $expectedString, string $expectedDefaultModuleString): void { if ($isUpdating) { - $this->config->expects($this->once()) - ->method('setAppValue') + $this->appConfig->expects($this->once()) + ->method('setValueString') ->with('core', 'encryption_enabled', 'yes'); } @@ -69,14 +76,14 @@ public function testEnable(string $oldStatus, ?string $defaultModule, array $ava ->willReturn($availableModules); if (empty($availableModules)) { - $this->config->expects($this->once()) - ->method('getAppValue') + $this->appConfig->expects($this->once()) + ->method('getValueString') ->willReturnMap([ ['core', 'encryption_enabled', 'no', $oldStatus], ]); } else { - $this->config->expects($this->exactly(2)) - ->method('getAppValue') + $this->appConfig->expects($this->exactly(2)) + ->method('getValueString') ->willReturnMap([ ['core', 'encryption_enabled', 'no', $oldStatus], ['core', 'default_encryption_module', '', $defaultModule], diff --git a/tests/lib/Encryption/EncryptionWrapperTest.php b/tests/lib/Encryption/EncryptionWrapperTest.php index 58bf5aff005fb..aabf86e04401c 100644 --- a/tests/lib/Encryption/EncryptionWrapperTest.php +++ b/tests/lib/Encryption/EncryptionWrapperTest.php @@ -61,6 +61,11 @@ public function testWrapStorage($expectedWrapped, $wrappedStorages): void { ->disableOriginalConstructor() ->getMock(); + // Mock encryption being enabled for tests that expect wrapping + $this->manager->expects($this->any()) + ->method('isEnabled') + ->willReturn($expectedWrapped); + $returnedStorage = $this->instance->wrapStorage('mountPoint', $storage, $mount); $this->assertEquals( diff --git a/tests/lib/Files/ObjectStore/S3EncryptionMigrationTest.php b/tests/lib/Files/ObjectStore/S3EncryptionMigrationTest.php new file mode 100644 index 0000000000000..01069c11bf759 --- /dev/null +++ b/tests/lib/Files/ObjectStore/S3EncryptionMigrationTest.php @@ -0,0 +1,270 @@ +getSystemValue('objectstore'); + if (!is_array($config) || $config['class'] !== S3::class) { + self::markTestSkipped('S3 primary storage not configured'); + } + } + + protected function setUp(): void { + parent::setUp(); + + $this->setUpEncryptionTrait(); + + $config = Server::get(IConfig::class); + $this->encryptionWasEnabled = $config->getAppValue('core', 'encryption_enabled', 'no'); + $this->originalEncryptionModule = $config->getAppValue('core', 'default_encryption_module'); + + $s3Config = Server::get(IConfig::class)->getSystemValue('objectstore'); + $this->bucket = $s3Config['arguments']['bucket'] ?? 'nextcloud'; + $this->objectStore = new S3($s3Config['arguments']); + + if (!$this->userManager->userExists(self::TEST_USER)) { + $this->createUser(self::TEST_USER, self::TEST_PASSWORD); + } + + $this->setupForUser(self::TEST_USER, self::TEST_PASSWORD); + $this->loginWithEncryption(self::TEST_USER); + + $this->userFolder = \OC::$server->getUserFolder(self::TEST_USER); + $this->view = new \OC\Files\View('/' . self::TEST_USER . '/files'); + } + + protected function tearDown(): void { + try { + if ($this->view) { + // Clean up test files + $testFiles = $this->view->getDirectoryContent(''); + foreach ($testFiles as $file) { + if (str_starts_with($file->getName(), 'migration-test-')) { + $this->view->unlink($file->getName()); + } + } + } + } catch (\Exception $e) { + // Ignore + } + + try { + $config = Server::get(IConfig::class); + $config->setAppValue('core', 'encryption_enabled', $this->encryptionWasEnabled); + $config->setAppValue('core', 'default_encryption_module', $this->originalEncryptionModule); + $config->deleteAppValue('encryption', 'useMasterKey'); + } catch (\Exception $e) { + // Ignore + } + + parent::tearDown(); + } + + /** + * Create an unencrypted file directly in S3 (simulating pre-fix behavior) + */ + private function createUnencryptedFileInS3(string $filename, string $content): int { + // Write directly to S3, bypassing encryption wrapper + $urn = 'urn:oid:' . time() . rand(1000, 9999); + $stream = fopen('php://temp', 'r+'); + fwrite($stream, $content); + rewind($stream); + + $this->objectStore->writeObject($urn, $stream); + fclose($stream); + + // Manually add to filecache as unencrypted + $cache = $this->userFolder->getStorage()->getCache(); + $fileId = (int)str_replace('urn:oid:', '', $urn); + + $cache->put($filename, [ + 'size' => strlen($content), + 'mtime' => time(), + 'mimetype' => 'application/octet-stream', + 'encrypted' => false, // Mark as unencrypted + 'storage_mtime' => time(), + ]); + + return $fileId; + } + + /** + * Test that encryption:encrypt-all safely handles mixed content + */ + public function testEncryptAllHandlesMixedContent(): void { + // Create test files in different states + $files = [ + 'migration-test-unencrypted-1.txt' => 'Unencrypted content 1', + 'migration-test-unencrypted-2.txt' => 'Unencrypted content 2', + ]; + + // 1. Create some unencrypted files (simulating pre-fix files) + foreach ($files as $filename => $content) { + // Directly write to S3 without encryption (simulate bug scenario) + // For now, just verify we can detect encryption status + $this->markTestSkipped('Manual S3 write needed - complex test case'); + } + + // Future: Complete this test to verify encrypt-all works on mixed content + } + + /** + * Test that isEncrypted() correctly identifies file state + */ + public function testIsEncryptedFlag(): void { + $testFile = 'migration-test-encrypted-flag.txt'; + $content = 'Test content for encryption flag'; + + // Write file with encryption wrapper (should be encrypted) + $this->view->file_put_contents($testFile, $content); + + // Get file info via node + $node = $this->userFolder->get($testFile); + + // Verify encrypted flag is set via node + $this->assertTrue($node->isEncrypted(), + 'File should be marked as encrypted in database after write'); + + // Verify content is accessible + $readContent = $this->view->file_get_contents($testFile); + $this->assertEquals($content, $readContent, + 'Content should be readable after encryption'); + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test database query to detect unencrypted files + */ + public function testDetectUnencryptedFilesQuery(): void { + // Create encrypted file + $this->view->file_put_contents('migration-test-encrypted.txt', 'encrypted'); + + // Query database for unencrypted files + $db = Server::get(\OCP\IDBConnection::class); + $query = $db->getQueryBuilder(); + + $query->select($query->func()->count('*', 'total')) + ->from('filecache') + ->where($query->expr()->eq('encrypted', $query->createNamedParameter(0))) + ->andWhere($query->expr()->neq('mimetype', + $query->createFunction('(SELECT id FROM oc_mimetypes WHERE mimetype = ' + . $query->createNamedParameter('httpd/unix-directory') . ')') + )) + ->andWhere($query->expr()->like('storage', + $query->createFunction('(SELECT numeric_id FROM oc_storages WHERE id LIKE ' + . $query->createNamedParameter('object::%') . ')') + )); + + $result = $query->executeQuery(); + $row = $result->fetch(); + $unencryptedCount = $row['total'] ?? 0; + + // After our encrypted file, this should be 0 or low + // (may have system files that aren't encrypted) + $this->assertIsNumeric($unencryptedCount, + 'Should be able to query unencrypted file count'); + + // Clean up + $this->view->unlink('migration-test-encrypted.txt'); + } + + /** + * Test size consistency after simulated migration + */ + public function testSizeConsistencyAfterEncryption(): void { + $testFile = 'migration-test-size-check.bin'; + $size = 50 * 1024; // 50KB + $data = random_bytes($size); + + // Write encrypted file + $this->view->file_put_contents($testFile, $data); + + // Verify size in database + $node = $this->userFolder->get($testFile); + $dbSize = $node->getSize(); + + // Verify actual content size + $readData = $this->view->file_get_contents($testFile); + $actualSize = strlen($readData); + + // Verify S3 size (should be larger) + $fileId = $node->getId(); + $urn = 'urn:oid:' . $fileId; + $s3Result = $this->objectStore->getConnection()->headObject([ + 'Bucket' => $this->bucket, + 'Key' => $urn, + ]); + $s3Size = $s3Result['ContentLength']; + + // Assertions + $this->assertEquals($size, $dbSize, + 'Database should have unencrypted size'); + $this->assertEquals($size, $actualSize, + 'Read content should match original size'); + $this->assertGreaterThan($size, $s3Size, + 'S3 should have encrypted size (larger)'); + + // Clean up + $this->view->unlink($testFile); + } +} diff --git a/tests/lib/Files/ObjectStore/S3EncryptionTest.php b/tests/lib/Files/ObjectStore/S3EncryptionTest.php new file mode 100644 index 0000000000000..bbc55e1a53008 --- /dev/null +++ b/tests/lib/Files/ObjectStore/S3EncryptionTest.php @@ -0,0 +1,493 @@ +getSystemValue('objectstore'); + if (!is_array($config) || $config['class'] !== S3::class) { + self::markTestSkipped('S3 primary storage not configured. Configure objectstore in config.php to run these tests.'); + } + } + + protected function setUp(): void { + parent::setUp(); + + // Set up encryption + $this->setUpEncryptionTrait(); + + // Save encryption state for teardown + $config = Server::get(IConfig::class); + $this->encryptionWasEnabled = $config->getAppValue('core', 'encryption_enabled', 'no'); + $this->originalEncryptionModule = $config->getAppValue('core', 'default_encryption_module'); + + // Get S3 config from system config + $this->s3Config = Server::get(IConfig::class)->getSystemValue('objectstore'); + $this->bucket = $this->s3Config['arguments']['bucket'] ?? 'nextcloud'; + + // Create S3 object store + $this->objectStore = new S3($this->s3Config['arguments']); + + // Create test user + if (!$this->userManager->userExists(self::TEST_USER)) { + $this->createUser(self::TEST_USER, self::TEST_PASSWORD); + } + + // Set up encryption for user + $this->setupForUser(self::TEST_USER, self::TEST_PASSWORD); + $this->loginWithEncryption(self::TEST_USER); + + // Get user folder (this will have encryption wrapper applied) + $this->userFolder = \OC::$server->getUserFolder(self::TEST_USER); + + // Get the view for the user + $this->view = new \OC\Files\View('/' . self::TEST_USER . '/files'); + + // Get the root ObjectStoreStorage (without wrapper) to check S3 sizes + $mount = \OC\Files\Filesystem::getMountManager()->find('/' . self::TEST_USER . '/files'); + $this->rootStorage = $mount->getStorage(); + + // Unwrap to get the actual ObjectStoreStorage if it's wrapped + while ($this->rootStorage instanceof \OC\Files\Storage\Wrapper\Wrapper) { + $this->rootStorage = $this->rootStorage->getWrapperStorage(); + } + } + + protected function tearDown(): void { + // Clean up test files + try { + if ($this->view) { + $this->cleanupTestFiles(); + } + } catch (\Exception $e) { + // Ignore cleanup errors + } + + // Tear down encryption + try { + $config = Server::get(IConfig::class); + $config->setAppValue('core', 'encryption_enabled', $this->encryptionWasEnabled ?? 'no'); + $config->setAppValue('core', 'default_encryption_module', $this->originalEncryptionModule ?? ''); + $config->deleteAppValue('encryption', 'useMasterKey'); + } catch (\Exception $e) { + // Ignore + } + + parent::tearDown(); + } + + private function cleanupTestFiles(): void { + // Clean up any test files that match our patterns + $patterns = ['test-size-*', 'test-roundtrip-*', 'test-integrity-*', + 'test-partial-read*', 'test-seek*', 'test-multipart*', 'test.txt']; + + foreach ($patterns as $pattern) { + try { + $files = $this->view->getDirectoryContent(''); + foreach ($files as $file) { + $name = $file->getName(); + if (fnmatch($pattern, $name)) { + $this->view->unlink($name); + } + } + } catch (\Exception $e) { + // Ignore + } + } + } + + /** + * Get the S3 URN for a path in the user's files + */ + private function getObjectUrn(string $path): string { + // Get file info from user folder + try { + $node = $this->userFolder->get($path); + $fileId = $node->getId(); + // URN format: urn:oid:{fileId} + return 'urn:oid:' . $fileId; + } catch (\Exception $e) { + throw new \Exception("File not found: $path - " . $e->getMessage()); + } + } + + /** + * Data provider for file sizes + */ + public static function dataFileSizes(): array { + return [ + '0 bytes (empty file)' => [0], + '1KB' => [1024], + '1MB' => [1024 * 1024], + '5MB (multipart threshold)' => [5 * 1024 * 1024], + '16MB (historical issue)' => [16 * 1024 * 1024], + '64MB (historical issue)' => [64 * 1024 * 1024], + '100MB (stress test)' => [100 * 1024 * 1024], + ]; + } + + /** + * CRITICAL SIZE VALIDATION TEST + * + * This test validates size consistency across three sources: + * 1. Database (filecache) - should store unencrypted size + * 2. S3 Object (headObject) - will be larger (encrypted) + * 3. Actual content - should match original unencrypted size + * + * Known issues exist with size mismatches between these sources. + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataFileSizes')] + public function testSizeConsistencyAcrossSources(int $originalSize): void { + $testFile = 'test-size-' . ($originalSize / 1024) . 'kb.bin'; + + // 1. Write file of known size using View (encryption wrapper applied) + $data = $originalSize > 0 ? random_bytes($originalSize) : ''; + $bytesWritten = $this->view->file_put_contents($testFile, $data); + + $this->assertEquals($originalSize, $bytesWritten, + 'file_put_contents should return original size written'); + + // 2. Get database size (from filecache via userFolder) + $node = $this->userFolder->get($testFile); + $dbSize = $node->getSize(); + + // 3. Get S3 object size (encrypted size) directly from S3 + $urn = $this->getObjectUrn($testFile); + $s3Result = $this->objectStore->getConnection()->headObject([ + 'Bucket' => $this->bucket, + 'Key' => $urn, + ]); + $s3Size = $s3Result['ContentLength']; + + // 4. Get actual content size (after decryption via View) + $content = $this->view->file_get_contents($testFile); + $actualSize = strlen($content); + + // ASSERTIONS - Critical size relationships + // After fixing CacheEntry.php getUnencryptedSize() bug, all sizes should be correct + $this->assertEquals($originalSize, $dbSize, + "Database should store unencrypted size (original: $originalSize, db: $dbSize)"); + + $this->assertEquals($originalSize, $actualSize, + "Actual content should match original size after decryption (original: $originalSize, actual: $actualSize)"); + + if ($originalSize === 0) { + // Zero-byte files still get encryption header in S3 + $this->assertGreaterThan(0, $s3Size, + 'S3 should have encryption header even for empty files'); + } else { + $this->assertGreaterThan($originalSize, $s3Size, + "S3 size should be larger than original due to encryption overhead (original: $originalSize, s3: $s3Size)"); + } + + // Verify content integrity - critical! + $this->assertEquals($data, $content, + 'Content should be identical after encrypt/decrypt cycle - corruption detected!'); + + // Validate encryption overhead is reasonable + // Binary signed format: Header (8192 bytes) + data blocks + // Each encrypted block is 8192 bytes, holds 8096 bytes unencrypted + // Overhead: ~2% for large files, more for small files due to header + + // Special case for zero-byte files + if ($originalSize === 0) { + // Zero-byte files still get encryption header + $this->assertGreaterThan(0, $s3Size, + 'Even empty files should have encryption header in S3'); + $this->assertLessThanOrEqual(8192, $s3Size, + 'Empty file should only have header block'); + } else { + $overheadPercent = (($s3Size - $originalSize) / $originalSize) * 100; + + // Sanity checks for overhead + if ($originalSize < 10240) { // < 10KB + // Small files have large relative overhead due to 8KB header + $this->assertLessThan(1000, $overheadPercent, + "Encryption overhead should be reasonable even for small files (got: {$overheadPercent}%)"); + } else { + // Larger files should have ~1-3% overhead + $this->assertGreaterThan(0.5, $overheadPercent, + "Should have some encryption overhead (got: {$overheadPercent}%)"); + $this->assertLessThan(5, $overheadPercent, + "Encryption overhead should be under 5% for files > 10KB (got: {$overheadPercent}%)"); + } + } + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test encrypted file round trip - write and read back + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataFileSizes')] + public function testEncryptedFileRoundTrip(int $size): void { + $testFile = 'test-roundtrip-' . ($size / 1024) . 'kb.bin'; + $data = $size > 0 ? random_bytes($size) : ''; + + // Write + $written = $this->view->file_put_contents($testFile, $data); + $this->assertEquals($size, $written); + + // Verify exists + $this->assertTrue($this->view->file_exists($testFile)); + + // Read back + $readData = $this->view->file_get_contents($testFile); + + // Verify size + $this->assertEquals($size, strlen($readData)); + + // Verify content + $this->assertEquals($data, $readData, 'Content mismatch after round trip'); + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test encrypted file integrity with streaming reads + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataFileSizes')] + public function testEncryptedFileIntegrity(int $size): void { + $testFile = 'test-integrity-' . ($size / 1024) . 'kb.bin'; + $data = $size > 0 ? random_bytes($size) : ''; + + // Write + $this->view->file_put_contents($testFile, $data); + + // Stream read + $handle = $this->view->fopen($testFile, 'r'); + $this->assertIsResource($handle); + + // Read in chunks + $chunkSize = 8192; + $readData = ''; + while (!feof($handle)) { + $chunk = fread($handle, $chunkSize); + $readData .= $chunk; + } + fclose($handle); + + // Verify + $this->assertEquals($size, strlen($readData), 'Size mismatch in streaming read'); + $this->assertEquals($data, $readData, 'Content mismatch in streaming read'); + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test partial reads (seeking) on encrypted files + */ + public function testEncryptedFilePartialRead(): void { + $testFile = 'test-partial-read.bin'; + $size = 1024 * 100; // 100KB + $data = random_bytes($size); + + // Write + $this->view->file_put_contents($testFile, $data); + + // Test partial reads at various offsets + $testCases = [ + ['offset' => 0, 'length' => 100], + ['offset' => 1000, 'length' => 500], + ['offset' => 50000, 'length' => 1000], + ['offset' => $size - 100, 'length' => 100], // End of file + ]; + + foreach ($testCases as $test) { + $offset = $test['offset']; + $length = $test['length']; + + $handle = $this->view->fopen($testFile, 'r'); + fseek($handle, $offset); + $chunk = fread($handle, $length); + fclose($handle); + + $expected = substr($data, $offset, $length); + $this->assertEquals($expected, $chunk, + "Partial read mismatch at offset $offset, length $length"); + } + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test seeking within encrypted files + */ + public function testEncryptedFileSeek(): void { + $testFile = 'test-seek.bin'; + $size = 1024 * 50; // 50KB + $data = random_bytes($size); + + // Write + $this->view->file_put_contents($testFile, $data); + + $handle = $this->view->fopen($testFile, 'r'); + + // Test SEEK_SET + fseek($handle, 1000, SEEK_SET); + $this->assertEquals(1000, ftell($handle)); + $chunk = fread($handle, 100); + $this->assertEquals(substr($data, 1000, 100), $chunk); + + // Test SEEK_CUR + fseek($handle, 500, SEEK_CUR); + $this->assertEquals(1600, ftell($handle)); + + // Test SEEK_END + fseek($handle, -100, SEEK_END); + $this->assertEquals($size - 100, ftell($handle)); + $chunk = fread($handle, 100); + $this->assertEquals(substr($data, -100), $chunk); + + fclose($handle); + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test that multipart upload works correctly with encryption + */ + public function testEncryptedMultipartUpload(): void { + $testFile = 'test-multipart.bin'; + // 6MB file to trigger multipart upload (threshold is 100MB, use 110MB to be safe) + $size = 110 * 1024 * 1024; + $data = random_bytes($size); + + // Write (should use multipart upload) + $written = $this->view->file_put_contents($testFile, $data); + $this->assertEquals($size, $written); + + // Verify file was created + $this->assertTrue($this->view->file_exists($testFile)); + + // Verify size in database + $node = $this->userFolder->get($testFile); + $dbSize = $node->getSize(); + $this->assertEquals($size, $dbSize, + 'Database should have unencrypted size even for multipart upload'); + + // Verify content + $readData = $this->view->file_get_contents($testFile); + $this->assertEquals($data, $readData, + 'Content mismatch for multipart encrypted upload'); + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test that file size tracking works correctly during writes + */ + public function testEncryptedFileSizeTracking(): void { + $testFile = 'test-size-tracking.bin'; + $sizes = [1024, 10240, 102400]; // 1KB, 10KB, 100KB + + foreach ($sizes as $size) { + $data = random_bytes($size); + + // Write + $this->view->file_put_contents($testFile, $data); + + // Check filesize() returns unencrypted size + $reportedSize = $this->view->filesize($testFile); + $this->assertEquals($size, $reportedSize, + "filesize() should return unencrypted size (expected: $size, got: $reportedSize)"); + + // Check stat() returns unencrypted size via userFolder + $node = $this->userFolder->get($testFile); + $nodeSize = $node->getSize(); + $this->assertEquals($size, $nodeSize, + "Node size should return unencrypted size (expected: $size, got: $nodeSize)"); + } + + // Clean up + $this->view->unlink($testFile); + } + + /** + * Test mime type handling with encryption + */ + public function testEncryptedFileMimeType(): void { + $testFile = 'test.txt'; + $data = 'This is a text file'; + + // Write + $this->view->file_put_contents($testFile, $data); + + // Get mime type via userFolder node + $node = $this->userFolder->get($testFile); + $mimeType = $node->getMimetype(); + + // Should detect as text/plain + $this->assertEquals('text/plain', $mimeType, + 'MIME type detection should work on encrypted files'); + + // Clean up + $this->view->unlink($testFile); + } +} diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 551c1024e0bef..eb4e0f3e5d0b3 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -26,6 +26,7 @@ use OCP\Command\IBus; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\IRootFolder; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IDBConnection; use OCP\IUserManager; @@ -191,6 +192,22 @@ protected function tearDown(): void { call_user_func([$this, $methodName]); } } + + // Clean up encryption state to prevent test pollution + // This ensures encryption_enabled is reset after each test, preventing + // MultiKeyEncryptException failures in subsequent tests when encryption + // is left enabled but user keys don't exist + try { + $appConfig = Server::get(IAppConfig::class); + $currentValue = $appConfig->getValueString('core', 'encryption_enabled', 'no'); + if ($currentValue === 'yes') { + $appConfig->setValueString('core', 'encryption_enabled', 'no'); + $appConfig->deleteKey('core', 'default_encryption_module'); + $appConfig->deleteKey('encryption', 'useMasterKey'); + } + } catch (\Throwable $e) { + // Ignore - may be called before bootstrap completes + } } /** diff --git a/tests/lib/Traits/EncryptionTrait.php b/tests/lib/Traits/EncryptionTrait.php index 2dc9213ff2431..2d67e4af8aad5 100644 --- a/tests/lib/Traits/EncryptionTrait.php +++ b/tests/lib/Traits/EncryptionTrait.php @@ -109,18 +109,20 @@ protected function setUpEncryptionTrait() { $this->encryptionApp = new Application([], $isReady); $this->config = Server::get(IConfig::class); - $this->encryptionWasEnabled = $this->config->getAppValue('core', 'encryption_enabled', 'no'); - $this->originalEncryptionModule = $this->config->getAppValue('core', 'default_encryption_module'); - $this->config->setAppValue('core', 'default_encryption_module', Encryption::ID); - $this->config->setAppValue('core', 'encryption_enabled', 'yes'); + $appConfig = Server::get(\OCP\IAppConfig::class); + $this->encryptionWasEnabled = $appConfig->getValueString('core', 'encryption_enabled', 'no'); + $this->originalEncryptionModule = $appConfig->getValueString('core', 'default_encryption_module', ''); + $appConfig->setValueString('core', 'default_encryption_module', Encryption::ID); + $appConfig->setValueString('core', 'encryption_enabled', 'yes'); $this->assertTrue(Server::get(\OCP\Encryption\IManager::class)->isEnabled()); } protected function tearDownEncryptionTrait() { if ($this->config) { - $this->config->setAppValue('core', 'encryption_enabled', $this->encryptionWasEnabled); - $this->config->setAppValue('core', 'default_encryption_module', $this->originalEncryptionModule); - $this->config->deleteAppValue('encryption', 'useMasterKey'); + $appConfig = Server::get(\OCP\IAppConfig::class); + $appConfig->setValueString('core', 'encryption_enabled', $this->encryptionWasEnabled); + $appConfig->setValueString('core', 'default_encryption_module', $this->originalEncryptionModule); + $appConfig->deleteKey('encryption', 'useMasterKey'); } } }