aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--apps/encryption/lib/Crypto/EncryptAll.php30
-rw-r--r--apps/encryption/tests/Crypto/EncryptAllTest.php35
-rw-r--r--lib/private/Files/Storage/Wrapper/Encryption.php48
3 files changed, 80 insertions, 33 deletions
diff --git a/apps/encryption/lib/Crypto/EncryptAll.php b/apps/encryption/lib/Crypto/EncryptAll.php
index 310f37aba83..2cf7b51cdbf 100644
--- a/apps/encryption/lib/Crypto/EncryptAll.php
+++ b/apps/encryption/lib/Crypto/EncryptAll.php
@@ -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)) {
diff --git a/apps/encryption/tests/Crypto/EncryptAllTest.php b/apps/encryption/tests/Crypto/EncryptAllTest.php
index a63b826bf3c..5ec0c1baf86 100644
--- a/apps/encryption/tests/Crypto/EncryptAllTest.php
+++ b/apps/encryption/tests/Crypto/EncryptAllTest.php
@@ -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'])
diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php
index 265a554b240..7cbfa8a126a 100644
--- a/lib/private/Files/Storage/Wrapper/Encryption.php
+++ b/lib/private/Files/Storage/Wrapper/Encryption.php
@@ -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);