mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Keep "encryptedVersion" when calling `\OC\Files\View::copy` 

When calling `\OC\Files\View::copy` we should also keep the version to ensure that the file will always have the correct version attached and can be successfully decrypted.

To test this the following steps are necessary (from https://github.com/owncloud/core/issues/22781#issuecomment-191328982):

1. setup a new ownCloud 9.0 beta2
2. enable encryption
2. upload a docx (5.7MB large)
3. upload the same file again and overwrite the existing file
4. I can download the original file and the first version
5. I restore the first version
6. restored version can no longer be downloaded with the error described above

The manual cache operation in `\OCA\Files_Versions\Storage` is unfortunately necessary since `\OCA\Files_Versions\Storage::copyFileContents` is not using `\OCP\Files\Storage::moveFromStorage` in the case when an object storage is used. Due to the workaround added in 54cea05271 the stream is directly copied and thus bypassing the FS.
This commit is contained in:
Lukas Reschke 2016-03-02 20:37:13 +01:00
parent 4f25f34178
commit 72c8187cbb
3 changed files with 61 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
apps/files_versions/lib/storage.php
View File

@ -191,12 +191,7 @@ class Storage {
$mtime = $users_view->filemtime('files/' . $filename);
$users_view->copy('files/' . $filename, 'files_versions/' . $filename . '.v' . $mtime);
// call getFileInfo to enforce a file cache entry for the new version
$newFileInfo = $users_view->getFileInfo('files_versions/' . $filename . '.v' . $mtime);
// Keep the "encrypted" value of the original file
$oldVersion = $files_view->getFileInfo($filename)->getEncryptedVersion();
$cache = $newFileInfo->getStorage()->getCache();
$cache->update($newFileInfo->getId(), ['encrypted' => $oldVersion, 'encryptedVersion' => $oldVersion]);
$users_view->getFileInfo('files_versions/' . $filename . '.v' . $mtime);
}
}
@ -332,14 +327,21 @@ class Storage {
//first create a new version
$version = 'files_versions'.$filename.'.v'.$users_view->filemtime('files'.$filename);
if (!$users_view->file_exists($version)) {
$users_view->copy('files'.$filename, 'files_versions'.$filename.'.v'.$users_view->filemtime('files'.$filename));
$versionCreated = true;
}
$fileToRestore = 'files_versions' . $filename . '.v' . $revision;
// Restore encrypted version of the old file for the newly restored file
// This has to happen manually here since the file is manually copied below
$oldVersion = $users_view->getFileInfo($fileToRestore)->getEncryptedVersion();
$newFileInfo = $files_view->getFileInfo($filename);
$cache = $newFileInfo->getStorage()->getCache();
$cache->update($newFileInfo->getId(), ['encrypted' => $oldVersion, 'encryptedVersion' => $oldVersion]);
// rollback
if (self::copyFileContents($users_view, 'files_versions' . $filename . '.v' . $revision, 'files' . $filename)) {
if (self::copyFileContents($users_view, $fileToRestore, 'files' . $filename)) {
$files_view->touch($file, $revision);
Storage::scheduleExpire($uid, $file);
\OC_Hook::emit('\OCP\Versions', 'rollback', array(

37
lib/private/files/storage/wrapper/encryption.php
View File

@ -620,6 +620,32 @@ class Encryption extends Wrapper {
return $this->copyBetweenStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, false);
}
/**
* Update the encrypted cache version in the database
*
* @param Storage $sourceStorage
* @param string $sourceInternalPath
* @param string $targetInternalPath
* @param bool $isRename
*/
private function updateEncryptedVersion(Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename) {
$isEncrypted = $this->encryptionManager->isEnabled() && $this->mount->getOption('encrypt', true) ? 1 : 0;
$cacheInformation = [
'encrypted' => (bool)$isEncrypted,
];
if($isEncrypted === 1) {
$cacheInformation['encryptedVersion'] = $sourceStorage->getCache()->get($sourceInternalPath)['encryptedVersion'];
}
// in case of a rename we need to manipulate the source cache because
// this information will be kept for the new target
if ($isRename) {
$sourceStorage->getCache()->put($sourceInternalPath, $cacheInformation);
} else {
$this->getCache()->put($targetInternalPath, $cacheInformation);
}
}
/**
* copy file between two storages
*
@ -647,6 +673,7 @@ class Encryption extends Wrapper {
$info['size']
);
}
$this->updateEncryptedVersion($sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename);
}
return $result;
}
@ -689,15 +716,7 @@ class Encryption extends Wrapper {
if ($preserveMtime) {
$this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath));
}
$isEncrypted = $this->encryptionManager->isEnabled() && $this->mount->getOption('encrypt', true) ? 1 : 0;
// in case of a rename we need to manipulate the source cache because
// this information will be kept for the new target
if ($isRename) {
$sourceStorage->getCache()->put($sourceInternalPath, ['encrypted' => $isEncrypted]);
} else {
$this->getCache()->put($targetInternalPath, ['encrypted' => $isEncrypted]);
}
$this->updateEncryptedVersion($sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename);
} else {
// delete partially written target file
$this->unlink($targetInternalPath);

27
tests/lib/files/storage/wrapper/encryption.php
View File

@ -693,11 +693,19 @@ class Encryption extends Storage {
$temp = \OC::$server->getTempManager();
return fopen($temp->getTemporaryFile(), $mode);
});
if($expectedEncrypted) {
$cache = $this->getMock('\OCP\Files\Cache\ICache');
$cache->expects($this->once())
->method('get')
->with($sourceInternalPath)
->willReturn(['encryptedVersion' => 12345]);
$storage2->expects($this->once())
->method('getCache')
->willReturn($cache);
}
$this->encryptionManager->expects($this->any())
->method('isEnabled')
->willReturn($encryptionEnabled);
// FIXME can not overwrite the return after definition
// $this->mount->expects($this->at(0))
// ->method('getOption')
@ -706,9 +714,16 @@ class Encryption extends Storage {
global $mockedMountPointEncryptionEnabled;
$mockedMountPointEncryptionEnabled = $mountPointEncryptionEnabled;
$expectedCachePut = [
'encrypted' => $expectedEncrypted,
];
if($expectedEncrypted === true) {
$expectedCachePut['encryptedVersion'] = 12345;
}
$this->cache->expects($this->once())
->method('put')
->with($sourceInternalPath, ['encrypted' => $expectedEncrypted]);
->with($sourceInternalPath, $expectedCachePut);
$this->invokePrivate($this->instance, 'copyBetweenStorage', [$storage2, $sourceInternalPath, $targetInternalPath, $preserveMtime, $isRename]);
@ -765,10 +780,10 @@ class Encryption extends Storage {
->with($sourceStorage, $sourceInternalPath, $targetInternalPath)
->willReturn($copyResult);
if ($copyResult) {
$instance->expects($this->once())->method('getCache')
->with('', $sourceStorage)
$instance->expects($this->any())->method('getCache')
->willReturn($cache);
if ($copyResult) {
$cache->expects($this->once())->method('get')
->with($sourceInternalPath)
->willReturn(['encrypted' => $encrypted, 'size' => 42]);