feature #15879 Deprecate the SecureRandom class (pierredup)

This PR was merged into the 2.8 branch. Discussion ---------- Deprecate the SecureRandom class | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Deprecate the `Symfony\Component\Security\Core\Util\SecureRandom` and encourage the usage of the `random_bytes` function instead Commits ------- f02973a Deprecate the SecureRandom class
symfony · Oct 6, 2015 · 141f9eb · 141f9eb
2 parents 7236571 + f02973a
commit 141f9eb
Show file tree

Hide file tree

Showing 16 changed files with 55 additions and 185 deletions.
diff --git a/composer.json b/composer.json
@@ -20,7 +20,8 @@
         "doctrine/common": "~2.4",
         "twig/twig": "~1.20|~2.0",
         "psr/log": "~1.0",
-        "symfony/security-acl": "~2.7"
+        "symfony/security-acl": "~2.7",
+        "paragonie/random_compat": "~1.0"
     },
     "replace": {
         "symfony/asset": "self.version",

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/security_csrf.xml
@@ -11,9 +11,7 @@
     </parameters>
 
     <services>
-        <service id="security.csrf.token_generator" class="%security.csrf.token_generator.class%" public="false">
-            <argument type="service" id="security.secure_random" />
-        </service>
+        <service id="security.csrf.token_generator" class="%security.csrf.token_generator.class%" public="false" />
 
         <service id="security.csrf.token_storage" class="%security.csrf.token_storage.class%" public="false">
             <argument type="service" id="session" />

diff --git a/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php b/src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php
@@ -164,6 +164,6 @@ private function createPasswordQuestion()
 
     private function generateSalt()
     {
-        return base64_encode($this->getContainer()->get('security.secure_random')->nextBytes(30));
+        return base64_encode(random_bytes(30));
     }
 }
diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_rememberme.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_rememberme.xml
@@ -45,9 +45,7 @@
         <service id="security.authentication.rememberme.services.persistent"
                  class="%security.authentication.rememberme.services.persistent.class%"
                  parent="security.authentication.rememberme.services.abstract"
-                 abstract="true">
-            <argument type="service" id="security.secure_random" />
-        </service>
+                 abstract="true" />
 
         <service id="security.authentication.rememberme.services.simplehash"
                  class="%security.authentication.rememberme.services.simplehash.class%"

diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md
@@ -12,6 +12,7 @@ CHANGELOG
    `Symfony\Component\Security\Http\Authentication\SimpleFormAuthenticatorInterface` instead
  * deprecated `Symfony\Component\Security\Core\Util\ClassUtils`, use
    `Symfony\Component\Security\Acl\Util\ClassUtils` instead
+ * deprecated the `Symfony\Component\Security\Core\Util\SecureRandom` class in favor of the `random_bytes()` function
  * deprecated `supportsAttribute()` and `supportsClass()` methods of
    `Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface` and
    `Symfony\Component\Security\Core\Authorization\Voter\VoterInterface`.

diff --git a/src/Symfony/Component/Security/Core/Tests/Util/SecureRandomTest.php b/src/Symfony/Component/Security/Core/Tests/Util/SecureRandomTest.php
@@ -13,26 +13,27 @@
 
 use Symfony\Component\Security\Core\Util\SecureRandom;
 
+/**
+ * @group legacy
+ */
 class SecureRandomTest extends \PHPUnit_Framework_TestCase
 {
     /**
      * T1: Monobit test.
-     *
-     * @dataProvider getSecureRandoms
      */
-    public function testMonobit($secureRandom)
+    public function testMonobit()
     {
+        $secureRandom = new SecureRandom();
         $nbOnBits = substr_count($this->getBitSequence($secureRandom, 20000), '1');
         $this->assertTrue($nbOnBits > 9654 && $nbOnBits < 10346, 'Monobit test failed, number of turned on bits: '.$nbOnBits);
     }
 
     /**
      * T2: Chi-square test with 15 degrees of freedom (chi-Quadrat-Anpassungstest).
-     *
-     * @dataProvider getSecureRandoms
      */
-    public function testPoker($secureRandom)
+    public function testPoker()
     {
+        $secureRandom = new SecureRandom();
         $b = $this->getBitSequence($secureRandom, 20000);
         $c = array();
         for ($i = 0; $i <= 15; ++$i) {
@@ -56,11 +57,10 @@ public function testPoker($secureRandom)
 
     /**
      * Run test.
-     *
-     * @dataProvider getSecureRandoms
      */
-    public function testRun($secureRandom)
+    public function testRun()
     {
+        $secureRandom = new SecureRandom();
         $b = $this->getBitSequence($secureRandom, 20000);
 
         $runs = array();
@@ -104,11 +104,10 @@ public function testRun($secureRandom)
 
     /**
      * Long-run test.
-     *
-     * @dataProvider getSecureRandoms
      */
-    public function testLongRun($secureRandom)
+    public function testLongRun()
     {
+        $secureRandom = new SecureRandom();
         $b = $this->getBitSequence($secureRandom, 20000);
 
         $longestRun = $currentRun = 0;
@@ -133,11 +132,10 @@ public function testLongRun($secureRandom)
 
     /**
      * Serial Correlation (Autokorrelationstest).
-     *
-     * @dataProvider getSecureRandoms
      */
-    public function testSerialCorrelation($secureRandom)
+    public function testSerialCorrelation()
     {
+        $secureRandom = new SecureRandom();
         $shift = mt_rand(1, 5000);
         $b = $this->getBitSequence($secureRandom, 20000);
 
@@ -149,44 +147,6 @@ public function testSerialCorrelation($secureRandom)
         $this->assertTrue($Z > 2326 && $Z < 2674, 'Failed serial correlation test: '.$Z);
     }
 
-    public function getSecureRandoms()
-    {
-        $secureRandoms = array();
-
-        // only add if openssl is indeed present
-        $secureRandom = new SecureRandom();
-        if ($this->hasOpenSsl($secureRandom)) {
-            $secureRandoms[] = array($secureRandom);
-        }
-
-        // no-openssl with custom seed provider
-        $secureRandom = new SecureRandom(sys_get_temp_dir().'/_sf2.seed');
-        $this->disableOpenSsl($secureRandom);
-        $secureRandoms[] = array($secureRandom);
-
-        return $secureRandoms;
-    }
-
-    protected function disableOpenSsl($secureRandom)
-    {
-        $ref = new \ReflectionProperty($secureRandom, 'useOpenSsl');
-        $ref->setAccessible(true);
-        $ref->setValue($secureRandom, false);
-        $ref->setAccessible(false);
-    }
-
-    protected function hasOpenSsl($secureRandom)
-    {
-        $ref = new \ReflectionProperty($secureRandom, 'useOpenSsl');
-        $ref->setAccessible(true);
-
-        $ret = $ref->getValue($secureRandom);
-
-        $ref->setAccessible(false);
-
-        return $ret;
-    }
-
     private function getBitSequence($secureRandom, $length)
     {
         $bitSequence = '';

diff --git a/src/Symfony/Component/Security/Core/Util/SecureRandom.php b/src/Symfony/Component/Security/Core/Util/SecureRandom.php
@@ -11,106 +11,23 @@
 
 namespace Symfony\Component\Security\Core\Util;
 
-use Psr\Log\LoggerInterface;
+@trigger_error('The '.__NAMESPACE__.'\SecureRandom class is deprecated since version 2.8 and will be removed in 3.0. Use the random_bytes() function instead.', E_USER_DEPRECATED);
 
 /**
  * A secure random number generator implementation.
  *
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
+ *
+ * @deprecated since version 2.8, to be removed in 3.0. Use the random_bytes function instead
  */
 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 (!function_exists('random_bytes') && !function_exists('openssl_random_pseudo_bytes')) {
-            if (null !== $this->logger) {
-                $this->logger->notice('It is recommended that you install the "paragonie/random_compat" library or enable the "openssl" extension for random number generation.');
-            }
-            $this->useOpenSsl = false;
-        } else {
-            $this->useOpenSsl = true;
-        }
-    }
-
     /**
      * {@inheritdoc}
      */
     public function nextBytes($nbBytes)
     {
-        if (function_exists('random_bytes')) {
-            return random_bytes($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;
+        return random_bytes($nbBytes);
     }
 }
diff --git a/src/Symfony/Component/Security/Core/Util/SecureRandomInterface.php b/src/Symfony/Component/Security/Core/Util/SecureRandomInterface.php
@@ -15,6 +15,8 @@
  * Interface that needs to be implemented by all secure random number generators.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ *
+ * @deprecated since version 2.8, to be removed in 3.0. Use the random_bytes function instead
  */
 interface SecureRandomInterface
 {

diff --git a/src/Symfony/Component/Security/Core/composer.json b/src/Symfony/Component/Security/Core/composer.json
@@ -16,7 +16,8 @@
         }
     ],
     "require": {
-        "php": ">=5.3.9"
+        "php": ">=5.3.9",
+        "paragonie/random_compat" : "~1.0"
     },
     "require-dev": {
         "symfony/phpunit-bridge": "~2.7|~3.0.0",

diff --git a/src/Symfony/Component/Security/Csrf/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php b/src/Symfony/Component/Security/Csrf/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php
@@ -44,8 +44,7 @@ public static function setUpBeforeClass()
 
     protected function setUp()
     {
-        $this->random = $this->getMock('Symfony\Component\Security\Core\Util\SecureRandomInterface');
-        $this->generator = new UriSafeTokenGenerator($this->random, self::ENTROPY);
+        $this->generator = new UriSafeTokenGenerator(self::ENTROPY);
     }
 
     protected function tearDown()
@@ -56,11 +55,6 @@ protected function tearDown()
 
     public function testGenerateToken()
     {
-        $this->random->expects($this->once())
-            ->method('nextBytes')
-            ->with(self::ENTROPY / 8)
-            ->will($this->returnValue(self::$bytes));
-
         $token = $this->generator->generateToken();
 
         $this->assertTrue(ctype_print($token), 'is printable');

diff --git a/src/Symfony/Component/Security/Csrf/TokenGenerator/UriSafeTokenGenerator.php b/src/Symfony/Component/Security/Csrf/TokenGenerator/UriSafeTokenGenerator.php
@@ -12,7 +12,6 @@
 namespace Symfony\Component\Security\Csrf\TokenGenerator;
 
 use Symfony\Component\Security\Core\Util\SecureRandomInterface;
-use Symfony\Component\Security\Core\Util\SecureRandom;
 
 /**
  * Generates CSRF tokens.
@@ -23,13 +22,6 @@
  */
 class UriSafeTokenGenerator implements TokenGeneratorInterface
 {
-    /**
-     * The generator for random values.
-     *
-     * @var SecureRandomInterface
-     */
-    private $random;
-
     /**
      * The amount of entropy collected for each token (in bits).
      *
@@ -40,15 +32,17 @@ class UriSafeTokenGenerator implements TokenGeneratorInterface
     /**
      * Generates URI-safe CSRF tokens.
      *
-     * @param SecureRandomInterface|null $random  The random value generator used for
-     *                                            generating entropy
-     * @param int                        $entropy The amount of entropy collected for
-     *                                            each token (in bits)
+     * @param int $entropy The amount of entropy collected for each token (in bits)
      */
-    public function __construct(SecureRandomInterface $random = null, $entropy = 256)
+    public function __construct($entropy = 256)
     {
-        $this->random = $random ?: new SecureRandom();
-        $this->entropy = $entropy;
+        if ($entropy instanceof SecureRandomInterface || func_num_args() === 2) {
+            @trigger_error('The '.__METHOD__.' method now requires the entropy to be given as the first argument. The SecureRandomInterface will be removed in 3.0.', E_USER_DEPRECATED);
+
+            $this->entropy = func_num_args() === 2 ? func_get_arg(1) : 256;
+        } else {
+            $this->entropy = $entropy;
+        }
     }
 
     /**
@@ -59,7 +53,7 @@ public function generateToken()
         // Generate an URI safe base64 encoded string that does not contain "+",
         // "/" or "=" which need to be URL encoded and make URLs unnecessarily
         // longer.
-        $bytes = $this->random->nextBytes($this->entropy / 8);
+        $bytes = random_bytes($this->entropy / 8);
 
         return rtrim(strtr(base64_encode($bytes), '+/', '-_'), '=');
     }

diff --git a/src/Symfony/Component/Security/Csrf/composer.json b/src/Symfony/Component/Security/Csrf/composer.json
@@ -17,7 +17,8 @@
     ],
     "require": {
         "php": ">=5.3.9",
-        "symfony/security-core": "~2.4|~3.0.0"
+        "symfony/security-core": "~2.4|~3.0.0",
+        "paragonie/random_compat" : "~1.0"
     },
     "require-dev": {
         "symfony/phpunit-bridge": "~2.7|~3.0.0",