From bceff8b409ebf72a082bbac8f35e12a50d975485 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 27 Feb 2017 14:58:12 +0100 Subject: [PATCH 1/3] Fix public link for master key In public link mode there is no session, so the code should use the public key instead. --- apps/encryption/lib/KeyManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php index 53a93d634b8c..1cd8986ca1a3 100644 --- a/apps/encryption/lib/KeyManager.php +++ b/apps/encryption/lib/KeyManager.php @@ -405,7 +405,7 @@ public function getFileKey($path, $uid) { return ''; } - if ($this->util->isMasterKeyEnabled()) { + if (!is_null($uid) && $this->util->isMasterKeyEnabled()) { $uid = $this->getMasterKeyId(); } From d53a291bdb8113b90a07ded9bf7c46da42007d29 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 27 Feb 2017 16:19:48 +0100 Subject: [PATCH 2/3] Add tests for null user --- apps/encryption/tests/KeyManagerTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/encryption/tests/KeyManagerTest.php b/apps/encryption/tests/KeyManagerTest.php index f31a3ef3500c..1d73e6f27866 100644 --- a/apps/encryption/tests/KeyManagerTest.php +++ b/apps/encryption/tests/KeyManagerTest.php @@ -413,7 +413,11 @@ public function dataTestGetFileKey() { ['', false, 'privateKey', true], ['', false, false, ''], ['', true, 'privateKey', true], - ['', true, false, ''] + ['', true, false, ''], + [null, false, 'privateKey', true], + [null, false, false, ''], + [null, true, 'privateKey', true], + [null, true, false, ''] ]; } From 715fd93329efcafb850ab1cf40569b97b5a84a82 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 2 Mar 2017 16:11:01 +0100 Subject: [PATCH 3/3] Use master key for public links as well --- apps/encryption/lib/KeyManager.php | 19 +++++++++--- apps/encryption/tests/KeyManagerTest.php | 39 ++++++++++++------------ 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php index 1cd8986ca1a3..cdfee683584c 100644 --- a/apps/encryption/lib/KeyManager.php +++ b/apps/encryption/lib/KeyManager.php @@ -399,17 +399,28 @@ public function getPrivateKey($userId) { * @return string */ public function getFileKey($path, $uid) { + if ($uid === '') { + $uid = null; + } + $publicAccess = is_null($uid); $encryptedFileKey = $this->keyStorage->getFileKey($path, $this->fileKeyId, Encryption::ID); if (empty($encryptedFileKey)) { return ''; } - if (!is_null($uid) && $this->util->isMasterKeyEnabled()) { + if ($this->util->isMasterKeyEnabled()) { $uid = $this->getMasterKeyId(); - } - - if (is_null($uid)) { + $shareKey = $this->getShareKey($path, $uid); + if ($publicAccess) { + $privateKey = $this->getSystemPrivateKey($uid); + $privateKey = $this->crypt->decryptPrivateKey($privateKey, $this->getMasterKeyPassword(), $uid); + } else { + // when logged in, the master key is already decrypted in the session + $privateKey = $this->session->getPrivateKey(); + } + } else if ($publicAccess) { + // use public share key for public links $uid = $this->getPublicShareKeyId(); $shareKey = $this->getShareKey($path, $uid); $privateKey = $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.privateKey', Encryption::ID); diff --git a/apps/encryption/tests/KeyManagerTest.php b/apps/encryption/tests/KeyManagerTest.php index 1d73e6f27866..a7bab029a5cf 100644 --- a/apps/encryption/tests/KeyManagerTest.php +++ b/apps/encryption/tests/KeyManagerTest.php @@ -344,6 +344,19 @@ public function testGetEncryptedFileKey() { $this->assertTrue($this->instance->getEncryptedFileKey('/')); } + 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, ''] + ]; + } + /** * @dataProvider dataTestGetFileKey * @@ -358,6 +371,10 @@ public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $expected if ($isMasterKeyEnabled) { $expectedUid = 'masterKeyId'; + $this->configMock->expects($this->any())->method('getSystemValue')->with('secret') + ->willReturn('password'); + } else if (!$uid) { + $expectedUid = 'systemKeyId'; } else { $expectedUid = $uid; } @@ -374,6 +391,9 @@ public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $expected ->with($path, $expectedUid . '.shareKey', 'OC_DEFAULT_MODULE') ->willReturn(true); + $this->utilMock->expects($this->any())->method('isMasterKeyEnabled') + ->willReturn($isMasterKeyEnabled); + if (is_null($uid)) { $this->keyStorageMock->expects($this->once()) ->method('getSystemUserKey') @@ -384,8 +404,6 @@ public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $expected } else { $this->keyStorageMock->expects($this->never()) ->method('getSystemUserKey'); - $this->utilMock->expects($this->once())->method('isMasterKeyEnabled') - ->willReturn($isMasterKeyEnabled); $this->sessionMock->expects($this->once())->method('getPrivateKey')->willReturn($privateKey); } @@ -404,23 +422,6 @@ public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $expected } - public function dataTestGetFileKey() { - return [ - ['user1', false, 'privateKey', true], - ['user1', false, false, ''], - ['user1', true, 'privateKey', true], - ['user1', true, false, ''], - ['', false, 'privateKey', true], - ['', false, false, ''], - ['', true, 'privateKey', true], - ['', true, false, ''], - [null, false, 'privateKey', true], - [null, false, false, ''], - [null, true, 'privateKey', true], - [null, true, false, ''] - ]; - } - public function testDeletePrivateKey() { $this->keyStorageMock->expects($this->once()) ->method('deleteUserKey')