Merge pull request #18121 from owncloud/enc_improve_privkey_encryption

use password hash to encrypt private key
This commit is contained in:
Lukas Reschke 2015-08-24 12:03:27 +02:00
commit cca35f0c3e
11 changed files with 345 additions and 39 deletions

View File

@ -100,10 +100,10 @@ class SettingsController extends Controller {
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);

View File

@ -220,8 +220,7 @@ class UserHooks implements IHook {
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 @@ class UserHooks implements IHook {
$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);

View File

@ -30,6 +30,7 @@ use OC\Encryption\Exceptions\DecryptionFailedException;
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 @@ class Crypt {
$this->logger = $logger;
$this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser() : false;
$this->config = $config;
$this->supportedKeyFormats = ['hash', 'password'];
}
/**
@ -161,10 +172,23 @@ class Crypt {
/**
* 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 @@ class Crypt {
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 @@ class Crypt {
}
/**
* 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 @@ class Crypt {
$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,20 +376,31 @@ class Crypt {
$password,
$cipher);
// Check if this is a valid private key
$res = openssl_get_privatekey($plainKey);
if (is_resource($res)) {
$sslInfo = openssl_pkey_get_details($res);
if (!isset($sslInfo['key'])) {
return false;
}
} else {
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 true;
}
}
return true;
}
/**
* @param $keyFileContents
* @param string $passPhrase
@ -358,7 +482,7 @@ class Crypt {
* @param $data
* @return array
*/
private function parseHeader($data) {
protected function parseHeader($data) {
$result = [];
if (substr($data, 0, strlen(self::HEADER_START)) === self::HEADER_START) {

View File

@ -146,7 +146,7 @@ class KeyManager {
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 @@ class KeyManager {
*/
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 @@ class KeyManager {
// 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 @@ class KeyManager {
$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 @@ class KeyManager {
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) {

View File

@ -136,7 +136,7 @@ class Recovery {
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 @@ class Recovery {
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);
}

View File

@ -188,7 +188,7 @@ class SettingsControllerTest extends TestCase {
$this->cryptMock
->expects($this->once())
->method('symmetricEncryptFileContent')
->method('encryptPrivateKey')
->willReturn('encryptedKey');
$this->cryptMock

View File

@ -114,7 +114,7 @@ class UserHooksTest extends TestCase {
->willReturnOnConsecutiveCalls(true, false);
$this->cryptMock->expects($this->exactly(4))
->method('symmetricEncryptFileContent')
->method('encryptPrivateKey')
->willReturn(true);
$this->cryptMock->expects($this->any())

View File

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

View File

@ -96,7 +96,7 @@ class RecoveryTest extends TestCase {
->method('decryptPrivateKey');
$this->cryptMock->expects($this->once())
->method('symmetricEncryptFileContent')
->method('encryptPrivateKey')
->willReturn(true);
$this->assertTrue($this->instance->changeRecoveryKeyPassword('password',

View File

@ -98,18 +98,41 @@ class cryptTest extends TestCase {
/**
* test generateHeader
* test generateHeader with valid key formats
*
* @dataProvider dataTestGenerateHeader
*/
public function testGenerateHeader() {
public function testGenerateHeader($keyFormat, $expected) {
$this->config->expects($this->once())
->method('getSystemValue')
->with($this->equalTo('cipher'), $this->equalTo('AES-256-CFB'))
->willReturn('AES-128-CFB');
$this->assertSame('HBEGIN:cipher:AES-128-CFB:HEND',
$this->crypt->generateHeader()
);
if ($keyFormat) {
$result = $this->crypt->generateHeader($keyFormat);
} else {
$result = $this->crypt->generateHeader();
}
$this->assertSame($expected, $result);
}
/**
* test generateHeader with invalid key format
*
* @expectedException \InvalidArgumentException
*/
public function testGenerateHeaderInvalid() {
$this->crypt->generateHeader('unknown');
}
public function dataTestGenerateHeader() {
return [
[null, 'HBEGIN:cipher:AES-128-CFB:keyFormat:hash:HEND'],
['password', 'HBEGIN:cipher:AES-128-CFB:keyFormat:password:HEND'],
['hash', 'HBEGIN:cipher:AES-128-CFB:keyFormat:hash:HEND']
];
}
/**
@ -262,4 +285,82 @@ class cryptTest extends TestCase {
}
/**
* test return values of valid ciphers
*
* @dataProvider dataTestGetKeySize
*/
public function testGetKeySize($cipher, $expected) {
$result = $this->invokePrivate($this->crypt, 'getKeySize', [$cipher]);
$this->assertSame($expected, $result);
}
/**
* test exception if cipher is unknown
*
* @expectedException \InvalidArgumentException
*/
public function testGetKeySizeFailure() {
$this->invokePrivate($this->crypt, 'getKeySize', ['foo']);
}
public function dataTestGetKeySize() {
return [
['AES-256-CFB', 32],
['AES-128-CFB', 16],
];
}
/**
* @dataProvider dataTestDecryptPrivateKey
*/
public function testDecryptPrivateKey($header, $privateKey, $expectedCipher, $isValidKey, $expected) {
/** @var \OCA\Encryption\Crypto\Crypt | \PHPUnit_Framework_MockObject_MockObject $crypt */
$crypt = $this->getMockBuilder('OCA\Encryption\Crypto\Crypt')
->setConstructorArgs(
[
$this->logger,
$this->userSession,
$this->config
]
)
->setMethods(
[
'parseHeader',
'generatePasswordHash',
'symmetricDecryptFileContent',
'isValidPrivateKey'
]
)
->getMock();
$crypt->expects($this->once())->method('parseHeader')->willReturn($header);
if (isset($header['keyFormat']) && $header['keyFormat'] === 'hash') {
$crypt->expects($this->once())->method('generatePasswordHash')->willReturn('hash');
$password = 'hash';
} else {
$crypt->expects($this->never())->method('generatePasswordHash');
$password = 'password';
}
$crypt->expects($this->once())->method('symmetricDecryptFileContent')
->with('privateKey', $password, $expectedCipher)->willReturn('key');
$crypt->expects($this->once())->method('isValidPrivateKey')->willReturn($isValidKey);
$result = $crypt->decryptPrivateKey($privateKey, 'password');
$this->assertSame($expected, $result);
}
public function dataTestDecryptPrivateKey() {
return [
[['cipher' => 'AES-128-CFB', 'keyFormat' => 'password'], 'HBEGIN:HENDprivateKey', 'AES-128-CFB', true, 'key'],
[['cipher' => 'AES-256-CFB', 'keyFormat' => 'password'], 'HBEGIN:HENDprivateKey', 'AES-256-CFB', true, 'key'],
[['cipher' => 'AES-256-CFB', 'keyFormat' => 'password'], 'HBEGIN:HENDprivateKey', 'AES-256-CFB', false, false],
[['cipher' => 'AES-256-CFB', 'keyFormat' => 'hash'], 'HBEGIN:HENDprivateKey', 'AES-256-CFB', true, 'key'],
[['cipher' => 'AES-256-CFB'], 'HBEGIN:HENDprivateKey', 'AES-256-CFB', true, 'key'],
[[], 'privateKey', 'AES-128-CFB', true, 'key'],
];
}
}

View File

@ -0,0 +1,87 @@
<?php
/* Note; This class can be removed as soon as we drop PHP 5.4 support.
*
*
* Password Hashing With PBKDF2 (http://crackstation.net/hashing-security.htm).
* Copyright (c) 2013, Taylor Hornby
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/
namespace OCA\Encryption\Vendor;
class PBKDF2Fallback {
/*
* PBKDF2 key derivation function as defined by RSA's PKCS #5: https://www.ietf.org/rfc/rfc2898.txt
* $algorithm - The hash algorithm to use. Recommended: SHA256
* $password - The password.
* $salt - A salt that is unique to the password.
* $count - Iteration count. Higher is better, but slower. Recommended: At least 1000.
* $key_length - The length of the derived key in bytes.
* $raw_output - If true, the key is returned in raw binary format. Hex encoded otherwise.
* Returns: A $key_length-byte key derived from the password and salt.
*
* Test vectors can be found here: https://www.ietf.org/rfc/rfc6070.txt
*
* This implementation of PBKDF2 was originally created by https://defuse.ca
* With improvements by http://www.variations-of-shadow.com
*/
public function pbkdf2($algorithm, $password, $salt, $count, $key_length, $raw_output = false) {
$algorithm = strtolower($algorithm);
if (!in_array($algorithm, hash_algos(), true))
trigger_error('PBKDF2 ERROR: Invalid hash algorithm.', E_USER_ERROR);
if ($count <= 0 || $key_length <= 0)
trigger_error('PBKDF2 ERROR: Invalid parameters.', E_USER_ERROR);
if (function_exists("hash_pbkdf2")) {
// The output length is in NIBBLES (4-bits) if $raw_output is false!
if (!$raw_output) {
$key_length = $key_length * 2;
}
return hash_pbkdf2($algorithm, $password, $salt, $count, $key_length, $raw_output);
}
$hash_length = strlen(hash($algorithm, "", true));
$block_count = ceil($key_length / $hash_length);
$output = "";
for ($i = 1; $i <= $block_count; $i++) {
// $i encoded as 4 bytes, big endian.
$last = $salt . pack("N", $i);
// first iteration
$last = $xorsum = hash_hmac($algorithm, $last, $password, true);
// perform the other $count - 1 iterations
for ($j = 1; $j < $count; $j++) {
$xorsum ^= ($last = hash_hmac($algorithm, $last, $password, true));
}
$output .= $xorsum;
}
if ($raw_output)
return substr($output, 0, $key_length);
else
return bin2hex(substr($output, 0, $key_length));
}
}