From 9524badccc6fd1e9d423f8dd8316b32ea576863b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 25 Jul 2017 11:54:14 +0200 Subject: [PATCH 1/4] extend the identity proof manager to allow system wide key pairs Signed-off-by: Bjoern Schiessle --- .../DependencyInjection/DIContainer.php | 3 +- .../Security/IdentityProof/Manager.php | 55 +++++++++++--- .../Security/IdentityProof/ManagerTest.php | 73 ++++++++++++++++--- 3 files changed, 108 insertions(+), 23 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index d0c69c3bf3..78400d8082 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -171,7 +171,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { $this->registerService(\OC\Security\IdentityProof\Manager::class, function ($c) { return new \OC\Security\IdentityProof\Manager( $this->getServer()->query(\OC\Files\AppData\Factory::class), - $this->getServer()->getCrypto() + $this->getServer()->getCrypto(), + $this->getServer()->getConfig() ); }); diff --git a/lib/private/Security/IdentityProof/Manager.php b/lib/private/Security/IdentityProof/Manager.php index 73edac5f74..a8c204c84b 100644 --- a/lib/private/Security/IdentityProof/Manager.php +++ b/lib/private/Security/IdentityProof/Manager.php @@ -23,6 +23,7 @@ namespace OC\Security\IdentityProof; use OC\Files\AppData\Factory; use OCP\Files\IAppData; +use OCP\IConfig; use OCP\IUser; use OCP\Security\ICrypto; @@ -31,15 +32,21 @@ class Manager { private $appData; /** @var ICrypto */ private $crypto; + /** @var IConfig */ + private $config; /** * @param Factory $appDataFactory * @param ICrypto $crypto + * @param IConfig $config */ public function __construct(Factory $appDataFactory, - ICrypto $crypto) { + ICrypto $crypto, + IConfig $config + ) { $this->appData = $appDataFactory->get('identityproof'); $this->crypto = $crypto; + $this->config = $config; } /** @@ -66,20 +73,20 @@ class Manager { } /** - * Generate a key for $user + * Generate a key for a given ID * Note: If a key already exists it will be overwritten * - * @param IUser $user + * @param string $id key id * @return Key */ - protected function generateKey(IUser $user) { + protected function generateKey($id) { list($publicKey, $privateKey) = $this->generateKeyPair(); // Write the private and public key to the disk try { - $this->appData->newFolder($user->getUID()); + $this->appData->newFolder($id); } catch (\Exception $e) {} - $folder = $this->appData->getFolder($user->getUID()); + $folder = $this->appData->getFolder($id); $folder->newFile('private') ->putContent($this->crypto->encrypt($privateKey)); $folder->newFile('public') @@ -89,21 +96,47 @@ class Manager { } /** - * Get public and private key for $user + * Get key for a specific id * - * @param IUser $user + * @param string $id * @return Key */ - public function getKey(IUser $user) { + protected function retrieveKey($id) { try { - $folder = $this->appData->getFolder($user->getUID()); + $folder = $this->appData->getFolder($id); $privateKey = $this->crypto->decrypt( $folder->getFile('private')->getContent() ); $publicKey = $folder->getFile('public')->getContent(); return new Key($publicKey, $privateKey); } catch (\Exception $e) { - return $this->generateKey($user); + return $this->generateKey($id); } } + + /** + * Get public and private key for $user + * + * @param IUser $user + * @return Key + */ + public function getKey(IUser $user) { + return $this->retrieveKey($user->getUID()); + } + + /** + * Get instance wide public and private key + * + * @return Key + * @throws \RuntimeException + */ + public function getSystemKey() { + $instanceId = $this->config->getSystemValue('instanceid', null); + if ($instanceId === null) { + throw new \RuntimeException('no instance id!'); + } + return $this->retrieveKey($instanceId); + } + + } diff --git a/tests/lib/Security/IdentityProof/ManagerTest.php b/tests/lib/Security/IdentityProof/ManagerTest.php index b46ca30705..5ab9ce63fb 100644 --- a/tests/lib/Security/IdentityProof/ManagerTest.php +++ b/tests/lib/Security/IdentityProof/ManagerTest.php @@ -27,8 +27,10 @@ use OC\Security\IdentityProof\Manager; use OCP\Files\IAppData; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\IConfig; use OCP\IUser; use OCP\Security\ICrypto; +use SebastianBergmann\Comparator\MockObjectComparator; use Test\TestCase; class ManagerTest extends TestCase { @@ -40,6 +42,8 @@ class ManagerTest extends TestCase { private $crypto; /** @var Manager|\PHPUnit_Framework_MockObject_MockObject */ private $manager; + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + private $config; public function setUp() { parent::setUp(); @@ -47,19 +51,37 @@ class ManagerTest extends TestCase { /** @var Factory|\PHPUnit_Framework_MockObject_MockObject $factory */ $this->factory = $this->createMock(Factory::class); $this->appData = $this->createMock(IAppData::class); + $this->config = $this->createMock(IConfig::class); $this->factory->expects($this->any()) ->method('get') ->with('identityproof') ->willReturn($this->appData); $this->crypto = $this->createMock(ICrypto::class); - $this->manager = $this->getMockBuilder(Manager::class) - ->setConstructorArgs([ + $this->manager = $this->getManager(['generateKeyPair']); + } + + /** + * create manager object + * + * @param array $setMethods + * @return Manager|\PHPUnit_Framework_MockObject_MockObject + */ + protected function getManager($setMethods = []) { + if (empty($setMethods)) { + return new Manager( $this->factory, - $this->crypto - ]) - ->setMethods(['generateKeyPair']) - ->getMock(); + $this->crypto, + $this->config + ); + } else { + return $this->getMockBuilder(Manager::class) + ->setConstructorArgs([ + $this->factory, + $this->crypto, + $this->config + ])->setMethods($setMethods)->getMock(); + } } public function testGetKeyWithExistingKey() { @@ -107,7 +129,7 @@ class ManagerTest extends TestCase { public function testGetKeyWithNotExistingKey() { $user = $this->createMock(IUser::class); $user - ->expects($this->exactly(3)) + ->expects($this->once()) ->method('getUID') ->willReturn('MyUid'); $this->appData @@ -161,10 +183,7 @@ class ManagerTest extends TestCase { } public function testGenerateKeyPair() { - $manager = new Manager( - $this->factory, - $this->crypto - ); + $manager = $this->getManager(); $data = 'MyTestData'; list($resultPublicKey, $resultPrivateKey) = self::invokePrivate($manager, 'generateKeyPair'); @@ -174,4 +193,36 @@ class ManagerTest extends TestCase { $this->assertSame(1, openssl_verify($data, $signature, $resultPublicKey)); $this->assertSame(2048, $details['bits']); } + + public function testGetSystemKey() { + $manager = $this->getManager(['retrieveKey']); + + /** @var Key|\PHPUnit_Framework_MockObject_MockObject $key */ + $key = $this->createMock(Key::class); + + $this->config->expects($this->once())->method('getSystemValue') + ->with('instanceid', null)->willReturn('instanceId'); + + $manager->expects($this->once())->method('retrieveKey')->with('instanceId') + ->willReturn($key); + + $this->assertSame($key, $manager->getSystemKey()); + + } + + + /** + * @expectedException \RuntimeException + */ + public function testGetSystemKeyFailure() { + $manager = $this->getManager(['retrieveKey']); + + /** @var Key|\PHPUnit_Framework_MockObject_MockObject $key */ + $key = $this->createMock(Key::class); + + $this->config->expects($this->once())->method('getSystemValue') + ->with('instanceid', null)->willReturn(null); + + $manager->getSystemKey(); + } } From bae5be3dc14085a5061cd38e74c07453ed19b66b Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 27 Jul 2017 16:52:28 +0200 Subject: [PATCH 2/4] add prefix to user and system keys to avoid name collisions Signed-off-by: Bjoern Schiessle --- lib/private/Repair.php | 5 +- .../NC13/RepairIdentityProofKeyFolders.php | 110 ++++++++++++++++++ .../Security/IdentityProof/Manager.php | 5 +- .../Security/IdentityProof/ManagerTest.php | 10 +- 4 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 lib/private/Repair/NC13/RepairIdentityProofKeyFolders.php diff --git a/lib/private/Repair.php b/lib/private/Repair.php index dae328e634..4864ad77b8 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -31,6 +31,7 @@ namespace OC; use OC\App\AppStore\Bundles\BundleFetcher; +use OC\Files\AppData\Factory; use OC\Repair\CleanTags; use OC\Repair\Collation; use OC\Repair\MoveUpdaterStepFile; @@ -39,6 +40,7 @@ use OC\Repair\NC11\FixMountStorages; use OC\Repair\NC11\MoveAvatars; use OC\Repair\NC12\InstallCoreBundle; use OC\Repair\NC12\UpdateLanguageCodes; +use OC\Repair\NC13\RepairIdentityProofKeyFolders; use OC\Repair\OldGroupMembershipShares; use OC\Repair\Owncloud\SaveAccountsTableData; use OC\Repair\RemoveRootShares; @@ -145,7 +147,8 @@ class Repair implements IOutput{ \OC::$server->getConfig(), \OC::$server->query(Installer::class) ), - new RepairInvalidPaths(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig()) + new RepairInvalidPaths(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig()), + new RepairIdentityProofKeyFolders(\OC::$server->getConfig(), \OC::$server->query(Factory::class), \OC::$server->getRootFolder()), ]; } diff --git a/lib/private/Repair/NC13/RepairIdentityProofKeyFolders.php b/lib/private/Repair/NC13/RepairIdentityProofKeyFolders.php new file mode 100644 index 0000000000..93a135b5cf --- /dev/null +++ b/lib/private/Repair/NC13/RepairIdentityProofKeyFolders.php @@ -0,0 +1,110 @@ + + * + * @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\NC13; + + +use OC\Files\AppData\Factory; +use OCP\Files\IRootFolder; +use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\IConfig; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +class RepairIdentityProofKeyFolders implements IRepairStep { + + /** @var IConfig */ + private $config; + + /** @var \OC\Files\AppData\AppData */ + private $appDataIdentityProof; + + /** @var IRootFolder */ + private $rootFolder; + + /** @var string */ + private $identityProofDir; + + /** + * RepairIdentityProofKeyFolders constructor. + * + * @param IConfig $config + * @param Factory $appDataFactory + * @param IRootFolder $rootFolder + */ + public function __construct(IConfig $config, Factory $appDataFactory, IRootFolder $rootFolder) { + $this->config = $config; + $this->appDataIdentityProof = $appDataFactory->get('identityproof'); + $this->rootFolder = $rootFolder; + + $instanceId = $this->config->getSystemValue('instanceid', null); + if ($instanceId === null) { + throw new \RuntimeException('no instance id!'); + } + $this->identityProofDir = 'appdata_' . $instanceId . '/identityproof/'; + } + + /** + * Returns the step's name + * + * @return string + * @since 9.1.0 + */ + public function getName() { + return "Rename folder with user specific keys"; + } + + /** + * Run repair step. + * Must throw exception on error. + * + * @param IOutput $output + * @throws \Exception in case of failure + * @since 9.1.0 + */ + public function run(IOutput $output) { + $versionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0'); + if (version_compare($versionFromBeforeUpdate, '13.0.0.1', '<=')) { + $count = $this->repair(); + $output->info('Repaired ' . $count . ' folders'); + } + } + + /** + * rename all dirs with user specific keys to 'user-uid' + * + * @return int + */ + private function repair() { + $count = 0; + $dirListing = $this->appDataIdentityProof->getDirectoryListing(); + /** @var ISimpleFolder $folder */ + foreach ($dirListing as $folder) { + $name = $folder->getName(); + $node = $this->rootFolder->get($this->identityProofDir . $name); + $node->move($this->identityProofDir . 'user-' . $name); + $count++; + } + + return $count; + } +} diff --git a/lib/private/Security/IdentityProof/Manager.php b/lib/private/Security/IdentityProof/Manager.php index a8c204c84b..c5134e12b8 100644 --- a/lib/private/Security/IdentityProof/Manager.php +++ b/lib/private/Security/IdentityProof/Manager.php @@ -121,7 +121,8 @@ class Manager { * @return Key */ public function getKey(IUser $user) { - return $this->retrieveKey($user->getUID()); + $uid = $user->getUID(); + return $this->retrieveKey('user-' . $uid); } /** @@ -135,7 +136,7 @@ class Manager { if ($instanceId === null) { throw new \RuntimeException('no instance id!'); } - return $this->retrieveKey($instanceId); + return $this->retrieveKey('system-' . $instanceId); } diff --git a/tests/lib/Security/IdentityProof/ManagerTest.php b/tests/lib/Security/IdentityProof/ManagerTest.php index 5ab9ce63fb..290e7be5c9 100644 --- a/tests/lib/Security/IdentityProof/ManagerTest.php +++ b/tests/lib/Security/IdentityProof/ManagerTest.php @@ -119,7 +119,7 @@ class ManagerTest extends TestCase { $this->appData ->expects($this->once()) ->method('getFolder') - ->with('MyUid') + ->with('user-MyUid') ->willReturn($folder); $expected = new Key('MyPublicKey', 'MyPrivateKey'); @@ -135,7 +135,7 @@ class ManagerTest extends TestCase { $this->appData ->expects($this->at(0)) ->method('getFolder') - ->with('MyUid') + ->with('user-MyUid') ->willThrowException(new \Exception()); $this->manager ->expects($this->once()) @@ -144,7 +144,7 @@ class ManagerTest extends TestCase { $this->appData ->expects($this->at(1)) ->method('newFolder') - ->with('MyUid'); + ->with('user-MyUid'); $folder = $this->createMock(ISimpleFolder::class); $this->crypto ->expects($this->once()) @@ -174,7 +174,7 @@ class ManagerTest extends TestCase { $this->appData ->expects($this->at(2)) ->method('getFolder') - ->with('MyUid') + ->with('user-MyUid') ->willReturn($folder); @@ -203,7 +203,7 @@ class ManagerTest extends TestCase { $this->config->expects($this->once())->method('getSystemValue') ->with('instanceid', null)->willReturn('instanceId'); - $manager->expects($this->once())->method('retrieveKey')->with('instanceId') + $manager->expects($this->once())->method('retrieveKey')->with('system-instanceId') ->willReturn($key); $this->assertSame($key, $manager->getSystemKey()); From d34bd3f17c0b323bd6077c67afacedb2ddff2dc0 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 2 Aug 2017 13:36:50 +0200 Subject: [PATCH 3/4] Bump version Signed-off-by: Roeland Jago Douma --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 649c8ae738..03c03f43c8 100644 --- a/version.php +++ b/version.php @@ -26,7 +26,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = array(13, 0, 0, 2); +$OC_Version = array(13, 0, 0, 3); // The human readable string $OC_VersionString = '13.0.0 alpha'; From 31af5cc3212716b47a4587a94acdae264508f3fb Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 2 Aug 2017 13:37:06 +0200 Subject: [PATCH 4/4] Update autoloader Signed-off-by: Roeland Jago Douma --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 2df64663d6..ef337cddfa 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -756,6 +756,7 @@ return array( 'OC\\Repair\\NC11\\MoveAvatarsBackgroundJob' => $baseDir . '/lib/private/Repair/NC11/MoveAvatarsBackgroundJob.php', 'OC\\Repair\\NC12\\InstallCoreBundle' => $baseDir . '/lib/private/Repair/NC12/InstallCoreBundle.php', 'OC\\Repair\\NC12\\UpdateLanguageCodes' => $baseDir . '/lib/private/Repair/NC12/UpdateLanguageCodes.php', + 'OC\\Repair\\NC13\\RepairIdentityProofKeyFolders' => $baseDir . '/lib/private/Repair/NC13/RepairIdentityProofKeyFolders.php', 'OC\\Repair\\NC13\\RepairInvalidPaths' => $baseDir . '/lib/private/Repair/NC13/RepairInvalidPaths.php', 'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.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 9f87c1060a..70fa8baf5e 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -786,6 +786,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Repair\\NC11\\MoveAvatarsBackgroundJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC11/MoveAvatarsBackgroundJob.php', 'OC\\Repair\\NC12\\InstallCoreBundle' => __DIR__ . '/../../..' . '/lib/private/Repair/NC12/InstallCoreBundle.php', 'OC\\Repair\\NC12\\UpdateLanguageCodes' => __DIR__ . '/../../..' . '/lib/private/Repair/NC12/UpdateLanguageCodes.php', + 'OC\\Repair\\NC13\\RepairIdentityProofKeyFolders' => __DIR__ . '/../../..' . '/lib/private/Repair/NC13/RepairIdentityProofKeyFolders.php', 'OC\\Repair\\NC13\\RepairInvalidPaths' => __DIR__ . '/../../..' . '/lib/private/Repair/NC13/RepairInvalidPaths.php', 'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php', 'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',