From 7ee9c1004cde3755df0c520baaabb72a80e02b02 Mon Sep 17 00:00:00 2001 From: nuxsmin Date: Thu, 15 Jun 2017 15:07:44 +0200 Subject: [PATCH] * [MOD] Related #637. Improved random bytes generation to prevent cache-timing attacks (this does not affec to to cryptographic functions, which were already safe). Thanks to @LeSuisse for the feedback. * [MOD] Temporary master password will never be saved in the event log. --- .../ConfigActionController.class.php | 1 - inc/SP/Core/CryptMasterPass.class.php | 4 ++- inc/SP/Util/Util.class.php | 34 +++---------------- 3 files changed, 8 insertions(+), 31 deletions(-) diff --git a/inc/SP/Controller/ConfigActionController.class.php b/inc/SP/Controller/ConfigActionController.class.php index 3c94d0a2..742bd100 100644 --- a/inc/SP/Controller/ConfigActionController.class.php +++ b/inc/SP/Controller/ConfigActionController.class.php @@ -647,7 +647,6 @@ class ConfigActionController implements ItemControllerInterface if ($tempMasterPass !== false && !empty($tempMasterPass)) { $this->LogMessage->addDescription(__('Clave Temporal Generada', false)); - $this->LogMessage->addDetails(__('Clave', false), $tempMasterPass); if ($tempMasterEmail) { $Message = new NoticeMessage(); diff --git a/inc/SP/Core/CryptMasterPass.class.php b/inc/SP/Core/CryptMasterPass.class.php index 611f79d7..71facb88 100644 --- a/inc/SP/Core/CryptMasterPass.class.php +++ b/inc/SP/Core/CryptMasterPass.class.php @@ -95,7 +95,9 @@ class CryptMasterPass Log::writeNewLog(__FUNCTION__, __('Clave temporal caducada', false), Log::INFO); return false; - } elseif ((!empty($passTime) && time() > $passMaxTime) + } + + if ((!empty($passTime) && time() > $passMaxTime) || $attempts >= self::MAX_ATTEMPTS ) { ConfigDB::setCacheConfigValue('tempmaster_pass', ''); diff --git a/inc/SP/Util/Util.class.php b/inc/SP/Util/Util.class.php index a177de79..c0f29b17 100644 --- a/inc/SP/Util/Util.class.php +++ b/inc/SP/Util/Util.class.php @@ -24,6 +24,9 @@ namespace SP\Util; +use Defuse\Crypto\Core; +use Defuse\Crypto\Crypto; +use Defuse\Crypto\Encoding; use SP\Config\Config; use SP\Config\ConfigDB; use SP\Core\Exceptions\SPException; @@ -119,38 +122,11 @@ class Util * * @param int $length opcional, con la longitud de la cadena * @return string + * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException */ public static function generateRandomBytes($length = 30) { - if (function_exists('random_bytes')) { - return bin2hex(random_bytes($length)); - } - - // Try to use openssl_random_pseudo_bytes - if (function_exists('openssl_random_pseudo_bytes')) { - $pseudo_byte = bin2hex(openssl_random_pseudo_bytes($length, $strong)); - return substr($pseudo_byte, 0, $length); // Truncate it to match the length - } - - // Try to use /dev/urandom - $fp = @file_get_contents('/dev/urandom', false, null, 0, $length); - if ($fp !== false) { - return substr(bin2hex($fp), 0, $length); - } - - // Fallback to mt_rand() - $characters = '0123456789'; - $characters .= 'abcdefghijklmnopqrstuvwxyz'; - $characters .= strtoupper('abcdefghijklmnopqrstuvwxyz'); - $charactersLength = strlen($characters) - 1; - $pseudo_byte = ''; - - // Select some random characters - for ($i = 0; $i < $length; $i++) { - $pseudo_byte .= $characters[mt_rand(0, $charactersLength)]; - } - - return $pseudo_byte; + return Encoding::binToHex(Core::secureRandom($length)); }