upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-06-10 17:23:59 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 57

Merge pull request #53666 from nextcloud/backport/53665/stable31

Browse source 
[stable31] fix(encryption): Catch exceptions in encrypt-all command and continue
This commit is contained in:
Côme Chilliet 2025-07-03 16:55:26 +02:00 committed by GitHub
commit e423fb46c9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 89 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
apps/encryption/lib/Crypto/EncryptAll.php
View file

@ -20,6 +20,7 @@ use OCP\L10N\IFactory;
use OCP\Mail\Headers\AutoSubmitted;
use OCP\Mail\IMailer;
use OCP\Security\ISecureRandom;
use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Helper\ProgressBar;
use Symfony\Component\Console\Helper\QuestionHelper;
use Symfony\Component\Console\Helper\Table;
@ -50,6 +51,7 @@ class EncryptAll {
protected IFactory $l10nFactory,
protected QuestionHelper $questionHelper,
protected ISecureRandom $secureRandom,
protected LoggerInterface $logger,
) {
// store one time passwords for the users
$this->userPasswords = [];
@ -207,9 +209,22 @@ class EncryptAll {
} else {
$progress->setMessage("encrypt files for user $userCount: $path");
$progress->advance();
if ($this->encryptFile($path) === false) {
$progress->setMessage("encrypt files for user $userCount: $path (already encrypted)");
try {
if ($this->encryptFile($path) === false) {
$progress->setMessage("encrypt files for user $userCount: $path (already encrypted)");
$progress->advance();
}
} catch (\Exception $e) {
$progress->setMessage("Failed to encrypt path $path: " . $e->getMessage());
$progress->advance();
$this->logger->error(
'Failed to encrypt path {path}',
[
'user' => $uid,
'path' => $path,
'exception' => $e,
]
);
}
}
}
@ -234,7 +249,14 @@ class EncryptAll {
$target = $path . '.encrypted.' . time();
try {
$this->rootView->copy($source, $target);
$copySuccess = $this->rootView->copy($source, $target);
if ($copySuccess === false) {
/* Copy failed, abort */
if ($this->rootView->file_exists($target)) {
$this->rootView->unlink($target);
}
throw new \Exception('Copy failed for ' . $source);
}
$this->rootView->rename($target, $source);
} catch (DecryptionFailedException $e) {
if ($this->rootView->file_exists($target)) {

84
apps/encryption/tests/Crypto/EncryptAllTest.php
View file

@ -20,6 +20,8 @@ use OCP\L10N\IFactory;
use OCP\Mail\IMailer;
use OCP\Security\ISecureRandom;
use OCP\UserInterface;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Formatter\OutputFormatterInterface;
use Symfony\Component\Console\Helper\ProgressBar;
use Symfony\Component\Console\Helper\QuestionHelper;
@ -28,48 +30,21 @@ use Symfony\Component\Console\Output\OutputInterface;
use Test\TestCase;
class EncryptAllTest extends TestCase {
/** @var \PHPUnit\Framework\MockObject\MockObject|KeyManager */
protected $keyManager;
/** @var \PHPUnit\Framework\MockObject\MockObject|Util */
protected $util;
/** @var \PHPUnit\Framework\MockObject\MockObject|IUserManager */
protected $userManager;
/** @var \PHPUnit\Framework\MockObject\MockObject|Setup */
protected $setupUser;
/** @var \PHPUnit\Framework\MockObject\MockObject|View */
protected $view;
/** @var \PHPUnit\Framework\MockObject\MockObject|IConfig */
protected $config;
/** @var \PHPUnit\Framework\MockObject\MockObject|IMailer */
protected $mailer;
/** @var \PHPUnit\Framework\MockObject\MockObject|IL10N */
protected $l;
/** @var \PHPUnit\Framework\MockObject\MockObject | IFactory */
protected $l10nFactory;
/** @var \PHPUnit\Framework\MockObject\MockObject | \Symfony\Component\Console\Helper\QuestionHelper */
protected $questionHelper;
/** @var \PHPUnit\Framework\MockObject\MockObject | \Symfony\Component\Console\Input\InputInterface */
protected $inputInterface;
/** @var \PHPUnit\Framework\MockObject\MockObject | \Symfony\Component\Console\Output\OutputInterface */
protected $outputInterface;
/** @var \PHPUnit\Framework\MockObject\MockObject|UserInterface */
protected $userInterface;
/** @var \PHPUnit\Framework\MockObject\MockObject|ISecureRandom */
protected $secureRandom;
protected KeyManager&MockObject $keyManager;
protected Util&MockObject $util;
protected IUserManager&MockObject $userManager;
protected Setup&MockObject $setupUser;
protected View&MockObject $view;
protected IConfig&MockObject $config;
protected IMailer&MockObject $mailer;
protected IL10N&MockObject $l;
protected IFactory&MockObject $l10nFactory;
protected \Symfony\Component\Console\Helper\QuestionHelper&MockObject $questionHelper;
protected \Symfony\Component\Console\Input\InputInterface&MockObject $inputInterface;
protected \Symfony\Component\Console\Output\OutputInterface&MockObject $outputInterface;
protected UserInterface&MockObject $userInterface;
protected ISecureRandom&MockObject $secureRandom;
protected LoggerInterface&MockObject $logger;
/** @var EncryptAll */
protected $encryptAll;
@ -101,6 +76,7 @@ class EncryptAllTest extends TestCase {
->disableOriginalConstructor()->getMock();
$this->userInterface = $this->getMockBuilder(UserInterface::class)
->disableOriginalConstructor()->getMock();
$this->logger = $this->createMock(LoggerInterface::class);
/**
* We need format method to return a string
@ -131,12 +107,13 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
);
}
public function testEncryptAll(): void {
/** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */
/** @var EncryptAll&MockObject $encryptAll */
$encryptAll = $this->getMockBuilder(EncryptAll::class)
->setConstructorArgs(
[
@ -150,7 +127,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords'])
@ -165,7 +143,7 @@ class EncryptAllTest extends TestCase {
}
public function testEncryptAllWithMasterKey(): void {
/** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */
/** @var EncryptAll&MockObject $encryptAll */
$encryptAll = $this->getMockBuilder(EncryptAll::class)
->setConstructorArgs(
[
@ -179,7 +157,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords'])
@ -195,7 +174,7 @@ class EncryptAllTest extends TestCase {
}
public function testCreateKeyPairs(): void {
/** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */
/** @var EncryptAll&MockObject $encryptAll */
$encryptAll = $this->getMockBuilder(EncryptAll::class)
->setConstructorArgs(
[
@ -209,7 +188,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['setupUserFS', 'generateOneTimePassword'])
@ -259,7 +239,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['encryptUsersFiles'])
@ -295,7 +276,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['encryptFile', 'setupUserFS'])

48
lib/private/Files/Storage/Wrapper/Encryption.php
View file

@ -23,6 +23,7 @@ use OCP\Encryption\IFile;
use OCP\Encryption\IManager;
use OCP\Encryption\Keys\IStorage;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\GenericFileException;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Storage;
use Psr\Log\LoggerInterface;
@ -185,11 +186,11 @@ class Encryption extends Wrapper {
public function rename(string $source, string $target): bool {
$result = $this->storage->rename($source, $target);
if ($result &&
if ($result
// versions always use the keys from the original file, so we can skip
// this step for versions
$this->isVersion($target) === false &&
$this->encryptionManager->isEnabled()) {
&& $this->isVersion($target) === false
&& $this->encryptionManager->isEnabled()) {
$sourcePath = $this->getFullPath($source);
if (!$this->util->isExcluded($sourcePath)) {
$targetPath = $this->getFullPath($target);
@ -210,9 +211,9 @@ class Encryption extends Wrapper {
public function rmdir(string $path): bool {
$result = $this->storage->rmdir($path);
$fullPath = $this->getFullPath($path);
if ($result &&
$this->util->isExcluded($fullPath) === false &&
$this->encryptionManager->isEnabled()
if ($result
&& $this->util->isExcluded($fullPath) === false
&& $this->encryptionManager->isEnabled()
) {
$this->keyStorage->deleteAllFileKeys($fullPath);
}
@ -225,9 +226,9 @@ class Encryption extends Wrapper {
$metaData = $this->getMetaData($path);
if (
!$this->is_dir($path) &&
isset($metaData['encrypted']) &&
$metaData['encrypted'] === true
!$this->is_dir($path)
&& isset($metaData['encrypted'])
&& $metaData['encrypted'] === true
) {
$fullPath = $this->getFullPath($path);
$module = $this->getEncryptionModule($path);
@ -384,9 +385,9 @@ class Encryption extends Wrapper {
$size = $this->storage->filesize($path);
$result = $unencryptedSize;
if ($unencryptedSize < 0 ||
($size > 0 && $unencryptedSize === $size) ||
$unencryptedSize > $size
if ($unencryptedSize < 0
|| ($size > 0 && $unencryptedSize === $size)
|| $unencryptedSize > $size
) {
// check if we already calculate the unencrypted size for the
// given path to avoid recursions
@ -622,8 +623,8 @@ class Encryption extends Wrapper {
): bool {
// for versions we have nothing to do, because versions should always use the
// key from the original file. Just create a 1:1 copy and done
if ($this->isVersion($targetInternalPath) ||
$this->isVersion($sourceInternalPath)) {
if ($this->isVersion($targetInternalPath)
|| $this->isVersion($sourceInternalPath)) {
// remember that we try to create a version so that we can detect it during
// fopen($sourceInternalPath) and by-pass the encryption in order to
// create a 1:1 copy of the file
@ -673,12 +674,16 @@ class Encryption extends Wrapper {
try {
$source = $sourceStorage->fopen($sourceInternalPath, 'r');
$target = $this->fopen($targetInternalPath, 'w');
[, $result] = \OC_Helper::streamCopy($source, $target);
if ($source === false || $target === false) {
$result = false;
} else {
[, $result] = \OC_Helper::streamCopy($source, $target);
}
} finally {
if (is_resource($source)) {
if (isset($source) && $source !== false) {
fclose($source);
}
if (is_resource($target)) {
if (isset($target) && $target !== false) {
fclose($target);
}
}
@ -728,6 +733,9 @@ class Encryption extends Wrapper {
public function hash(string $type, string $path, bool $raw = false): string|false {
$fh = $this->fopen($path, 'rb');
if ($fh === false) {
return false;
}
$ctx = hash_init($type);
hash_update_stream($ctx, $fh);
fclose($fh);
@ -752,6 +760,9 @@ class Encryption extends Wrapper {
$firstBlock = '';
if ($this->storage->is_file($path)) {
$handle = $this->storage->fopen($path, 'r');
if ($handle === false) {
return '';
}
$firstBlock = fread($handle, $this->util->getHeaderSize());
fclose($handle);
}
@ -882,6 +893,9 @@ class Encryption extends Wrapper {
public function writeStream(string $path, $stream, ?int $size = null): int {
// always fall back to fopen
$target = $this->fopen($path, 'w');
if ($target === false) {
throw new GenericFileException("Failed to open $path for writing");
}
[$count, $result] = \OC_Helper::streamCopy($stream, $target);
fclose($stream);
fclose($target);