From 8928bbe96901b9374ecd42cb6cf9e3e7914ebafa Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 12 Aug 2020 13:22:24 +0200 Subject: [PATCH] Make legacy cipher opt in * Systems that upgrade have this enabled by default * New systems disable it * We'll have to add some wargning in the setup checks if this is enabled Signed-off-by: Roeland Jago Douma --- apps/encryption/lib/Crypto/Crypt.php | 17 ++++- apps/encryption/tests/Crypto/CryptTest.php | 10 +++ apps/encryption/tests/Settings/AdminTest.php | 23 +++---- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Repair.php | 2 + .../Repair/NC20/EncryptionLegacyCipher.php | 62 +++++++++++++++++++ version.php | 2 +- 8 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 lib/private/Repair/NC20/EncryptionLegacyCipher.php diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php index 7723b63a66..02059f632f 100644 --- a/apps/encryption/lib/Crypto/Crypt.php +++ b/apps/encryption/lib/Crypto/Crypt.php @@ -32,6 +32,7 @@ namespace OCA\Encryption\Crypto; use OC\Encryption\Exceptions\DecryptionFailedException; use OC\Encryption\Exceptions\EncryptionFailedException; +use OC\ServerNotAvailableException; use OCA\Encryption\Exceptions\MultiKeyDecryptException; use OCA\Encryption\Exceptions\MultiKeyEncryptException; use OCP\Encryption\Exceptions\GenericEncryptionException; @@ -89,6 +90,9 @@ class Crypt { 'AES-128-CFB' => 16, ]; + /** @var bool */ + private $supportLegacy; + /** * @param ILogger $logger * @param IUserSession $userSession @@ -101,6 +105,8 @@ class Crypt { $this->config = $config; $this->l = $l; $this->supportedKeyFormats = ['hash', 'password']; + + $this->supportLegacy = $this->config->getSystemValueBool('encryption.legacy_format_support', false); } /** @@ -299,6 +305,10 @@ class Crypt { * @return string */ public function getLegacyCipher() { + if (!$this->supportLegacy) { + throw new ServerNotAvailableException('Legacy cipher is no longer supported!'); + } + return self::LEGACY_CIPHER; } @@ -391,7 +401,7 @@ class Crypt { if (isset($header['cipher'])) { $cipher = $header['cipher']; } else { - $cipher = self::LEGACY_CIPHER; + $cipher = $this->getLegacyCipher(); } if (isset($header['keyFormat'])) { @@ -570,6 +580,11 @@ class Crypt { $meta = substr($catFile, -93); $signaturePosition = strpos($meta, '00sig00'); + // If we no longer support the legacy format then everything needs a signature + if (!$skipSignatureCheck && !$this->supportLegacy && $signaturePosition === false) { + throw new GenericEncryptionException('Missing Signature', $this->l->t('Missing Signature')); + } + // enforce signature for the new 'CTR' ciphers if (!$skipSignatureCheck && $signaturePosition === false && stripos($cipher, 'ctr') !== false) { throw new GenericEncryptionException('Missing Signature', $this->l->t('Missing Signature')); diff --git a/apps/encryption/tests/Crypto/CryptTest.php b/apps/encryption/tests/Crypto/CryptTest.php index 4af41a9aac..a49a37bea0 100644 --- a/apps/encryption/tests/Crypto/CryptTest.php +++ b/apps/encryption/tests/Crypto/CryptTest.php @@ -209,6 +209,9 @@ class CryptTest extends TestCase { * @dataProvider dataTestSplitMetaData */ public function testSplitMetaData($data, $expected) { + $this->config->method('getSystemValue') + ->with('encryption_skip_signature_check', false) + ->willReturn(true); $result = self::invokePrivate($this->crypt, 'splitMetaData', [$data, 'AES-256-CFB']); $this->assertTrue(is_array($result)); $this->assertSame(3, count($result)); @@ -233,6 +236,9 @@ class CryptTest extends TestCase { * @dataProvider dataTestHasSignature */ public function testHasSignature($data, $expected) { + $this->config->method('getSystemValue') + ->with('encryption_skip_signature_check', false) + ->willReturn(true); $this->assertSame($expected, $this->invokePrivate($this->crypt, 'hasSignature', [$data, 'AES-256-CFB']) ); @@ -385,6 +391,10 @@ class CryptTest extends TestCase { * @dataProvider dataTestDecryptPrivateKey */ public function testDecryptPrivateKey($header, $privateKey, $expectedCipher, $isValidKey, $expected) { + $this->config->method('getSystemValueBool') + ->with('encryption.legacy_format_support', false) + ->willReturn(true); + /** @var \OCA\Encryption\Crypto\Crypt | \PHPUnit\Framework\MockObject\MockObject $crypt */ $crypt = $this->getMockBuilder(Crypt::class) ->setConstructorArgs( diff --git a/apps/encryption/tests/Settings/AdminTest.php b/apps/encryption/tests/Settings/AdminTest.php index 78ea6158fd..82eed2d78a 100644 --- a/apps/encryption/tests/Settings/AdminTest.php +++ b/apps/encryption/tests/Settings/AdminTest.php @@ -74,20 +74,21 @@ class AdminTest extends TestCase { public function testGetForm() { $this->config - ->expects($this->at(0)) ->method('getAppValue') - ->with('encryption', 'recoveryAdminEnabled', '0') - ->willReturn(1); - $this->config - ->expects($this->at(1)) - ->method('getAppValue') - ->with('encryption', 'encryptHomeStorage', '1') - ->willReturn(1); + ->will($this->returnCallback(function ($app, $key, $default) { + if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') { + return '1'; + } + if ($app === 'encryption' && $key === 'encryptHomeStorage' && $default === '1') { + return '1'; + } + return $default; + })); $params = [ - 'recoveryEnabled' => 1, + 'recoveryEnabled' => '1', 'initStatus' => '0', - 'encryptHomeStorage' => false, - 'masterKeyEnabled' => false + 'encryptHomeStorage' => true, + 'masterKeyEnabled' => true ]; $expected = new TemplateResponse('encryption', 'settings-admin', $params, ''); $this->assertEquals($expected, $this->admin->getForm()); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index e063fe0b71..03732c6315 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1245,6 +1245,7 @@ return array( 'OC\\Repair\\NC16\\CleanupCardDAVPhotoCache' => $baseDir . '/lib/private/Repair/NC16/CleanupCardDAVPhotoCache.php', 'OC\\Repair\\NC16\\ClearCollectionsAccessCache' => $baseDir . '/lib/private/Repair/NC16/ClearCollectionsAccessCache.php', 'OC\\Repair\\NC18\\ResetGeneratedAvatarFlag' => $baseDir . '/lib/private/Repair/NC18/ResetGeneratedAvatarFlag.php', + 'OC\\Repair\\NC20\\EncryptionLegacyCipher' => $baseDir . '/lib/private/Repair/NC20/EncryptionLegacyCipher.php', 'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\DropAccountTermsTable' => $baseDir . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php', 'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 20394b4ae3..41a68ef8dc 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1274,6 +1274,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Repair\\NC16\\CleanupCardDAVPhotoCache' => __DIR__ . '/../../..' . '/lib/private/Repair/NC16/CleanupCardDAVPhotoCache.php', 'OC\\Repair\\NC16\\ClearCollectionsAccessCache' => __DIR__ . '/../../..' . '/lib/private/Repair/NC16/ClearCollectionsAccessCache.php', 'OC\\Repair\\NC18\\ResetGeneratedAvatarFlag' => __DIR__ . '/../../..' . '/lib/private/Repair/NC18/ResetGeneratedAvatarFlag.php', + 'OC\\Repair\\NC20\\EncryptionLegacyCipher' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionLegacyCipher.php', 'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\DropAccountTermsTable' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php', 'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php', diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 60609de417..6151812d31 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -48,6 +48,7 @@ use OC\Repair\NC16\AddClenupLoginFlowV2BackgroundJob; use OC\Repair\NC16\CleanupCardDAVPhotoCache; use OC\Repair\NC16\ClearCollectionsAccessCache; use OC\Repair\NC18\ResetGeneratedAvatarFlag; +use OC\Repair\NC20\EncryptionLegacyCipher; use OC\Repair\OldGroupMembershipShares; use OC\Repair\Owncloud\DropAccountTermsTable; use OC\Repair\Owncloud\SaveAccountsTableData; @@ -156,6 +157,7 @@ class Repair implements IOutput { new RemoveLinkShares(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig(), \OC::$server->getGroupManager(), \OC::$server->getNotificationManager(), \OC::$server->query(ITimeFactory::class)), new ClearCollectionsAccessCache(\OC::$server->getConfig(), \OC::$server->query(IManager::class)), \OC::$server->query(ResetGeneratedAvatarFlag::class), + \OC::$server->query(EncryptionLegacyCipher::class), ]; } diff --git a/lib/private/Repair/NC20/EncryptionLegacyCipher.php b/lib/private/Repair/NC20/EncryptionLegacyCipher.php new file mode 100644 index 0000000000..4e743f3f1d --- /dev/null +++ b/lib/private/Repair/NC20/EncryptionLegacyCipher.php @@ -0,0 +1,62 @@ + + * + * @author Roeland Jago Douma + * + * @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 . + * + */ + +namespace OC\Repair\NC20; + +use OCP\Encryption\IManager; +use OCP\IConfig; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class EncryptionLegacyCipher implements IRepairStep { + + /** @var IConfig */ + private $config; + /** @var IManager */ + private $manager; + + public function __construct(IConfig $config, + IManager $manager) { + $this->config = $config; + $this->manager = $manager; + } + + public function getName(): string { + return 'Keep legacy encryption enabled'; + } + + private function shouldRun(): bool { + $versionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0.0'); + return version_compare($versionFromBeforeUpdate, '20.0.0.0', '<='); + } + + public function run(IOutput $output): void { + if ($this->manager->isEnabled()) { + if ($this->config->getSystemValue('encryption.legacy_format_support', '') === '') { + $this->config->setSystemValue('encryption.legacy_format_support', true); + } + } + } +} diff --git a/version.php b/version.php index c60039b454..9df6d8fbec 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [20, 0, 0, 0]; +$OC_Version = [20, 0, 0, 1]; // The human readable string $OC_VersionString = '20.0.0 alpha';