New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2.2][Security] Add a PRNG (closes #3595) #4763
Changes from all commits
e5dc7af
c0c8972
248703f
5849855
234f725
5cdf696
ca567b5
aecc9b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Security\Core\Util; | ||
|
||
use Symfony\Component\HttpKernel\Log\LoggerInterface; | ||
|
||
/** | ||
* A secure random number generator implementation. | ||
* | ||
* @author Fabien Potencier <fabien@symfony.com> | ||
* @author Johannes M. Schmitt <schmittjoh@gmail.com> | ||
*/ | ||
final class SecureRandom implements SecureRandomInterface | ||
{ | ||
private $logger; | ||
private $useOpenSsl; | ||
private $seed; | ||
private $seedUpdated; | ||
private $seedLastUpdatedAt; | ||
private $seedFile; | ||
|
||
/** | ||
* Constructor. | ||
* | ||
* Be aware that a guessable seed will severely compromise the PRNG | ||
* algorithm that is employed. | ||
* | ||
* @param string $seedFile | ||
* @param LoggerInterface $logger | ||
*/ | ||
public function __construct($seedFile = null, LoggerInterface $logger = null) | ||
{ | ||
$this->seedFile = $seedFile; | ||
$this->logger = $logger; | ||
|
||
// determine whether to use OpenSSL | ||
if (defined('PHP_WINDOWS_VERSION_BUILD') && version_compare(PHP_VERSION, '5.3.4', '<')) { | ||
$this->useOpenSsl = false; | ||
} elseif (!function_exists('openssl_random_pseudo_bytes')) { | ||
if (null !== $this->logger) { | ||
$this->logger->notice('It is recommended that you enable the "openssl" extension for random number generation.'); | ||
} | ||
$this->useOpenSsl = false; | ||
} else { | ||
$this->useOpenSsl = true; | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function nextBytes($nbBytes) | ||
{ | ||
// try OpenSSL | ||
if ($this->useOpenSsl) { | ||
$bytes = openssl_random_pseudo_bytes($nbBytes, $strong); | ||
|
||
if (false !== $bytes && true === $strong) { | ||
return $bytes; | ||
} | ||
|
||
if (null !== $this->logger) { | ||
$this->logger->info('OpenSSL did not produce a secure random number.'); | ||
} | ||
} | ||
|
||
// initialize seed | ||
if (null === $this->seed) { | ||
if (null === $this->seedFile) { | ||
throw new \RuntimeException('You need to specify a file path to store the seed.'); | ||
} | ||
|
||
if (is_file($this->seedFile)) { | ||
list($this->seed, $this->seedLastUpdatedAt) = $this->readSeed(); | ||
} else { | ||
$this->seed = uniqid(mt_rand(), true); | ||
$this->updateSeed(); | ||
} | ||
} | ||
|
||
$bytes = ''; | ||
while (strlen($bytes) < $nbBytes) { | ||
static $incr = 1; | ||
$bytes .= hash('sha512', $incr++.$this->seed.uniqid(mt_rand(), true).$nbBytes, true); | ||
$this->seed = base64_encode(hash('sha512', $this->seed.$bytes.$nbBytes, true)); | ||
$this->updateSeed(); | ||
} | ||
|
||
return substr($bytes, 0, $nbBytes); | ||
} | ||
|
||
private function readSeed() | ||
{ | ||
return json_decode(file_get_contents($this->seedFile)); | ||
} | ||
|
||
private function updateSeed() | ||
{ | ||
if (!$this->seedUpdated && $this->seedLastUpdatedAt < time() - mt_rand(1, 10)) { | ||
file_put_contents($this->seedFile, json_encode(array($this->seed, microtime(true)))); | ||
} | ||
|
||
$this->seedUpdated = true; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Security\Core\Util; | ||
|
||
use Symfony\Component\HttpKernel\Log\LoggerInterface; | ||
|
||
/** | ||
* Interface that needs to be implemented by all secure random number generators. | ||
* | ||
* @author Fabien Potencier <fabien@symfony.com> | ||
*/ | ||
interface SecureRandomInterface | ||
{ | ||
/** | ||
* Generates the specified number of secure random bytes. | ||
* | ||
* @param integer $nbBytes | ||
* | ||
* @return string | ||
*/ | ||
public function nextBytes($nbBytes); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Security\Core\Util; | ||
|
||
/** | ||
* String utility functions. | ||
* | ||
* @author Fabien Potencier <fabien@symfony.com> | ||
*/ | ||
final class StringUtils | ||
{ | ||
final private function __construct() | ||
{ | ||
} | ||
|
||
/** | ||
* Compares two strings. | ||
* | ||
* This method implements a constant-time algorithm to compare strings. | ||
* | ||
* @param string $str1 The first string | ||
* @param string $str2 The second string | ||
* | ||
* @return Boolean true if the two strings are the same, false otherwise | ||
*/ | ||
public static function equals($str1, $str2) | ||
{ | ||
if (strlen($str1) !== $c = strlen($str2)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say that Also the computation for the case with (@ircmaxell has a very good implementation: https://github.com/ircmaxell/PHP-PasswordLib/blob/master/lib/PasswordLib/Password/AbstractPassword.php#L77) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That constant time comparitor that I use in PasswordLib is good if you're not calling it a lot of times. The only problem with it is that since it uses 2 hashes every time, it can be a little bit of overhead. For general usages, I have another implementation here: http://blog.ircmaxell.com/2012/12/seven-ways-to-screw-up-bcrypt.html The only implementation detail is the order of input matters (the string in the users control must be the second parameter for it to not leak any information. I have added a PR to address this issue: #6510 |
||
return false; | ||
} | ||
|
||
$result = 0; | ||
for ($i = 0; $i < $c; $i++) { | ||
$result |= ord($str1[$i]) ^ ord($str2[$i]); | ||
} | ||
|
||
return 0 === $result; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
use Symfony\Component\Security\Core\Exception\CookieTheftException; | ||
use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentToken; | ||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; | ||
use Symfony\Component\Security\Core\Util\SecureRandomInterface; | ||
|
||
/** | ||
* Concrete implementation of the RememberMeServicesInterface which needs | ||
|
@@ -30,6 +31,24 @@ | |
class PersistentTokenBasedRememberMeServices extends AbstractRememberMeServices | ||
{ | ||
private $tokenProvider; | ||
private $secureRandom; | ||
|
||
/** | ||
* Constructor. | ||
* | ||
* @param array $userProviders | ||
* @param string $key | ||
* @param string $providerKey | ||
* @param array $options | ||
* @param LoggerInterface $logger | ||
* @param SecureRandomInterface $secureRandom | ||
*/ | ||
public function __construct(array $userProviders, $key, $providerKey, array $options = array(), LoggerInterface $logger = null, SecureRandomInterface $secureRandom) | ||
{ | ||
parent::__construct($userProviders, $key, $providerKey, $options, $logger); | ||
|
||
$this->secureRandom = $secureRandom; | ||
} | ||
|
||
/** | ||
* Sets the token provider | ||
|
@@ -79,7 +98,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request) | |
} | ||
|
||
$series = $persistentToken->getSeries(); | ||
$tokenValue = $this->generateRandomValue(); | ||
$tokenValue = $this->secureRandom->nextBytes(64); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still broken if the setter has not been called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
$this->tokenProvider->updateToken($series, $tokenValue, new \DateTime()); | ||
$request->attributes->set(self::COOKIE_ATTR_NAME, | ||
new Cookie( | ||
|
@@ -101,8 +120,8 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request) | |
*/ | ||
protected function onLoginSuccess(Request $request, Response $response, TokenInterface $token) | ||
{ | ||
$series = $this->generateRandomValue(); | ||
$tokenValue = $this->generateRandomValue(); | ||
$series = $this->secureRandom->nextBytes(64); | ||
$tokenValue = $this->secureRandom->nextBytes(64); | ||
|
||
$this->tokenProvider->createNewToken( | ||
new PersistentToken( | ||
|
@@ -126,26 +145,4 @@ protected function onLoginSuccess(Request $request, Response $response, TokenInt | |
) | ||
); | ||
} | ||
|
||
/** | ||
* Generates a cryptographically strong random value | ||
* | ||
* @return string | ||
*/ | ||
protected function generateRandomValue() | ||
{ | ||
if (function_exists('openssl_random_pseudo_bytes')) { | ||
$bytes = openssl_random_pseudo_bytes(64, $strong); | ||
|
||
if (true === $strong && false !== $bytes) { | ||
return base64_encode($bytes); | ||
} | ||
} | ||
|
||
if (null !== $this->logger) { | ||
$this->logger->warn('Could not produce a cryptographically strong random value. Please install/update the OpenSSL extension.'); | ||
} | ||
|
||
return base64_encode(hash('sha512', uniqid(mt_rand(), true), true)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be moved to a
2.2.0
sectionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done