From d26a9c3c5819be48b76586c2fa60da9a7a9829dd Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 26 Aug 2014 19:02:40 +0200 Subject: [PATCH 1/9] Add some security utilities This adds some security utilities to core including: - A library for basic crypto operations (e.g. to encrypt passwords) - A better library for cryptographic actions which allows you to specify the charset - A library for secure string comparisions Remove .htaccess Remove .htaccess Fix typo Add public API Use timing constant comparision Remove CBC constant Adjust code Remove confusing $this --- config/config.sample.php | 3 + core/setup/controller.php | 1 - core/templates/installation.php | 7 -- lib/private/repair.php | 1 + lib/private/security/crypto.php | 104 ++++++++++++++++++++++++++ lib/private/security/securerandom.php | 79 +++++++++++++++++++ lib/private/security/stringutils.php | 38 ++++++++++ lib/private/server.php | 26 +++++++ lib/private/setup.php | 17 +++-- lib/private/util.php | 54 ++----------- lib/public/security/icrypto.php | 46 ++++++++++++ lib/public/security/isecurerandom.php | 53 +++++++++++++ lib/public/security/stringutils.php | 25 +++++++ lib/public/util.php | 1 + lib/repair/repairconfig.php | 37 +++++++++ tests/lib/security/crypto.php | 63 ++++++++++++++++ tests/lib/security/securerandom.php | 51 +++++++++++++ tests/lib/security/stringutils.php | 21 ++++++ 18 files changed, 565 insertions(+), 62 deletions(-) create mode 100644 lib/private/security/crypto.php create mode 100644 lib/private/security/securerandom.php create mode 100644 lib/private/security/stringutils.php create mode 100644 lib/public/security/icrypto.php create mode 100644 lib/public/security/isecurerandom.php create mode 100644 lib/public/security/stringutils.php create mode 100644 lib/repair/repairconfig.php create mode 100644 tests/lib/security/crypto.php create mode 100644 tests/lib/security/securerandom.php create mode 100644 tests/lib/security/stringutils.php diff --git a/config/config.sample.php b/config/config.sample.php index 9656555691..02dbb1fcf7 100755 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -35,6 +35,9 @@ $CONFIG = array( /* Define the salt used to hash the user passwords. All your user passwords are lost if you lose this string. */ "passwordsalt" => "", +/* Secret used by ownCloud for various purposes, e.g. to encrypt data. If you lose this string there will be data corruption. */ +"secret" => "", + /* Force use of HTTPS connection (true = use HTTPS) */ "forcessl" => false, diff --git a/core/setup/controller.php b/core/setup/controller.php index e764b232e8..c72f06fc2d 100644 --- a/core/setup/controller.php +++ b/core/setup/controller.php @@ -153,7 +153,6 @@ class Controller { 'hasMSSQL' => $hasMSSQL, 'databases' => $databases, 'directory' => $datadir, - 'secureRNG' => \OC_Util::secureRNGAvailable(), 'htaccessWorking' => $htaccessWorking, 'vulnerableToNullByte' => $vulnerableToNullByte, 'errors' => $errors, diff --git a/core/templates/installation.php b/core/templates/installation.php index f934e3a86c..b74d4caf10 100644 --- a/core/templates/installation.php +++ b/core/templates/installation.php @@ -27,13 +27,6 @@ t('Please update your PHP installation to use %s securely.', $theme->getName() )); ?>

- -
- t('Security Warning'));?> -

t('No secure random number generator is available, please enable the PHP OpenSSL extension.'));?>
- t('Without a secure random number generator an attacker may be able to predict password reset tokens and take over your account.'));?>

-
-
t('Security Warning'));?> diff --git a/lib/private/repair.php b/lib/private/repair.php index 46b5ae4639..4ff446f860 100644 --- a/lib/private/repair.php +++ b/lib/private/repair.php @@ -71,6 +71,7 @@ class Repair extends BasicEmitter { return array( new \OC\Repair\RepairMimeTypes(), new \OC\Repair\RepairLegacyStorages(\OC::$server->getConfig(), \OC_DB::getConnection()), + new \OC\Repair\RepairConfig(), ); } diff --git a/lib/private/security/crypto.php b/lib/private/security/crypto.php new file mode 100644 index 0000000000..659b0170ec --- /dev/null +++ b/lib/private/security/crypto.php @@ -0,0 +1,104 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + + +namespace OC\Security; + +use Crypt_AES; +use Crypt_Hash; +use OCP\Security\ICrypto; +use OCP\Security\StringUtils; + +/** + * Class Crypto provides a high-level encryption layer using AES-CBC. If no key has been provided + * it will use the secret defined in config.php as key. Additionally the message will be HMAC'd. + * + * Usage: + * $encryptWithDefaultPassword = \OC::$server->getCrypto()->encrypt('EncryptedText'); + * $encryptWithCustompassword = \OC::$server->getCrypto()->encrypt('EncryptedText', 'password'); + * + * @package OC\Security + */ +class Crypto implements ICrypto { + /** @var Crypt_AES $cipher */ + private $cipher; + /** @var int */ + private $ivLength = 16; + + function __construct() { + $this->cipher = new Crypt_AES(); + } + + /** + * @param string $message The message to authenticate + * @param string $password Password to use (defaults to `secret` in config.php) + * @return string Calculated HMAC + */ + public function calculateHMAC($message, $password = '') { + if($password === '') { + $password = \OC::$server->getConfig()->getSystemValue('secret'); + } + + $hash = new Crypt_Hash('sha512'); + $hash->setKey($password); + return $hash->hash($message); + } + + /** + * Encrypts a value and adds an HMAC (Encrypt-Then-MAC) + * @param string $plaintext + * @param string $password Password to encrypt, if not specified the secret from config.php will be taken + * @return string Authenticated ciphertext + */ + public function encrypt($plaintext, $password = '') { + if($password === '') { + $password = \OC::$server->getConfig()->getSystemValue('secret'); + } + $this->cipher->setPassword($password); + + $iv = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate($this->ivLength); + $this->cipher->setIV($iv); + + $ciphertext = bin2hex($this->cipher->encrypt($plaintext)); + $hmac = bin2hex($this->calculateHMAC($ciphertext.$iv, $password)); + + return $ciphertext.'|'.$iv.'|'.$hmac; + } + + /** + * Decrypts a value and verifies the HMAC (Encrypt-Then-Mac) + * @param string $authenticatedCiphertext + * @param string $password Password to encrypt, if not specified the secret from config.php will be taken + * @return string plaintext + * @throws \Exception If the HMAC does not match + */ + public function decrypt($authenticatedCiphertext, $password = '') { + if($password === '') { + $password = \OC::$server->getConfig()->getSystemValue('secret'); + } + $this->cipher->setPassword($password); + + $parts = explode('|', $authenticatedCiphertext); + if(sizeof($parts) !== 3) { + throw new \Exception('Authenticated ciphertext could not be decoded.'); + } + + $ciphertext = hex2bin($parts[0]); + $iv = $parts[1]; + $hmac = hex2bin($parts[2]); + + $this->cipher->setIV($iv); + + if(!StringUtils::equals($this->calculateHMAC($parts[0].$parts[1], $password), $hmac)) { + throw new \Exception('HMAC does not match.'); + } + + return $this->cipher->decrypt($ciphertext); + } + +} diff --git a/lib/private/security/securerandom.php b/lib/private/security/securerandom.php new file mode 100644 index 0000000000..2402e863fb --- /dev/null +++ b/lib/private/security/securerandom.php @@ -0,0 +1,79 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\Security; + +use RandomLib; +use Sabre\DAV\Exception; +use OCP\Security\ISecureRandom; + +/** + * Class SecureRandom provides a layer around RandomLib to generate + * secure random strings. + * + * Usage: + * \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(10); + * + * @package OC\Security + */ +class SecureRandom implements ISecureRandom { + + /** @var \RandomLib\Factory */ + var $factory; + /** @var \RandomLib\Generator */ + var $generator; + + function __construct() { + $this->factory = new RandomLib\Factory; + } + + /** + * Convenience method to get a low strength random number generator. + * + * Low Strength should be used anywhere that random strings are needed + * in a non-cryptographical setting. They are not strong enough to be + * used as keys or salts. They are however useful for one-time use tokens. + * + * @return $this + */ + public function getLowStrengthGenerator() { + $this->generator = $this->factory->getLowStrengthGenerator(); + return $this; + } + + /** + * Convenience method to get a medium strength random number generator. + * + * Medium Strength should be used for most needs of a cryptographic nature. + * They are strong enough to be used as keys and salts. However, they do + * take some time and resources to generate, so they should not be over-used + * + * @return $this + */ + public function getMediumStrengthGenerator() { + $this->generator = $this->factory->getMediumStrengthGenerator(); + return $this; + } + + /** + * Generate a random string of specified length. + * @param string $length The length of the generated string + * @param string $characters An optional list of characters to use if no characterlist is + * specified 0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ./ + * is used. + * @return string + * @throws \Exception If the generator is not initialized. + */ + public function generate($length, $characters = '') { + if(is_null($this->generator)) { + throw new \Exception('Generator is not initialized.'); + } + + return $this->generator->generateString($length, $characters); + } +} diff --git a/lib/private/security/stringutils.php b/lib/private/security/stringutils.php new file mode 100644 index 0000000000..32dff50fa8 --- /dev/null +++ b/lib/private/security/stringutils.php @@ -0,0 +1,38 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\Security; + +class StringUtils { + + /** + * Compares whether two strings are equal. To prevent guessing of the string + * length this is done by comparing two hashes against each other and afterwards + * a comparison of the real string to prevent against the unlikely chance of + * collisions. + * @param string $expected The expected value + * @param string $input The input to compare against + * @return bool True if the two strings are equal, otherwise false. + */ + public static function equals($expected, $input) { + + if(function_exists('hash_equals')) { + return hash_equals($expected, $input); + } + + $randomString = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(10); + + if(hash('sha512', $expected.$randomString) === hash('sha512', $input.$randomString)) { + if($expected === $input) { + return true; + } + } + + return false; + } +} \ No newline at end of file diff --git a/lib/private/server.php b/lib/private/server.php index aab3c82bfe..86fead1daf 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -9,6 +9,8 @@ use OC\Cache\UserCache; use OC\DB\ConnectionWrapper; use OC\Files\Node\Root; use OC\Files\View; +use OC\Security\Crypto; +use OC\Security\SecureRandom; use OCP\IServerContainer; /** @@ -199,6 +201,12 @@ class Server extends SimpleContainer implements IServerContainer { $this->registerService('Search', function ($c) { return new Search(); }); + $this->registerService('SecureRandom', function($c) { + return new SecureRandom(); + }); + $this->registerService('Crypto', function($c) { + return new Crypto(); + }); $this->registerService('Db', function ($c) { return new Db(); }); @@ -455,6 +463,24 @@ class Server extends SimpleContainer implements IServerContainer { return $this->query('Search'); } + /** + * Returns a SecureRandom instance + * + * @return \OCP\Security\ISecureRandom + */ + function getSecureRandom() { + return $this->query('SecureRandom'); + } + + /** + * Returns a Crypto instance + * + * @return \OCP\Security\ICrypto + */ + function getCrypto() { + return $this->query('Crypto'); + } + /** * Returns an instance of the db facade * diff --git a/lib/private/setup.php b/lib/private/setup.php index fdf98ab095..9ea1690b6d 100644 --- a/lib/private/setup.php +++ b/lib/private/setup.php @@ -67,14 +67,19 @@ class OC_Setup { } //generate a random salt that is used to salt the local user passwords - $salt = OC_Util::generateRandomBytes(30); - OC_Config::setValue('passwordsalt', $salt); + $salt = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(30); + \OC::$server->getConfig()->setSystemValue('passwordsalt', $salt); + + // generate a secret + $secret = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(48); + \OC::$server->getConfig()->setSystemValue('secret', $secret); //write the config file - OC_Config::setValue('trusted_domains', $trustedDomains); - OC_Config::setValue('datadirectory', $datadir); - OC_Config::setValue('dbtype', $dbtype); - OC_Config::setValue('version', implode('.', OC_Util::getVersion())); + \OC::$server->getConfig()->setSystemValue('trusted_domains', $trustedDomains); + \OC::$server->getConfig()->setSystemValue('datadirectory', $datadir); + \OC::$server->getConfig()->setSystemValue('dbtype', $dbtype); + \OC::$server->getConfig()->setSystemValue('version', implode('.', OC_Util::getVersion())); + try { $dbSetup->initialize($options); $dbSetup->setupDatabase($username); diff --git a/lib/private/util.php b/lib/private/util.php index 4307560a92..b2a9aecb5d 100755 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -905,7 +905,7 @@ class OC_Util { $id = OC_Config::getValue('instanceid', null); if (is_null($id)) { // We need to guarantee at least one letter in instanceid so it can be used as the session_name - $id = 'oc' . self::generateRandomBytes(10); + $id = 'oc' . \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(10); OC_Config::$object->setValue('instanceid', $id); } return $id; @@ -1208,62 +1208,20 @@ class OC_Util { * * @param int $length of the random string * @return string - * Please also update secureRNGAvailable if you change something here + * @deprecated Use \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate($length); instead */ public static function generateRandomBytes($length = 30) { - // Try to use openssl_random_pseudo_bytes - if (function_exists('openssl_random_pseudo_bytes')) { - $pseudoByte = bin2hex(openssl_random_pseudo_bytes($length, $strong)); - if ($strong == true) { - return substr($pseudoByte, 0, $length); // Truncate it to match the length - } - } - - // Try to use /dev/urandom - if (!self::runningOnWindows()) { - $fp = @file_get_contents('/dev/urandom', false, null, 0, $length); - if ($fp !== false) { - $string = substr(bin2hex($fp), 0, $length); - return $string; - } - } - - // Fallback to mt_rand() - $characters = '0123456789'; - $characters .= 'abcdefghijklmnopqrstuvwxyz'; - $charactersLength = strlen($characters) - 1; - $pseudoByte = ""; - - // Select some random characters - for ($i = 0; $i < $length; $i++) { - $pseudoByte .= $characters[mt_rand(0, $charactersLength)]; - } - return $pseudoByte; + return \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate($length); } /** * Checks if a secure random number generator is available * - * @return bool + * @return true + * @deprecated Function will be removed in the future and does only return true. */ public static function secureRNGAvailable() { - // Check openssl_random_pseudo_bytes - if (function_exists('openssl_random_pseudo_bytes')) { - openssl_random_pseudo_bytes(1, $strong); - if ($strong == true) { - return true; - } - } - - // Check /dev/urandom - if (!self::runningOnWindows()) { - $fp = @file_get_contents('/dev/urandom', false, null, 0, 1); - if ($fp !== false) { - return true; - } - } - - return false; + return true; } /** diff --git a/lib/public/security/icrypto.php b/lib/public/security/icrypto.php new file mode 100644 index 0000000000..e55eea8dd6 --- /dev/null +++ b/lib/public/security/icrypto.php @@ -0,0 +1,46 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCP\Security; + +/** + * Class Crypto provides a high-level encryption layer using AES-CBC. If no key has been provided + * it will use the secret defined in config.php as key. Additionally the message will be HMAC'd. + * + * Usage: + * $encryptWithDefaultPassword = \OC::$server->getCrypto()->encrypt('EncryptedText'); + * $encryptWithCustomPassword = \OC::$server->getCrypto()->encrypt('EncryptedText', 'password'); + * + * @package OCP\Security + */ +interface ICrypto { + + /** + * @param string $message The message to authenticate + * @param string $password Password to use (defaults to `secret` in config.php) + * @return string Calculated HMAC + */ + public function calculateHMAC($message, $password = ''); + + /** + * Encrypts a value and adds an HMAC (Encrypt-Then-MAC) + * @param string $plaintext + * @param string $password Password to encrypt, if not specified the secret from config.php will be taken + * @return string Authenticated ciphertext + */ + public function encrypt($plaintext, $password = ''); + + /** + * Decrypts a value and verifies the HMAC (Encrypt-Then-Mac) + * @param string $authenticatedCiphertext + * @param string $password Password to encrypt, if not specified the secret from config.php will be taken + * @return string plaintext + * @throws \Exception If the HMAC does not match + */ + public function decrypt($authenticatedCiphertext, $password = ''); +} diff --git a/lib/public/security/isecurerandom.php b/lib/public/security/isecurerandom.php new file mode 100644 index 0000000000..ae6e1d5845 --- /dev/null +++ b/lib/public/security/isecurerandom.php @@ -0,0 +1,53 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCP\Security; + +/** + * Class SecureRandom provides a layer around RandomLib to generate + * secure random numbers. + * + * Usage: + * $rng = new \OC\Security\SecureRandom(); + * $randomString = $rng->getMediumStrengthGenerator()->generateString(30); + * + * @package OCP\Security + */ +interface ISecureRandom { + + /** + * Convenience method to get a low strength random number generator. + * + * Low Strength should be used anywhere that random strings are needed + * in a non-cryptographical setting. They are not strong enough to be + * used as keys or salts. They are however useful for one-time use tokens. + * + * @return $this + */ + public function getLowStrengthGenerator(); + + /** + * Convenience method to get a medium strength random number generator. + * + * Medium Strength should be used for most needs of a cryptographic nature. + * They are strong enough to be used as keys and salts. However, they do + * take some time and resources to generate, so they should not be over-used + * + * @return $this + */ + public function getMediumStrengthGenerator(); + + /** + * Generate a random string of specified length. + * @param string $length The length of the generated string + * @param string $characters An optional list of characters to use + * @return string + * @throws \Exception + */ + public function generate($length, $characters = ''); +} diff --git a/lib/public/security/stringutils.php b/lib/public/security/stringutils.php new file mode 100644 index 0000000000..8e7b132724 --- /dev/null +++ b/lib/public/security/stringutils.php @@ -0,0 +1,25 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + + +namespace OCP\Security; + +class StringUtils { + /** + * Compares whether two strings are equal. To prevent guessing of the string + * length this is done by comparing two hashes against each other and afterwards + * a comparison of the real string to prevent against the unlikely chance of + * collisions. + * @param string $expected The expected value + * @param string $input The input to compare against + * @return bool True if the two strings are equal, otherwise false. + */ + public static function equals($expected, $input) { + return \OC\Security\StringUtils::equals($expected, $input); + } +} diff --git a/lib/public/util.php b/lib/public/util.php index 87b7a4f19d..83a6155685 100644 --- a/lib/public/util.php +++ b/lib/public/util.php @@ -505,6 +505,7 @@ class Util { * Generates a cryptographic secure pseudo-random string * @param int $length of the random string * @return string + * @deprecated Use \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate($length); instead */ public static function generateRandomBytes($length = 30) { return \OC_Util::generateRandomBytes($length); diff --git a/lib/repair/repairconfig.php b/lib/repair/repairconfig.php new file mode 100644 index 0000000000..e09d8e8fe7 --- /dev/null +++ b/lib/repair/repairconfig.php @@ -0,0 +1,37 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\Repair; + +use OC\Hooks\BasicEmitter; +use OC\RepairStep; +use Sabre\DAV\Exception; + +class RepairConfig extends BasicEmitter implements RepairStep { + + public function getName() { + return 'Repair config'; + } + + /** + * Updates the configuration after running an update + */ + public function run() { + $this->addSecret(); + } + + /** + * Adds a secret to config.php + */ + private function addSecret() { + if(\OC::$server->getConfig()->getSystemValue('secret', null) === null) { + $secret = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(48); + \OC::$server->getConfig()->setSystemValue('secret', $secret); + } + } +} diff --git a/tests/lib/security/crypto.php b/tests/lib/security/crypto.php new file mode 100644 index 0000000000..e07a60267e --- /dev/null +++ b/tests/lib/security/crypto.php @@ -0,0 +1,63 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +use \OC\Security\Crypto; + +class CryptoTest extends \PHPUnit_Framework_TestCase { + + function testDefaultEncrypt() { + $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; + $crypto = new Crypto(); + $ciphertext = $crypto->encrypt($stringToEncrypt); + $this->assertEquals($stringToEncrypt, $crypto->decrypt($ciphertext)); + + $stringToEncrypt = ''; + $ciphertext = $crypto->encrypt($stringToEncrypt); + $this->assertEquals($stringToEncrypt, $crypto->decrypt($ciphertext)); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage HMAC does not match. + */ + function testWrongPassword() { + $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; + $encryptCrypto = new Crypto(); + $ciphertext = $encryptCrypto->encrypt($stringToEncrypt); + + $decryptCrypto = new Crypto(); + $this->assertFalse($decryptCrypto->decrypt($ciphertext, 'A wrong password!')); + } + + function testLaterDecryption() { + $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; + $encryptedString = '560f5436ba864b9f12f7f7ca6d41c327554a6f2c0a160a03316b202af07c65163274993f3a46e7547c07ba89304f00594a2f3bd99f83859097c58049c39d0d4ade10e0de914ff0604961e7c849d0271ed6c0b23f984ba16e7d033e3305fb0910e7b6a2a65c988d17dbee71d8f953684d|d2kdFUspVjC0Y0sr|1a5feacf87eaa6869a6abdfba9a296e7bbad45b6ad89f7dce67cdc98e2da5dc4379cc672cc655e52bbf19599bf59482fbea13a73937697fa656bf10f3fc4f1aa'; + $crypto = new Crypto(); + $this->assertEquals($stringToEncrypt, $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd')); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage HMAC does not match. + */ + function testWrongIV() { + $encryptedString = '560f5436ba864b9f12f7f7ca6d41c327554a6f2c0a160a03316b202af07c65163274993f3a46e7547c07ba89304f00594a2f3bd99f83859097c58049c39d0d4ade10e0de914ff0604961e7c849d0271ed6c0b23f984ba16e7d033e3305fb0910e7b6a2a65c988d17dbee71d8f953684d|d2kdFUspVjC0o0sr|1a5feacf87eaa6869a6abdfba9a296e7bbad45b6ad89f7dce67cdc98e2da5dc4379cc672cc655e52bbf19599bf59482fbea13a73937697fa656bf10f3fc4f1aa'; + $crypto = new Crypto(); + $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Authenticated ciphertext could not be decoded. + */ + function testWrongParameters() { + $encryptedString = '1|2'; + $crypto = new Crypto(); + $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); + } +} diff --git a/tests/lib/security/securerandom.php b/tests/lib/security/securerandom.php new file mode 100644 index 0000000000..75f8f56fb9 --- /dev/null +++ b/tests/lib/security/securerandom.php @@ -0,0 +1,51 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +class SecureRandomTest extends \PHPUnit_Framework_TestCase { + + public function stringGenerationProvider() { + return array( + array(0, 0), + array(1, 1), + array(128, 128), + array(256, 256), + array(1024, 1024), + array(2048, 2048), + array(64000, 64000), + ); + } + + /** + * @dataProvider stringGenerationProvider + */ + function testGetLowStrengthGeneratorLength($length, $expectedLength) { + $rng = new \OC\Security\SecureRandom(); + $generator = $rng->getLowStrengthGenerator(); + + $this->assertEquals($expectedLength, strlen($generator->generate($length))); + } + + /** + * @dataProvider stringGenerationProvider + */ + function testMediumLowStrengthGeneratorLength($length, $expectedLength) { + $rng = new \OC\Security\SecureRandom(); + $generator = $rng->getMediumStrengthGenerator(); + + $this->assertEquals($expectedLength, strlen($generator->generate($length))); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Generator is not initialized + */ + function testUninitializedGenerate() { + $rng = new \OC\Security\SecureRandom(); + $rng->generate(30); + } +} diff --git a/tests/lib/security/stringutils.php b/tests/lib/security/stringutils.php new file mode 100644 index 0000000000..72293124eb --- /dev/null +++ b/tests/lib/security/stringutils.php @@ -0,0 +1,21 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +use \OC\Security\StringUtils; + +class StringUtilsTest extends \PHPUnit_Framework_TestCase { + + function testEquals() { + $this->assertTrue(StringUtils::equals('GpKY9fSnWRaeFNJbES99zVGvA', 'GpKY9fSnWRaeFNJbES99zVGvA')); + $this->assertFalse(StringUtils::equals('GpKY9fSnWNJbES99zVGvA', 'GpKY9fSnWRaeFNJbES99zVGvA')); + $this->assertFalse(StringUtils::equals('', 'GpKY9fSnWRaeFNJbES99zVGvA')); + $this->assertFalse(StringUtils::equals('GpKY9fSnWRaeFNJbES99zVGvA', '')); + $this->assertTrue(StringUtils::equals('', '')); + } + +} From 3329e0f2b22207a24ddb4953bbf11964b23682d9 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 27 Aug 2014 00:49:53 +0200 Subject: [PATCH 2/9] Use DI --- lib/private/security/crypto.php | 18 +++++++++++++----- lib/private/server.php | 2 +- tests/lib/security/crypto.php | 12 ++++++------ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/private/security/crypto.php b/lib/private/security/crypto.php index 659b0170ec..34f0d4e617 100644 --- a/lib/private/security/crypto.php +++ b/lib/private/security/crypto.php @@ -12,7 +12,9 @@ namespace OC\Security; use Crypt_AES; use Crypt_Hash; use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; use OCP\Security\StringUtils; +use OCP\IConfig; /** * Class Crypto provides a high-level encryption layer using AES-CBC. If no key has been provided @@ -29,9 +31,15 @@ class Crypto implements ICrypto { private $cipher; /** @var int */ private $ivLength = 16; + /** @var IConfig */ + private $config; + /** @var ISecureRandom */ + private $random; - function __construct() { + function __construct(IConfig $config, ISecureRandom $random) { $this->cipher = new Crypt_AES(); + $this->config = $config; + $this->random = $random; } /** @@ -41,7 +49,7 @@ class Crypto implements ICrypto { */ public function calculateHMAC($message, $password = '') { if($password === '') { - $password = \OC::$server->getConfig()->getSystemValue('secret'); + $password = $this->config->getSystemValue('secret'); } $hash = new Crypt_Hash('sha512'); @@ -57,11 +65,11 @@ class Crypto implements ICrypto { */ public function encrypt($plaintext, $password = '') { if($password === '') { - $password = \OC::$server->getConfig()->getSystemValue('secret'); + $password = $this->config->getSystemValue('secret'); } $this->cipher->setPassword($password); - $iv = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate($this->ivLength); + $iv = $this->random->getLowStrengthGenerator()->generate($this->ivLength); $this->cipher->setIV($iv); $ciphertext = bin2hex($this->cipher->encrypt($plaintext)); @@ -79,7 +87,7 @@ class Crypto implements ICrypto { */ public function decrypt($authenticatedCiphertext, $password = '') { if($password === '') { - $password = \OC::$server->getConfig()->getSystemValue('secret'); + $password = $this->config->getSystemValue('secret'); } $this->cipher->setPassword($password); diff --git a/lib/private/server.php b/lib/private/server.php index 86fead1daf..d67517f13e 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -205,7 +205,7 @@ class Server extends SimpleContainer implements IServerContainer { return new SecureRandom(); }); $this->registerService('Crypto', function($c) { - return new Crypto(); + return new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); }); $this->registerService('Db', function ($c) { return new Db(); diff --git a/tests/lib/security/crypto.php b/tests/lib/security/crypto.php index e07a60267e..6211e3e822 100644 --- a/tests/lib/security/crypto.php +++ b/tests/lib/security/crypto.php @@ -12,7 +12,7 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { function testDefaultEncrypt() { $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; - $crypto = new Crypto(); + $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); $ciphertext = $crypto->encrypt($stringToEncrypt); $this->assertEquals($stringToEncrypt, $crypto->decrypt($ciphertext)); @@ -27,17 +27,17 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { */ function testWrongPassword() { $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; - $encryptCrypto = new Crypto(); + $encryptCrypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); $ciphertext = $encryptCrypto->encrypt($stringToEncrypt); - $decryptCrypto = new Crypto(); + $decryptCrypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); $this->assertFalse($decryptCrypto->decrypt($ciphertext, 'A wrong password!')); } function testLaterDecryption() { $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; $encryptedString = '560f5436ba864b9f12f7f7ca6d41c327554a6f2c0a160a03316b202af07c65163274993f3a46e7547c07ba89304f00594a2f3bd99f83859097c58049c39d0d4ade10e0de914ff0604961e7c849d0271ed6c0b23f984ba16e7d033e3305fb0910e7b6a2a65c988d17dbee71d8f953684d|d2kdFUspVjC0Y0sr|1a5feacf87eaa6869a6abdfba9a296e7bbad45b6ad89f7dce67cdc98e2da5dc4379cc672cc655e52bbf19599bf59482fbea13a73937697fa656bf10f3fc4f1aa'; - $crypto = new Crypto(); + $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); $this->assertEquals($stringToEncrypt, $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd')); } @@ -47,7 +47,7 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { */ function testWrongIV() { $encryptedString = '560f5436ba864b9f12f7f7ca6d41c327554a6f2c0a160a03316b202af07c65163274993f3a46e7547c07ba89304f00594a2f3bd99f83859097c58049c39d0d4ade10e0de914ff0604961e7c849d0271ed6c0b23f984ba16e7d033e3305fb0910e7b6a2a65c988d17dbee71d8f953684d|d2kdFUspVjC0o0sr|1a5feacf87eaa6869a6abdfba9a296e7bbad45b6ad89f7dce67cdc98e2da5dc4379cc672cc655e52bbf19599bf59482fbea13a73937697fa656bf10f3fc4f1aa'; - $crypto = new Crypto(); + $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); } @@ -57,7 +57,7 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { */ function testWrongParameters() { $encryptedString = '1|2'; - $crypto = new Crypto(); + $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); } } From fe74b397a53b8a568c15d1ccf779bc8b0425b3c5 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sun, 31 Aug 2014 15:27:15 +0200 Subject: [PATCH 3/9] Use correct 3rdparty --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index 428579e7a1..82d02dd48a 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit 428579e7a14ce465cb3596c91110ad7d13de2b06 +Subproject commit 82d02dd48ad11312bd740c57720dc84b4d66fa8a From 50b430ee7cadd6be1520d63acdac27bc06581e09 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 3 Sep 2014 11:03:27 +0200 Subject: [PATCH 4/9] Add char consts, hash the specified password for the HMAC --- lib/private/security/crypto.php | 3 +++ lib/private/security/stringutils.php | 6 +++++- lib/public/security/isecurerandom.php | 8 ++++++++ tests/lib/security/crypto.php | 2 +- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/private/security/crypto.php b/lib/private/security/crypto.php index 34f0d4e617..6fdff8d92a 100644 --- a/lib/private/security/crypto.php +++ b/lib/private/security/crypto.php @@ -52,6 +52,9 @@ class Crypto implements ICrypto { $password = $this->config->getSystemValue('secret'); } + // Append an "a" behind the password and hash it to prevent reusing the same password as for encryption + $password = hash('sha512', $password . 'a'); + $hash = new Crypt_Hash('sha512'); $hash->setKey($password); return $hash->hash($message); diff --git a/lib/private/security/stringutils.php b/lib/private/security/stringutils.php index 32dff50fa8..33a3a70801 100644 --- a/lib/private/security/stringutils.php +++ b/lib/private/security/stringutils.php @@ -15,6 +15,10 @@ class StringUtils { * length this is done by comparing two hashes against each other and afterwards * a comparison of the real string to prevent against the unlikely chance of * collisions. + * + * Be aware that this function may leak whether the string to compare have a different + * length. + * * @param string $expected The expected value * @param string $input The input to compare against * @return bool True if the two strings are equal, otherwise false. @@ -25,7 +29,7 @@ class StringUtils { return hash_equals($expected, $input); } - $randomString = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(10); + $randomString = \OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(10); if(hash('sha512', $expected.$randomString) === hash('sha512', $input.$randomString)) { if($expected === $input) { diff --git a/lib/public/security/isecurerandom.php b/lib/public/security/isecurerandom.php index ae6e1d5845..8856b45780 100644 --- a/lib/public/security/isecurerandom.php +++ b/lib/public/security/isecurerandom.php @@ -20,6 +20,14 @@ namespace OCP\Security; */ interface ISecureRandom { + /** + * Flags for characters that can be used for generate($length, $characters) + */ + const CHAR_UPPER = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; + const CHAR_LOWER = 'abcdefghijklmnopqrstuvwxyz'; + const CHAR_DIGITS = '0123456789'; + const CHAR_SYMBOLS = 'CHAR_SYMBOLS'; + /** * Convenience method to get a low strength random number generator. * diff --git a/tests/lib/security/crypto.php b/tests/lib/security/crypto.php index 6211e3e822..55f9d2fe53 100644 --- a/tests/lib/security/crypto.php +++ b/tests/lib/security/crypto.php @@ -36,7 +36,7 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { function testLaterDecryption() { $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; - $encryptedString = '560f5436ba864b9f12f7f7ca6d41c327554a6f2c0a160a03316b202af07c65163274993f3a46e7547c07ba89304f00594a2f3bd99f83859097c58049c39d0d4ade10e0de914ff0604961e7c849d0271ed6c0b23f984ba16e7d033e3305fb0910e7b6a2a65c988d17dbee71d8f953684d|d2kdFUspVjC0Y0sr|1a5feacf87eaa6869a6abdfba9a296e7bbad45b6ad89f7dce67cdc98e2da5dc4379cc672cc655e52bbf19599bf59482fbea13a73937697fa656bf10f3fc4f1aa'; + $encryptedString = '44a35023cca2e7a6125e06c29fc4b2ad9d8a33d0873a8b45b0de4ef9284f260c6c46bf25dc62120644c59b8bafe4281ddc47a70c35ae6c29ef7a63d79eefacc297e60b13042ac582733598d0a6b4de37311556bb5c480fd2633de4e6ebafa868c2d1e2d80a5d24f9660360dba4d6e0c8|lhrFgK0zd9U160Wo|a75e57ab701f9124e1113543fd1dc596f21e20d456a0d1e813d5a8aaec9adcb11213788e96598b67fe9486a9f0b99642c18296d0175db44b1ae426e4e91080ee'; $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); $this->assertEquals($stringToEncrypt, $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd')); } From 20a7fb03340dbeceb5613642cb9ebb87ca9dfed1 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 3 Sep 2014 12:22:21 +0200 Subject: [PATCH 5/9] Fix CHAR_SYMBOLS --- lib/public/security/isecurerandom.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/public/security/isecurerandom.php b/lib/public/security/isecurerandom.php index 8856b45780..a6d24571ab 100644 --- a/lib/public/security/isecurerandom.php +++ b/lib/public/security/isecurerandom.php @@ -26,7 +26,7 @@ interface ISecureRandom { const CHAR_UPPER = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; const CHAR_LOWER = 'abcdefghijklmnopqrstuvwxyz'; const CHAR_DIGITS = '0123456789'; - const CHAR_SYMBOLS = 'CHAR_SYMBOLS'; + const CHAR_SYMBOLS = "!\"#$%&\'()* +,-./:;<=>?@[\]^_`{|}~"; /** * Convenience method to get a low strength random number generator. From 1ccbaae84636dabdd59cb0d9d945a6d2f22d10e8 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 3 Sep 2014 13:51:44 +0200 Subject: [PATCH 6/9] Refactor tests a little bit --- tests/lib/security/crypto.php | 45 +++++++++++++++++------------ tests/lib/security/securerandom.php | 18 ++++++++---- tests/lib/security/stringutils.php | 29 +++++++++++++++---- 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/tests/lib/security/crypto.php b/tests/lib/security/crypto.php index 55f9d2fe53..0f89253839 100644 --- a/tests/lib/security/crypto.php +++ b/tests/lib/security/crypto.php @@ -10,15 +10,28 @@ use \OC\Security\Crypto; class CryptoTest extends \PHPUnit_Framework_TestCase { - function testDefaultEncrypt() { - $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; - $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); - $ciphertext = $crypto->encrypt($stringToEncrypt); - $this->assertEquals($stringToEncrypt, $crypto->decrypt($ciphertext)); + public function defaultEncryptionProvider() + { + return array( + array('Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'), + array(''), + array('我看这本书。 我看這本書') + ); + } - $stringToEncrypt = ''; - $ciphertext = $crypto->encrypt($stringToEncrypt); - $this->assertEquals($stringToEncrypt, $crypto->decrypt($ciphertext)); + /** @var Crypto */ + protected $crypto; + + protected function setUp() { + $this->crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); + } + + /** + * @dataProvider defaultEncryptionProvider + */ + function testDefaultEncrypt($stringToEncrypt) { + $ciphertext = $this->crypto->encrypt($stringToEncrypt); + $this->assertEquals($stringToEncrypt, $this->crypto->decrypt($ciphertext)); } /** @@ -27,18 +40,14 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { */ function testWrongPassword() { $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; - $encryptCrypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); - $ciphertext = $encryptCrypto->encrypt($stringToEncrypt); - - $decryptCrypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); - $this->assertFalse($decryptCrypto->decrypt($ciphertext, 'A wrong password!')); + $ciphertext = $this->crypto->encrypt($stringToEncrypt); + $this->crypto->decrypt($ciphertext, 'A wrong password!'); } function testLaterDecryption() { $stringToEncrypt = 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'; $encryptedString = '44a35023cca2e7a6125e06c29fc4b2ad9d8a33d0873a8b45b0de4ef9284f260c6c46bf25dc62120644c59b8bafe4281ddc47a70c35ae6c29ef7a63d79eefacc297e60b13042ac582733598d0a6b4de37311556bb5c480fd2633de4e6ebafa868c2d1e2d80a5d24f9660360dba4d6e0c8|lhrFgK0zd9U160Wo|a75e57ab701f9124e1113543fd1dc596f21e20d456a0d1e813d5a8aaec9adcb11213788e96598b67fe9486a9f0b99642c18296d0175db44b1ae426e4e91080ee'; - $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); - $this->assertEquals($stringToEncrypt, $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd')); + $this->assertEquals($stringToEncrypt, $this->crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd')); } /** @@ -47,8 +56,7 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { */ function testWrongIV() { $encryptedString = '560f5436ba864b9f12f7f7ca6d41c327554a6f2c0a160a03316b202af07c65163274993f3a46e7547c07ba89304f00594a2f3bd99f83859097c58049c39d0d4ade10e0de914ff0604961e7c849d0271ed6c0b23f984ba16e7d033e3305fb0910e7b6a2a65c988d17dbee71d8f953684d|d2kdFUspVjC0o0sr|1a5feacf87eaa6869a6abdfba9a296e7bbad45b6ad89f7dce67cdc98e2da5dc4379cc672cc655e52bbf19599bf59482fbea13a73937697fa656bf10f3fc4f1aa'; - $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); - $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); + $this->crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); } /** @@ -57,7 +65,6 @@ class CryptoTest extends \PHPUnit_Framework_TestCase { */ function testWrongParameters() { $encryptedString = '1|2'; - $crypto = new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom()); - $crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); + $this->crypto->decrypt($encryptedString, 'ThisIsAVeryS3cur3P4ssw0rd'); } } diff --git a/tests/lib/security/securerandom.php b/tests/lib/security/securerandom.php index 75f8f56fb9..52c1dec92e 100644 --- a/tests/lib/security/securerandom.php +++ b/tests/lib/security/securerandom.php @@ -6,6 +6,8 @@ * See the COPYING-README file. */ +use \OC\Security\SecureRandom; + class SecureRandomTest extends \PHPUnit_Framework_TestCase { public function stringGenerationProvider() { @@ -20,12 +22,18 @@ class SecureRandomTest extends \PHPUnit_Framework_TestCase { ); } + /** @var SecureRandom */ + protected $rng; + + protected function setUp() { + $this->rng = new \OC\Security\SecureRandom(); + } + /** * @dataProvider stringGenerationProvider */ function testGetLowStrengthGeneratorLength($length, $expectedLength) { - $rng = new \OC\Security\SecureRandom(); - $generator = $rng->getLowStrengthGenerator(); + $generator = $this->rng->getLowStrengthGenerator(); $this->assertEquals($expectedLength, strlen($generator->generate($length))); } @@ -34,8 +42,7 @@ class SecureRandomTest extends \PHPUnit_Framework_TestCase { * @dataProvider stringGenerationProvider */ function testMediumLowStrengthGeneratorLength($length, $expectedLength) { - $rng = new \OC\Security\SecureRandom(); - $generator = $rng->getMediumStrengthGenerator(); + $generator = $this->rng->getMediumStrengthGenerator(); $this->assertEquals($expectedLength, strlen($generator->generate($length))); } @@ -45,7 +52,6 @@ class SecureRandomTest extends \PHPUnit_Framework_TestCase { * @expectedExceptionMessage Generator is not initialized */ function testUninitializedGenerate() { - $rng = new \OC\Security\SecureRandom(); - $rng->generate(30); + $this->rng->generate(30); } } diff --git a/tests/lib/security/stringutils.php b/tests/lib/security/stringutils.php index 72293124eb..039f3d3756 100644 --- a/tests/lib/security/stringutils.php +++ b/tests/lib/security/stringutils.php @@ -10,12 +10,29 @@ use \OC\Security\StringUtils; class StringUtilsTest extends \PHPUnit_Framework_TestCase { - function testEquals() { - $this->assertTrue(StringUtils::equals('GpKY9fSnWRaeFNJbES99zVGvA', 'GpKY9fSnWRaeFNJbES99zVGvA')); - $this->assertFalse(StringUtils::equals('GpKY9fSnWNJbES99zVGvA', 'GpKY9fSnWRaeFNJbES99zVGvA')); - $this->assertFalse(StringUtils::equals('', 'GpKY9fSnWRaeFNJbES99zVGvA')); - $this->assertFalse(StringUtils::equals('GpKY9fSnWRaeFNJbES99zVGvA', '')); - $this->assertTrue(StringUtils::equals('', '')); + public function dataProvider() + { + return array( + array('Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.', 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt.'), + array('', ''), + array('我看这本书。 我看這本書', '我看这本书。 我看這本書'), + array('GpKY9fSnWNJbES99zVGvA', 'GpKY9fSnWNJbES99zVGvA') + ); + } + + /** + * @dataProvider dataProvider + */ + function testWrongEquals($string) { + $this->assertFalse(StringUtils::equals($string, 'A Completely Wrong String')); + $this->assertFalse(StringUtils::equals($string, null)); + } + + /** + * @dataProvider dataProvider + */ + function testTrueEquals($string, $expected) { + $this->assertTrue(StringUtils::equals($string, $expected)); } } From a54af89d8a579ead1b26f617fa0d177925aee6e8 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 3 Sep 2014 14:13:12 +0200 Subject: [PATCH 7/9] Add test for the second argument --- lib/public/security/isecurerandom.php | 2 +- tests/lib/security/securerandom.php | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/public/security/isecurerandom.php b/lib/public/security/isecurerandom.php index a6d24571ab..46d82dd5f1 100644 --- a/lib/public/security/isecurerandom.php +++ b/lib/public/security/isecurerandom.php @@ -26,7 +26,7 @@ interface ISecureRandom { const CHAR_UPPER = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; const CHAR_LOWER = 'abcdefghijklmnopqrstuvwxyz'; const CHAR_DIGITS = '0123456789'; - const CHAR_SYMBOLS = "!\"#$%&\'()* +,-./:;<=>?@[\]^_`{|}~"; + const CHAR_SYMBOLS = '!\"#$%&\\'()* +,-./:;<=>?@[\]^_`{|}~'; /** * Convenience method to get a low strength random number generator. diff --git a/tests/lib/security/securerandom.php b/tests/lib/security/securerandom.php index 52c1dec92e..2920077fa1 100644 --- a/tests/lib/security/securerandom.php +++ b/tests/lib/security/securerandom.php @@ -22,6 +22,14 @@ class SecureRandomTest extends \PHPUnit_Framework_TestCase { ); } + public static function charCombinations() { + return array( + array('CHAR_LOWER', '[a-z]'), + array('CHAR_UPPER', '[A-Z]'), + array('CHAR_DIGITS', '[0-9]'), + ); + } + /** @var SecureRandom */ protected $rng; @@ -54,4 +62,15 @@ class SecureRandomTest extends \PHPUnit_Framework_TestCase { function testUninitializedGenerate() { $this->rng->generate(30); } + + /** + * @dataProvider charCombinations + */ + public function testScheme($charName, $chars) { + $generator = $this->rng->getMediumStrengthGenerator(); + $scheme = constant('OCP\Security\ISecureRandom::' . $charName); + $randomString = $generator->generate(100, $scheme); + $matchesRegex = preg_match('/^'.$chars.'+$/', $randomString); + $this->assertSame(1, $matchesRegex); + } } From dcea6de26ad9d640a91064f35719d4a5a80cc64d Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 3 Sep 2014 14:16:55 +0200 Subject: [PATCH 8/9] Fix quoting --- lib/public/security/isecurerandom.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/public/security/isecurerandom.php b/lib/public/security/isecurerandom.php index 46d82dd5f1..3de60f8d71 100644 --- a/lib/public/security/isecurerandom.php +++ b/lib/public/security/isecurerandom.php @@ -26,7 +26,7 @@ interface ISecureRandom { const CHAR_UPPER = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; const CHAR_LOWER = 'abcdefghijklmnopqrstuvwxyz'; const CHAR_DIGITS = '0123456789'; - const CHAR_SYMBOLS = '!\"#$%&\\'()* +,-./:;<=>?@[\]^_`{|}~'; + const CHAR_SYMBOLS = '!\"#$%&\\\'()* +,-./:;<=>?@[\]^_`{|}~'; /** * Convenience method to get a low strength random number generator. From dbbdcff862663373711d968821bb79a10aeb52a6 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 3 Sep 2014 14:21:35 +0200 Subject: [PATCH 9/9] Increment version --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 2442943411..4421a06bea 100644 --- a/version.php +++ b/version.php @@ -3,7 +3,7 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version=array(7, 8, 0, 0); +$OC_Version=array(7, 8, 1, 0); // The human readable string $OC_VersionString='8.0 pre alpha';