From 2b98eea1297a8024650c456fccbc33824721053e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 9 Sep 2019 20:39:19 +0200 Subject: Harden identifyproof openssl code Signed-off-by: Roeland Jago Douma --- lib/private/Security/IdentityProof/Manager.php | 35 ++++++++++++++++++++------ lib/private/Server.php | 8 ------ 2 files changed, 28 insertions(+), 15 deletions(-) (limited to 'lib') diff --git a/lib/private/Security/IdentityProof/Manager.php b/lib/private/Security/IdentityProof/Manager.php index fb27f04d873..6db5d4ab2eb 100644 --- a/lib/private/Security/IdentityProof/Manager.php +++ b/lib/private/Security/IdentityProof/Manager.php @@ -29,6 +29,7 @@ namespace OC\Security\IdentityProof; use OC\Files\AppData\Factory; use OCP\Files\IAppData; use OCP\IConfig; +use OCP\ILogger; use OCP\IUser; use OCP\Security\ICrypto; @@ -39,19 +40,18 @@ class Manager { private $crypto; /** @var IConfig */ private $config; + /** @var ILogger */ + private $logger; - /** - * @param Factory $appDataFactory - * @param ICrypto $crypto - * @param IConfig $config - */ public function __construct(Factory $appDataFactory, ICrypto $crypto, - IConfig $config + IConfig $config, + ILogger $logger ) { $this->appData = $appDataFactory->get('identityproof'); $this->crypto = $crypto; $this->config = $config; + $this->logger = $logger; } /** @@ -59,6 +59,7 @@ class Manager { * In a separate function for unit testing purposes. * * @return array [$publicKey, $privateKey] + * @throws \RuntimeException */ protected function generateKeyPair(): array { $config = [ @@ -68,7 +69,16 @@ class Manager { // Generate new key $res = openssl_pkey_new($config); - openssl_pkey_export($res, $privateKey); + + if ($res === false) { + $this->logOpensslError(); + throw new \RuntimeException('OpenSSL reported a problem'); + } + + if (openssl_pkey_export($res, $privateKey, null, $config) === false) { + $this->logOpensslError(); + throw new \RuntimeException('OpenSSL reported a problem'); + } // Extract the public key from $res to $pubKey $publicKey = openssl_pkey_get_details($res); @@ -83,6 +93,7 @@ class Manager { * * @param string $id key id * @return Key + * @throws \RuntimeException */ protected function generateKey(string $id): Key { list($publicKey, $privateKey) = $this->generateKeyPair(); @@ -105,6 +116,7 @@ class Manager { * * @param string $id * @return Key + * @throws \RuntimeException */ protected function retrieveKey(string $id): Key { try { @@ -124,6 +136,7 @@ class Manager { * * @param IUser $user * @return Key + * @throws \RuntimeException */ public function getKey(IUser $user): Key { $uid = $user->getUID(); @@ -144,5 +157,13 @@ class Manager { return $this->retrieveKey('system-' . $instanceId); } + private function logOpensslError(): void { + $errors = []; + while ($error = openssl_error_string()) { + $errors[] = $error; + } + $this->logger->critical('Something is wrong with your openssl setup: ' . implode(', ', $errors)); + } + } diff --git a/lib/private/Server.php b/lib/private/Server.php index 433ee044fa4..b4af17ba288 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1186,14 +1186,6 @@ class Server extends ServerContainer implements IServerContainer { $this->registerAlias(IDashboardManager::class, DashboardManager::class); $this->registerAlias(IFullTextSearchManager::class, FullTextSearchManager::class); - $this->registerService(\OC\Security\IdentityProof\Manager::class, function (Server $c) { - return new \OC\Security\IdentityProof\Manager( - $c->query(\OC\Files\AppData\Factory::class), - $c->getCrypto(), - $c->getConfig() - ); - }); - $this->registerAlias(ISubAdmin::class, SubAdmin::class); $this->registerAlias(IInitialStateService::class, InitialStateService::class); -- cgit v1.2.3