Skip to content
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

Merged
merged 8 commits into from Oct 28, 2012
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -138,5 +138,12 @@
<argument type="service" id="security.context" />
<argument type="service" id="security.encoder_factory" />
</service>

<!-- Pseudorandom Number Generator -->
<service id="security.secure_random" class="Symfony\Component\Security\Core\Util\SecureRandom">
<tag name="monolog.logger" channel="security" />
<argument>%kernel.cache_dir%/secure_random.seed</argument>
<argument type="service" id="logger" on-invalid="ignore" />
</service>
</services>
</container>
Expand Up @@ -45,6 +45,12 @@
class="%security.authentication.rememberme.services.persistent.class%"
parent="security.authentication.rememberme.services.abstract"
abstract="true">
<argument type="collection" /> <!-- User Providers -->
<argument /> <!-- Shared Token Key -->
<argument /> <!-- Shared Provider Key -->
<argument type="collection" /> <!-- Options -->
<argument type="service" id="logger" on-invalid="null" />
<argument type="service" id="security.secure_random" />
</service>

<service id="security.authentication.rememberme.services.simplehash"
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Security/CHANGELOG.md
Expand Up @@ -4,7 +4,8 @@ CHANGELOG
2.2.0
-----

* Added PBKDF2 Password encoder
* added secure random number generator
Copy link
Member

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 section

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

* added PBKDF2 Password encoder

2.1.0
-----
Expand Down
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Security\Core\Encoder;

use Symfony\Component\Security\Core\Util\StringUtils;

/**
* BasePasswordEncoder is the base class for all password encoders.
*
Expand Down Expand Up @@ -77,15 +79,6 @@ protected function mergePasswordAndSalt($password, $salt)
*/
protected function comparePasswords($password1, $password2)
{
if (strlen($password1) !== strlen($password2)) {
return false;
}

$result = 0;
for ($i = 0; $i < strlen($password1); $i++) {
$result |= ord($password1[$i]) ^ ord($password2[$i]);
}

return 0 === $result;
return StringUtils::equals($password1, $password2);
}
}
114 changes: 114 additions & 0 deletions src/Symfony/Component/Security/Core/Util/SecureRandom.php
@@ -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;
}
}
31 changes: 31 additions & 0 deletions src/Symfony/Component/Security/Core/Util/SecureRandomInterface.php
@@ -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);
}
48 changes: 48 additions & 0 deletions src/Symfony/Component/Security/Core/Util/StringUtils.php
@@ -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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that strlen is not in constant time. The computation time of this if-clause therfore still depends on the length of both strings.

Also the computation for the case with (strlen($str1) === strlen($str2)) is more costly (because it does no fast-exit). Thefore an attacker can try an increasing length of password input to check for the length of password.

(@ircmaxell has a very good implementation: https://github.com/ircmaxell/PHP-PasswordLib/blob/master/lib/PasswordLib/Password/AbstractPassword.php#L77)

Copy link
Contributor

Choose a reason for hiding this comment

The 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;
}
}
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -79,7 +98,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
}

$series = $persistentToken->getSeries();
$tokenValue = $this->generateRandomValue();
$tokenValue = $this->secureRandom->nextBytes(64);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still broken if the setter has not been called

Copy link
Member Author

Choose a reason for hiding this comment

The 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(
Expand All @@ -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(
Expand All @@ -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));
}
}
@@ -1,5 +1,14 @@
<?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\Tests\Core\Util
{
use Symfony\Component\Security\Core\Util\ClassUtils;
Expand Down