diff options
20 files changed, 402 insertions, 133 deletions
diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index a6c9b8b4ebe..6b6f622a5a7 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -436,7 +436,7 @@ class FilesPlugin extends ServerPlugin { \OC::$server->get(LoggerInterface::class)->debug('Inefficient fetching of metadata'); } - return json_encode((object)$sizeMetadata->getMetadata(), JSON_THROW_ON_ERROR); + return $sizeMetadata->getValue(); }); } } diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php index fe9813a6cfa..22a697a1232 100644 --- a/apps/encryption/lib/Crypto/Crypt.php +++ b/apps/encryption/lib/Crypto/Crypt.php @@ -185,14 +185,9 @@ 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, $position) { + public function symmetricEncryptFileContent(string $plainContent, string $passPhrase, int $version, string $position): string|false { if (!$plainContent) { $this->logger->error('Encryption Library, symmetrical encryption failed no content given', ['app' => 'encryption']); @@ -409,7 +404,7 @@ class Crypt { $privateKey, $hash, 0, - 0 + '0' ); return $encryptedKey; @@ -537,12 +532,8 @@ class Crypt { /** * create signature - * - * @param string $data - * @param string $passPhrase - * @return string */ - private function createSignature($data, $passPhrase) { + private function createSignature(string $data, string $passPhrase): string { $passPhrase = hash('sha512', $passPhrase . 'a', true); return hash_hmac('sha256', $data, $passPhrase); } @@ -695,13 +686,25 @@ class Crypt { } /** - * @param string $encKeyFile - * @param string $shareKey * @param \OpenSSLAsymmetricKey|\OpenSSLCertificate|array|string $privateKey - * @return string * @throws MultiKeyDecryptException */ - public function multiKeyDecrypt($encKeyFile, $shareKey, $privateKey) { + public function multiKeyDecrypt(string $shareKey, $privateKey): string { + $plainContent = ''; + + // decrypt the intermediate key with RSA + if (openssl_private_decrypt($shareKey, $intermediate, $privateKey, OPENSSL_PKCS1_OAEP_PADDING)) { + return $intermediate; + } else { + throw new MultiKeyDecryptException('multikeydecrypt with share key failed:' . openssl_error_string()); + } + } + + /** + * @param \OpenSSLAsymmetricKey|\OpenSSLCertificate|array|string $privateKey + * @throws MultiKeyDecryptException + */ + public function multiKeyDecryptLegacy(string $encKeyFile, string $shareKey, $privateKey): string { if (!$encKeyFile) { throw new MultiKeyDecryptException('Cannot multikey decrypt empty plain content'); } @@ -715,12 +718,55 @@ class Crypt { } /** + * @param array<string,\OpenSSLAsymmetricKey|\OpenSSLCertificate|array|string> $keyFiles + * @throws MultiKeyEncryptException + */ + public function multiKeyEncrypt(string $plainContent, array $keyFiles): array { + if (empty($plainContent)) { + throw new MultiKeyEncryptException('Cannot multikeyencrypt empty plain content'); + } + + // Set empty vars to be set by openssl by reference + $shareKeys = []; + $mappedShareKeys = []; + + // make sure that there is at least one public key to use + if (count($keyFiles) >= 1) { + // prepare the encrypted keys + $shareKeys = []; + + // iterate over the public keys and encrypt the intermediate + // for each of them with RSA + foreach ($keyFiles as $tmp_key) { + if (openssl_public_encrypt($plainContent, $tmp_output, $tmp_key, OPENSSL_PKCS1_OAEP_PADDING)) { + $shareKeys[] = $tmp_output; + } + } + + // set the result if everything worked fine + if (count($keyFiles) === count($shareKeys)) { + $i = 0; + + // Ensure each shareKey is labelled with its corresponding key id + foreach ($keyFiles as $userId => $publicKey) { + $mappedShareKeys[$userId] = $shareKeys[$i]; + $i++; + } + + return $mappedShareKeys; + } + } + throw new MultiKeyEncryptException('multikeyencryption failed ' . openssl_error_string()); + } + + /** * @param string $plainContent * @param array $keyFiles * @return array * @throws MultiKeyEncryptException + * @deprecated 27.0.0 use multiKeyEncrypt */ - public function multiKeyEncrypt($plainContent, array $keyFiles) { + public function multiKeyEncryptLegacy($plainContent, array $keyFiles) { // openssl_seal returns false without errors if plaincontent is empty // so trigger our own error if (empty($plainContent)) { @@ -809,6 +855,7 @@ class Crypt { /** * Custom implementation of openssl_seal() * + * @deprecated 27.0.0 use multiKeyEncrypt * @throws EncryptionFailedException */ private function opensslSeal(string $data, string &$sealed_data, array &$encrypted_keys, array $public_key, string $cipher_algo): int|false { diff --git a/apps/encryption/lib/Crypto/Encryption.php b/apps/encryption/lib/Crypto/Encryption.php index b44472fd04a..465856fd28d 100644 --- a/apps/encryption/lib/Crypto/Encryption.php +++ b/apps/encryption/lib/Crypto/Encryption.php @@ -108,6 +108,8 @@ class Encryption implements IEncryptionModule { /** @var int Current version of the file */ private $version = 0; + private bool $useLegacyFileKey = true; + /** @var array remember encryption signature version */ private static $rememberVersion = []; @@ -182,6 +184,8 @@ class Encryption implements IEncryptionModule { $this->writeCache = ''; $this->useLegacyBase64Encoding = true; + $this->useLegacyFileKey = ($header['useLegacyFileKey'] ?? 'true') !== 'false'; + if (isset($header['encoding'])) { $this->useLegacyBase64Encoding = $header['encoding'] !== Crypt::BINARY_ENCODING_FORMAT; } @@ -195,13 +199,17 @@ class Encryption implements IEncryptionModule { } if ($this->session->decryptAllModeActivated()) { - $encryptedFileKey = $this->keyManager->getEncryptedFileKey($this->path); $shareKey = $this->keyManager->getShareKey($this->path, $this->session->getDecryptAllUid()); - $this->fileKey = $this->crypt->multiKeyDecrypt($encryptedFileKey, - $shareKey, - $this->session->getDecryptAllKey()); + if ($this->useLegacyFileKey) { + $encryptedFileKey = $this->keyManager->getEncryptedFileKey($this->path); + $this->fileKey = $this->crypt->multiKeyDecryptLegacy($encryptedFileKey, + $shareKey, + $this->session->getDecryptAllKey()); + } else { + $this->fileKey = $this->crypt->multiKeyDecrypt($shareKey, $this->session->getDecryptAllKey()); + } } else { - $this->fileKey = $this->keyManager->getFileKey($this->path, $this->user); + $this->fileKey = $this->keyManager->getFileKey($this->path, $this->user, $this->useLegacyFileKey); } // always use the version from the original file, also part files @@ -239,7 +247,11 @@ class Encryption implements IEncryptionModule { $this->cipher = $this->crypt->getLegacyCipher(); } - $result = ['cipher' => $this->cipher, 'signed' => 'true']; + $result = [ + 'cipher' => $this->cipher, + 'signed' => 'true', + 'useLegacyFileKey' => 'false', + ]; if ($this->useLegacyBase64Encoding !== true) { $result['encoding'] = Crypt::BINARY_ENCODING_FORMAT; @@ -254,14 +266,14 @@ class Encryption implements IEncryptionModule { * buffer. * * @param string $path to the file - * @param int $position + * @param string $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, $position = 0) { + public function end($path, $position = '0') { $result = ''; if ($this->isWriteOperation) { // in case of a part file we remember the new signature versions @@ -296,10 +308,13 @@ class Encryption implements IEncryptionModule { } $publicKeys = $this->keyManager->addSystemKeys($this->accessList, $publicKeys, $this->getOwner($path)); - $encryptedKeyfiles = $this->crypt->multiKeyEncrypt($this->fileKey, $publicKeys); - $this->keyManager->setAllFileKeys($this->path, $encryptedKeyfiles); + $shareKeys = $this->crypt->multiKeyEncrypt($this->fileKey, $publicKeys); + $this->keyManager->deleteLegacyFileKey($this->path); + foreach ($shareKeys as $uid => $keyFile) { + $this->keyManager->setShareKey($this->path, $uid, $keyFile); + } } - return $result; + return $result ?: ''; } @@ -315,7 +330,6 @@ class Encryption implements IEncryptionModule { // If extra data is left over from the last round, make sure it // is integrated into the next block if ($this->writeCache) { - // Concat writeCache to start of $data $data = $this->writeCache . $data; @@ -327,7 +341,6 @@ class Encryption implements IEncryptionModule { $encrypted = ''; // While there still remains some data to be processed & written while (strlen($data) > 0) { - // Remaining length for this iteration, not of the // entire file (may be greater than 8192 bytes) $remainingLength = strlen($data); @@ -335,7 +348,6 @@ class Encryption implements IEncryptionModule { // If data remaining to be written is less than the // size of 1 unencrypted block if ($remainingLength < $this->getUnencryptedBlockSize(true)) { - // Set writeCache to contents of $data // The writeCache will be carried over to the // next write round, and added to the start of @@ -349,11 +361,10 @@ class Encryption implements IEncryptionModule { // Clear $data ready for next round $data = ''; } else { - // Read the chunk from the start of $data $chunk = substr($data, 0, $this->getUnencryptedBlockSize(true)); - $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version + 1, $position); + $encrypted .= $this->crypt->symmetricEncryptFileContent($chunk, $this->fileKey, $this->version + 1, (string)$position); // Remove the chunk we just processed from // $data, leaving only unprocessed data in $data @@ -391,7 +402,7 @@ class Encryption implements IEncryptionModule { * @param string $path path to the file which should be updated * @param string $uid of the user who performs the operation * @param array $accessList who has access to the file contains the key 'users' and 'public' - * @return boolean + * @return bool */ public function update($path, $uid, array $accessList) { if (empty($accessList)) { @@ -399,10 +410,10 @@ class Encryption implements IEncryptionModule { $this->keyManager->setVersion($path, self::$rememberVersion[$path], new View()); unset(self::$rememberVersion[$path]); } - return; + return false; } - $fileKey = $this->keyManager->getFileKey($path, $uid); + $fileKey = $this->keyManager->getFileKey($path, $uid, null); if (!empty($fileKey)) { $publicKeys = []; @@ -420,11 +431,13 @@ class Encryption implements IEncryptionModule { $publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $this->getOwner($path)); - $encryptedFileKey = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys); + $shareKeys = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys); $this->keyManager->deleteAllFileKeys($path); - $this->keyManager->setAllFileKeys($path, $encryptedFileKey); + foreach ($shareKeys as $uid => $keyFile) { + $this->keyManager->setShareKey($this->path, $uid, $keyFile); + } } else { $this->logger->debug('no file key found, we assume that the file "{file}" is not encrypted', ['file' => $path, 'app' => 'encryption']); @@ -503,7 +516,7 @@ class Encryption implements IEncryptionModule { * @throws DecryptionFailedException */ public function isReadable($path, $uid) { - $fileKey = $this->keyManager->getFileKey($path, $uid); + $fileKey = $this->keyManager->getFileKey($path, $uid, null); if (empty($fileKey)) { $owner = $this->util->getOwner($path); if ($owner !== $uid) { diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php index 2c6487d062a..f0005b45761 100644 --- a/apps/encryption/lib/KeyManager.php +++ b/apps/encryption/lib/KeyManager.php @@ -44,7 +44,6 @@ use OCP\IUserSession; use OCP\Lock\ILockingProvider; class KeyManager { - /** * @var Session */ @@ -441,17 +440,21 @@ class KeyManager { /** * @param string $path * @param $uid + * @param ?bool $useLegacyFileKey null means try both * @return string */ - public function getFileKey($path, $uid) { + public function getFileKey(string $path, ?string $uid, ?bool $useLegacyFileKey): string { if ($uid === '') { $uid = null; } $publicAccess = is_null($uid); - $encryptedFileKey = $this->keyStorage->getFileKey($path, $this->fileKeyId, Encryption::ID); + $encryptedFileKey = ''; + if ($useLegacyFileKey ?? true) { + $encryptedFileKey = $this->keyStorage->getFileKey($path, $this->fileKeyId, Encryption::ID); - if (empty($encryptedFileKey)) { - return ''; + if (empty($encryptedFileKey) && $useLegacyFileKey) { + return ''; + } } if ($this->util->isMasterKeyEnabled()) { @@ -475,10 +478,17 @@ class KeyManager { $privateKey = $this->session->getPrivateKey(); } - if ($encryptedFileKey && $shareKey && $privateKey) { - return $this->crypt->multiKeyDecrypt($encryptedFileKey, - $shareKey, - $privateKey); + if ($useLegacyFileKey ?? true) { + if ($encryptedFileKey && $shareKey && $privateKey) { + return $this->crypt->multiKeyDecryptLegacy($encryptedFileKey, + $shareKey, + $privateKey); + } + } + if (!($useLegacyFileKey ?? false)) { + if ($shareKey && $privateKey) { + return $this->crypt->multiKeyDecrypt($shareKey, $privateKey); + } } return ''; @@ -656,6 +666,10 @@ class KeyManager { return $this->keyStorage->deleteAllFileKeys($path); } + public function deleteLegacyFileKey(string $path): bool { + return $this->keyStorage->deleteFileKey($path, $this->fileKeyId, Encryption::ID); + } + /** * @param array $userIds * @return array diff --git a/apps/encryption/lib/Recovery.php b/apps/encryption/lib/Recovery.php index f4336ec7c4e..25738dabf89 100644 --- a/apps/encryption/lib/Recovery.php +++ b/apps/encryption/lib/Recovery.php @@ -35,8 +35,6 @@ use OCP\IUserSession; use OCP\PreConditionNotMetException; class Recovery { - - /** * @var null|IUser */ @@ -102,7 +100,7 @@ class Recovery { } if ($keyManager->checkRecoveryPassword($password)) { - $appConfig->setAppValue('encryption', 'recoveryAdminEnabled', 1); + $appConfig->setAppValue('encryption', 'recoveryAdminEnabled', '1'); return true; } @@ -140,7 +138,7 @@ class Recovery { if ($keyManager->checkRecoveryPassword($recoveryPassword)) { // Set recoveryAdmin as disabled - $this->config->setAppValue('encryption', 'recoveryAdminEnabled', 0); + $this->config->setAppValue('encryption', 'recoveryAdminEnabled', '0'); return true; } return false; @@ -169,7 +167,7 @@ class Recovery { * @return bool */ public function isRecoveryKeyEnabled() { - $enabled = $this->config->getAppValue('encryption', 'recoveryAdminEnabled', 0); + $enabled = $this->config->getAppValue('encryption', 'recoveryAdminEnabled', '0'); return ($enabled === '1'); } @@ -199,16 +197,15 @@ class Recovery { /** * add recovery key to all encrypted files - * @param string $path */ - private function addRecoveryKeys($path) { + private function addRecoveryKeys(string $path): void { $dirContent = $this->view->getDirectoryContent($path); foreach ($dirContent as $item) { $filePath = $item->getPath(); if ($item['type'] === 'dir') { $this->addRecoveryKeys($filePath . '/'); } else { - $fileKey = $this->keyManager->getFileKey($filePath, $this->user->getUID()); + $fileKey = $this->keyManager->getFileKey($filePath, $this->user->getUID(), null); if (!empty($fileKey)) { $accessList = $this->file->getAccessList($filePath); $publicKeys = []; @@ -218,8 +215,11 @@ class Recovery { $publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $this->user->getUID()); - $encryptedKeyfiles = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys); - $this->keyManager->setAllFileKeys($filePath, $encryptedKeyfiles); + $shareKeys = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys); + $this->keyManager->deleteLegacyFileKey($filePath); + foreach ($shareKeys as $uid => $keyFile) { + $this->keyManager->setShareKey($filePath, $uid, $keyFile); + } } } } @@ -227,9 +227,8 @@ class Recovery { /** * remove recovery key to all encrypted files - * @param string $path */ - private function removeRecoveryKeys($path) { + private function removeRecoveryKeys(string $path): void { $dirContent = $this->view->getDirectoryContent($path); foreach ($dirContent as $item) { $filePath = $item->getPath(); @@ -243,11 +242,8 @@ class Recovery { /** * recover users files with the recovery key - * - * @param string $recoveryPassword - * @param string $user */ - public function recoverUsersFiles($recoveryPassword, $user) { + public function recoverUsersFiles(string $recoveryPassword, string $user): void { $encryptedKey = $this->keyManager->getSystemPrivateKey($this->keyManager->getRecoveryKeyId()); $privateKey = $this->crypt->decryptPrivateKey($encryptedKey, $recoveryPassword); @@ -258,12 +254,8 @@ class Recovery { /** * recover users files - * - * @param string $path - * @param string $privateKey - * @param string $uid */ - private function recoverAllFiles($path, $privateKey, $uid) { + private function recoverAllFiles(string $path, string $privateKey, string $uid): void { $dirContent = $this->view->getDirectoryContent($path); foreach ($dirContent as $item) { @@ -279,19 +271,17 @@ class Recovery { /** * recover file - * - * @param string $path - * @param string $privateKey - * @param string $uid */ - private function recoverFile($path, $privateKey, $uid) { + private function recoverFile(string $path, string $privateKey, string $uid): void { $encryptedFileKey = $this->keyManager->getEncryptedFileKey($path); $shareKey = $this->keyManager->getShareKey($path, $this->keyManager->getRecoveryKeyId()); if ($encryptedFileKey && $shareKey && $privateKey) { - $fileKey = $this->crypt->multiKeyDecrypt($encryptedFileKey, + $fileKey = $this->crypt->multiKeyDecryptLegacy($encryptedFileKey, $shareKey, $privateKey); + } elseif ($shareKey && $privateKey) { + $fileKey = $this->crypt->multiKeyDecrypt($shareKey, $privateKey); } if (!empty($fileKey)) { @@ -303,8 +293,11 @@ class Recovery { $publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $uid); - $encryptedKeyfiles = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys); - $this->keyManager->setAllFileKeys($path, $encryptedKeyfiles); + $shareKeys = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys); + $this->keyManager->deleteLegacyFileKey($path); + foreach ($shareKeys as $uid => $keyFile) { + $this->keyManager->setShareKey($path, $uid, $keyFile); + } } } } diff --git a/apps/encryption/tests/Crypto/CryptTest.php b/apps/encryption/tests/Crypto/CryptTest.php index 08d0bba2668..dd41c67e8ad 100644 --- a/apps/encryption/tests/Crypto/CryptTest.php +++ b/apps/encryption/tests/Crypto/CryptTest.php @@ -34,8 +34,6 @@ use OCP\IUserSession; use Test\TestCase; class CryptTest extends TestCase { - - /** @var \OCP\ILogger|\PHPUnit\Framework\MockObject\MockObject */ private $logger; @@ -155,7 +153,7 @@ class CryptTest extends TestCase { ->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()); + $this->assertSame('AES-256-CTR', $this->crypt->getCipher()); } /** @@ -396,7 +394,7 @@ class CryptTest extends TestCase { public function testDecryptPrivateKey($header, $privateKey, $expectedCipher, $isValidKey, $expected) { $this->config->method('getSystemValueBool') ->withConsecutive(['encryption.legacy_format_support', false], - ['encryption.use_legacy_base64_encoding', false]) + ['encryption.use_legacy_base64_encoding', false]) ->willReturnOnConsecutiveCalls(true, false); /** @var \OCA\Encryption\Crypto\Crypt | \PHPUnit\Framework\MockObject\MockObject $crypt */ @@ -465,4 +463,17 @@ class CryptTest extends TestCase { $this->invokePrivate($this->crypt, 'isValidPrivateKey', ['foo']) ); } + + public function testMultiKeyEncrypt() { + $res = openssl_pkey_new(); + openssl_pkey_export($res, $privateKey); + $publicKeyPem = openssl_pkey_get_details($res)['key']; + $publicKey = openssl_pkey_get_public($publicKeyPem); + + $shareKeys = $this->crypt->multiKeyEncrypt('content', ['user1' => $publicKey]); + $this->assertEquals( + 'content', + $this->crypt->multiKeyDecrypt($shareKeys['user1'], $privateKey) + ); + } } diff --git a/apps/encryption/tests/Crypto/EncryptionTest.php b/apps/encryption/tests/Crypto/EncryptionTest.php index 63698dcdc63..675a8bf8e29 100644 --- a/apps/encryption/tests/Crypto/EncryptionTest.php +++ b/apps/encryption/tests/Crypto/EncryptionTest.php @@ -42,7 +42,6 @@ use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class EncryptionTest extends TestCase { - /** @var Encryption */ private $instance; @@ -156,7 +155,7 @@ class EncryptionTest extends TestCase { ->willReturnCallback([$this, 'addSystemKeysCallback']); $this->cryptMock->expects($this->any()) ->method('multiKeyEncrypt') - ->willReturn(true); + ->willReturn([]); $this->instance->end('/foo/bar'); } @@ -276,7 +275,7 @@ class EncryptionTest extends TestCase { ->with($path, $recoveryKeyId) ->willReturn($recoveryShareKey); $this->cryptMock->expects($this->once()) - ->method('multiKeyDecrypt') + ->method('multiKeyDecryptLegacy') ->with('encryptedFileKey', $recoveryShareKey, $decryptAllKey) ->willReturn($fileKey); @@ -378,6 +377,7 @@ class EncryptionTest extends TestCase { function ($fileKey, $publicKeys) { $this->assertEmpty($publicKeys); $this->assertSame('fileKey', $fileKey); + return []; } ); diff --git a/apps/encryption/tests/KeyManagerTest.php b/apps/encryption/tests/KeyManagerTest.php index c08f8b576d9..be116318185 100644 --- a/apps/encryption/tests/KeyManagerTest.php +++ b/apps/encryption/tests/KeyManagerTest.php @@ -267,7 +267,6 @@ class KeyManagerTest extends TestCase { * @param bool $useMasterKey */ public function testInit($useMasterKey) { - /** @var \OCA\Encryption\KeyManager|\PHPUnit\Framework\MockObject\MockObject $instance */ $instance = $this->getMockBuilder(KeyManager::class) ->setConstructorArgs( @@ -373,14 +372,22 @@ class KeyManagerTest extends TestCase { public function dataTestGetFileKey() { return [ - ['user1', false, 'privateKey', true], - ['user1', false, false, ''], - ['user1', true, 'privateKey', true], - ['user1', true, false, ''], - [null, false, 'privateKey', true], - [null, false, false, ''], - [null, true, 'privateKey', true], - [null, true, false, ''] + ['user1', false, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], + ['user1', false, 'privateKey', '', 'multiKeyDecryptResult'], + ['user1', false, false, 'legacyKey', ''], + ['user1', false, false, '', ''], + ['user1', true, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], + ['user1', true, 'privateKey', '', 'multiKeyDecryptResult'], + ['user1', true, false, 'legacyKey', ''], + ['user1', true, false, '', ''], + [null, false, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], + [null, false, 'privateKey', '', 'multiKeyDecryptResult'], + [null, false, false, 'legacyKey', ''], + [null, false, false, '', ''], + [null, true, 'privateKey', 'legacyKey', 'multiKeyDecryptResult'], + [null, true, 'privateKey', '', 'multiKeyDecryptResult'], + [null, true, false, 'legacyKey', ''], + [null, true, false, '', ''], ]; } @@ -392,7 +399,7 @@ class KeyManagerTest extends TestCase { * @param $privateKey * @param $expected */ - public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $expected) { + public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $encryptedFileKey, $expected) { $path = '/foo.txt'; if ($isMasterKeyEnabled) { @@ -414,8 +421,8 @@ class KeyManagerTest extends TestCase { [$path, $expectedUid . '.shareKey', 'OC_DEFAULT_MODULE'], ) ->willReturnOnConsecutiveCalls( - true, - true, + $encryptedFileKey, + 'fileKey', ); $this->utilMock->expects($this->any())->method('isMasterKeyEnabled') @@ -434,17 +441,32 @@ class KeyManagerTest extends TestCase { $this->sessionMock->expects($this->once())->method('getPrivateKey')->willReturn($privateKey); } - if ($privateKey) { - $this->cryptMock->expects($this->once()) - ->method('multiKeyDecrypt') - ->willReturn(true); - } else { + if (!empty($encryptedFileKey)) { $this->cryptMock->expects($this->never()) ->method('multiKeyDecrypt'); + if ($privateKey) { + $this->cryptMock->expects($this->once()) + ->method('multiKeyDecryptLegacy') + ->willReturn('multiKeyDecryptResult'); + } else { + $this->cryptMock->expects($this->never()) + ->method('multiKeyDecryptLegacy'); + } + } else { + $this->cryptMock->expects($this->never()) + ->method('multiKeyDecryptLegacy'); + if ($privateKey) { + $this->cryptMock->expects($this->once()) + ->method('multiKeyDecrypt') + ->willReturn('multiKeyDecryptResult'); + } else { + $this->cryptMock->expects($this->never()) + ->method('multiKeyDecrypt'); + } } $this->assertSame($expected, - $this->instance->getFileKey($path, $uid) + $this->instance->getFileKey($path, $uid, null) ); } @@ -562,7 +584,6 @@ class KeyManagerTest extends TestCase { * @param $masterKey */ public function testValidateMasterKey($masterKey) { - /** @var \OCA\Encryption\KeyManager | \PHPUnit\Framework\MockObject\MockObject $instance */ $instance = $this->getMockBuilder(KeyManager::class) ->setConstructorArgs( diff --git a/apps/encryption/tests/RecoveryTest.php b/apps/encryption/tests/RecoveryTest.php index 37b6671d5cb..af053515f8c 100644 --- a/apps/encryption/tests/RecoveryTest.php +++ b/apps/encryption/tests/RecoveryTest.php @@ -211,7 +211,8 @@ class RecoveryTest extends TestCase { ->willReturn([]); $this->cryptMock->expects($this->once()) - ->method('decryptPrivateKey'); + ->method('decryptPrivateKey') + ->willReturn('privateKey'); $this->instance->recoverUsersFiles('password', 'admin'); $this->addToAssertionCount(1); } @@ -226,8 +227,8 @@ class RecoveryTest extends TestCase { ->willReturn(true); $this->cryptMock->expects($this->once()) - ->method('multiKeyDecrypt') - ->willReturn(true); + ->method('multiKeyDecryptLegacy') + ->willReturn('multiKeyDecryptLegacyResult'); $this->fileMock->expects($this->once()) ->method('getAccessList') @@ -244,10 +245,13 @@ class RecoveryTest extends TestCase { $this->cryptMock->expects($this->once()) - ->method('multiKeyEncrypt'); + ->method('multiKeyEncrypt') + ->willReturn(['admin' => 'shareKey']); $this->keyManagerMock->expects($this->once()) - ->method('setAllFileKeys'); + ->method('deleteLegacyFileKey'); + $this->keyManagerMock->expects($this->once()) + ->method('setShareKey'); $this->assertNull(self::invokePrivate($this->instance, 'recoverFile', diff --git a/core/Migrations/Version24000Date20220404230027.php b/core/Migrations/Version24000Date20220404230027.php index d53a43af959..26af9a4c6cc 100644 --- a/core/Migrations/Version24000Date20220404230027.php +++ b/core/Migrations/Version24000Date20220404230027.php @@ -52,11 +52,15 @@ class Version24000Date20220404230027 extends SimpleMigrationStep { 'notnull' => true, 'length' => 50, ]); - $table->addColumn('metadata', Types::JSON, [ - 'notnull' => true, + $table->addColumn('value', Types::TEXT, [ + 'notnull' => false, + 'default' => '', ]); $table->setPrimaryKey(['id', 'group_name'], 'file_metadata_idx'); + + return $schema; } - return $schema; + + return null; } } diff --git a/core/Migrations/Version27000Date20230309104325.php b/core/Migrations/Version27000Date20230309104325.php new file mode 100644 index 00000000000..e11b37b4b29 --- /dev/null +++ b/core/Migrations/Version27000Date20230309104325.php @@ -0,0 +1,90 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Louis Chmn <louis@chmn.me> + * + * @author Louis Chmn <louis@chmn.me> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Migrate oc_file_metadata.metadata as JSON type to oc_file_metadata.value a STRING type + * @see \OC\Metadata\FileMetadata + */ +class Version27000Date20230309104325 extends SimpleMigrationStep { + public function __construct( + private IDBConnection $connection + ) { + } + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $metadataTable = $schema->getTable('file_metadata'); + + if ($metadataTable->hasColumn('value')) { + return null; + } + + $metadataTable->addColumn('value', Types::TEXT, [ + 'notnull' => false, + 'default' => '', + ]); + return $schema; + } + + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return void + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $metadataTable = $schema->getTable('file_metadata'); + + if (!$metadataTable->hasColumn('metadata')) { + return; + } + + $this->connection + ->getQueryBuilder() + ->update('file_metadata') + ->set('value', 'metadata') + ->executeStatement(); + } +} diff --git a/core/Migrations/Version27000Date20230309104802.php b/core/Migrations/Version27000Date20230309104802.php new file mode 100644 index 00000000000..4bd50fe0396 --- /dev/null +++ b/core/Migrations/Version27000Date20230309104802.php @@ -0,0 +1,57 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2023 Louis Chmn <louis@chmn.me> + * + * @author Louis Chmn <louis@chmn.me> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Migrate oc_file_metadata.metadata as JSON type to oc_file_metadata.value a STRING type + * @see \OC\Metadata\FileMetadata + */ +class Version27000Date20230309104802 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $metadataTable = $schema->getTable('file_metadata'); + + if ($metadataTable->hasColumn('metadata')) { + $metadataTable->dropColumn('metadata'); + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 40d0c4f0cd1..c2d460d1511 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1089,6 +1089,8 @@ return array( 'OC\\Core\\Migrations\\Version25000Date20220602190540' => $baseDir . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20220905140840' => $baseDir . '/core/Migrations/Version25000Date20220905140840.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => $baseDir . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104325' => $baseDir . '/core/Migrations/Version27000Date20230309104325.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104802' => $baseDir . '/core/Migrations/Version27000Date20230309104802.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 65021f54adb..33a0d49a2dd 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1122,6 +1122,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version25000Date20220602190540' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220602190540.php', 'OC\\Core\\Migrations\\Version25000Date20220905140840' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20220905140840.php', 'OC\\Core\\Migrations\\Version25000Date20221007010957' => __DIR__ . '/../../..' . '/core/Migrations/Version25000Date20221007010957.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104325' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104325.php', + 'OC\\Core\\Migrations\\Version27000Date20230309104802' => __DIR__ . '/../../..' . '/core/Migrations/Version27000Date20230309104802.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', diff --git a/lib/private/Metadata/FileMetadata.php b/lib/private/Metadata/FileMetadata.php index 9ad0f9d35c6..a9808a86998 100644 --- a/lib/private/Metadata/FileMetadata.php +++ b/lib/private/Metadata/FileMetadata.php @@ -28,16 +28,24 @@ use OCP\DB\Types; /** * @method string getGroupName() * @method void setGroupName(string $groupName) - * @method array getMetadata() - * @method void setMetadata(array $metadata) + * @method string getValue() + * @method void setValue(string $value) * @see \OC\Core\Migrations\Version240000Date20220404230027 */ class FileMetadata extends Entity { protected ?string $groupName = null; - protected ?array $metadata = null; + protected ?string $value = null; public function __construct() { $this->addType('groupName', 'string'); - $this->addType('metadata', Types::JSON); + $this->addType('value', Types::STRING); + } + + public function getDecodedValue(): array { + return json_decode($this->getValue(), true) ?? []; + } + + public function setArrayAsValue(array $value): void { + $this->setValue(json_encode($value, JSON_THROW_ON_ERROR)); } } diff --git a/lib/private/Metadata/FileMetadataMapper.php b/lib/private/Metadata/FileMetadataMapper.php index f8f8df4bf3b..f3120e5e515 100644 --- a/lib/private/Metadata/FileMetadataMapper.php +++ b/lib/private/Metadata/FileMetadataMapper.php @@ -89,7 +89,7 @@ class FileMetadataMapper extends QBMapper { continue; } $empty = new FileMetadata(); - $empty->setMetadata([]); + $empty->setValue(''); $empty->setGroupName($groupName); $empty->setId($id); $metadata[$id] = $empty; @@ -132,13 +132,13 @@ class FileMetadataMapper extends QBMapper { $idType = $this->getParameterTypeForProperty($entity, 'id'); $groupNameType = $this->getParameterTypeForProperty($entity, 'groupName'); - $metadataValue = $entity->getMetadata(); - $metadataType = $this->getParameterTypeForProperty($entity, 'metadata'); + $value = $entity->getValue(); + $valueType = $this->getParameterTypeForProperty($entity, 'value'); $qb = $this->db->getQueryBuilder(); $qb->update($this->tableName) - ->set('metadata', $qb->createNamedParameter($metadataValue, $metadataType)) + ->set('value', $qb->createNamedParameter($value, $valueType)) ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, $idType))) ->andWhere($qb->expr()->eq('group_name', $qb->createNamedParameter($groupName, $groupNameType))) ->executeStatement(); diff --git a/lib/private/Metadata/MetadataManager.php b/lib/private/Metadata/MetadataManager.php index 77407a2f529..6d96ff1ab68 100644 --- a/lib/private/Metadata/MetadataManager.php +++ b/lib/private/Metadata/MetadataManager.php @@ -78,6 +78,9 @@ class MetadataManager implements IMetadataManager { $this->fileMetadataMapper->clear($fileId); } + /** + * @return array<int, FileMetadata> + */ public function fetchMetadataFor(string $group, array $fileIds): array { return $this->fileMetadataMapper->findForGroupForFiles($fileIds, $group); } diff --git a/lib/private/Metadata/Provider/ExifProvider.php b/lib/private/Metadata/Provider/ExifProvider.php index ae2c57ba7e5..4e211e7b6c4 100644 --- a/lib/private/Metadata/Provider/ExifProvider.php +++ b/lib/private/Metadata/Provider/ExifProvider.php @@ -65,12 +65,12 @@ class ExifProvider implements IMetadataProvider { $size = new FileMetadata(); $size->setGroupName('size'); $size->setId($file->getId()); - $size->setMetadata([]); + $size->setArrayAsValue([]); if (!$data) { $sizeResult = getimagesizefromstring($file->getContent()); if ($sizeResult !== false) { - $size->setMetadata([ + $size->setArrayAsValue([ 'width' => $sizeResult[0], 'height' => $sizeResult[1], ]); @@ -79,7 +79,7 @@ class ExifProvider implements IMetadataProvider { } } elseif (array_key_exists('COMPUTED', $data)) { if (array_key_exists('Width', $data['COMPUTED']) && array_key_exists('Height', $data['COMPUTED'])) { - $size->setMetadata([ + $size->setArrayAsValue([ 'width' => $data['COMPUTED']['Width'], 'height' => $data['COMPUTED']['Height'], ]); @@ -95,7 +95,7 @@ class ExifProvider implements IMetadataProvider { $gps = new FileMetadata(); $gps->setGroupName('gps'); $gps->setId($file->getId()); - $gps->setMetadata([ + $gps->setArrayAsValue([ 'latitude' => $this->gpsDegreesToDecimal($data['GPS']['GPSLatitude'], $data['GPS']['GPSLatitudeRef']), 'longitude' => $this->gpsDegreesToDecimal($data['GPS']['GPSLongitude'], $data['GPS']['GPSLongitudeRef']), ]); diff --git a/lib/public/Encryption/IEncryptionModule.php b/lib/public/Encryption/IEncryptionModule.php index d0ae430a81a..37db2729335 100644 --- a/lib/public/Encryption/IEncryptionModule.php +++ b/lib/public/Encryption/IEncryptionModule.php @@ -60,7 +60,7 @@ interface IEncryptionModule { * @param array $header contains the header data read from the file * @param array $accessList who has access to the file contains the key 'users' and 'public' * - * $return array $header contain data as key-value pairs which should be + * @return array $header contain data as key-value pairs which should be * written to the header, in case of a write operation * or if no additional data is needed return a empty array * @since 8.1.0 diff --git a/tests/lib/Metadata/FileMetadataMapperTest.php b/tests/lib/Metadata/FileMetadataMapperTest.php index 1a005f24b8a..4f7708ab9a9 100644 --- a/tests/lib/Metadata/FileMetadataMapperTest.php +++ b/tests/lib/Metadata/FileMetadataMapperTest.php @@ -51,23 +51,23 @@ class FileMetadataMapperTest extends \Test\TestCase { $file1 = new FileMetadata(); $file1->setId(1); $file1->setGroupName('size'); - $file1->setMetadata([]); + $file1->setArrayAsValue([]); $file2 = new FileMetadata(); $file2->setId(2); $file2->setGroupName('size'); - $file2->setMetadata(['width' => 293, 'height' => 23]); + $file2->setArrayAsValue(['width' => 293, 'height' => 23]); // not added, it's the default $file3 = new FileMetadata(); $file3->setId(3); $file3->setGroupName('size'); - $file3->setMetadata([]); + $file3->setArrayAsValue([]); $file4 = new FileMetadata(); $file4->setId(4); $file4->setGroupName('size'); - $file4->setMetadata(['complex' => ["yes", "maybe" => 34.0]]); + $file4->setArrayAsValue(['complex' => ["yes", "maybe" => 34.0]]); $this->mapper->insert($file1); $this->mapper->insert($file2); @@ -75,10 +75,10 @@ class FileMetadataMapperTest extends \Test\TestCase { $files = $this->mapper->findForGroupForFiles([1, 2, 3, 4], 'size'); - $this->assertEquals($files[1]->getMetadata(), $file1->getMetadata()); - $this->assertEquals($files[2]->getMetadata(), $file2->getMetadata()); - $this->assertEquals($files[3]->getMetadata(), $file3->getMetadata()); - $this->assertEquals($files[4]->getMetadata(), $file4->getMetadata()); + $this->assertEquals($files[1]->getValue(), $file1->getValue()); + $this->assertEquals($files[2]->getValue(), $file2->getValue()); + $this->assertEquals($files[3]->getDecodedValue(), $file3->getDecodedValue()); + $this->assertEquals($files[4]->getValue(), $file4->getValue()); $this->mapper->clear(1); $this->mapper->clear(2); |