Merge pull request #53699 from nextcloud/backport/53665/stable30

[stable30] fix(encryption): Catch exceptions in encrypt-all command and continue
This commit is contained in:
Ferdinand Thiessen 2025-07-03 16:59:22 +02:00 committed by GitHub
commit a37bbc51c0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 80 additions and 33 deletions

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;
@ -82,7 +83,8 @@ class EncryptAll {
IL10N $l,
IFactory $l10nFactory,
QuestionHelper $questionHelper,
ISecureRandom $secureRandom
ISecureRandom $secureRandom,
protected LoggerInterface $logger,
) {
$this->userSetup = $userSetup;
$this->userManager = $userManager;
@ -251,9 +253,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,
]
);
}
}
}
@ -278,7 +293,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)) {

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;
@ -71,6 +73,8 @@ class EncryptAllTest extends TestCase {
/** @var \PHPUnit\Framework\MockObject\MockObject | \OCP\Security\ISecureRandom */
protected $secureRandom;
protected LoggerInterface&MockObject $logger;
/** @var EncryptAll */
protected $encryptAll;
@ -101,6 +105,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 +136,13 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
);
}
public function testEncryptAll() {
/** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */
public function testEncryptAll(): void {
/** @var EncryptAll&MockObject $encryptAll */
$encryptAll = $this->getMockBuilder(EncryptAll::class)
->setConstructorArgs(
[
@ -150,7 +156,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords'])
@ -164,8 +171,8 @@ class EncryptAllTest extends TestCase {
$encryptAll->encryptAll($this->inputInterface, $this->outputInterface);
}
public function testEncryptAllWithMasterKey() {
/** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */
public function testEncryptAllWithMasterKey(): void {
/** @var EncryptAll&MockObject $encryptAll */
$encryptAll = $this->getMockBuilder(EncryptAll::class)
->setConstructorArgs(
[
@ -179,7 +186,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['createKeyPairs', 'encryptAllUsersFiles', 'outputPasswords'])
@ -194,8 +202,8 @@ class EncryptAllTest extends TestCase {
$encryptAll->encryptAll($this->inputInterface, $this->outputInterface);
}
public function testCreateKeyPairs() {
/** @var EncryptAll | \PHPUnit\Framework\MockObject\MockObject $encryptAll */
public function testCreateKeyPairs(): void {
/** @var EncryptAll&MockObject $encryptAll */
$encryptAll = $this->getMockBuilder(EncryptAll::class)
->setConstructorArgs(
[
@ -209,7 +217,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['setupUserFS', 'generateOneTimePassword'])
@ -259,7 +268,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['encryptUsersFiles'])
@ -295,7 +305,8 @@ class EncryptAllTest extends TestCase {
$this->l,
$this->l10nFactory,
$this->questionHelper,
$this->secureRandom
$this->secureRandom,
$this->logger,
]
)
->setMethods(['encryptFile', 'setupUserFS'])

View file

@ -24,6 +24,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;
@ -265,11 +266,11 @@ class Encryption extends Wrapper {
public function rename($source, $target) {
$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);
@ -296,9 +297,9 @@ class Encryption extends Wrapper {
public function rmdir($path) {
$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);
}
@ -317,9 +318,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);
@ -493,9 +494,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
@ -764,8 +765,8 @@ class Encryption extends Wrapper {
) {
// 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
@ -815,12 +816,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);
}
}
@ -870,6 +875,9 @@ class Encryption extends Wrapper {
public function hash($type, $path, $raw = false) {
$fh = $this->fopen($path, 'rb');
if ($fh === false) {
return false;
}
$ctx = hash_init($type);
hash_update_stream($ctx, $fh);
fclose($fh);
@ -897,6 +905,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);
}
@ -1046,6 +1057,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);