Merge pull request #18121 from owncloud/enc_improve_privkey_encryption

use password hash to encrypt private key

apps/encryption/controller/settingscontroller.php

@@ -100,10 +100,10 @@ public function updatePrivateKeyPassword($oldPassword, $newPassword) {
 
  if ($passwordCorrect !== false) {
  $encryptedKey = $this->keyManager->getPrivateKey($uid);
- $decryptedKey = $this->crypt->decryptPrivateKey($encryptedKey, $oldPassword);
+ $decryptedKey = $this->crypt->decryptPrivateKey($encryptedKey, $oldPassword, $uid);
 
  if ($decryptedKey) {
- $encryptedKey = $this->crypt->symmetricEncryptFileContent($decryptedKey, $newPassword);
+ $encryptedKey = $this->crypt->encryptPrivateKey($decryptedKey, $newPassword, $uid);
  $header = $this->crypt->generateHeader();
  if ($encryptedKey) {
  $this->keyManager->setPrivateKey($uid, $header . $encryptedKey);

apps/encryption/hooks/userhooks.php

@@ -220,8 +220,7 @@ public function setPassphrase($params) {
  if ($user && $params['uid'] === $user->getUID() && $privateKey) {
 
  // Encrypt private key with new user pwd as passphrase
- $encryptedPrivateKey = $this->crypt->symmetricEncryptFileContent($privateKey,
- $params['password']);
+ $encryptedPrivateKey = $this->crypt->encryptPrivateKey($privateKey, $params['password'], $params['uid']);
 
  // Save private key
  if ($encryptedPrivateKey) {
@@ -259,8 +258,7 @@ public function setPassphrase($params) {
  $this->keyManager->setPublicKey($user, $keyPair['publicKey']);
 
  // Encrypt private key with new password
- $encryptedKey = $this->crypt->symmetricEncryptFileContent($keyPair['privateKey'],
- $newUserPassword);
+ $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], $newUserPassword, $user);
 
  if ($encryptedKey) {
  $this->keyManager->setPrivateKey($user, $this->crypt->generateHeader() . $encryptedKey);

apps/encryption/lib/crypto/crypt.php

@@ -30,6 +30,7 @@
 use OC\Encryption\Exceptions\EncryptionFailedException;
 use OCA\Encryption\Exceptions\MultiKeyDecryptException;
 use OCA\Encryption\Exceptions\MultiKeyEncryptException;
+use OCA\Encryption\Vendor\PBKDF2Fallback;
 use OCP\Encryption\Exceptions\GenericEncryptionException;
 use OCP\IConfig;
 use OCP\ILogger;
@@ -42,6 +43,10 @@ class Crypt {
  // default cipher from old ownCloud versions
  const LEGACY_CIPHER = 'AES-128-CFB';
 
+ // default key format, old ownCloud version encrypted the private key directly
+ // with the user password
+ const LEGACY_KEY_FORMAT = 'password';
+
  const HEADER_START = 'HBEGIN';
  const HEADER_END = 'HEND';
  /**
@@ -57,6 +62,11 @@ class Crypt {
  */
  private $config;
 
+ /**
+ * @var array
+ */
+ private $supportedKeyFormats;
+
  /**
  * @param ILogger $logger
  * @param IUserSession $userSession
@@ -66,6 +76,7 @@ public function __construct(ILogger $logger, IUserSession $userSession, IConfig
  $this->logger = $logger;
  $this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser() : false;
  $this->config = $config;
+ $this->supportedKeyFormats = ['hash', 'password'];
  }
 
  /**
@@ -161,10 +172,23 @@ public function symmetricEncryptFileContent($plainContent, $passPhrase) {
 
  /**
  * generate header for encrypted file
+ *
+ * @param string $keyFormat (can be 'hash' or 'password')
+ * @return string
+ * @throws \InvalidArgumentException
  */
- public function generateHeader() {
+ public function generateHeader($keyFormat = 'hash') {
+
+ if (in_array($keyFormat, $this->supportedKeyFormats, true) === false) {
+ throw new \InvalidArgumentException('key format "' . $keyFormat . '" is not supported');
+ }
+
  $cipher = $this->getCipher();
- $header = self::HEADER_START . ':cipher:' . $cipher . ':' . self::HEADER_END;
+
+ $header = self::HEADER_START
+ . ':cipher:' . $cipher
+ . ':keyFormat:' . $keyFormat
+ . ':' . self::HEADER_END;
 
  return $header;
  }
@@ -211,6 +235,25 @@ public function getCipher() {
  return $cipher;
  }
 
+ /**
+ * get key size depending on the cipher
+ *
+ * @param string $cipher supported ('AES-256-CFB' and 'AES-128-CFB')
+ * @return int
+ * @throws \InvalidArgumentException
+ */
+ protected function getKeySize($cipher) {
+ if ($cipher === 'AES-256-CFB') {
+ return 32;
+ } else if ($cipher === 'AES-128-CFB') {
+ return 16;
+ }
+
+ throw new \InvalidArgumentException(
+ 'Wrong cipher defined only AES-128-CFB and AES-256-CFB are supported.'
+ );
+ }
+
  /**
  * get legacy cipher
  *
@@ -238,11 +281,71 @@ private function addPadding($data) {
  }
 
  /**
+ * generate password hash used to encrypt the users private key
+ *
+ * @param string $password
+ * @param string $cipher
+ * @param string $uid only used for user keys
+ * @return string
+ */
+ protected function generatePasswordHash($password, $cipher, $uid = '') {
+ $instanceId = $this->config->getSystemValue('instanceid');
+ $instanceSecret = $this->config->getSystemValue('secret');
+ $salt = hash('sha256', $uid . $instanceId . $instanceSecret, true);
+ $keySize = $this->getKeySize($cipher);
+
+ if (function_exists('hash_pbkdf2')) {
+ $hash = hash_pbkdf2(
+ 'sha256',
+ $password,
+ $salt,
+ 100000,
+ $keySize,
+ true
+ );
+ } else {
+ // fallback to 3rdparty lib for PHP <= 5.4.
+ // FIXME: Can be removed as soon as support for PHP 5.4 was dropped
+ $fallback = new PBKDF2Fallback();
+ $hash = $fallback->pbkdf2(
+ 'sha256',
+ $password,
+ $salt,
+ 100000,
+ $keySize,
+ true
+ );
+ }
+
+ return $hash;
+ }
+
+ /**
+ * encrypt private key
+ *
  * @param string $privateKey
  * @param string $password
+ * @param string $uid for regular users, empty for system keys
  * @return bool|string
  */
- public function decryptPrivateKey($privateKey, $password = '') {
+ public function encryptPrivateKey($privateKey, $password, $uid = '') {
+ $cipher = $this->getCipher();
+ $hash = $this->generatePasswordHash($password, $cipher, $uid);
+ $encryptedKey = $this->symmetricEncryptFileContent(
+ $privateKey,
+ $hash
+ );
+
+ return $encryptedKey;
+ }
+
+ /**
+ * @param string $privateKey
+ * @param string $password
+ * @param string $uid for regular users, empty for system keys
+ * @return bool|string
+ */
+ public function decryptPrivateKey($privateKey, $password = '', $uid = '') {
 
  $header = $this->parseHeader($privateKey);
 
@@ -252,6 +355,16 @@ public function decryptPrivateKey($privateKey, $password = '') {
  $cipher = self::LEGACY_CIPHER;
  }
 
+ if (isset($header['keyFormat'])) {
+ $keyFormat = $header['keyFormat'];
+ } else {
+ $keyFormat = self::LEGACY_KEY_FORMAT;
+ }
+
+ if ($keyFormat === 'hash') {
+ $password = $this->generatePasswordHash($password, $cipher, $uid);
+ }
+
  // If we found a header we need to remove it from the key we want to decrypt
  if (!empty($header)) {
  $privateKey = substr($privateKey,
@@ -263,18 +376,29 @@ public function decryptPrivateKey($privateKey, $password = '') {
  $password,
  $cipher);
 
- // Check if this is a valid private key
+ if ($this->isValidPrivateKey($plainKey) === false) {
+ return false;
+ }
+
+ return $plainKey;
+ }
+
+ /**
+ * check if it is a valid private key
+ *
+ * @param $plainKey
+ * @return bool
+ */
+ protected function isValidPrivateKey($plainKey) {
  $res = openssl_get_privatekey($plainKey);
  if (is_resource($res)) {
  $sslInfo = openssl_pkey_get_details($res);
- if (!isset($sslInfo['key'])) {
- return false;
+ if (isset($sslInfo['key'])) {
+ return true;
  }
- } else {
- return false;
  }
 
- return $plainKey;
+ return true;
  }
 
  /**
@@ -358,7 +482,7 @@ private function decrypt($encryptedContent, $iv, $passPhrase = '', $cipher = sel
  * @param $data
  * @return array
  */
- private function parseHeader($data) {
+ protected function parseHeader($data) {
  $result = [];
 
  if (substr($data, 0, strlen(self::HEADER_START)) === self::HEADER_START) {

apps/encryption/lib/keymanager.php

@@ -146,7 +146,7 @@ public function validateShareKey() {
  Encryption::ID);
 
  // Encrypt private key empty passphrase
- $encryptedKey = $this->crypt->symmetricEncryptFileContent($keyPair['privateKey'], '');
+ $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], '');
  $header = $this->crypt->generateHeader();
  $this->setSystemPrivateKey($this->publicShareKeyId, $header . $encryptedKey);
  }
@@ -184,8 +184,7 @@ public function getRecoveryKeyId() {
  */
  public function checkRecoveryPassword($password) {
  $recoveryKey = $this->keyStorage->getSystemUserKey($this->recoveryKeyId . '.privateKey', Encryption::ID);
- $decryptedRecoveryKey = $this->crypt->decryptPrivateKey($recoveryKey,
- $password);
+ $decryptedRecoveryKey = $this->crypt->decryptPrivateKey($recoveryKey, $password);
 
  if ($decryptedRecoveryKey) {
  return true;
@@ -203,8 +202,8 @@ public function storeKeyPair($uid, $password, $keyPair) {
  // Save Public Key
  $this->setPublicKey($uid, $keyPair['publicKey']);
 
- $encryptedKey = $this->crypt->symmetricEncryptFileContent($keyPair['privateKey'],
- $password);
+ $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], $password, $uid);
+
  $header = $this->crypt->generateHeader();
 
  if ($encryptedKey) {
@@ -226,8 +225,7 @@ public function setRecoveryKey($password, $keyPair) {
  $keyPair['publicKey'],
  Encryption::ID);
 
- $encryptedKey = $this->crypt->symmetricEncryptFileContent($keyPair['privateKey'],
- $password);
+ $encryptedKey = $this->crypt->encryptPrivateKey($keyPair['privateKey'], $password);
  $header = $this->crypt->generateHeader();
 
  if ($encryptedKey) {
@@ -308,8 +306,7 @@ public function init($uid, $passPhrase) {
 
  try {
  $privateKey = $this->getPrivateKey($uid);
- $privateKey = $this->crypt->decryptPrivateKey($privateKey,
- $passPhrase);
+ $privateKey = $this->crypt->decryptPrivateKey($privateKey, $passPhrase, $uid);
  } catch (PrivateKeyMissingException $e) {
  return false;
  } catch (DecryptionFailedException $e) {

apps/encryption/lib/recovery.php

@@ -136,7 +136,7 @@ public function enableAdminRecovery($password) {
  public function changeRecoveryKeyPassword($newPassword, $oldPassword) {
  $recoveryKey = $this->keyManager->getSystemPrivateKey($this->keyManager->getRecoveryKeyId());
  $decryptedRecoveryKey = $this->crypt->decryptPrivateKey($recoveryKey, $oldPassword);
- $encryptedRecoveryKey = $this->crypt->symmetricEncryptFileContent($decryptedRecoveryKey, $newPassword);
+ $encryptedRecoveryKey = $this->crypt->encryptPrivateKey($decryptedRecoveryKey, $newPassword);
  $header = $this->crypt->generateHeader();
  if ($encryptedRecoveryKey) {
  $this->keyManager->setSystemPrivateKey($this->keyManager->getRecoveryKeyId(), $header . $encryptedRecoveryKey);
@@ -263,8 +263,7 @@ private function removeRecoveryKeys($path) {
  public function recoverUsersFiles($recoveryPassword, $user) {
  $encryptedKey = $this->keyManager->getSystemPrivateKey($this->keyManager->getRecoveryKeyId());
 
- $privateKey = $this->crypt->decryptPrivateKey($encryptedKey,
- $recoveryPassword);
+ $privateKey = $this->crypt->decryptPrivateKey($encryptedKey, $recoveryPassword);
 
  $this->recoverAllFiles('/' . $user . '/files/', $privateKey, $user);
  }

apps/encryption/tests/controller/SettingsControllerTest.php

@@ -188,7 +188,7 @@ public function testUpdatePrivateKeyPassword() {
 
  $this->cryptMock
  ->expects($this->once())
- ->method('symmetricEncryptFileContent')
+ ->method('encryptPrivateKey')
  ->willReturn('encryptedKey');
 
  $this->cryptMock

apps/encryption/tests/hooks/UserHooksTest.php

@@ -114,7 +114,7 @@ public function testSetPassphrase() {
  ->willReturnOnConsecutiveCalls(true, false);
 
  $this->cryptMock->expects($this->exactly(4))
- ->method('symmetricEncryptFileContent')
+ ->method('encryptPrivateKey')
  ->willReturn(true);
 
  $this->cryptMock->expects($this->any())

apps/encryption/tests/lib/KeyManagerTest.php

@@ -260,7 +260,7 @@ public function testSetRecoveryKey() {
  ->method('setSystemUserKey')
  ->willReturn(true);
  $this->cryptMock->expects($this->any())
- ->method('symmetricEncryptFileContent')
+ ->method('encryptPrivateKey')
  ->with($this->equalTo('privateKey'), $this->equalTo('pass'))
  ->willReturn('decryptedPrivateKey');
 

apps/encryption/tests/lib/RecoveryTest.php

@@ -96,7 +96,7 @@ public function testChangeRecoveryKeyPassword() {
  ->method('decryptPrivateKey');
 
  $this->cryptMock->expects($this->once())
- ->method('symmetricEncryptFileContent')
+ ->method('encryptPrivateKey')
  ->willReturn(true);
 
  $this->assertTrue($this->instance->changeRecoveryKeyPassword('password',