Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/nextcloud/server.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulius Härtl <jus@bitgrid.net>2020-07-22 11:05:51 +0300
committerbackportbot[bot] <backportbot[bot]@users.noreply.github.com>2020-10-28 17:14:01 +0300
commitdd4dc60f4c9b7efa61b63fc4d8e4fd74c82cb8ce (patch)
tree2a6c775b310588593dc2eedd5fd3a19797fe1fef /apps/encryption
parent4d20a63d4f1fd5603ccc17c22dd06e51a4caf100 (diff)
Harden key generation
There might be cases where multiple requests trigger the key generation at the same time and the instance ends up with a non-fitting public/private key pair. Therefore the whole key generation should be locked. Other than that this makes sure that user key generation return values are properly validated. Signed-off-by: Julius Härtl <jus@bitgrid.net>
Diffstat (limited to 'apps/encryption')
-rw-r--r--apps/encryption/lib/AppInfo/Application.php3
-rw-r--r--apps/encryption/lib/KeyManager.php104
-rw-r--r--apps/encryption/lib/Users/Setup.php4
3 files changed, 77 insertions, 34 deletions
diff --git a/apps/encryption/lib/AppInfo/Application.php b/apps/encryption/lib/AppInfo/Application.php
index 688564887d3..1e3b2e43d78 100644
--- a/apps/encryption/lib/AppInfo/Application.php
+++ b/apps/encryption/lib/AppInfo/Application.php
@@ -152,7 +152,8 @@ class Application extends \OCP\AppFramework\App {
$server->getUserSession(),
new Session($server->getSession()),
$server->getLogger(),
- $c->query('Util')
+ $c->query('Util'),
+ $server->getLockingProvider()
);
});
diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php
index 09bcf9371d2..0cc29a6a6ab 100644
--- a/apps/encryption/lib/KeyManager.php
+++ b/apps/encryption/lib/KeyManager.php
@@ -41,6 +41,7 @@ use OCP\Encryption\Keys\IStorage;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUserSession;
+use OCP\Lock\ILockingProvider;
class KeyManager {
@@ -104,6 +105,11 @@ class KeyManager {
private $util;
/**
+ * @var ILockingProvider
+ */
+ private $lockingProvider;
+
+ /**
* @param IStorage $keyStorage
* @param Crypt $crypt
* @param IConfig $config
@@ -119,7 +125,8 @@ class KeyManager {
IUserSession $userSession,
Session $session,
ILogger $log,
- Util $util
+ Util $util,
+ ILockingProvider $lockingProvider
) {
$this->util = $util;
$this->session = $session;
@@ -127,6 +134,7 @@ class KeyManager {
$this->crypt = $crypt;
$this->config = $config;
$this->log = $log;
+ $this->lockingProvider = $lockingProvider;
$this->recoveryKeyId = $this->config->getAppValue('encryption',
'recoveryKeyId');
@@ -161,17 +169,24 @@ class KeyManager {
public function validateShareKey() {
$shareKey = $this->getPublicShareKey();
if (empty($shareKey)) {
- $keyPair = $this->crypt->createKeyPair();
-
- // Save public key
- $this->keyStorage->setSystemUserKey(
- $this->publicShareKeyId . '.publicKey', $keyPair['publicKey'],
- Encryption::ID);
-
- // Encrypt private key empty passphrase
- $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], '');
- $header = $this->crypt->generateHeader();
- $this->setSystemPrivateKey($this->publicShareKeyId, $header . $encryptedKey);
+ $this->lockingProvider->acquireLock('encryption-generateSharedKey', ILockingProvider::LOCK_EXCLUSIVE, 'Encryption: shared key generation');
+ try {
+ $keyPair = $this->crypt->createKeyPair();
+
+ // Save public key
+ $this->keyStorage->setSystemUserKey(
+ $this->publicShareKeyId . '.' . $this->publicKeyId, $keyPair['publicKey'],
+ Encryption::ID);
+
+ // Encrypt private key empty passphrase
+ $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], '');
+ $header = $this->crypt->generateHeader();
+ $this->setSystemPrivateKey($this->publicShareKeyId, $header . $encryptedKey);
+ } catch (\Throwable $e) {
+ $this->lockingProvider->releaseLock('encryption-generateSharedKey', ILockingProvider::LOCK_EXCLUSIVE);
+ throw $e;
+ }
+ $this->lockingProvider->releaseLock('encryption-generateSharedKey', ILockingProvider::LOCK_EXCLUSIVE);
}
}
@@ -184,18 +199,36 @@ class KeyManager {
}
$publicMasterKey = $this->getPublicMasterKey();
- if (empty($publicMasterKey)) {
- $keyPair = $this->crypt->createKeyPair();
-
- // Save public key
- $this->keyStorage->setSystemUserKey(
- $this->masterKeyId . '.publicKey', $keyPair['publicKey'],
- Encryption::ID);
-
- // Encrypt private key with system password
- $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], $this->getMasterKeyPassword(), $this->masterKeyId);
- $header = $this->crypt->generateHeader();
- $this->setSystemPrivateKey($this->masterKeyId, $header . $encryptedKey);
+ $privateMasterKey = $this->getPrivateMasterKey();
+
+ if (empty($publicMasterKey) && empty($privateMasterKey)) {
+ // There could be a race condition here if two requests would trigger
+ // the generation the second one would enter the key generation as long
+ // as the first one didn't write the key to the keystorage yet
+ $this->lockingProvider->acquireLock('encryption-generateMasterKey', ILockingProvider::LOCK_EXCLUSIVE, 'Encryption: master key generation');
+ try {
+ $keyPair = $this->crypt->createKeyPair();
+
+ // Save public key
+ $this->keyStorage->setSystemUserKey(
+ $this->masterKeyId . '.' . $this->publicKeyId, $keyPair['publicKey'],
+ Encryption::ID);
+
+ // Encrypt private key with system password
+ $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], $this->getMasterKeyPassword(), $this->masterKeyId);
+ $header = $this->crypt->generateHeader();
+ $this->setSystemPrivateKey($this->masterKeyId, $header . $encryptedKey);
+ } catch (\Throwable $e) {
+ $this->lockingProvider->releaseLock('encryption-generateMasterKey', ILockingProvider::LOCK_EXCLUSIVE);
+ throw $e;
+ }
+ $this->lockingProvider->releaseLock('encryption-generateMasterKey', ILockingProvider::LOCK_EXCLUSIVE);
+ } elseif (empty($publicMasterKey)) {
+ $this->log->error('A private master key is available but the public key could not be found. This should never happen.');
+ return;
+ } elseif (empty($privateMasterKey)) {
+ $this->log->error('A private master key is available but the public key could not be found. This should never happen.');
+ return;
}
if (!$this->session->isPrivateKeySet()) {
@@ -222,7 +255,7 @@ class KeyManager {
* @return string
*/
public function getRecoveryKey() {
- return $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.publicKey', Encryption::ID);
+ return $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.' . $this->publicKeyId, Encryption::ID);
}
/**
@@ -239,7 +272,7 @@ class KeyManager {
* @return bool
*/
public function checkRecoveryPassword($password) {
- $recoveryKey = $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.privateKey', Encryption::ID);
+ $recoveryKey = $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.' . $this->privateKeyId, Encryption::ID);
$decryptedRecoveryKey = $this->crypt->decryptPrivateKey($recoveryKey, $password);
if ($decryptedRecoveryKey) {
@@ -251,7 +284,7 @@ class KeyManager {
/**
* @param string $uid
* @param string $password
- * @param string $keyPair
+ * @param array $keyPair
* @return bool
*/
public function storeKeyPair($uid, $password, $keyPair) {
@@ -277,7 +310,7 @@ class KeyManager {
public function setRecoveryKey($password, $keyPair) {
// Save Public Key
$this->keyStorage->setSystemUserKey($this->getRecoveryKeyId().
- '.publicKey',
+ '.' . $this->publicKeyId,
$keyPair['publicKey'],
Encryption::ID);
@@ -435,7 +468,7 @@ class KeyManager {
// use public share key for public links
$uid = $this->getPublicShareKeyId();
$shareKey = $this->getShareKey($path, $uid);
- $privateKey = $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.privateKey', Encryption::ID);
+ $privateKey = $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.' . $this->privateKeyId, Encryption::ID);
$privateKey = $this->crypt->decryptPrivateKey($privateKey);
} else {
$shareKey = $this->getShareKey($path, $uid);
@@ -578,7 +611,7 @@ class KeyManager {
* @return string
*/
public function getPublicShareKey() {
- return $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.publicKey', Encryption::ID);
+ return $this->keyStorage->getSystemUserKey($this->publicShareKeyId . '.' . $this->publicKeyId, Encryption::ID);
}
/**
@@ -718,6 +751,15 @@ class KeyManager {
* @return string
*/
public function getPublicMasterKey() {
- return $this->keyStorage->getSystemUserKey($this->masterKeyId . '.publicKey', Encryption::ID);
+ return $this->keyStorage->getSystemUserKey($this->masterKeyId . '.' . $this->publicKeyId, Encryption::ID);
+ }
+
+ /**
+ * get public master key
+ *
+ * @return string
+ */
+ public function getPrivateMasterKey() {
+ return $this->keyStorage->getSystemUserKey($this->masterKeyId . '.' . $this->privateKeyId, Encryption::ID);
}
}
diff --git a/apps/encryption/lib/Users/Setup.php b/apps/encryption/lib/Users/Setup.php
index 5406c4e51cc..e80435ac698 100644
--- a/apps/encryption/lib/Users/Setup.php
+++ b/apps/encryption/lib/Users/Setup.php
@@ -73,8 +73,8 @@ class Setup {
*/
public function setupUser($uid, $password) {
if (!$this->keyManager->userHasKeys($uid)) {
- return $this->keyManager->storeKeyPair($uid, $password,
- $this->crypt->createKeyPair());
+ $keyPair = $this->crypt->createKeyPair();
+ return is_array($keyPair) ? $this->keyManager->storeKeyPair($uid, $password, $keyPair) : false;
}
return true;
}