From d25b8dacb36dd251bd7002930a9ce6ba6a50b7a6 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 4 Jan 2016 21:00:55 +0100 Subject: [PATCH 01/20] Use AES-256-CTR as default CTR is recommended over CFB mode. --- apps/encryption/lib/crypto/crypt.php | 60 ++++++++++++------- .../encryption/tests/lib/crypto/cryptTest.php | 34 +++++++++-- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index e387380cd9..4bed565d02 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -36,9 +36,21 @@ use OCP\IConfig; use OCP\ILogger; use OCP\IUserSession; +/** + * Class Crypt provides the encryption implementation of the default ownCloud + * encryption module. As default AES-256-CTR is used, it does however offer support + * for the following modes: + * + * - AES-256-CTR + * - AES-128-CTR + * - AES-256-CFB + * - AES-128-CFB + * + * @package OCA\Encryption\Crypto + */ class Crypt { - const DEFAULT_CIPHER = 'AES-256-CFB'; + const DEFAULT_CIPHER = 'AES-256-CTR'; // default cipher from old ownCloud versions const LEGACY_CIPHER = 'AES-128-CFB'; @@ -48,23 +60,21 @@ class Crypt { const HEADER_START = 'HBEGIN'; const HEADER_END = 'HEND'; - /** - * @var ILogger - */ + /** @var ILogger */ private $logger; - /** - * @var string - */ + /** @var string */ private $user; - /** - * @var IConfig - */ + /** @var IConfig */ private $config; - - /** - * @var array - */ + /** @var array */ private $supportedKeyFormats; + /** @var array */ + private $supportedCiphersAndKeySize = [ + 'AES-256-CTR' => 32, + 'AES-128-CTR' => 16, + 'AES-256-CFB' => 32, + 'AES-128-CFB' => 16, + ]; /** * @param ILogger $logger @@ -225,8 +235,13 @@ class Crypt { */ public function getCipher() { $cipher = $this->config->getSystemValue('cipher', self::DEFAULT_CIPHER); - if ($cipher !== 'AES-256-CFB' && $cipher !== 'AES-128-CFB') { - $this->logger->warning('Wrong cipher defined in config.php only AES-128-CFB and AES-256-CFB are supported. Fall back' . self::DEFAULT_CIPHER, + if (!isset($this->supportedCiphersAndKeySize[$cipher])) { + $this->logger->warning( + sprintf( + 'Unsupported cipher (%s) defined in config.php supported. Falling back to %s', + $cipher, + self::DEFAULT_CIPHER + ), ['app' => 'encryption']); $cipher = self::DEFAULT_CIPHER; } @@ -237,19 +252,20 @@ class Crypt { /** * get key size depending on the cipher * - * @param string $cipher supported ('AES-256-CFB' and 'AES-128-CFB') + * @param string $cipher * @return int * @throws \InvalidArgumentException */ protected function getKeySize($cipher) { - if ($cipher === 'AES-256-CFB') { - return 32; - } else if ($cipher === 'AES-128-CFB') { - return 16; + if(isset($this->supportedCiphersAndKeySize[$cipher])) { + return $this->supportedCiphersAndKeySize[$cipher]; } throw new \InvalidArgumentException( - 'Wrong cipher defined only AES-128-CFB and AES-256-CFB are supported.' + sprintf( + 'Unsupported cipher (%s) defined.', + $cipher + ) ); } diff --git a/apps/encryption/tests/lib/crypto/cryptTest.php b/apps/encryption/tests/lib/crypto/cryptTest.php index e599cc2896..ce4dfb68cf 100644 --- a/apps/encryption/tests/lib/crypto/cryptTest.php +++ b/apps/encryption/tests/lib/crypto/cryptTest.php @@ -105,7 +105,7 @@ class cryptTest extends TestCase { $this->config->expects($this->once()) ->method('getSystemValue') - ->with($this->equalTo('cipher'), $this->equalTo('AES-256-CFB')) + ->with($this->equalTo('cipher'), $this->equalTo('AES-256-CTR')) ->willReturn('AES-128-CFB'); if ($keyFormat) { @@ -126,6 +126,9 @@ class cryptTest extends TestCase { $this->crypt->generateHeader('unknown'); } + /** + * @return array + */ public function dataTestGenerateHeader() { return [ [null, 'HBEGIN:cipher:AES-128-CFB:keyFormat:hash:HEND'], @@ -134,16 +137,28 @@ class cryptTest extends TestCase { ]; } + public function testGetCipherWithInvalidCipher() { + $this->config->expects($this->once()) + ->method('getSystemValue') + ->with($this->equalTo('cipher'), $this->equalTo('AES-256-CTR')) + ->willReturn('Not-Existing-Cipher'); + $this->logger + ->expects($this->once()) + ->method('warning') + ->with('Unsupported cipher (Not-Existing-Cipher) defined in config.php supported. Falling back to AES-256-CTR'); + + $this->assertSame('AES-256-CTR', $this->crypt->getCipher()); + } + /** * @dataProvider dataProviderGetCipher * @param string $configValue * @param string $expected */ public function testGetCipher($configValue, $expected) { - $this->config->expects($this->once()) ->method('getSystemValue') - ->with($this->equalTo('cipher'), $this->equalTo('AES-256-CFB')) + ->with($this->equalTo('cipher'), $this->equalTo('AES-256-CTR')) ->willReturn($configValue); $this->assertSame($expected, @@ -161,7 +176,10 @@ class cryptTest extends TestCase { return array( array('AES-128-CFB', 'AES-128-CFB'), array('AES-256-CFB', 'AES-256-CFB'), - array('unknown', 'AES-256-CFB') + array('AES-128-CTR', 'AES-128-CTR'), + array('AES-256-CTR', 'AES-256-CTR'), + + array('unknown', 'AES-256-CTR') ); } @@ -303,10 +321,15 @@ class cryptTest extends TestCase { $this->invokePrivate($this->crypt, 'getKeySize', ['foo']); } + /** + * @return array + */ public function dataTestGetKeySize() { return [ ['AES-256-CFB', 32], ['AES-128-CFB', 16], + ['AES-256-CTR', 32], + ['AES-128-CTR', 16], ]; } @@ -351,6 +374,9 @@ class cryptTest extends TestCase { $this->assertSame($expected, $result); } + /** + * @return array + */ public function dataTestDecryptPrivateKey() { return [ [['cipher' => 'AES-128-CFB', 'keyFormat' => 'password'], 'HBEGIN:HENDprivateKey', 'AES-128-CFB', true, 'key'], From 59ebad0b538ae173f9781cec0f524c8ca407a181 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 4 Jan 2016 23:06:23 +0100 Subject: [PATCH 02/20] Use an actual 16 byte long IV The previous IV was actually 12 byte extended to 16 byte using base64. As the encrypted file should be fine with containing binary data as well we can simply remove the encoding like that here. --- apps/encryption/lib/crypto/crypt.php | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index 4bed565d02..ffb64d640d 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -156,7 +156,7 @@ class Crypt { * @param string $plainContent * @param string $passPhrase * @return false|string - * @throws GenericEncryptionException + * @throws EncryptionFailedException */ public function symmetricEncryptFileContent($plainContent, $passPhrase) { @@ -512,22 +512,7 @@ class Crypt { * @throws GenericEncryptionException */ private function generateIv() { - $random = openssl_random_pseudo_bytes(12, $strong); - if ($random) { - if (!$strong) { - // If OpenSSL indicates randomness is insecure log error - $this->logger->error('Encryption Library: Insecure symmetric key was generated using openssl_random_psudo_bytes()', - ['app' => 'encryption']); - } - - /* - * We encode the iv purely for string manipulation - * purposes -it gets decoded before use - */ - return base64_encode($random); - } - // If we ever get here we've failed anyway no need for an else - throw new GenericEncryptionException('Generating IV Failed'); + return random_bytes(16); } /** From db8f267647a8d6682f132f8d6dc1334daf90fa4e Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 4 Jan 2016 23:10:51 +0100 Subject: [PATCH 03/20] Add note about the addPadding function --- apps/encryption/lib/crypto/crypt.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index ffb64d640d..1cbf39a82b 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -288,6 +288,10 @@ class Crypt { } /** + * Note: This is _NOT_ a padding used for encryption purposes. It is solely + * used to achieve the PHP stream size. It has _NOTHING_ to do with the + * encrypted content and is not used in any crypto primitive. + * * @param string $data * @return string */ From 40a5ba72fc868207356c9143c99a947f1a6e6500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 5 Jan 2016 12:51:05 +0100 Subject: [PATCH 04/20] sign all encrypted blocks and check signature on decrypt --- apps/encryption/appinfo/application.php | 3 +- apps/encryption/appinfo/register_command.php | 3 +- apps/encryption/lib/crypto/crypt.php | 131 +++++++++++++++--- apps/encryption/lib/crypto/encryption.php | 13 +- apps/encryption/settings/settings-admin.php | 3 +- .../encryption/settings/settings-personal.php | 3 +- .../encryption/tests/lib/crypto/cryptTest.php | 7 +- settings/changepassword/controller.php | 3 +- 8 files changed, 133 insertions(+), 33 deletions(-) diff --git a/apps/encryption/appinfo/application.php b/apps/encryption/appinfo/application.php index 433e9e8628..6d01d3e835 100644 --- a/apps/encryption/appinfo/application.php +++ b/apps/encryption/appinfo/application.php @@ -131,7 +131,8 @@ class Application extends \OCP\AppFramework\App { $server = $c->getServer(); return new Crypt($server->getLogger(), $server->getUserSession(), - $server->getConfig()); + $server->getConfig(), + $server->getL10N($c->getAppName())); }); $container->registerService('Session', diff --git a/apps/encryption/appinfo/register_command.php b/apps/encryption/appinfo/register_command.php index 2bb49d55c2..5f32718cdf 100644 --- a/apps/encryption/appinfo/register_command.php +++ b/apps/encryption/appinfo/register_command.php @@ -25,11 +25,12 @@ use Symfony\Component\Console\Helper\QuestionHelper; $userManager = OC::$server->getUserManager(); $view = new \OC\Files\View(); $config = \OC::$server->getConfig(); +$l = \OC::$server->getL10N('encryption'); $userSession = \OC::$server->getUserSession(); $connection = \OC::$server->getDatabaseConnection(); $logger = \OC::$server->getLogger(); $questionHelper = new QuestionHelper(); -$crypt = new \OCA\Encryption\Crypto\Crypt($logger, $userSession, $config); +$crypt = new \OCA\Encryption\Crypto\Crypt($logger, $userSession, $config, $l); $util = new \OCA\Encryption\Util($view, $crypt, $logger, $userSession, $config, $userManager); $application->add(new MigrateKeys($userManager, $view, $connection, $config, $logger)); diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index 1cbf39a82b..f4c47d33f0 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -29,10 +29,12 @@ namespace OCA\Encryption\Crypto; use OC\Encryption\Exceptions\DecryptionFailedException; use OC\Encryption\Exceptions\EncryptionFailedException; +use OC\HintException; use OCA\Encryption\Exceptions\MultiKeyDecryptException; use OCA\Encryption\Exceptions\MultiKeyEncryptException; use OCP\Encryption\Exceptions\GenericEncryptionException; use OCP\IConfig; +use OCP\IL10N; use OCP\ILogger; use OCP\IUserSession; @@ -60,14 +62,22 @@ class Crypt { const HEADER_START = 'HBEGIN'; const HEADER_END = 'HEND'; + /** @var ILogger */ private $logger; + /** @var string */ private $user; + /** @var IConfig */ private $config; + /** @var array */ private $supportedKeyFormats; + + /** @var IL10N */ + private $l; + /** @var array */ private $supportedCiphersAndKeySize = [ 'AES-256-CTR' => 32, @@ -80,11 +90,13 @@ class Crypt { * @param ILogger $logger * @param IUserSession $userSession * @param IConfig $config + * @param IL10N $l */ - public function __construct(ILogger $logger, IUserSession $userSession, IConfig $config) { + public function __construct(ILogger $logger, IUserSession $userSession, IConfig $config, IL10N $l) { $this->logger = $logger; $this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : '"no user given"'; $this->config = $config; + $this->l = $l; $this->supportedKeyFormats = ['hash', 'password']; } @@ -172,8 +184,12 @@ class Crypt { $iv, $passPhrase, $this->getCipher()); + + $sig = $this->createSignature($encryptedContent, $passPhrase); + // combine content to encrypt the IV identifier and actual IV $catFile = $this->concatIV($encryptedContent, $iv); + $catFile = $this->concatSig($catFile, $sig); $padded = $this->addPadding($catFile); return $padded; @@ -287,6 +303,15 @@ class Crypt { return $encryptedContent . '00iv00' . $iv; } + /** + * @param string $encryptedContent + * @param string $signature + * @return string + */ + private function concatSig($encryptedContent, $signature) { + return $encryptedContent . '00sig00' . $signature; + } + /** * Note: This is _NOT_ a padding used for encryption purposes. It is solely * used to achieve the PHP stream size. It has _NOTHING_ to do with the @@ -296,7 +321,7 @@ class Crypt { * @return string */ private function addPadding($data) { - return $data . 'xx'; + return $data . 'xxx'; } /** @@ -414,10 +439,12 @@ class Crypt { * @throws DecryptionFailedException */ public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER) { - // Remove Padding - $noPadding = $this->removePadding($keyFileContents); - $catFile = $this->splitIv($noPadding); + $catFile = $this->splitMetaData($keyFileContents, $cipher); + + if ($catFile['signature']) { + $this->checkSignature($catFile['encrypted'], $passPhrase, $catFile['signature']); + } return $this->decrypt($catFile['encrypted'], $catFile['iv'], @@ -425,42 +452,102 @@ class Crypt { $cipher); } + /** + * check for valid signature + * + * @param string $data + * @param string $passPhrase + * @param string $expectedSignature + * @throws HintException + */ + private function checkSignature($data, $passPhrase, $expectedSignature) { + $signature = $this->createSignature($data, $passPhrase); + if (hash_equals($expectedSignature, $signature)) { + throw new HintException('Bad Signature', $this->l->t('Bad Signature')); + } + } + + /** + * create signature + * + * @param string $data + * @param string $passPhrase + * @return string + */ + private function createSignature($data, $passPhrase) { + $signature = hash_hmac('sha256', $data, $passPhrase); + return $signature; + } + + /** * remove padding * - * @param $padded + * @param string $padded + * @param bool $hasSignature did the block contain a signature, in this case we use a different padding * @return string|false */ - private function removePadding($padded) { - if (substr($padded, -2) === 'xx') { + private function removePadding($padded, $hasSignature = false) { + if ($hasSignature === false && substr($padded, -2) === 'xx') { return substr($padded, 0, -2); + } elseif ($hasSignature === true && substr($padded, -3) === 'xxx') { + return substr($padded, 0, -3); } return false; } /** - * split iv from encrypted content + * split meta data from encrypted file + * Note: for now, we assume that the meta data always start with the iv + * followed by the signature, if available * - * @param string|false $catFile - * @return string + * @param string $catFile + * @param string $cipher + * @return array */ - private function splitIv($catFile) { - // Fetch encryption metadata from end of file - $meta = substr($catFile, -22); - - // Fetch IV from end of file - $iv = substr($meta, -16); - - // Remove IV and IV Identifier text to expose encrypted content - - $encrypted = substr($catFile, 0, -22); + private function splitMetaData($catFile, $cipher) { + if ($this->hasSignature($catFile, $cipher)) { + $catFile = $this->removePadding($catFile, true); + $meta = substr($catFile, -93); + $iv = substr($meta, strlen('00iv00'), 16); + $sig = substr($meta, 22 + strlen('00sig00')); + $encrypted = substr($catFile, 0, -93); + } else { + $catFile = $this->removePadding($catFile); + $meta = substr($catFile, -22); + $iv = substr($meta, -16); + $sig = false; + $encrypted = substr($catFile, 0, -93); + } return [ 'encrypted' => $encrypted, - 'iv' => $iv + 'iv' => $iv, + 'signature' => $sig ]; } + /** + * check if encrypted block is signed + * + * @param string $catFile + * @param string $cipher + * @return bool + * @throws HintException + */ + private function hasSignature($catFile, $cipher) { + $meta = substr($catFile, 93); + $signaturePosition = strpos($meta, '00sig00'); + + // enforce signature for the new 'CTR' ciphers + if ($signaturePosition === false && strpos(strtolower($cipher), 'ctr') !== false) { + throw new HintException('Missing Signature', $this->l->t('Missing Signature')); + } + + return ($signaturePosition !== false); + } + + /** * @param string $encryptedContent * @param string $iv diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index 3b66684a7f..7099f53e2a 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -94,6 +94,9 @@ class Encryption implements IEncryptionModule { /** @var DecryptAll */ private $decryptAll; + /** @var int unencrypted block size */ + private $unencryptedBlockSize = 6072; + /** * * @param Crypt $crypt @@ -253,7 +256,7 @@ class Encryption implements IEncryptionModule { public function encrypt($data) { // If extra data is left over from the last round, make sure it - // is integrated into the next 6126 / 8192 block + // is integrated into the next block if ($this->writeCache) { // Concat writeCache to start of $data @@ -275,7 +278,7 @@ class Encryption implements IEncryptionModule { // If data remaining to be written is less than the // size of 1 6126 byte block - if ($remainingLength < 6126) { + if ($remainingLength < $this->unencryptedBlockSize) { // Set writeCache to contents of $data // The writeCache will be carried over to the @@ -293,14 +296,14 @@ class Encryption implements IEncryptionModule { } else { // Read the chunk from the start of $data - $chunk = substr($data, 0, 6126); + $chunk = substr($data, 0, $this->unencryptedBlockSize); $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey); // Remove the chunk we just processed from // $data, leaving only unprocessed data in $data // var, for handling on the next round - $data = substr($data, 6126); + $data = substr($data, $this->unencryptedBlockSize); } @@ -410,7 +413,7 @@ class Encryption implements IEncryptionModule { * @return integer */ public function getUnencryptedBlockSize() { - return 6126; + return $this->unencryptedBlockSize; } /** diff --git a/apps/encryption/settings/settings-admin.php b/apps/encryption/settings/settings-admin.php index c3d523f27d..6c7c0987fd 100644 --- a/apps/encryption/settings/settings-admin.php +++ b/apps/encryption/settings/settings-admin.php @@ -29,7 +29,8 @@ $tmpl = new OCP\Template('encryption', 'settings-admin'); $crypt = new \OCA\Encryption\Crypto\Crypt( \OC::$server->getLogger(), \OC::$server->getUserSession(), - \OC::$server->getConfig()); + \OC::$server->getConfig(), + \OC::$server->getL10N('encryption')); $util = new \OCA\Encryption\Util( new \OC\Files\View(), diff --git a/apps/encryption/settings/settings-personal.php b/apps/encryption/settings/settings-personal.php index 2dff590485..0f6e935370 100644 --- a/apps/encryption/settings/settings-personal.php +++ b/apps/encryption/settings/settings-personal.php @@ -28,7 +28,8 @@ $template = new OCP\Template('encryption', 'settings-personal'); $crypt = new \OCA\Encryption\Crypto\Crypt( \OC::$server->getLogger(), $userSession, - \OC::$server->getConfig()); + \OC::$server->getConfig(), + \OC::$server->getL10N('encryption')); $util = new \OCA\Encryption\Util( new \OC\Files\View(), diff --git a/apps/encryption/tests/lib/crypto/cryptTest.php b/apps/encryption/tests/lib/crypto/cryptTest.php index ce4dfb68cf..c774da1836 100644 --- a/apps/encryption/tests/lib/crypto/cryptTest.php +++ b/apps/encryption/tests/lib/crypto/cryptTest.php @@ -39,6 +39,10 @@ class cryptTest extends TestCase { /** @var \PHPUnit_Framework_MockObject_MockObject */ private $config; + + /** @var \PHPUnit_Framework_MockObject_MockObject */ + private $l; + /** @var Crypt */ private $crypt; @@ -57,8 +61,9 @@ class cryptTest extends TestCase { $this->config = $this->getMockBuilder('OCP\IConfig') ->disableOriginalConstructor() ->getMock(); + $this->l = $this->getMock('OCP\IL10N'); - $this->crypt = new Crypt($this->logger, $this->userSession, $this->config); + $this->crypt = new Crypt($this->logger, $this->userSession, $this->config, $this->l); } /** diff --git a/settings/changepassword/controller.php b/settings/changepassword/controller.php index bfa599c1d0..8469ec1423 100644 --- a/settings/changepassword/controller.php +++ b/settings/changepassword/controller.php @@ -89,7 +89,8 @@ class Controller { $crypt = new \OCA\Encryption\Crypto\Crypt( \OC::$server->getLogger(), \OC::$server->getUserSession(), - \OC::$server->getConfig()); + \OC::$server->getConfig(), + \OC::$server->getL10N('encryption')); $keyStorage = \OC::$server->getEncryptionKeyStorage(); $util = new \OCA\Encryption\Util( new \OC\Files\View(), From cf3a8f274f05170b69cb3872d0d8c9045a5876d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 5 Jan 2016 15:29:44 +0100 Subject: [PATCH 05/20] make it backward compatible to work with signed and un-signed files --- apps/encryption/lib/crypto/crypt.php | 4 +-- apps/encryption/lib/crypto/encryption.php | 25 +++++++++++++------ .../files/storage/wrapper/encryption.php | 6 +++-- lib/private/files/stream/encryption.php | 10 +++++--- lib/public/encryption/iencryptionmodule.php | 7 +++--- 5 files changed, 34 insertions(+), 18 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index f4c47d33f0..e935f36455 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -462,7 +462,7 @@ class Crypt { */ private function checkSignature($data, $passPhrase, $expectedSignature) { $signature = $this->createSignature($data, $passPhrase); - if (hash_equals($expectedSignature, $signature)) { + if (!hash_equals($expectedSignature, $signature)) { throw new HintException('Bad Signature', $this->l->t('Bad Signature')); } } @@ -517,7 +517,7 @@ class Crypt { $meta = substr($catFile, -22); $iv = substr($meta, -16); $sig = false; - $encrypted = substr($catFile, 0, -93); + $encrypted = substr($catFile, 0, -22); } return [ diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index 7099f53e2a..4843284f7a 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -94,8 +94,12 @@ class Encryption implements IEncryptionModule { /** @var DecryptAll */ private $decryptAll; + /** @var int unencrypted block size if block contains signature */ + private $unencryptedBlockSizeSigned = 6072; + /** @var int unencrypted block size */ - private $unencryptedBlockSize = 6072; + private $unencryptedBlockSize = 6126; + /** * @@ -198,7 +202,7 @@ class Encryption implements IEncryptionModule { $this->cipher = $this->crypt->getLegacyCipher(); } - return array('cipher' => $this->cipher); + return array('cipher' => $this->cipher, 'signed' => 'true'); } /** @@ -278,7 +282,7 @@ class Encryption implements IEncryptionModule { // If data remaining to be written is less than the // size of 1 6126 byte block - if ($remainingLength < $this->unencryptedBlockSize) { + if ($remainingLength < $this->unencryptedBlockSizeSigned) { // Set writeCache to contents of $data // The writeCache will be carried over to the @@ -296,14 +300,14 @@ class Encryption implements IEncryptionModule { } else { // Read the chunk from the start of $data - $chunk = substr($data, 0, $this->unencryptedBlockSize); + $chunk = substr($data, 0, $this->unencryptedBlockSizeSigned); $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey); // Remove the chunk we just processed from // $data, leaving only unprocessed data in $data // var, for handling on the next round - $data = substr($data, $this->unencryptedBlockSize); + $data = substr($data, $this->unencryptedBlockSizeSigned); } @@ -410,10 +414,15 @@ class Encryption implements IEncryptionModule { * get size of the unencrypted payload per block. * ownCloud read/write files with a block size of 8192 byte * - * @return integer + * @param bool $signed + * @return int */ - public function getUnencryptedBlockSize() { - return $this->unencryptedBlockSize; + public function getUnencryptedBlockSize($signed = false) { + if ($signed === false) { + return $this->unencryptedBlockSize; + } + + return $this->unencryptedBlockSizeSigned; } /** diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index f358bd5923..96d642e778 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -343,6 +343,7 @@ class Encryption extends Wrapper { $shouldEncrypt = false; $encryptionModule = null; $header = $this->getHeader($path); + $signed = (isset($header['signed']) && $header['signed'] === 'true') ? true : false; $fullPath = $this->getFullPath($path); $encryptionModuleId = $this->util->getEncryptionModuleId($header); @@ -377,7 +378,7 @@ class Encryption extends Wrapper { || $mode === 'wb' || $mode === 'wb+' ) { - // don't overwrite encrypted files if encyption is not enabled + // don't overwrite encrypted files if encryption is not enabled if ($targetIsEncrypted && $encryptionEnabled === false) { throw new GenericEncryptionException('Tried to access encrypted file but encryption is not enabled'); } @@ -385,6 +386,7 @@ class Encryption extends Wrapper { // if $encryptionModuleId is empty, the default module will be used $encryptionModule = $this->encryptionManager->getEncryptionModule($encryptionModuleId); $shouldEncrypt = $encryptionModule->shouldEncrypt($fullPath); + $signed = true; } } else { $info = $this->getCache()->get($path); @@ -422,7 +424,7 @@ class Encryption extends Wrapper { } $handle = \OC\Files\Stream\Encryption::wrap($source, $path, $fullPath, $header, $this->uid, $encryptionModule, $this->storage, $this, $this->util, $this->fileHelper, $mode, - $size, $unencryptedSize, $headerSize); + $size, $unencryptedSize, $headerSize, $signed); return $handle; } diff --git a/lib/private/files/stream/encryption.php b/lib/private/files/stream/encryption.php index c884cd8fa0..11e2b218d1 100644 --- a/lib/private/files/stream/encryption.php +++ b/lib/private/files/stream/encryption.php @@ -110,7 +110,8 @@ class Encryption extends Wrapper { 'size', 'unencryptedSize', 'encryptionStorage', - 'headerSize' + 'headerSize', + 'signed' ); } @@ -132,6 +133,7 @@ class Encryption extends Wrapper { * @param int $size * @param int $unencryptedSize * @param int $headerSize + * @param bool $signed * @param string $wrapper stream wrapper class * @return resource * @@ -148,6 +150,7 @@ class Encryption extends Wrapper { $size, $unencryptedSize, $headerSize, + $signed, $wrapper = 'OC\Files\Stream\Encryption') { $context = stream_context_create(array( @@ -164,7 +167,8 @@ class Encryption extends Wrapper { 'size' => $size, 'unencryptedSize' => $unencryptedSize, 'encryptionStorage' => $encStorage, - 'headerSize' => $headerSize + 'headerSize' => $headerSize, + 'signed' => $signed ) )); @@ -225,7 +229,7 @@ class Encryption extends Wrapper { $this->position = 0; $this->cache = ''; $this->writeFlag = false; - $this->unencryptedBlockSize = $this->encryptionModule->getUnencryptedBlockSize(); + $this->unencryptedBlockSize = $this->encryptionModule->getUnencryptedBlockSize($this->signed); if ( $mode === 'w' diff --git a/lib/public/encryption/iencryptionmodule.php b/lib/public/encryption/iencryptionmodule.php index 426e4ddecc..45e0b79c2a 100644 --- a/lib/public/encryption/iencryptionmodule.php +++ b/lib/public/encryption/iencryptionmodule.php @@ -119,10 +119,11 @@ interface IEncryptionModule { * get size of the unencrypted payload per block. * ownCloud read/write files with a block size of 8192 byte * - * @return integer - * @since 8.1.0 + * @param bool $signed + * @return int + * @since 8.1.0 optional parameter $signed was added in 9.0.0 */ - public function getUnencryptedBlockSize(); + public function getUnencryptedBlockSize($signed = false); /** * check if the encryption module is able to read the file, From e7ff84df5cac2a7917360c316f817f754ce8e5a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 5 Jan 2016 16:34:40 +0100 Subject: [PATCH 06/20] always use default cipher for write operations, no matter how the file was encrypted before --- apps/encryption/lib/crypto/encryption.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index 4843284f7a..dc60c09478 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -192,10 +192,10 @@ class Encryption implements IEncryptionModule { } } - if (isset($header['cipher'])) { - $this->cipher = $header['cipher']; - } elseif ($this->isWriteOperation) { + if ($this->isWriteOperation) { $this->cipher = $this->crypt->getCipher(); + } elseif (isset($header['cipher'])) { + $this->cipher = $header['cipher']; } else { // if we read a file without a header we fall-back to the legacy cipher // which was used in <=oC6 From 61dd191253fb3bead23143a503c9d25780f2e63d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 5 Jan 2016 16:55:58 +0100 Subject: [PATCH 07/20] meta data are at the end of the file --- apps/encryption/lib/crypto/crypt.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index e935f36455..e071819b16 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -536,7 +536,7 @@ class Crypt { * @throws HintException */ private function hasSignature($catFile, $cipher) { - $meta = substr($catFile, 93); + $meta = substr($catFile, -93); $signaturePosition = strpos($meta, '00sig00'); // enforce signature for the new 'CTR' ciphers From b9ff16498befe211205ea3c20c2eb341f97919fa Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 5 Jan 2016 17:08:57 +0100 Subject: [PATCH 08/20] Use random_bytes instead OpenSSL --- apps/encryption/lib/crypto/crypt.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index e071819b16..d5a41c2900 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -614,14 +614,7 @@ class Crypt { * @throws \Exception */ public function generateFileKey() { - // Generate key - $key = base64_encode(openssl_random_pseudo_bytes(32, $strong)); - if (!$key || !$strong) { - // If OpenSSL indicates randomness is insecure, log error - throw new \Exception('Encryption library, Insecure symmetric key was generated using openssl_random_pseudo_bytes()'); - } - - return $key; + return base64_encode(random_bytes(32)); } /** From 9bb97c714bb2158fd019ba9efc24a8bc8595b499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Schie=C3=9Fle?= Date: Tue, 5 Jan 2016 19:01:03 +0100 Subject: [PATCH 09/20] fixing unit tests --- apps/encryption/lib/crypto/crypt.php | 2 +- .../encryption/tests/lib/crypto/cryptTest.php | 63 ++++++++++++++++--- .../tests/lib/crypto/encryptionTest.php | 2 +- lib/private/files/stream/encryption.php | 3 + tests/lib/files/stream/encryption.php | 1 + 5 files changed, 60 insertions(+), 11 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index d5a41c2900..f27f55619a 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -442,7 +442,7 @@ class Crypt { $catFile = $this->splitMetaData($keyFileContents, $cipher); - if ($catFile['signature']) { + if ($catFile['signature'] !== false) { $this->checkSignature($catFile['encrypted'], $passPhrase, $catFile['signature']); } diff --git a/apps/encryption/tests/lib/crypto/cryptTest.php b/apps/encryption/tests/lib/crypto/cryptTest.php index c774da1836..d94aea463c 100644 --- a/apps/encryption/tests/lib/crypto/cryptTest.php +++ b/apps/encryption/tests/lib/crypto/cryptTest.php @@ -204,17 +204,61 @@ class cryptTest extends TestCase { } /** - * test splitIV() + * @dataProvider dataTestSplitMetaData */ - public function testSplitIV() { - $data = 'encryptedContent00iv001234567890123456'; - $result = self::invokePrivate($this->crypt, 'splitIV', array($data)); + public function testSplitMetaData($data, $expected) { + $result = self::invokePrivate($this->crypt, 'splitMetaData', array($data, 'AES-256-CFB')); $this->assertTrue(is_array($result)); - $this->assertSame(2, count($result)); + $this->assertSame(3, count($result)); $this->assertArrayHasKey('encrypted', $result); $this->assertArrayHasKey('iv', $result); - $this->assertSame('encryptedContent', $result['encrypted']); - $this->assertSame('1234567890123456', $result['iv']); + $this->assertArrayHasKey('signature', $result); + $this->assertSame($expected['encrypted'], $result['encrypted']); + $this->assertSame($expected['iv'], $result['iv']); + $this->assertSame($expected['signature'], $result['signature']); + } + + public function dataTestSplitMetaData() { + return [ + ['encryptedContent00iv001234567890123456xx', + ['encrypted' => 'encryptedContent', 'iv' => '1234567890123456', 'signature' => false]], + ['encryptedContent00iv00123456789012345600sig00e1992521e437f6915f9173b190a512cfc38a00ac24502db44e0ba10c2bb0cc86xxx', + ['encrypted' => 'encryptedContent', 'iv' => '1234567890123456', 'signature' => 'e1992521e437f6915f9173b190a512cfc38a00ac24502db44e0ba10c2bb0cc86']], + ]; + } + + /** + * @dataProvider dataTestHasSignature + */ + public function testHasSignature($data, $expected) { + $this->assertSame($expected, + $this->invokePrivate($this->crypt, 'hasSignature', array($data, 'AES-256-CFB')) + ); + } + + public function dataTestHasSignature() { + return [ + ['encryptedContent00iv001234567890123456xx', false], + ['encryptedContent00iv00123456789012345600sig00e1992521e437f6915f9173b190a512cfc38a00ac24502db44e0ba10c2bb0cc86xxx', true] + ]; + } + + /** + * @dataProvider dataTestHasSignatureFail + * @expectedException \OC\HintException + */ + public function testHasSignatureFail($cipher) { + $data = 'encryptedContent00iv001234567890123456xx'; + $this->invokePrivate($this->crypt, 'hasSignature', array($data, $cipher)); + } + + public function dataTestHasSignatureFail() { + return [ + ['AES-256-CTR'], + ['aes-256-ctr'], + ['AES-128-CTR'], + ['ctr-256-ctr'] + ]; } /** @@ -222,7 +266,7 @@ class cryptTest extends TestCase { */ public function testAddPadding() { $result = self::invokePrivate($this->crypt, 'addPadding', array('data')); - $this->assertSame('dataxx', $result); + $this->assertSame('dataxxx', $result); } /** @@ -348,7 +392,8 @@ class cryptTest extends TestCase { [ $this->logger, $this->userSession, - $this->config + $this->config, + $this->l ] ) ->setMethods( diff --git a/apps/encryption/tests/lib/crypto/encryptionTest.php b/apps/encryption/tests/lib/crypto/encryptionTest.php index 62e77c742d..ad943ab6e4 100644 --- a/apps/encryption/tests/lib/crypto/encryptionTest.php +++ b/apps/encryption/tests/lib/crypto/encryptionTest.php @@ -229,7 +229,7 @@ class EncryptionTest extends TestCase { public function dataTestBegin() { return array( - array('w', ['cipher' => 'myCipher'], 'legacyCipher', 'defaultCipher', 'fileKey', 'myCipher'), + array('w', ['cipher' => 'myCipher'], 'legacyCipher', 'defaultCipher', 'fileKey', 'defaultCipher'), array('r', ['cipher' => 'myCipher'], 'legacyCipher', 'defaultCipher', 'fileKey', 'myCipher'), array('w', [], 'legacyCipher', 'defaultCipher', '', 'defaultCipher'), array('r', [], 'legacyCipher', 'defaultCipher', 'file_key', 'legacyCipher'), diff --git a/lib/private/files/stream/encryption.php b/lib/private/files/stream/encryption.php index 11e2b218d1..bc771a91eb 100644 --- a/lib/private/files/stream/encryption.php +++ b/lib/private/files/stream/encryption.php @@ -72,6 +72,9 @@ class Encryption extends Wrapper { /** @var string */ protected $fullPath; + /** @var bool */ + protected $signed; + /** * header data returned by the encryption module, will be written to the file * in case of a write operation diff --git a/tests/lib/files/stream/encryption.php b/tests/lib/files/stream/encryption.php index f9d8f076b6..f67dd09bc4 100644 --- a/tests/lib/files/stream/encryption.php +++ b/tests/lib/files/stream/encryption.php @@ -117,6 +117,7 @@ class Encryption extends \Test\TestCase { $header->setAccessible(true); $header->setValue($streamWrapper, array()); $header->setAccessible(false); + $this->invokePrivate($streamWrapper, 'signed', [true]); // call stream_open, that's the method we want to test $dummyVar = 'foo'; From 3b62459c41fe56db00f2156535b0fe689bb43177 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 8 Jan 2016 18:07:19 +0100 Subject: [PATCH 10/20] Use hash with appended "a" of the original password for the authentication --- apps/encryption/lib/crypto/crypt.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index f27f55619a..790541bc07 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -475,6 +475,7 @@ class Crypt { * @return string */ private function createSignature($data, $passPhrase) { + $passPhrase = hash('sha512', $passPhrase . 'a', true); $signature = hash_hmac('sha256', $data, $passPhrase); return $signature; } @@ -607,14 +608,14 @@ class Crypt { } /** - * Generate a cryptographically secure pseudo-random base64 encoded 256-bit - * ASCII key, used as file key + * Generate a cryptographically secure pseudo-random 256-bit ASCII key, used + * as file key * * @return string * @throws \Exception */ public function generateFileKey() { - return base64_encode(random_bytes(32)); + return random_bytes(32); } /** From d5c15968878f896caf7b2c197ecc8e87f0b584c8 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 8 Jan 2016 18:12:22 +0100 Subject: [PATCH 11/20] Clarify documentation --- apps/encryption/lib/crypto/crypt.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index 790541bc07..69d8757eb8 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -48,6 +48,8 @@ use OCP\IUserSession; * - AES-256-CFB * - AES-128-CFB * + * For integrity protection Encrypt-Then-MAC using HMAC-SHA256 is used. + * * @package OCA\Encryption\Crypto */ class Crypt { From b5824f024a1008b0195b6e8f4803774cfe644b7b Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 2 Feb 2016 20:00:36 +0100 Subject: [PATCH 12/20] Keep track of file version This way it is not possible anymore for an external storage admin to put up old versions of the file. --- apps/encryption/lib/crypto/crypt.php | 22 ++++++++++++-------- apps/encryption/lib/crypto/encryption.php | 25 ++++++++++++++++++----- apps/encryption/lib/keymanager.php | 19 +++++++++++++++++ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index 69d8757eb8..6c3aee47a5 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -169,10 +169,11 @@ class Crypt { /** * @param string $plainContent * @param string $passPhrase + * @param int $version * @return false|string * @throws EncryptionFailedException */ - public function symmetricEncryptFileContent($plainContent, $passPhrase) { + public function symmetricEncryptFileContent($plainContent, $passPhrase, $version) { if (!$plainContent) { $this->logger->error('Encryption Library, symmetrical encryption failed no content given', @@ -187,7 +188,8 @@ class Crypt { $passPhrase, $this->getCipher()); - $sig = $this->createSignature($encryptedContent, $passPhrase); + // Create a signature based on the key as well as the current version + $sig = $this->createSignature($encryptedContent, $passPhrase.$version); // combine content to encrypt the IV identifier and actual IV $catFile = $this->concatIV($encryptedContent, $iv); @@ -365,7 +367,8 @@ class Crypt { $hash = $this->generatePasswordHash($password, $cipher, $uid); $encryptedKey = $this->symmetricEncryptFileContent( $privateKey, - $hash + $hash, + 0 ); return $encryptedKey; @@ -404,9 +407,12 @@ class Crypt { self::HEADER_END) + strlen(self::HEADER_END)); } - $plainKey = $this->symmetricDecryptFileContent($privateKey, + $plainKey = $this->symmetricDecryptFileContent( + $privateKey, $password, - $cipher); + $cipher, + 0 + ); if ($this->isValidPrivateKey($plainKey) === false) { return false; @@ -437,15 +443,15 @@ class Crypt { * @param string $keyFileContents * @param string $passPhrase * @param string $cipher + * @param int $version * @return string * @throws DecryptionFailedException */ - public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER) { - + public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER, $version = 0) { $catFile = $this->splitMetaData($keyFileContents, $cipher); if ($catFile['signature'] !== false) { - $this->checkSignature($catFile['encrypted'], $passPhrase, $catFile['signature']); + $this->checkSignature($catFile['encrypted'], $passPhrase.$version, $catFile['signature']); } return $this->decrypt($catFile['encrypted'], diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index dc60c09478..90c60b8e0d 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -100,6 +100,9 @@ class Encryption implements IEncryptionModule { /** @var int unencrypted block size */ private $unencryptedBlockSize = 6126; + /** @var int Current version of the file */ + private $version = 0; + /** * @@ -163,7 +166,6 @@ class Encryption implements IEncryptionModule { * or if no additional data is needed return a empty array */ public function begin($path, $user, $mode, array $header, array $accessList) { - $this->path = $this->getPathToRealFile($path); $this->accessList = $accessList; $this->user = $user; @@ -180,6 +182,8 @@ class Encryption implements IEncryptionModule { $this->fileKey = $this->keyManager->getFileKey($this->path, $this->user); } + $this->version = (int)$this->keyManager->getVersion($this->path); + if ( $mode === 'w' || $mode === 'w+' @@ -220,8 +224,13 @@ class Encryption implements IEncryptionModule { public function end($path) { $result = ''; if ($this->isWriteOperation) { + // Partial files do not increase the version + if(\OC\Files\Cache\Scanner::isPartialFile($path)) { + $this->version = $this->version-1; + } + $this->keyManager->setVersion($this->path, $this->version+1); if (!empty($this->writeCache)) { - $result = $this->crypt->symmetricEncryptFileContent($this->writeCache, $this->fileKey); + $result = $this->crypt->symmetricEncryptFileContent($this->writeCache, $this->fileKey, $this->version+1); $this->writeCache = ''; } $publicKeys = array(); @@ -258,7 +267,6 @@ class Encryption implements IEncryptionModule { * @return string encrypted data */ public function encrypt($data) { - // If extra data is left over from the last round, make sure it // is integrated into the next block if ($this->writeCache) { @@ -302,7 +310,11 @@ class Encryption implements IEncryptionModule { // Read the chunk from the start of $data $chunk = substr($data, 0, $this->unencryptedBlockSizeSigned); - $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey); + // Partial files do not increase the version + if(\OC\Files\Cache\Scanner::isPartialFile($this->path)) { + $this->version = $this->version - 1; + } + $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version+1); // Remove the chunk we just processed from // $data, leaving only unprocessed data in $data @@ -334,7 +346,7 @@ class Encryption implements IEncryptionModule { $result = ''; if (!empty($data)) { - $result = $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher); + $result = $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher, $this->version); } return $result; } @@ -349,6 +361,7 @@ class Encryption implements IEncryptionModule { */ public function update($path, $uid, array $accessList) { $fileKey = $this->keyManager->getFileKey($path, $uid); + $version = $this->keyManager->getVersion($path); if (!empty($fileKey)) { @@ -369,6 +382,8 @@ class Encryption implements IEncryptionModule { $this->keyManager->setAllFileKeys($path, $encryptedFileKey); + $this->keyManager->setVersion($path, $version); + } else { $this->logger->debug('no file key found, we assume that the file "{file}" is not encrypted', array('file' => $path, 'app' => 'encryption')); diff --git a/apps/encryption/lib/keymanager.php b/apps/encryption/lib/keymanager.php index b6365cf2cc..4cbb377a43 100644 --- a/apps/encryption/lib/keymanager.php +++ b/apps/encryption/lib/keymanager.php @@ -412,6 +412,24 @@ class KeyManager { return ''; } + /** + * Get the current version of a file + * + * @param string $path + * @return mixed + */ + public function getVersion($path) { + return $this->keyStorage->getFileKey($path, 'version', Encryption::ID); + } + + /** + * @param string $path + * @param string $version + */ + public function setVersion($path, $version) { + $this->keyStorage->setFileKey($path, 'version', $version, Encryption::ID); + } + /** * get the encrypted file key * @@ -546,6 +564,7 @@ class KeyManager { /** * @param string $path + * @return bool */ public function deleteAllFileKeys($path) { return $this->keyStorage->deleteAllFileKeys($path); From 3badf5caf579f8ff10c9917f62cb41cd9b0c68f8 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 3 Feb 2016 14:32:04 +0100 Subject: [PATCH 13/20] Use number of chunk for HMAC as well Prevents switching single blocks within the encrypted file. --- apps/encryption/lib/crypto/crypt.php | 11 +++++++---- apps/encryption/lib/crypto/encryption.php | 19 +++++++++---------- lib/private/files/stream/encryption.php | 18 +++++++++++++----- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/apps/encryption/lib/crypto/crypt.php b/apps/encryption/lib/crypto/crypt.php index 6c3aee47a5..b4c10f4279 100644 --- a/apps/encryption/lib/crypto/crypt.php +++ b/apps/encryption/lib/crypto/crypt.php @@ -170,10 +170,11 @@ class Crypt { * @param string $plainContent * @param string $passPhrase * @param int $version + * @param int $position * @return false|string * @throws EncryptionFailedException */ - public function symmetricEncryptFileContent($plainContent, $passPhrase, $version) { + public function symmetricEncryptFileContent($plainContent, $passPhrase, $version, $position) { if (!$plainContent) { $this->logger->error('Encryption Library, symmetrical encryption failed no content given', @@ -189,7 +190,7 @@ class Crypt { $this->getCipher()); // Create a signature based on the key as well as the current version - $sig = $this->createSignature($encryptedContent, $passPhrase.$version); + $sig = $this->createSignature($encryptedContent, $passPhrase.$version.$position); // combine content to encrypt the IV identifier and actual IV $catFile = $this->concatIV($encryptedContent, $iv); @@ -368,6 +369,7 @@ class Crypt { $encryptedKey = $this->symmetricEncryptFileContent( $privateKey, $hash, + 0, 0 ); @@ -444,14 +446,15 @@ class Crypt { * @param string $passPhrase * @param string $cipher * @param int $version + * @param int $position * @return string * @throws DecryptionFailedException */ - public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER, $version = 0) { + public function symmetricDecryptFileContent($keyFileContents, $passPhrase, $cipher = self::DEFAULT_CIPHER, $version = 0, $position = 0) { $catFile = $this->splitMetaData($keyFileContents, $cipher); if ($catFile['signature'] !== false) { - $this->checkSignature($catFile['encrypted'], $passPhrase.$version, $catFile['signature']); + $this->checkSignature($catFile['encrypted'], $passPhrase.$version.$position, $catFile['signature']); } return $this->decrypt($catFile['encrypted'], diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index 90c60b8e0d..d4e8087c4b 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -215,13 +215,14 @@ class Encryption implements IEncryptionModule { * buffer. * * @param string $path to the file + * @param int $position * @return string remained data which should be written to the file in case * of a write operation * @throws PublicKeyMissingException * @throws \Exception * @throws \OCA\Encryption\Exceptions\MultiKeyEncryptException */ - public function end($path) { + public function end($path, $position = 0) { $result = ''; if ($this->isWriteOperation) { // Partial files do not increase the version @@ -230,7 +231,7 @@ class Encryption implements IEncryptionModule { } $this->keyManager->setVersion($this->path, $this->version+1); if (!empty($this->writeCache)) { - $result = $this->crypt->symmetricEncryptFileContent($this->writeCache, $this->fileKey, $this->version+1); + $result = $this->crypt->symmetricEncryptFileContent($this->writeCache, $this->fileKey, $this->version+1, $position); $this->writeCache = ''; } $publicKeys = array(); @@ -264,9 +265,10 @@ class Encryption implements IEncryptionModule { * encrypt data * * @param string $data you want to encrypt + * @param int $position * @return string encrypted data */ - public function encrypt($data) { + public function encrypt($data, $position = 0) { // If extra data is left over from the last round, make sure it // is integrated into the next block if ($this->writeCache) { @@ -314,7 +316,7 @@ class Encryption implements IEncryptionModule { if(\OC\Files\Cache\Scanner::isPartialFile($this->path)) { $this->version = $this->version - 1; } - $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version+1); + $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version+1, $position); // Remove the chunk we just processed from // $data, leaving only unprocessed data in $data @@ -332,10 +334,11 @@ class Encryption implements IEncryptionModule { * decrypt data * * @param string $data you want to decrypt + * @param int $position * @return string decrypted data * @throws DecryptionFailedException */ - public function decrypt($data) { + public function decrypt($data, $position = 0) { if (empty($this->fileKey)) { $msg = 'Can not decrypt this file, probably this is a shared file. Please ask the file owner to reshare the file with you.'; $hint = $this->l->t('Can not decrypt this file, probably this is a shared file. Please ask the file owner to reshare the file with you.'); @@ -344,11 +347,7 @@ class Encryption implements IEncryptionModule { throw new DecryptionFailedException($msg, $hint); } - $result = ''; - if (!empty($data)) { - $result = $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher, $this->version); - } - return $result; + return $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher, $this->version, $position); } /** diff --git a/lib/private/files/stream/encryption.php b/lib/private/files/stream/encryption.php index bc771a91eb..63949035b5 100644 --- a/lib/private/files/stream/encryption.php +++ b/lib/private/files/stream/encryption.php @@ -399,8 +399,9 @@ class Encryption extends Wrapper { } public function stream_close() { - $this->flush(); - $remainingData = $this->encryptionModule->end($this->fullPath); + $this->flush('end'); + $position = (int)floor($this->position/$this->unencryptedBlockSize); + $remainingData = $this->encryptionModule->end($this->fullPath, $position . 'end'); if ($this->readOnly === false) { if(!empty($remainingData)) { parent::stream_write($remainingData); @@ -412,15 +413,17 @@ class Encryption extends Wrapper { /** * write block to file + * @param string $positionPrefix */ - protected function flush() { + protected function flush($positionPrefix = '') { // write to disk only when writeFlag was set to 1 if ($this->writeFlag) { // Disable the file proxies so that encryption is not // automatically attempted when the file is written to disk - // we are handling that separately here and we don't want to // get into an infinite loop - $encrypted = $this->encryptionModule->encrypt($this->cache); + $position = (int)floor($this->position/$this->unencryptedBlockSize); + $encrypted = $this->encryptionModule->encrypt($this->cache, $position . $positionPrefix); $bytesWritten = parent::stream_write($encrypted); $this->writeFlag = false; // Check whether the write concerns the last block @@ -447,7 +450,12 @@ class Encryption extends Wrapper { if ($this->cache === '' && !($this->position === $this->unencryptedSize && ($this->position % $this->unencryptedBlockSize) === 0)) { // Get the data from the file handle $data = parent::stream_read($this->util->getBlockSize()); - $this->cache = $this->encryptionModule->decrypt($data); + $position = (int)floor($this->position/$this->unencryptedBlockSize); + $numberOfChunks = (int)($this->unencryptedSize / $this->unencryptedBlockSize); + if($numberOfChunks === $position) { + $position .= 'end'; + } + $this->cache = $this->encryptionModule->decrypt($data, $position); } } From 5ccb9dfa7e35d78d61d7a973ee2a5fddfda7d766 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 8 Feb 2016 20:35:33 +0100 Subject: [PATCH 14/20] Use database for keeping track of the version --- apps/encryption/lib/crypto/encryption.php | 11 ++++++-- apps/encryption/lib/keymanager.php | 25 ++++++++++++++++--- apps/files_versions/lib/storage.php | 10 +++++++- lib/private/files/cache/cache.php | 9 +++++-- lib/private/files/fileinfo.php | 9 +++++++ .../files/storage/wrapper/encryption.php | 3 ++- 6 files changed, 58 insertions(+), 9 deletions(-) diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index d4e8087c4b..b640f9a7a0 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -55,6 +55,9 @@ class Encryption implements IEncryptionModule { /** @var string */ private $path; + /** @var string */ + private $realPath; + /** @var string */ private $user; @@ -167,6 +170,7 @@ class Encryption implements IEncryptionModule { */ public function begin($path, $user, $mode, array $header, array $accessList) { $this->path = $this->getPathToRealFile($path); + $this->realPath = $this->path; $this->accessList = $accessList; $this->user = $user; $this->isWriteOperation = false; @@ -182,7 +186,7 @@ class Encryption implements IEncryptionModule { $this->fileKey = $this->keyManager->getFileKey($this->path, $this->user); } - $this->version = (int)$this->keyManager->getVersion($this->path); + $this->version = (int)$this->keyManager->getVersion($this->realPath); if ( $mode === 'w' @@ -360,7 +364,10 @@ class Encryption implements IEncryptionModule { */ public function update($path, $uid, array $accessList) { $fileKey = $this->keyManager->getFileKey($path, $uid); - $version = $this->keyManager->getVersion($path); + if(empty($this->realPath)) { + $this->realPath = $path; + } + $version = $this->keyManager->getVersion($this->realPath); if (!empty($fileKey)) { diff --git a/apps/encryption/lib/keymanager.php b/apps/encryption/lib/keymanager.php index 4cbb377a43..7d8bd8485e 100644 --- a/apps/encryption/lib/keymanager.php +++ b/apps/encryption/lib/keymanager.php @@ -25,12 +25,14 @@ namespace OCA\Encryption; use OC\Encryption\Exceptions\DecryptionFailedException; +use OC\Files\View; use OCA\Encryption\Crypto\Encryption; use OCA\Encryption\Exceptions\PrivateKeyMissingException; use OCA\Encryption\Exceptions\PublicKeyMissingException; use OCA\Encryption\Crypto\Crypt; use OCP\Encryption\Keys\IStorage; use OCP\IConfig; +use OCP\IDBConnection; use OCP\ILogger; use OCP\IUserSession; @@ -416,18 +418,35 @@ class KeyManager { * Get the current version of a file * * @param string $path - * @return mixed + * @return int */ public function getVersion($path) { - return $this->keyStorage->getFileKey($path, 'version', Encryption::ID); + $view = new \OC\Files\View(); + $fileInfo = $view->getFileInfo($path); + if($fileInfo === false) { + return 0; + } + return $fileInfo->getEncryptedVersion(); } /** + * Set the current version of a file + * * @param string $path * @param string $version */ public function setVersion($path, $version) { - $this->keyStorage->setFileKey($path, 'version', $version, Encryption::ID); + $view = new \OC\Files\View(); + $fileInfo= $view->getFileInfo($path); + + if($fileInfo !== false) { + $fileId = $fileInfo->getId(); + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $qb->update('filecache') + ->set('encrypted', $qb->createNamedParameter($version)) + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))) + ->execute(); + } } /** diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php index 47acec1d76..0b121c344f 100644 --- a/apps/files_versions/lib/storage.php +++ b/apps/files_versions/lib/storage.php @@ -165,7 +165,15 @@ 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 - $users_view->getFileInfo('files_versions/' . $filename . '.v' . $mtime); + $newFileInfo = $users_view->getFileInfo('files_versions/' . $filename . '.v' . $mtime); + + // Keep the "encrypted" value of the original file + $oldVersion = $files_view->getFileInfo($filename)->getEncryptedVersion(); + $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $qb->update('filecache') + ->set('encrypted', $qb->createNamedParameter($oldVersion)) + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($newFileInfo->getId()))) + ->execute(); } } diff --git a/lib/private/files/cache/cache.php b/lib/private/files/cache/cache.php index 22b9f49e52..b30666d48d 100644 --- a/lib/private/files/cache/cache.php +++ b/lib/private/files/cache/cache.php @@ -145,6 +145,7 @@ class Cache implements ICache { $data['size'] = 0 + $data['size']; $data['mtime'] = (int)$data['mtime']; $data['storage_mtime'] = (int)$data['storage_mtime']; + $data['encryptedVersion'] = (int)$data['encrypted']; $data['encrypted'] = (bool)$data['encrypted']; $data['storage'] = $this->storageId; $data['mimetype'] = $this->mimetypeLoader->getMimetypeById($data['mimetype']); @@ -345,8 +346,12 @@ class Cache implements ICache { $queryParts[] = '`mtime`'; } } elseif ($name === 'encrypted') { - // Boolean to integer conversion - $value = $value ? 1 : 0; + if(isset($data['encryptedVersion'])) { + $value = $data['encryptedVersion']; + } else { + // Boolean to integer conversion + $value = $value ? 1 : 0; + } } $params[] = $value; $queryParts[] = '`' . $name . '`'; diff --git a/lib/private/files/fileinfo.php b/lib/private/files/fileinfo.php index f22e1099e2..1d722a4673 100644 --- a/lib/private/files/fileinfo.php +++ b/lib/private/files/fileinfo.php @@ -193,6 +193,15 @@ class FileInfo implements \OCP\Files\FileInfo, \ArrayAccess { return $this->data['encrypted']; } + /** + * Return the currently version used for the HMAC in the encryption app + * + * @return int + */ + public function getEncryptedVersion() { + return isset($this->data['encryptedVersion']) ? (int) $this->data['encryptedVersion'] : 1; + } + /** * @return int */ diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index 96d642e778..3307599aa5 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -131,11 +131,12 @@ class Encryption extends Wrapper { // update file cache if ($info) { $info = $info->getData(); + $info['encrypted'] = $info['encryptedVersion']; } else { $info = []; + $info['encrypted'] = true; } - $info['encrypted'] = true; $info['size'] = $size; $this->getCache()->put($path, $info); From 966eb4b0844aff453a48bcad7b342854f531f500 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 9 Feb 2016 16:32:34 +0100 Subject: [PATCH 15/20] realPath should contain the path to the file we want to read, e.g. the version and not the original file --- apps/encryption/lib/crypto/encryption.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index b640f9a7a0..403b617c5b 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -170,7 +170,7 @@ class Encryption implements IEncryptionModule { */ public function begin($path, $user, $mode, array $header, array $accessList) { $this->path = $this->getPathToRealFile($path); - $this->realPath = $this->path; + $this->realPath = $path; $this->accessList = $accessList; $this->user = $user; $this->isWriteOperation = false; From 3736f1382632716102ab2c99a1028ba7bdede5f4 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 9 Feb 2016 18:07:07 +0100 Subject: [PATCH 16/20] Check if partial cache entry or not in encryption wrapper --- lib/private/files/storage/wrapper/encryption.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index 3307599aa5..14d3b15bba 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -39,6 +39,7 @@ use OCP\Encryption\Keys\IStorage; use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage; use OCP\ILogger; +use OCP\Files\Cache\ICacheEntry; class Encryption extends Wrapper { @@ -129,11 +130,13 @@ class Encryption extends Wrapper { if (isset($this->unencryptedSize[$fullPath])) { $size = $this->unencryptedSize[$fullPath]; // update file cache - if ($info) { + if ($info instanceof ICacheEntry) { $info = $info->getData(); $info['encrypted'] = $info['encryptedVersion']; } else { - $info = []; + if (!is_array($info)) { + $info = []; + } $info['encrypted'] = true; } From 377d7fb8a817f337552e444c5aea9eb82bcfada4 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 9 Feb 2016 20:05:07 +0100 Subject: [PATCH 17/20] don't decrease ->version for part files but only a local variable, otherwise it can happen that we decrease it twice and end up with the wrong value --- apps/encryption/lib/crypto/encryption.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index 403b617c5b..645d17f818 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -231,11 +231,13 @@ class Encryption implements IEncryptionModule { if ($this->isWriteOperation) { // Partial files do not increase the version if(\OC\Files\Cache\Scanner::isPartialFile($path)) { - $this->version = $this->version-1; + $version = $this->version; + } else { + $version = $this->version + 1; } $this->keyManager->setVersion($this->path, $this->version+1); if (!empty($this->writeCache)) { - $result = $this->crypt->symmetricEncryptFileContent($this->writeCache, $this->fileKey, $this->version+1, $position); + $result = $this->crypt->symmetricEncryptFileContent($this->writeCache, $this->fileKey, $version, $position); $this->writeCache = ''; } $publicKeys = array(); @@ -318,9 +320,11 @@ class Encryption implements IEncryptionModule { // Partial files do not increase the version if(\OC\Files\Cache\Scanner::isPartialFile($this->path)) { - $this->version = $this->version - 1; + $version = $this->version; + } else { + $version = $this->version + 1; } - $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version+1, $position); + $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $version, $position); // Remove the chunk we just processed from // $data, leaving only unprocessed data in $data From 6724f765733fa67f6f3e3fd0fb6a6a23eaaa127b Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 9 Feb 2016 22:27:23 +0100 Subject: [PATCH 18/20] Use cache and add tests --- apps/encryption/lib/crypto/encryption.php | 9 ++++--- apps/encryption/lib/keymanager.php | 18 +++++-------- apps/encryption/tests/lib/KeyManagerTest.php | 27 ++++++++++++++++++++ 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php index 645d17f818..a637f52a86 100644 --- a/apps/encryption/lib/crypto/encryption.php +++ b/apps/encryption/lib/crypto/encryption.php @@ -29,6 +29,7 @@ namespace OCA\Encryption\Crypto; use OC\Encryption\Exceptions\DecryptionFailedException; +use OC\Files\View; use OCA\Encryption\Exceptions\PublicKeyMissingException; use OCA\Encryption\Session; use OCA\Encryption\Util; @@ -186,7 +187,7 @@ class Encryption implements IEncryptionModule { $this->fileKey = $this->keyManager->getFileKey($this->path, $this->user); } - $this->version = (int)$this->keyManager->getVersion($this->realPath); + $this->version = (int)$this->keyManager->getVersion($this->realPath, new View()); if ( $mode === 'w' @@ -235,7 +236,7 @@ class Encryption implements IEncryptionModule { } else { $version = $this->version + 1; } - $this->keyManager->setVersion($this->path, $this->version+1); + $this->keyManager->setVersion($this->path, $this->version+1, new View()); if (!empty($this->writeCache)) { $result = $this->crypt->symmetricEncryptFileContent($this->writeCache, $this->fileKey, $version, $position); $this->writeCache = ''; @@ -371,7 +372,7 @@ class Encryption implements IEncryptionModule { if(empty($this->realPath)) { $this->realPath = $path; } - $version = $this->keyManager->getVersion($this->realPath); + $version = $this->keyManager->getVersion($this->realPath, new View()); if (!empty($fileKey)) { @@ -392,7 +393,7 @@ class Encryption implements IEncryptionModule { $this->keyManager->setAllFileKeys($path, $encryptedFileKey); - $this->keyManager->setVersion($path, $version); + $this->keyManager->setVersion($path, $version, new View()); } else { $this->logger->debug('no file key found, we assume that the file "{file}" is not encrypted', diff --git a/apps/encryption/lib/keymanager.php b/apps/encryption/lib/keymanager.php index 7d8bd8485e..57a8155e7b 100644 --- a/apps/encryption/lib/keymanager.php +++ b/apps/encryption/lib/keymanager.php @@ -418,10 +418,10 @@ class KeyManager { * Get the current version of a file * * @param string $path + * @param View $view * @return int */ - public function getVersion($path) { - $view = new \OC\Files\View(); + public function getVersion($path, View $view) { $fileInfo = $view->getFileInfo($path); if($fileInfo === false) { return 0; @@ -433,19 +433,15 @@ class KeyManager { * Set the current version of a file * * @param string $path - * @param string $version + * @param int $version + * @param View $view */ - public function setVersion($path, $version) { - $view = new \OC\Files\View(); + public function setVersion($path, $version, View $view) { $fileInfo= $view->getFileInfo($path); if($fileInfo !== false) { - $fileId = $fileInfo->getId(); - $qb = \OC::$server->getDatabaseConnection()->getQueryBuilder(); - $qb->update('filecache') - ->set('encrypted', $qb->createNamedParameter($version)) - ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId))) - ->execute(); + $cache = $fileInfo->getStorage()->getCache(); + $cache->put($path, ['fileid' => $fileInfo->getId(), 'encrypted' => $version, 'encryptedVersion' => $version]); } } diff --git a/apps/encryption/tests/lib/KeyManagerTest.php b/apps/encryption/tests/lib/KeyManagerTest.php index c69610fb54..a8cb2dcc78 100644 --- a/apps/encryption/tests/lib/KeyManagerTest.php +++ b/apps/encryption/tests/lib/KeyManagerTest.php @@ -579,4 +579,31 @@ class KeyManagerTest extends TestCase { ]; } + public function testGetVersionWithoutFileInfo() { + $view = $this->getMockBuilder('\\OC\\Files\\View') + ->disableOriginalConstructor()->getMock(); + $view->expects($this->once()) + ->method('getFileInfo') + ->with('/admin/files/myfile.txt') + ->willReturn(false); + + $this->assertSame(0, $this->instance->getVersion('/admin/files/myfile.txt', $view)); + } + + public function testGetVersionWithFileInfo() { + $view = $this->getMockBuilder('\\OC\\Files\\View') + ->disableOriginalConstructor()->getMock(); + $fileInfo = $this->getMockBuilder('\\OC\\Files\\FileInfo') + ->disableOriginalConstructor()->getMock(); + $fileInfo->expects($this->once()) + ->method('getEncryptedVersion') + ->willReturn(1337); + $view->expects($this->once()) + ->method('getFileInfo') + ->with('/admin/files/myfile.txt') + ->willReturn($fileInfo); + + $this->assertSame(1337, $this->instance->getVersion('/admin/files/myfile.txt', $view)); + } + } From 45c78476f5baed56e4e8158ed5283057f35366dd Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 9 Feb 2016 22:34:22 +0100 Subject: [PATCH 19/20] Use cache update instead of put for encryption version Saves a call to fetch the file id which didn't even work for a reason. This fix properly sets the version in the database. --- apps/encryption/lib/keymanager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/encryption/lib/keymanager.php b/apps/encryption/lib/keymanager.php index 57a8155e7b..0c957e1201 100644 --- a/apps/encryption/lib/keymanager.php +++ b/apps/encryption/lib/keymanager.php @@ -441,7 +441,7 @@ class KeyManager { if($fileInfo !== false) { $cache = $fileInfo->getStorage()->getCache(); - $cache->put($path, ['fileid' => $fileInfo->getId(), 'encrypted' => $version, 'encryptedVersion' => $version]); + $cache->update($fileInfo->getId(), ['encrypted' => $version, 'encryptedVersion' => $version]); } } From ca350294a624c4edac7c6ce23b514d57556e3e34 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 9 Feb 2016 22:47:15 +0100 Subject: [PATCH 20/20] Add tests for setVersion --- apps/encryption/tests/lib/KeyManagerTest.php | 40 ++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/apps/encryption/tests/lib/KeyManagerTest.php b/apps/encryption/tests/lib/KeyManagerTest.php index a8cb2dcc78..ea1830db4d 100644 --- a/apps/encryption/tests/lib/KeyManagerTest.php +++ b/apps/encryption/tests/lib/KeyManagerTest.php @@ -606,4 +606,44 @@ class KeyManagerTest extends TestCase { $this->assertSame(1337, $this->instance->getVersion('/admin/files/myfile.txt', $view)); } + public function testSetVersionWithFileInfo() { + $view = $this->getMockBuilder('\\OC\\Files\\View') + ->disableOriginalConstructor()->getMock(); + $cache = $this->getMockBuilder('\\OCP\\Files\\Cache\\ICache') + ->disableOriginalConstructor()->getMock(); + $cache->expects($this->once()) + ->method('update') + ->with(123, ['encrypted' => 5, 'encryptedVersion' => 5]); + $storage = $this->getMockBuilder('\\OCP\\Files\\Storage') + ->disableOriginalConstructor()->getMock(); + $storage->expects($this->once()) + ->method('getCache') + ->willReturn($cache); + $fileInfo = $this->getMockBuilder('\\OC\\Files\\FileInfo') + ->disableOriginalConstructor()->getMock(); + $fileInfo->expects($this->once()) + ->method('getStorage') + ->willReturn($storage); + $fileInfo->expects($this->once()) + ->method('getId') + ->willReturn(123); + $view->expects($this->once()) + ->method('getFileInfo') + ->with('/admin/files/myfile.txt') + ->willReturn($fileInfo); + + $this->instance->setVersion('/admin/files/myfile.txt', 5, $view); + } + + public function testSetVersionWithoutFileInfo() { + $view = $this->getMockBuilder('\\OC\\Files\\View') + ->disableOriginalConstructor()->getMock(); + $view->expects($this->once()) + ->method('getFileInfo') + ->with('/admin/files/myfile.txt') + ->willReturn(false); + + $this->instance->setVersion('/admin/files/myfile.txt', 5, $view); + } + }