Merge pull request #15938 from owncloud/enc_update_recovery_share_key

[encryption] check recovery key setting for the correct user
This commit is contained in:
Björn Schießle 2015-05-05 10:48:09 +02:00
commit 2ce01ee0bf
8 changed files with 92 additions and 44 deletions

View File

@ -190,7 +190,7 @@ class Encryption implements IEncryptionModule {
}
}
$publicKeys = $this->keyManager->addSystemKeys($this->accessList, $publicKeys);
$publicKeys = $this->keyManager->addSystemKeys($this->accessList, $publicKeys, $this->user);
$encryptedKeyfiles = $this->crypt->multiKeyEncrypt($this->fileKey, $publicKeys);
$this->keyManager->setAllFileKeys($this->path, $encryptedKeyfiles);
@ -295,7 +295,7 @@ class Encryption implements IEncryptionModule {
$publicKeys[$user] = $this->keyManager->getPublicKey($user);
}
$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys);
$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $uid);
$encryptedFileKey = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
@ -313,28 +313,6 @@ class Encryption implements IEncryptionModule {
return true;
}
/**
* add system keys such as the public share key and the recovery key
*
* @param array $accessList
* @param array $publicKeys
* @return array
*/
public function addSystemKeys(array $accessList, array $publicKeys) {
if (!empty($accessList['public'])) {
$publicKeys[$this->keyManager->getPublicShareKeyId()] = $this->keyManager->getPublicShareKey();
}
if ($this->keyManager->recoveryKeyExists() &&
$this->util->isRecoveryEnabledForUser()) {
$publicKeys[$this->keyManager->getRecoveryKeyId()] = $this->keyManager->getRecoveryKey();
}
return $publicKeys;
}
/**
* should the file be encrypted or not
*

View File

@ -531,10 +531,11 @@ class KeyManager {
*
* @param array $accessList
* @param array $publicKeys
* @param string $uid
* @return array
* @throws PublicKeyMissingException
*/
public function addSystemKeys(array $accessList, array $publicKeys) {
public function addSystemKeys(array $accessList, array $publicKeys, $uid) {
if (!empty($accessList['public'])) {
$publicShareKey = $this->getPublicShareKey();
if (empty($publicShareKey)) {
@ -544,7 +545,7 @@ class KeyManager {
}
if ($this->recoveryKeyExists() &&
$this->util->isRecoveryEnabledForUser()) {
$this->util->isRecoveryEnabledForUser($uid)) {
$publicKeys[$this->getRecoveryKeyId()] = $this->getRecoveryKey();
}

View File

@ -228,7 +228,7 @@ class Recovery {
$publicKeys[$uid] = $this->keyManager->getPublicKey($uid);
}
$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys);
$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $this->user->getUID());
$encryptedKeyfiles = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
$this->keyManager->setAllFileKeys($filePath, $encryptedKeyfiles);
@ -264,33 +264,39 @@ class Recovery {
$privateKey = $this->crypt->decryptPrivateKey($encryptedKey,
$recoveryPassword);
$this->recoverAllFiles('/' . $user . '/files/', $privateKey);
$this->recoverAllFiles('/' . $user . '/files/', $privateKey, $user);
}
/**
* @param $path
* @param $privateKey
* recover users files
*
* @param string $path
* @param string $privateKey
* @param string $uid
*/
private function recoverAllFiles($path, $privateKey) {
private function recoverAllFiles($path, $privateKey, $uid) {
$dirContent = $this->view->getDirectoryContent($path);
foreach ($dirContent as $item) {
// Get relative path from encryption/keyfiles
$filePath = $item->getPath();
if ($this->view->is_dir($filePath)) {
$this->recoverAllFiles($filePath . '/', $privateKey);
$this->recoverAllFiles($filePath . '/', $privateKey, $uid);
} else {
$this->recoverFile($filePath, $privateKey);
$this->recoverFile($filePath, $privateKey, $uid);
}
}
}
/**
* recover file
*
* @param string $path
* @param string $privateKey
* @param string $uid
*/
private function recoverFile($path, $privateKey) {
private function recoverFile($path, $privateKey, $uid) {
$encryptedFileKey = $this->keyManager->getEncryptedFileKey($path);
$shareKey = $this->keyManager->getShareKey($path, $this->keyManager->getRecoveryKeyId());
@ -303,11 +309,11 @@ class Recovery {
if (!empty($fileKey)) {
$accessList = $this->file->getAccessList($path);
$publicKeys = array();
foreach ($accessList['users'] as $uid) {
$publicKeys[$uid] = $this->keyManager->getPublicKey($uid);
foreach ($accessList['users'] as $user) {
$publicKeys[$user] = $this->keyManager->getPublicKey($user);
}
$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys);
$publicKeys = $this->keyManager->addSystemKeys($accessList, $publicKeys, $uid);
$encryptedKeyfiles = $this->crypt->multiKeyEncrypt($fileKey, $publicKeys);
$this->keyManager->setAllFileKeys($path, $encryptedKeyfiles);

View File

@ -77,10 +77,13 @@ class Util {
}
/**
* check if recovery key is enabled for user
*
* @param string $uid
* @return bool
*/
public function isRecoveryEnabledForUser() {
$recoveryMode = $this->config->getUserValue($this->user->getUID(),
public function isRecoveryEnabledForUser($uid) {
$recoveryMode = $this->config->getUserValue($uid,
'encryption',
'recoveryEnabled',
0);

View File

@ -56,7 +56,7 @@ $privateKeySet = $session->isPrivateKeySet();
$initialized = $session->getStatus();
$recoveryAdminEnabled = \OC::$server->getConfig()->getAppValue('encryption', 'recoveryAdminEnabled');
$recoveryEnabledForUser = $util->isRecoveryEnabledForUser();
$recoveryEnabledForUser = $util->isRecoveryEnabledForUser($user);
$result = false;

View File

@ -297,4 +297,63 @@ class KeyManagerTest extends TestCase {
$this->assertTrue($this->instance->deleteAllFileKeys('/'));
}
/**
* test add public share key and or recovery key to the list of public keys
*
* @dataProvider dataTestAddSystemKeys
*
* @param array $accessList
* @param array $publicKeys
* @param string $uid
* @param array $expectedKeys
*/
public function testAddSystemKeys($accessList, $publicKeys, $uid, $expectedKeys) {
$publicShareKeyId = 'publicShareKey';
$recoveryKeyId = 'recoveryKey';
$this->keyStorageMock->expects($this->any())
->method('getSystemUserKey')
->willReturnCallback(function($keyId, $encryptionModuleId) {
return $keyId;
});
$this->utilMock->expects($this->any())
->method('isRecoveryEnabledForUser')
->willReturnCallback(function($uid) {
if ($uid === 'user1') {
return true;
}
return false;
});
// set key IDs
\Test_Helper::invokePrivate($this->instance, 'publicShareKeyId', [$publicShareKeyId]);
\Test_Helper::invokePrivate($this->instance, 'recoveryKeyId', [$recoveryKeyId]);
$result = $this->instance->addSystemKeys($accessList, $publicKeys, $uid);
foreach ($expectedKeys as $expected) {
$this->assertArrayHasKey($expected, $result);
}
$this->assertSameSize($expectedKeys, $result);
}
/**
* data provider for testAddSystemKeys()
*
* @return array
*/
public function dataTestAddSystemKeys() {
return array(
array(['public' => true],[], 'user1', ['publicShareKey', 'recoveryKey']),
array(['public' => false], [], 'user1', ['recoveryKey']),
array(['public' => true],[], 'user2', ['publicShareKey']),
array(['public' => false], [], 'user2', []),
);
}
}

View File

@ -170,6 +170,7 @@ class RecoveryTest extends TestCase {
$this->keyManagerMock->expects($this->once())
->method('addSystemKeys')
->with($this->anything(), $this->anything(), $this->equalTo('admin'))
->willReturn(['admin' => 'publicKey']);
@ -181,7 +182,7 @@ class RecoveryTest extends TestCase {
$this->assertNull(\Test_Helper::invokePrivate($this->instance,
'recoverFile',
['/', 'testkey']));
['/', 'testkey', 'admin']));
}
protected function setUp() {

View File

@ -44,11 +44,11 @@ class UtilTest extends TestCase {
*
*/
public function testIsRecoveryEnabledForUser() {
$this->assertTrue($this->instance->isRecoveryEnabledForUser());
$this->assertTrue($this->instance->isRecoveryEnabledForUser('admin'));
// Assert recovery will return default value if not set
unset(self::$tempStorage['recoveryEnabled']);
$this->assertEquals(0, $this->instance->isRecoveryEnabledForUser());
$this->assertEquals(0, $this->instance->isRecoveryEnabledForUser('admin'));
}
public function testUserHasFiles() {