Skip to content

Commit a96fe1d

Browse files
joshtrichardsjoshtrichards
authored
feat(encryption): improve encryption:encrypt-all validation, safety & logging
- Add stronger pre-run validation and safer execution for the encryption:encrypt-all command: - more internal state checks before proceeding - require explicit "yes" (not just "yANYTHING" confirmation to proceed due to sensitivity of action - better guarantee restoration of state (maintenance/trashbin) via finally block - explicitly log key failures - improve user-facing help/warnings/guidance Signed-off-by: Josh <[email protected]>
1 parent 38792c8 commit a96fe1d

File tree

1 file changed

+90
-48
lines changed

1 file changed

+90
-48
lines changed

‎core/Command/Encryption/EncryptAll.php‎

Lines changed: 90 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OCP\App\IAppManager;
1111
use OCP\Encryption\IManager;
1212
use OCP\IConfig;
13+
use Psr\Log\LoggerInterface;
1314
use Symfony\Component\Console\Command\Command;
1415
use Symfony\Component\Console\Helper\QuestionHelper;
1516
use Symfony\Component\Console\Input\InputInterface;
@@ -24,81 +25,122 @@ public function __construct(
2425
protected IAppManager $appManager,
2526
protected IConfig $config,
2627
protected QuestionHelper $questionHelper,
28+
private LoggerInterface $logger,
2729
) {
2830
parent::__construct();
2931
}
3032

31-
/**
32-
* Set maintenance mode and disable the trashbin app
33-
*/
34-
protected function forceMaintenanceAndTrashbin(): void {
35-
$this->wasTrashbinEnabled = (bool)$this->appManager->isEnabledForUser('files_trashbin');
36-
$this->config->setSystemValue('maintenance', true);
37-
$this->appManager->disableApp('files_trashbin');
38-
}
39-
40-
/**
41-
* Reset the maintenance mode and re-enable the trashbin app
42-
*/
43-
protected function resetMaintenanceAndTrashbin(): void {
44-
$this->config->setSystemValue('maintenance', false);
45-
if ($this->wasTrashbinEnabled) {
46-
$this->appManager->enableApp('files_trashbin');
47-
}
48-
}
49-
50-
protected function configure() {
33+
protected function configure(): void {
5134
parent::configure();
5235

53-
$this->setName('encryption:encrypt-all');
54-
$this->setDescription('Encrypt all files for all users');
55-
$this->setHelp(
56-
'This will encrypt all files for all users. '
57-
. 'Please make sure that no user access his files during this process!'
36+
$this
37+
->setName('encryption:encrypt-all')
38+
->setDescription('Encrypt all users\' files using Nextcloud Server Side Encryption')
39+
->setHelp(
40+
"This will encrypt all files server-side for all users.\n" .
41+
"Maintenance mode will be enabled automatically.\n" .
42+
"Users should not access their files during this process!\n" .
43+
"WARNING: Please read the documentation prior to utilizing this feature to avoid data loss!"
5844
);
5945
}
6046

6147
protected function execute(InputInterface $input, OutputInterface $output): int {
48+
49+
// Handle container environment TTY problems to avoid confusion
6250
if (!$input->isInteractive()) {
63-
$output->writeln('Invalid TTY.');
64-
$output->writeln('If you are trying to execute the command in a Docker ');
65-
$output->writeln("container, do not forget to execute 'docker exec' with");
66-
$output->writeln("the '-i' and '-t' options.");
51+
$output->writeln('<error>Invalid TTY.</error>');
52+
$output->writeln("<comment>If running inside Docker, use 'docker exec -it' or equivalent.</comment>");
6753
$output->writeln('');
68-
return 1;
54+
return self::FAILURE;
6955
}
7056

57+
// Prevent running if SSE isn't enabled
7158
if ($this->encryptionManager->isEnabled() === false) {
72-
throw new \Exception('Server side encryption is not enabled');
59+
$output->writeln('<error>Server Side Encryption is not enabled; unable to encrypt files.</error>');
60+
return self::FAILURE;
61+
}
62+
63+
// Prevent running if no valid SSE modules are registered
64+
$modules = $this->encryptionManager->getEncryptionModules();
65+
if (empty($modules)) {
66+
$output->writeln('<error>No Server Side Encryption modules are registered; unable to encrypt files.</error>');
67+
return self::FAILURE;
68+
}
69+
70+
// Prevent running if a default SSE module isn't already configured
71+
$defaultModuleId = $this->encryptionManager->getDefaultEncryptionModuleId();
72+
if ($defaultModuleId === '') {
73+
$output->writeln('<error>A default Server Side Encryption module is not configured; unable to encrypt files.</error>');
74+
return self::FAILURE;
7375
}
7476

77+
// Prevent running if the default SSE module isn't valid
78+
if (!isset($modules[$defaultModuleId])) {
79+
$output->writeln('<error>The current default Server Side Encryption module does not exist: ' . $defaultModuleId . '; unable to encrypt files.</error>');
80+
return self::FAILURE;
81+
}
82+
83+
// Prevent running if maintenance mode is already enabled
7584
if ($this->config->getSystemValueBool('maintenance')) {
7685
$output->writeln('<error>This command cannot be run with maintenance mode enabled.</error>');
7786
return self::FAILURE;
7887
}
7988

89+
// TODO: Might make sense to add some additional readiness checks here such as the readiness of key storage/etc
90+
8091
$output->writeln("\n");
8192
$output->writeln('You are about to encrypt all files stored in your Nextcloud installation.');
82-
$output->writeln('Depending on the number of available files, and their size, this may take quite some time.');
83-
$output->writeln('Please ensure that no user accesses their files during this time!');
84-
$output->writeln('Note: The encryption module you use determines which files get encrypted.');
93+
$output->writeln('Depending on the number and size of files, this may take a long time.');
94+
$output->writeln('Please ensure that no user accesses their files during this process!');
95+
$output->writeln('Note: The encryption module you use and its settings determine which files get encrypted.');
96+
$output->writeln('Reminder: If External Storage is included in encryption, those files will no longer be accessible outside Nextcloud.');
97+
$output->writeln('WARNING: Please read the documentation prior to utilizing this feature to avoid data loss!');
8598
$output->writeln('');
86-
$question = new ConfirmationQuestion('Do you really want to continue? (y/n) ', false);
87-
if ($this->questionHelper->ask($input, $output, $question)) {
88-
$this->forceMaintenanceAndTrashbin();
89-
90-
try {
91-
$defaultModule = $this->encryptionManager->getEncryptionModule();
92-
$defaultModule->encryptAll($input, $output);
93-
} catch (\Exception $ex) {
94-
$this->resetMaintenanceAndTrashbin();
95-
throw $ex;
96-
}
9799

100+
// require "yes" to be typed in fully since this is a sensitive action
101+
$question = new ConfirmationQuestion('Do you really want to continue? (yes/NO) ', false, '/^yes$/i');
102+
if (!$this->questionHelper->ask($input, $output, $question)) {
103+
$output->writeln("\n" . 'Aborted.');
104+
return self::FAILURE;
105+
}
106+
107+
// Requirements before proceeding: disable trash bin, enable maintenance mode
108+
$this->forceMaintenanceAndTrashbin();
109+
110+
// Encrypt all the files
111+
try {
112+
$defaultModule = $this->encryptionManager->getEncryptionModule();
113+
$defaultModule->encryptAll($input, $output);
114+
} catch (\Throwable $ex) {
115+
$output->writeln('<error>Encryption failed: ' . $ex->getMessage() . '</error>');
116+
$this->logger->error('encryption:encrypt-all failed', ['exception' => $ex]);
117+
return self::FAILURE;
118+
} finally {
119+
// restore state no matter what (XXX: Better coverage than prior behavior; though I'm not convinced either is ideal)
98120
$this->resetMaintenanceAndTrashbin();
99-
return self::SUCCESS;
100121
}
101-
$output->writeln('aborted');
102-
return self::FAILURE;
122+
// If we made it here, we're good
123+
return self::SUCCESS;
124+
}
125+
126+
/**
127+
* Set maintenance mode and disable the trashbin app
128+
* TODO: The "why?" should be noted here.
129+
*/
130+
protected function forceMaintenanceAndTrashbin(): void {
131+
$this->wasTrashbinEnabled = (bool)$this->appManager->isEnabledForUser('files_trashbin');
132+
$this->config->setSystemValue('maintenance', true);
133+
$this->appManager->disableApp('files_trashbin');
134+
// TODO: Determine whether files_versions should be disabled temporarily too
135+
}
136+
137+
/**
138+
* Reset the maintenance mode and re-enable the trashbin app
139+
*/
140+
protected function resetMaintenanceAndTrashbin(): void {
141+
$this->config->setSystemValue('maintenance', false);
142+
if ($this->wasTrashbinEnabled) {
143+
$this->appManager->enableApp('files_trashbin');
144+
}
103145
}
104146
}

0 commit comments

Comments
 (0)