Harden identifyproof openssl code

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

2 files changed, 28 insertions, 15 deletions

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);