diff --git a/apps/encryption/lib/Settings/Admin.php b/apps/encryption/lib/Settings/Admin.php index 422d338d45e03..13cd9a4017e1a 100644 --- a/apps/encryption/lib/Settings/Admin.php +++ b/apps/encryption/lib/Settings/Admin.php @@ -53,6 +53,7 @@ public function getForm() { $crypt, $this->userSession, $this->config, + $this->appConfig, $this->userManager); // Check if an adminRecovery account is enabled for recovering files after lost pwd diff --git a/apps/encryption/lib/Util.php b/apps/encryption/lib/Util.php index ccbdcdcb242b1..bc6d50b86aa09 100644 --- a/apps/encryption/lib/Util.php +++ b/apps/encryption/lib/Util.php @@ -11,6 +11,7 @@ use OC\Files\View; use OCA\Encryption\Crypto\Crypt; use OCP\Files\Storage\IStorage; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; @@ -25,6 +26,7 @@ public function __construct( private Crypt $crypt, IUserSession $userSession, private IConfig $config, + private IAppConfig $appConfig, private IUserManager $userManager, ) { $this->user = $userSession->isLoggedIn() ? $userSession->getUser() : false; @@ -51,13 +53,7 @@ public function isRecoveryEnabledForUser($uid) { * @return bool */ public function shouldEncryptHomeStorage() { - $encryptHomeStorage = $this->config->getAppValue( - 'encryption', - 'encryptHomeStorage', - '1' - ); - - return ($encryptHomeStorage === '1'); + return $this->appConfig->getValueBool('encryption', 'encryptHomeStorage', true); } /** @@ -66,12 +62,7 @@ public function shouldEncryptHomeStorage() { * @param bool $encryptHomeStorage */ public function setEncryptHomeStorage($encryptHomeStorage) { - $value = $encryptHomeStorage ? '1' : '0'; - $this->config->setAppValue( - 'encryption', - 'encryptHomeStorage', - $value - ); + $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', (bool)$encryptHomeStorage); } /** diff --git a/apps/encryption/tests/Settings/AdminTest.php b/apps/encryption/tests/Settings/AdminTest.php index 10d2a61c5279e..f81dfe3e87115 100644 --- a/apps/encryption/tests/Settings/AdminTest.php +++ b/apps/encryption/tests/Settings/AdminTest.php @@ -62,7 +62,8 @@ public function testGetForm(): void { $this->appConfig ->method('getValueBool') ->willReturnMap([ - ['encryption', 'recoveryAdminEnabled', true] + ['encryption', 'recoveryAdminEnabled', true], + ['encryption', 'encryptHomeStorage', true, true], ]); $this->config ->method('getAppValue') @@ -70,9 +71,6 @@ public function testGetForm(): void { if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') { return '1'; } - if ($app === 'encryption' && $key === 'encryptHomeStorage' && $default === '1') { - return '1'; - } return $default; }); diff --git a/apps/encryption/tests/UtilTest.php b/apps/encryption/tests/UtilTest.php index ed274f74ab5ca..5f42f18ab4632 100644 --- a/apps/encryption/tests/UtilTest.php +++ b/apps/encryption/tests/UtilTest.php @@ -14,6 +14,7 @@ use OCA\Encryption\Util; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IUser; use OCP\IUserManager; @@ -26,6 +27,7 @@ class UtilTest extends TestCase { protected Util $instance; protected static $tempStorage = []; + protected IAppConfig&MockObject $appConfigMock; protected IConfig&MockObject $configMock; protected View&MockObject $filesMock; protected IUserManager&MockObject $userManagerMock; @@ -78,6 +80,7 @@ protected function setUp(): void { ->willReturn(true); $this->configMock = $this->createMock(IConfig::class); + $this->appConfigMock = $this->createMock(IAppConfig::class); $this->configMock->expects($this->any()) ->method('getUserValue') @@ -87,7 +90,7 @@ protected function setUp(): void { ->method('setUserValue') ->willReturnCallback([$this, 'setValueTester']); - $this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->userManagerMock); + $this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->appConfigMock, $this->userManagerMock); } /** @@ -136,13 +139,13 @@ public static function dataTestIsMasterKeyEnabled(): array { } /** - * @param string $returnValue return value from getAppValue() + * @param bool $returnValue return value from getValueBool() * @param bool $expected */ #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestShouldEncryptHomeStorage')] - public function testShouldEncryptHomeStorage($returnValue, $expected): void { - $this->configMock->expects($this->once())->method('getAppValue') - ->with('encryption', 'encryptHomeStorage', '1') + public function testShouldEncryptHomeStorage(bool $returnValue, bool $expected): void { + $this->appConfigMock->expects($this->once())->method('getValueBool') + ->with('encryption', 'encryptHomeStorage', true) ->willReturn($returnValue); $this->assertSame($expected, @@ -151,26 +154,26 @@ public function testShouldEncryptHomeStorage($returnValue, $expected): void { public static function dataTestShouldEncryptHomeStorage(): array { return [ - ['1', true], - ['0', false] + [true, true], + [false, false] ]; } /** - * @param $value - * @param $expected + * @param bool $value + * @param bool $expected */ #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestSetEncryptHomeStorage')] - public function testSetEncryptHomeStorage($value, $expected): void { - $this->configMock->expects($this->once())->method('setAppValue') + public function testSetEncryptHomeStorage(bool $value, bool $expected): void { + $this->appConfigMock->expects($this->once())->method('setValueBool') ->with('encryption', 'encryptHomeStorage', $expected); $this->instance->setEncryptHomeStorage($value); } public static function dataTestSetEncryptHomeStorage(): array { return [ - [true, '1'], - [false, '0'] + [true, true], + [false, false] ]; } diff --git a/apps/provisioning_api/lib/Controller/AppConfigController.php b/apps/provisioning_api/lib/Controller/AppConfigController.php index 0c53e3c009110..374d74ea9cd24 100644 --- a/apps/provisioning_api/lib/Controller/AppConfigController.php +++ b/apps/provisioning_api/lib/Controller/AppConfigController.php @@ -203,7 +203,8 @@ protected function verifyConfigKey(string $app, string $key, string $value) { throw new \InvalidArgumentException('The given key can not be set'); } - if ($app === 'core' && $key === 'encryption_enabled' && $value !== 'yes') { + if ($app === 'core' && $key === 'encryption_enabled' + && !in_array(strtolower(trim($value)), ['yes', '1', 'true', 'on'], true)) { throw new \InvalidArgumentException('The given key can not be set'); } diff --git a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php index 55885aabf0a42..a1253411871b0 100644 --- a/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php +++ b/apps/provisioning_api/tests/Controller/AppConfigControllerTest.php @@ -367,6 +367,10 @@ public static function dataVerifyConfigKey(): array { ['dav', 'public_route', ''], ['files', 'remote_route', ''], ['core', 'encryption_enabled', 'yes'], + ['core', 'encryption_enabled', '1'], + ['core', 'encryption_enabled', 'true'], + ['core', 'encryption_enabled', 'YES'], + ['core', 'encryption_enabled', 'on'], ]; } @@ -384,6 +388,8 @@ public static function dataVerifyConfigKeyThrows(): array { ['contacts', 'types', ''], ['core', 'encryption_enabled', 'no'], ['core', 'encryption_enabled', ''], + ['core', 'encryption_enabled', '0'], + ['core', 'encryption_enabled', 'false'], ['core', 'public_files', ''], ['core', 'public_dav', ''], ['core', 'remote_files', ''], diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 4b83ed5e23a90..2f8be2212060f 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1300,10 +1300,8 @@ - - @@ -3023,19 +3021,6 @@ - - - - - - - - - - - - - diff --git a/core/Command/Encryption/DecryptAll.php b/core/Command/Encryption/DecryptAll.php index 85a70b49cec24..56fd22d3b2a6b 100644 --- a/core/Command/Encryption/DecryptAll.php +++ b/core/Command/Encryption/DecryptAll.php @@ -9,6 +9,7 @@ namespace OC\Core\Command\Encryption; use OCP\App\IAppManager; +use OCP\Exceptions\AppConfigTypeConflictException; use OCP\IAppConfig; use OCP\IConfig; use Symfony\Component\Console\Command\Command; @@ -91,11 +92,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled'); + try { + $originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); + $originallyEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } try { if ($originallyEnabled) { $output->write('Disable server side encryption... '); - $this->appConfig->setValueBool('core', 'encryption_enabled', false); + $this->writeEncryptionEnabled(false); $output->writeln('done.'); } else { $output->writeln('Server side encryption not enabled. Nothing to do.'); @@ -123,18 +129,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln(' aborted.'); if ($originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); } } elseif (($uid !== '') && $originallyEnabled) { $output->writeln('Server side encryption remains enabled'); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); } $this->resetMaintenanceAndTrashbin(); return 0; } if ($originallyEnabled) { $output->write('Enable server side encryption... '); - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); $output->writeln('done.'); } $output->writeln('aborted'); @@ -142,10 +148,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int } catch (\Exception $e) { // enable server side encryption again if something went wrong if ($originallyEnabled) { - $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->writeEncryptionEnabled(true); } $this->resetMaintenanceAndTrashbin(); throw $e; } } + + private function writeEncryptionEnabled(bool $enabled): void { + try { + $this->appConfig->setValueBool('core', 'encryption_enabled', $enabled); + } catch (AppConfigTypeConflictException) { + $this->appConfig->setValueString('core', 'encryption_enabled', $enabled ? 'yes' : 'no'); + } + } } diff --git a/core/Command/Encryption/Disable.php b/core/Command/Encryption/Disable.php index fe280daa111b4..0f69be38f1011 100644 --- a/core/Command/Encryption/Disable.php +++ b/core/Command/Encryption/Disable.php @@ -9,14 +9,15 @@ */ namespace OC\Core\Command\Encryption; -use OCP\IConfig; +use OCP\Exceptions\AppConfigTypeConflictException; +use OCP\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class Disable extends Command { public function __construct( - protected IConfig $config, + protected IAppConfig $appConfig, ) { parent::__construct(); } @@ -31,10 +32,21 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') !== 'yes') { + try { + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); + $isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } + + if (!$isEnabled) { $output->writeln('Encryption is already disabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'no'); + try { + $this->appConfig->setValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $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 02c610250011c..66d30032f5842 100644 --- a/core/Command/Encryption/Enable.php +++ b/core/Command/Encryption/Enable.php @@ -10,14 +10,15 @@ namespace OC\Core\Command\Encryption; use OCP\Encryption\IManager; -use OCP\IConfig; +use OCP\Exceptions\AppConfigTypeConflictException; +use OCP\IAppConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; class Enable extends Command { public function __construct( - protected IConfig $config, + protected IAppConfig $appConfig, protected IManager $encryptionManager, ) { parent::__construct(); @@ -33,10 +34,21 @@ protected function configure() { #[\Override] protected function execute(InputInterface $input, OutputInterface $output): int { - if ($this->config->getAppValue('core', 'encryption_enabled', 'no') === 'yes') { + try { + $isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + $raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no'); + $isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } + + if ($isEnabled) { $output->writeln('Encryption is already enabled'); } else { - $this->config->setAppValue('core', 'encryption_enabled', 'yes'); + try { + $this->appConfig->setValueBool('core', 'encryption_enabled', true); + } catch (AppConfigTypeConflictException) { + $this->appConfig->setValueString('core', 'encryption_enabled', 'yes'); + } $output->writeln('Encryption enabled'); } $output->writeln(''); @@ -46,7 +58,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/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 6d57f1e487842..de2f29d2535f4 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -2092,6 +2092,7 @@ 'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairLogoDimension' => $baseDir . '/lib/private/Repair/RepairLogoDimension.php', 'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php', + 'OC\\Repair\\RetypeEncryptionConfigKeys' => $baseDir . '/lib/private/Repair/RetypeEncryptionConfigKeys.php', 'OC\\RichObjectStrings\\RichTextFormatter' => $baseDir . '/lib/private/RichObjectStrings/RichTextFormatter.php', 'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 564995dbfa757..1584ff0333dfa 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -2133,6 +2133,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php', 'OC\\Repair\\RepairLogoDimension' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairLogoDimension.php', 'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php', + 'OC\\Repair\\RetypeEncryptionConfigKeys' => __DIR__ . '/../../..' . '/lib/private/Repair/RetypeEncryptionConfigKeys.php', 'OC\\RichObjectStrings\\RichTextFormatter' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/RichTextFormatter.php', 'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php', 'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php', diff --git a/lib/private/Encryption/EncryptionWrapper.php b/lib/private/Encryption/EncryptionWrapper.php index 68d2efd8b8c0b..355257218e806 100644 --- a/lib/private/Encryption/EncryptionWrapper.php +++ b/lib/private/Encryption/EncryptionWrapper.php @@ -8,14 +8,17 @@ 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; use OCP\Encryption\IFile; use OCP\Encryption\Keys\IStorage as EncryptionKeysStorage; +use OCP\Exceptions\AppConfigTypeConflictException; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IDisableEncryptionStorage; use OCP\Files\Storage\IStorage; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IGroupManager; use OCP\IUserManager; @@ -57,32 +60,67 @@ public function wrapStorage(string $mountPoint, IStorage $storage, IMountPoint $ 'mount' => $mount ]; - if ($force || (!$storage->instanceOfStorage(IDisableEncryptionStorage::class) && $mountPoint !== '/')) { - $user = Server::get(IUserSession::class)->getUser(); - $mountManager = Filesystem::getMountManager(); - $uid = $user ? $user->getUID() : null; - $fileHelper = Server::get(IFile::class); - $keyStorage = 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(), - Server::get(IUserManager::class), - Server::get(IGroupManager::class), - Server::get(IConfig::class) - ); - return new Encryption( - $parameters, - $this->manager, - $util, - $this->logger, - $fileHelper, - $uid, - $keyStorage, - $mountManager, - $this->arrayCache + // Root mount point handling: skip encryption wrapper + if ($mountPoint === '/') { + return $storage; + } + + // Skip encryption for home mounts if encryptHomeStorage is disabled + if ($mount instanceof HomeMountPoint && !$this->shouldEncryptHomeStorage()) { + return $storage; + } + } + + // Apply encryption wrapper + $user = Server::get(IUserSession::class)->getUser(); + $mountManager = Filesystem::getMountManager(); + $uid = $user ? $user->getUID() : null; + $fileHelper = Server::get(IFile::class); + $keyStorage = Server::get(EncryptionKeysStorage::class); + + $util = new Util( + new View(), + Server::get(IUserManager::class), + Server::get(IGroupManager::class), + Server::get(IConfig::class) + ); + return new Encryption( + $parameters, + $this->manager, + $util, + $this->logger, + $fileHelper, + $uid, + $keyStorage, + $mountManager, + $this->arrayCache + ); + } + + private function shouldEncryptHomeStorage(): bool { + $appConfig = Server::get(IAppConfig::class); + try { + return $appConfig->getValueBool('encryption', 'encryptHomeStorage', true); + } catch (AppConfigTypeConflictException) { + // Stored as VALUE_STRING from a pre-upgrade installation. + // RetypeEncryptionConfigKeys repair step will fix the type on occ upgrade. + return $this->parseLegacyBoolString( + $appConfig->getValueString('encryption', 'encryptHomeStorage', '1') ); - } else { - return $storage; + } catch (\Throwable) { + // DB not ready (e.g. oc_appconfig does not yet exist during install). + return true; } } + + private function parseLegacyBoolString(string $value): bool { + return in_array(strtolower(trim($value)), ['1', 'true', 'yes', 'on'], true); + } } diff --git a/lib/private/Encryption/Manager.php b/lib/private/Encryption/Manager.php index ac27f0911b8da..f8b278d3d3b9e 100644 --- a/lib/private/Encryption/Manager.php +++ b/lib/private/Encryption/Manager.php @@ -16,10 +16,12 @@ use OC\ServiceUnavailableException; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; +use OCP\Exceptions\AppConfigTypeConflictException; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\IConfig; use OCP\IL10N; +use OCP\Server; use Psr\Log\LoggerInterface; class Manager implements IManager { @@ -48,8 +50,17 @@ public function isEnabled() { return false; } - $enabled = $this->config->getAppValue('core', 'encryption_enabled', 'no'); - return $enabled === 'yes'; + try { + return Server::get(\OCP\IAppConfig::class)->getValueBool('core', 'encryption_enabled', false); + } catch (AppConfigTypeConflictException) { + // Stored as VALUE_STRING from a pre-upgrade installation. + // RetypeEncryptionConfigKeys repair step will fix the type on occ upgrade. + $raw = Server::get(\OCP\IAppConfig::class)->getValueString('core', 'encryption_enabled', 'no'); + return in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true); + } catch (\Throwable) { + // DB not ready (e.g. oc_appconfig does not yet exist during install). + return false; + } } /** diff --git a/lib/private/Files/Cache/CacheEntry.php b/lib/private/Files/Cache/CacheEntry.php index 74af7d0a59800..5f20bfdd3e613 100644 --- a/lib/private/Files/Cache/CacheEntry.php +++ b/lib/private/Files/Cache/CacheEntry.php @@ -140,7 +140,7 @@ public function __clone() { #[\Override] 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 36c8c02a8bfca..df73ccd79d94d 100644 --- a/lib/private/Files/Cache/Scanner.php +++ b/lib/private/Files/Cache/Scanner.php @@ -183,8 +183,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']); } @@ -202,7 +206,12 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = $data['etag_changed'] = true; } } else { - unset($data['unencrypted_size']); + // For new files, preserve unencrypted_size only when the file is encrypted + // and the key is present; otherwise clear it so it won't be written stale. + if (!isset($data['encrypted']) || !$data['encrypted'] + || !isset($data['unencrypted_size'])) { + unset($data['unencrypted_size']); + } $newData = $data; $fileId = -1; } diff --git a/lib/private/Files/FileInfo.php b/lib/private/Files/FileInfo.php index 65280b0f31580..37d96ba27ff2b 100644 --- a/lib/private/Files/FileInfo.php +++ b/lib/private/Files/FileInfo.php @@ -173,7 +173,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 8edf25e9a5111..a256dfaa63d1d 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -400,6 +400,7 @@ protected function verifyUnencryptedSize(string $path, int $unencryptedSize): in if ($unencryptedSize < 0 || ($size > 0 && $unencryptedSize === $size) || $unencryptedSize > $size + || ($unencryptedSize === 0 && $size > $this->util->getHeaderSize()) ) { // 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 1c662aa023c6c..7b83ce0592a80 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/lib/private/Repair.php b/lib/private/Repair.php index 90e209cfe9085..3bc5827e4c44e 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -56,6 +56,7 @@ use OC\Repair\RepairInvalidShares; use OC\Repair\RepairLogoDimension; use OC\Repair\RepairMimeTypes; +use OC\Repair\RetypeEncryptionConfigKeys; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; @@ -157,6 +158,7 @@ public function addStep(IRepairStep|string $repairStep, bool $includeExpensive = */ public static function getRepairSteps(bool $includeExpensive = false): array { $repairSteps = [ + Server::get(RetypeEncryptionConfigKeys::class), new Collation(Server::get(IConfig::class), Server::get(LoggerInterface::class), Server::get(IDBConnection::class), false), Server::get(CleanTags::class), Server::get(RepairInvalidShares::class), diff --git a/lib/private/Repair/RetypeEncryptionConfigKeys.php b/lib/private/Repair/RetypeEncryptionConfigKeys.php new file mode 100644 index 0000000000000..48f0ce17c12c6 --- /dev/null +++ b/lib/private/Repair/RetypeEncryptionConfigKeys.php @@ -0,0 +1,55 @@ +appConfig->getValueType($app, $key); + } catch (AppConfigUnknownKeyException) { + continue; + } + + if (($type & IAppConfig::VALUE_BOOL) === IAppConfig::VALUE_BOOL) { + $output->info("$app.$key is already typed as boolean, skipping."); + continue; + } + + $raw = strtolower(trim($this->appConfig->getValueString($app, $key, $defaultBool ? '1' : '0'))); + $bool = in_array($raw, ['1', 'true', 'yes', 'on'], true); + + $this->appConfig->deleteKey($app, $key); + $this->appConfig->setValueBool($app, $key, $bool); + $output->info("Re-typed $app.$key from string '$raw' to boolean " . ($bool ? 'true' : 'false') . '.'); + } + } +} diff --git a/tests/Core/Command/Encryption/DecryptAllTest.php b/tests/Core/Command/Encryption/DecryptAllTest.php index 009eb36eb7fe2..74ccf0563bfbc 100644 --- a/tests/Core/Command/Encryption/DecryptAllTest.php +++ b/tests/Core/Command/Encryption/DecryptAllTest.php @@ -103,7 +103,7 @@ public function testExecute($encryptionEnabled, $continue): void { $this->appConfig->expects($this->once()) ->method('getValueBool') - ->with('core', 'encryption_enabled') + ->with('core', 'encryption_enabled', false) ->willReturn($encryptionEnabled); $this->consoleInput->expects($this->any()) @@ -168,7 +168,7 @@ public function testExecuteFailure(): void { ['core', 'encryption_enabled', true, false], ]; $this->appConfig->expects($this->exactly(2)) - ->method('setValuebool') + ->method('setValueBool') ->willReturnCallback(function () use (&$calls): bool { $expected = array_shift($calls); $this->assertEquals($expected, func_get_args()); @@ -176,7 +176,7 @@ public function testExecuteFailure(): void { }); $this->appConfig->expects($this->once()) ->method('getValueBool') - ->with('core', 'encryption_enabled') + ->with('core', 'encryption_enabled', false) ->willReturn(true); $this->consoleInput->expects($this->any()) diff --git a/tests/Core/Command/Encryption/DisableTest.php b/tests/Core/Command/Encryption/DisableTest.php index 96310a6c75ba3..b2ef86c2921f2 100644 --- a/tests/Core/Command/Encryption/DisableTest.php +++ b/tests/Core/Command/Encryption/DisableTest.php @@ -9,14 +9,14 @@ namespace Tests\Core\Command\Encryption; use OC\Core\Command\Encryption\Disable; -use OCP\IConfig; +use OCP\IAppConfig; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class DisableTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject */ - protected $config; + protected $appConfig; /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $consoleInput; /** @var \PHPUnit\Framework\MockObject\MockObject */ @@ -29,35 +29,35 @@ class DisableTest extends TestCase { protected function setUp(): void { parent::setUp(); - $config = $this->config = $this->getMockBuilder(IConfig::class) + $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($appConfig); } public static function dataDisable(): array { return [ - ['yes', true, 'Encryption disabled'], - ['no', false, 'Encryption is already disabled'], + [true, true, 'Encryption disabled'], + [false, false, 'Encryption is already disabled'], ]; } /** * - * @param string $oldStatus + * @param bool $oldStatus * @param bool $isUpdating * @param string $expectedString */ #[\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()) + public function testDisable(bool $oldStatus, bool $isUpdating, string $expectedString): void { + $this->appConfig->expects($this->once()) + ->method('getValueBool') + ->with('core', 'encryption_enabled', false) ->willReturn($oldStatus); $this->consoleOutput->expects($this->once()) @@ -65,9 +65,9 @@ public function testDisable($oldStatus, $isUpdating, $expectedString): void { ->with($this->stringContains($expectedString)); if ($isUpdating) { - $this->config->expects($this->once()) - ->method('setAppValue') - ->with('core', 'encryption_enabled', 'no'); + $this->appConfig->expects($this->once()) + ->method('setValueBool') + ->with('core', 'encryption_enabled', false); } self::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]); diff --git a/tests/Core/Command/Encryption/EnableTest.php b/tests/Core/Command/Encryption/EnableTest.php index 234984b1c0954..2acc1458e81aa 100644 --- a/tests/Core/Command/Encryption/EnableTest.php +++ b/tests/Core/Command/Encryption/EnableTest.php @@ -10,14 +10,14 @@ use OC\Core\Command\Encryption\Enable; use OCP\Encryption\IManager; -use OCP\IConfig; +use OCP\IAppConfig; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class EnableTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject */ - protected $config; + protected $appConfig; /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $manager; /** @var \PHPUnit\Framework\MockObject\MockObject */ @@ -32,7 +32,7 @@ class EnableTest extends TestCase { protected function setUp(): void { parent::setUp(); - $config = $this->config = $this->getMockBuilder(IConfig::class) + $appConfig = $this->appConfig = $this->getMockBuilder(IAppConfig::class) ->disableOriginalConstructor() ->getMock(); $manager = $this->manager = $this->getMockBuilder(IManager::class) @@ -41,47 +41,44 @@ protected function setUp(): void { $this->consoleInput = $this->getMockBuilder(InputInterface::class)->getMock(); $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($appConfig, $manager); } public static function dataEnable(): array { return [ - ['no', '', [], true, 'Encryption enabled', 'No encryption module is loaded'], - ['yes', '', [], false, 'Encryption is already enabled', 'No encryption module is loaded'], - ['no', '', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'No default module is set'], - ['no', 'OC_NO_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'The current default module does not exist: OC_NO_MODULE'], - ['no', 'OC_TEST_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'Default module: OC_TEST_MODULE'], + [false, '', [], true, 'Encryption enabled', 'No encryption module is loaded'], + [true, '', [], false, 'Encryption is already enabled', 'No encryption module is loaded'], + [false, '', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'No default module is set'], + [false, 'OC_NO_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'The current default module does not exist: OC_NO_MODULE'], + [false, 'OC_TEST_MODULE', ['OC_TEST_MODULE' => []], true, 'Encryption enabled', 'Default module: OC_TEST_MODULE'], ]; } #[\PHPUnit\Framework\Attributes\DataProvider('dataEnable')] - public function testEnable(string $oldStatus, ?string $defaultModule, array $availableModules, bool $isUpdating, string $expectedString, string $expectedDefaultModuleString): void { + public function testEnable(bool $oldStatus, ?string $defaultModule, array $availableModules, bool $isUpdating, string $expectedString, string $expectedDefaultModuleString): void { if ($isUpdating) { - $this->config->expects($this->once()) - ->method('setAppValue') - ->with('core', 'encryption_enabled', 'yes'); + $this->appConfig->expects($this->once()) + ->method('setValueBool') + ->with('core', 'encryption_enabled', true); } $this->manager->expects($this->atLeastOnce()) ->method('getEncryptionModules') ->willReturn($availableModules); - if (empty($availableModules)) { - $this->config->expects($this->once()) - ->method('getAppValue') - ->willReturnMap([ - ['core', 'encryption_enabled', 'no', $oldStatus], - ]); - } else { - $this->config->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['core', 'encryption_enabled', 'no', $oldStatus], - ['core', 'default_encryption_module', '', $defaultModule], - ]); + $this->appConfig->expects($this->once()) + ->method('getValueBool') + ->with('core', 'encryption_enabled', false) + ->willReturn($oldStatus); + + if (!empty($availableModules)) { + $this->appConfig->expects($this->once()) + ->method('getValueString') + ->with('core', 'default_encryption_module', '') + ->willReturn((string)$defaultModule); } $calls = [ diff --git a/tests/lib/Encryption/ManagerTest.php b/tests/lib/Encryption/ManagerTest.php index 5f3d1987dc3dc..2b414f0c64f91 100644 --- a/tests/lib/Encryption/ManagerTest.php +++ b/tests/lib/Encryption/ManagerTest.php @@ -14,8 +14,10 @@ use OC\Files\View; use OC\Memcache\ArrayCache; use OCP\Encryption\IEncryptionModule; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IL10N; +use OCP\Server; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -73,10 +75,18 @@ public function testManagerIsDisabledIfDisabledButModules(): void { $this->assertFalse($this->manager->isEnabled()); } + /** + * @group DB + */ public function testManagerIsEnabled(): void { + $appConfig = Server::get(IAppConfig::class); + $appConfig->setValueBool('core', 'encryption_enabled', true); + $this->config->expects($this->any())->method('getSystemValueBool')->willReturn(true); - $this->config->expects($this->any())->method('getAppValue')->willReturn('yes'); - $this->assertTrue($this->manager->isEnabled()); + $result = $this->manager->isEnabled(); + + $appConfig->deleteKey('core', 'encryption_enabled'); + $this->assertTrue($result); } public function testModuleRegistration() { diff --git a/tests/lib/Files/ObjectStore/S3EncryptionMigrationTest.php b/tests/lib/Files/ObjectStore/S3EncryptionMigrationTest.php new file mode 100644 index 0000000000000..8abbe35eba73d --- /dev/null +++ b/tests/lib/Files/ObjectStore/S3EncryptionMigrationTest.php @@ -0,0 +1,217 @@ +getSystemValue('objectstore'); + if (!is_array($config) || ($config['class'] ?? null) !== S3::class) { + self::markTestSkipped('S3 primary storage not configured'); + } + } + + protected function setUp(): void { + parent::setUp(); + + $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 + } + + parent::tearDown(); + } + + /** + * 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); + + // Resolve object-store storage IDs first + $storageQuery = $db->getQueryBuilder(); + $storageQuery->select('numeric_id') + ->from('storages') + ->where($storageQuery->expr()->like('id', $storageQuery->createNamedParameter('object::%'))); + $storageResult = $storageQuery->executeQuery(); + $objectStoreIds = array_column($storageResult->fetchAllAssociative(), 'numeric_id'); + $storageResult->closeCursor(); + + $query = $db->getQueryBuilder(); + $query->select($query->func()->count('*', 'total')) + ->from('filecache') + ->where($query->expr()->eq('encrypted', $query->createNamedParameter(0))); + + if (!empty($objectStoreIds)) { + $query->andWhere($query->expr()->in('storage', + $query->createNamedParameter($objectStoreIds, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT_ARRAY))); + } + + $result = $query->executeQuery(); + $row = $result->fetch(); + $unencryptedCount = $row['total'] ?? 0; + + // Verify the test file actually landed in filecache (query is hitting the right table). + $fileQuery = $db->getQueryBuilder(); + $fileQuery->select('fileid', 'encrypted') + ->from('filecache') + ->where($fileQuery->expr()->like('path', $fileQuery->createNamedParameter('%migration-test-encrypted%'))); + $fileResult = $fileQuery->executeQuery(); + $fileRow = $fileResult->fetch(); + $fileResult->closeCursor(); + + $this->assertNotFalse($fileRow, 'Test file should appear in filecache after write'); + + // The detection query must return a valid numeric result. + $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..ff95fa89e662c --- /dev/null +++ b/tests/lib/Files/ObjectStore/S3EncryptionTest.php @@ -0,0 +1,482 @@ +getSystemValue('objectstore'); + if (!is_array($config) || ($config['class'] ?? null) !== S3::class) { + self::markTestSkipped('S3 primary storage not configured. Configure objectstore in config.php to run these tests.'); + } + } + + protected function setUp(): void { + parent::setUp(); + + // 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'); + } + + protected function tearDown(): void { + // Clean up test files + try { + if ($this->view) { + $this->cleanupTestFiles(); + } + } catch (\Exception $e) { + // Ignore cleanup errors + } + + 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(), 0, $e); + } + } + + /** + * Data provider for file sizes + */ + public static function dataFileSizes(): array { + $sizes = [ + '0 bytes (empty file)' => [0], + '1KB' => [1024], + '1MB' => [1024 * 1024], + '5MB (multipart threshold)' => [5 * 1024 * 1024], + '16MB (historical issue)' => [16 * 1024 * 1024], + ]; + if (getenv('RUN_HEAVY_S3_TESTS')) { + $sizes['64MB (historical issue)'] = [64 * 1024 * 1024]; + $sizes['100MB (stress test)'] = [100 * 1024 * 1024]; + } + return $sizes; + } + + /** + * 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) : ''; + $expectedHash = hash('sha256', $data); + $bytesWritten = $this->view->file_put_contents($testFile, $data); + unset($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 via hash to avoid holding two large buffers simultaneously + $this->assertEquals($expectedHash, hash('sha256', $content), + 'Content should be identical after encrypt/decrypt cycle - corruption detected!'); + unset($content); + + // 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) : ''; + $expectedHash = hash('sha256', $data); + + // Write + $written = $this->view->file_put_contents($testFile, $data); + $this->assertEquals($size, $written); + unset($data); + + // 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 via hash to avoid holding two large buffers in memory + $this->assertEquals($expectedHash, hash('sha256', $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) : ''; + $expectedHash = hash('sha256', $data); + + // Write + $this->view->file_put_contents($testFile, $data); + unset($data); + + // Stream read using incremental hashing to avoid large in-memory buffers + $handle = $this->view->fopen($testFile, 'r'); + $this->assertIsResource($handle); + + $ctx = hash_init('sha256'); + $totalRead = 0; + $chunkSize = 8192; + while (!feof($handle)) { + $chunk = fread($handle, $chunkSize); + $totalRead += strlen($chunk); + hash_update($ctx, $chunk); + } + fclose($handle); + + // Verify + $this->assertEquals($size, $totalRead, 'Size mismatch in streaming read'); + $this->assertEquals($expectedHash, hash_final($ctx), '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 { + if (!getenv('RUN_HEAVY_S3_TESTS')) { + $this->markTestSkipped('Set RUN_HEAVY_S3_TESTS=1 to run large-file multipart tests'); + } + + $testFile = 'test-multipart.bin'; + // 110 MiB file to exceed the multipart upload threshold (~100 MiB); see S3 backend source. + $size = 110 * 1024 * 1024; + $data = random_bytes($size); + $expectedHash = hash('sha256', $data); + + // Write (should use multipart upload) + $written = $this->view->file_put_contents($testFile, $data); + $this->assertEquals($size, $written); + unset($data); + + // 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 via streaming hash to avoid loading 110MiB into memory + $stream = $this->view->fopen($testFile, 'rb'); + $this->assertNotFalse($stream, 'Failed to open multipart encrypted upload for reading'); + $hashCtx = hash_init('sha256'); + while (!feof($stream)) { + $chunk = fread($stream, 65536); + if ($chunk !== false && $chunk !== '') { + hash_update($hashCtx, $chunk); + } + } + fclose($stream); + $this->assertEquals($expectedHash, hash_final($hashCtx), + '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/Repair/RetypeEncryptionConfigKeysTest.php b/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php new file mode 100644 index 0000000000000..48bea2cb97739 --- /dev/null +++ b/tests/lib/Repair/RetypeEncryptionConfigKeysTest.php @@ -0,0 +1,101 @@ +appConfig = Server::get(IAppConfig::class); + $this->output = $this->createMock(IOutput::class); + $this->repair = new RetypeEncryptionConfigKeys($this->appConfig); + // Clean slate in case previous test runs or occ invocations left residue + $this->appConfig->deleteKey('core', 'encryption_enabled'); + $this->appConfig->deleteKey('encryption', 'encryptHomeStorage'); + } + + protected function tearDown(): void { + $this->appConfig->deleteKey('core', 'encryption_enabled'); + $this->appConfig->deleteKey('encryption', 'encryptHomeStorage'); + parent::tearDown(); + } + + public function testAbsentKeyIsNoOp(): void { + $this->output->expects($this->never())->method('info'); + $this->repair->run($this->output); + // No exception, no write + $this->assertTrue(true); + } + + public static function dataStringValues(): array { + return [ + ['yes', true], + ['no', false], + ['1', true], + ['0', false], + ['true', true], + ['false', false], + ['on', true], + ['YES', true], + ]; + } + + /** + * @dataProvider dataStringValues + */ + public function testEncryptionEnabledIsRetypedFromString(string $raw, bool $expected): void { + $this->appConfig->setValueString('core', 'encryption_enabled', $raw); + + $this->repair->run($this->output); + + $this->assertSame(IAppConfig::VALUE_BOOL, $this->appConfig->getValueType('core', 'encryption_enabled')); + $this->assertSame($expected, $this->appConfig->getValueBool('core', 'encryption_enabled', !$expected)); + } + + /** + * @dataProvider dataStringValues + */ + public function testEncryptHomeStorageIsRetypedFromString(string $raw, bool $expected): void { + $this->appConfig->setValueString('encryption', 'encryptHomeStorage', $raw); + + $this->repair->run($this->output); + + $this->assertSame(IAppConfig::VALUE_BOOL, $this->appConfig->getValueType('encryption', 'encryptHomeStorage')); + $this->assertSame($expected, $this->appConfig->getValueBool('encryption', 'encryptHomeStorage', !$expected)); + } + + public function testAlreadyBoolIsNoOp(): void { + $this->appConfig->setValueBool('core', 'encryption_enabled', true); + $this->appConfig->setValueBool('encryption', 'encryptHomeStorage', false); + + // Should log "already typed" messages but not re-write + $this->output->expects($this->exactly(2)) + ->method('info') + ->with($this->stringContains('already typed')); + + $this->repair->run($this->output); + + $this->assertTrue($this->appConfig->getValueBool('core', 'encryption_enabled', false)); + $this->assertFalse($this->appConfig->getValueBool('encryption', 'encryptHomeStorage', true)); + } +} diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index aed919a1e6fe0..b9e5dfc5500e3 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -27,6 +27,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; @@ -195,6 +196,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->getValueBool('core', 'encryption_enabled', false); + if ($currentValue) { + $appConfig->setValueBool('core', 'encryption_enabled', false); + $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..42d04376a53f8 100644 --- a/tests/lib/Traits/EncryptionTrait.php +++ b/tests/lib/Traits/EncryptionTrait.php @@ -34,7 +34,7 @@ abstract protected function registerStorageWrapper($name, $wrapper); abstract protected static function markTestSkipped(string $message = ''): void; abstract protected static function assertTrue($condition, string $message = ''): void; - private $encryptionWasEnabled; + private bool $encryptionWasEnabled = false; private $originalEncryptionModule; @@ -109,18 +109,27 @@ 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->getValueBool('core', 'encryption_enabled', false); + $this->originalEncryptionModule = $appConfig->getValueString('core', 'default_encryption_module', ''); + $appConfig->setValueString('core', 'default_encryption_module', Encryption::ID); + $appConfig->setValueBool('core', 'encryption_enabled', true); $this->assertTrue(Server::get(\OCP\Encryption\IManager::class)->isEnabled()); + + // Ensure system-wide share/master keys exist in the keystore. + // Setup::setupSystem() is cache-gated on 'keys-validated' and may no-op + // when the cache was primed while encryption was disabled. + $keyManager = Server::get(KeyManager::class); + $keyManager->validateShareKey(); + $keyManager->validateMasterKey(); } 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->setValueBool('core', 'encryption_enabled', $this->encryptionWasEnabled); + $appConfig->setValueString('core', 'default_encryption_module', $this->originalEncryptionModule); + $appConfig->deleteKey('encryption', 'useMasterKey'); } } }