From ec2e26b7dadfe1000b97eedd42827be4e346310d Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 13 Jan 2017 09:51:37 +0100 Subject: [PATCH] Setupfs before access a users keys (#26917) * The file system of a user has to be properly setup before accessing the keys * Fix unit test execution * Init mount points for user in more places in Keys\Storage * Fix encryption key storage tests to properly create required users * Added test that check if user home is mounted after resolving key Signed-off-by: Morris Jobke --- lib/private/Encryption/Keys/Storage.php | 33 ++++++++++-- lib/private/Server.php | 6 ++- tests/lib/Encryption/Keys/StorageTest.php | 63 +++++++++++++++++++++-- 3 files changed, 95 insertions(+), 7 deletions(-) diff --git a/lib/private/Encryption/Keys/Storage.php b/lib/private/Encryption/Keys/Storage.php index a4c3a26d7be48..c0af2d39bfcf3 100644 --- a/lib/private/Encryption/Keys/Storage.php +++ b/lib/private/Encryption/Keys/Storage.php @@ -29,6 +29,7 @@ use OC\Files\View; use OCP\Encryption\Keys\IStorage; use OC\User\NoUserException; +use OCP\IUserSession; class Storage implements IStorage { @@ -58,11 +59,15 @@ class Storage implements IStorage { /** @var array */ private $keyCache = []; + /** @var string */ + private $currentUser = null; + /** - * @param View $view - * @param Util $util + * @param View $view view + * @param Util $util encryption util class + * @param IUserSession $session user session */ - public function __construct(View $view, Util $util) { + public function __construct(View $view, Util $util, IUserSession $session) { $this->view = $view; $this->util = $util; @@ -70,6 +75,10 @@ public function __construct(View $view, Util $util) { $this->keys_base_dir = $this->encryption_base_dir .'/keys'; $this->backup_base_dir = $this->encryption_base_dir .'/backup'; $this->root_dir = $this->util->getKeyStorageRoot(); + + if (!is_null($session) && !is_null($session->getUser())) { + $this->currentUser = $session->getUser()->getUID(); + } } /** @@ -189,6 +198,7 @@ protected function constructUserKeyPath($encryptionModuleId, $keyId, $uid) { if ($uid === null) { $path = $this->root_dir . '/' . $this->encryption_base_dir . '/' . $encryptionModuleId . '/' . $keyId; } else { + $this->setupUserMounts($uid); $path = $this->root_dir . '/' . $uid . $this->encryption_base_dir . '/' . $encryptionModuleId . '/' . $uid . '.' . $keyId; } @@ -254,6 +264,7 @@ private function getFileKeyDir($encryptionModuleId, $path) { if ($this->util->isSystemWideMountPoint($filename, $owner)) { $keyPath = $this->root_dir . '/' . $this->keys_base_dir . $filename . '/'; } else { + $this->setupUserMounts($owner); $keyPath = $this->root_dir . '/' . $owner . $this->keys_base_dir . $filename . '/'; } @@ -348,6 +359,7 @@ protected function getPathToKeys($path) { if ($systemWideMountPoint) { $systemPath = $this->root_dir . '/' . $this->keys_base_dir . $relativePath . '/'; } else { + $this->setupUserMounts($owner); $systemPath = $this->root_dir . '/' . $owner . $this->keys_base_dir . $relativePath . '/'; } @@ -373,4 +385,19 @@ protected function keySetPreparation($path) { } } + /** + * Setup the mounts of the given user if different than + * the current user. + * + * This is needed because in many cases the keys are stored + * within the user's home storage. + * + * @param string $uid user id + */ + protected function setupUserMounts($uid) { + if (!is_null($uid) && $uid !== '' && $uid !== $this->currentUser) { + \OC\Files\Filesystem::initMountPoints($uid); + } + } + } diff --git a/lib/private/Server.php b/lib/private/Server.php index c21ff650b24a4..b749033b7b0e3 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -183,7 +183,11 @@ public function __construct($webRoot, \OC\Config $config) { $c->getConfig() ); - return new Encryption\Keys\Storage($view, $util); + return new Encryption\Keys\Storage( + $view, + $util, + $c->getUserSession() + ); }); $this->registerService('TagMapper', function (Server $c) { return new TagMapper($c->getDatabaseConnection()); diff --git a/tests/lib/Encryption/Keys/StorageTest.php b/tests/lib/Encryption/Keys/StorageTest.php index 4e9719351f30b..4bf18ea3232ce 100644 --- a/tests/lib/Encryption/Keys/StorageTest.php +++ b/tests/lib/Encryption/Keys/StorageTest.php @@ -24,17 +24,31 @@ namespace Test\Encryption\Keys; use OC\Encryption\Keys\Storage; +use OC\Encryption\Util; +use OC\Files\View; use Test\TestCase; +use Test\Traits\UserTrait; +use OCP\IUser; +use OCP\IUserSession; +/** + * Class StorageTest + * + * @group DB + * + * @package Test\Encryption\Keys + */ class StorageTest extends TestCase { + use UserTrait; + /** @var Storage */ protected $storage; - /** @var \PHPUnit_Framework_MockObject_MockObject */ + /** @var \PHPUnit_Framework_MockObject_MockObject | Util */ protected $util; - /** @var \PHPUnit_Framework_MockObject_MockObject */ + /** @var \PHPUnit_Framework_MockObject_MockObject | View */ protected $view; /** @var \PHPUnit_Framework_MockObject_MockObject */ @@ -55,7 +69,14 @@ public function setUp() { ->disableOriginalConstructor() ->getMock(); - $this->storage = new Storage($this->view, $this->util); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user1'); + $userSession = $this->createMock(IUserSession::class); + $userSession->method('getUser')->willReturn($user); + + $this->createUser('user1', '123456'); + $this->createUser('user2', '123456'); + $this->storage = new Storage($this->view, $this->util, $userSession); } public function testSetFileKey() { @@ -237,6 +258,42 @@ public function testGetUserKey() { ); } + public function testGetUserKeyShared() { + $this->view->expects($this->once()) + ->method('file_get_contents') + ->with($this->equalTo('/user2/files_encryption/encModule/user2.publicKey')) + ->willReturn('key'); + $this->view->expects($this->once()) + ->method('file_exists') + ->with($this->equalTo('/user2/files_encryption/encModule/user2.publicKey')) + ->willReturn(true); + + $this->assertFalse($this->isUserHomeMounted('user2')); + + $this->assertSame('key', + $this->storage->getUserKey('user2', 'publicKey', 'encModule') + ); + + $this->assertTrue($this->isUserHomeMounted('user2')); + } + + /** + * Returns whether the home of the given user was mounted + * + * @param string $userId + * @return boolean true if mounted, false otherwise + */ + private function isUserHomeMounted($userId) { + // verify that user2's FS got mounted when retrieving the key + $mountManager = \OC::$server->getMountManager(); + $mounts = $mountManager->getAll(); + $mounts = array_filter($mounts, function($mount) use ($userId) { + return ($mount->getMountPoint() === "/$userId/"); + }); + + return !empty($mounts); + } + public function testDeleteUserKey() { $this->view->expects($this->once()) ->method('file_exists')