Clean up CSPRNG support for PHP7
authorMax Semenik <maxsem.wiki@gmail.com>
Fri, 19 Jan 2018 22:42:56 +0000 (14:42 -0800)
committerKrinkle <krinklemail@gmail.com>
Sun, 10 Jun 2018 00:52:04 +0000 (00:52 +0000)
Replace it all with random_bytes(), leave
only MWCryptRand::generateHex() as a convenience helper.

Change-Id: Ic30376a90e66d8f00dab86e7e6466fb3a750b87d

15 files changed:
RELEASE-NOTES-1.32
includes/MediaWikiServices.php
includes/OutputPage.php
includes/ServiceWiring.php
includes/installer/Installer.php
includes/installer/i18n/en.json
includes/installer/i18n/qqq.json
includes/libs/CryptHKDF.php
includes/libs/CryptRand.php
includes/password/BcryptPassword.php
includes/password/EncryptedPassword.php
includes/password/Pbkdf2Password.php
includes/session/Session.php
includes/utils/MWCryptRand.php
tests/phpunit/includes/session/SessionTest.php

index 4e2bdfa..cdc2582 100644 (file)
@@ -186,6 +186,9 @@ because of Phabricator reports.
 * The ApiQueryContributions class has been renamed to ApiQueryUserContribs.
 * The XMPInfo, XMPReader, and XMPValidate classes have been deprecated in favor
   of the namespaced classes provided by the wikimedia/xmp-reader library.
+* Class CryptRand, everything in MWCryptRand except generateHex() and function
+  MediaWikiServices::getCryptRand() are deprecated, use random_bytes() to
+  generate cryptographically secure random byte sequences.
 
 === Other changes in 1.32 ===
 * …
index ac98683..dbb18a7 100644 (file)
@@ -539,6 +539,7 @@ class MediaWikiServices extends ServiceContainer {
 
        /**
         * @since 1.28
+        * @deprecated since 1.32, use random_bytes()/random_int()
         * @return CryptRand
         */
        public function getCryptRand() {
index f92f4f3..6700cdb 100644 (file)
@@ -4018,8 +4018,8 @@ class OutputPage extends ContextSource {
                }
                if ( $this->CSPNonce === null ) {
                        // XXX It might be expensive to generate randomness
-                       // on every request, on windows.
-                       $rand = MWCryptRand::generate( 15 );
+                       // on every request, on Windows.
+                       $rand = random_bytes( 15 );
                        $this->CSPNonce = base64_encode( $rand );
                }
                return $this->CSPNonce;
index ace64ab..3f8ba18 100644 (file)
@@ -188,29 +188,8 @@ return [
                );
        },
 
-       'CryptRand' => function ( MediaWikiServices $services ) {
-               $secretKey = $services->getMainConfig()->get( 'SecretKey' );
-               return new CryptRand(
-                       [
-                               // To try vary the system information of the state a bit more
-                               // by including the system's hostname into the state
-                               'wfHostname',
-                               // It's mostly worthless but throw the wiki's id into the data
-                               // for a little more variance
-                               'wfWikiID',
-                               // If we have a secret key set then throw it into the state as well
-                               function () use ( $secretKey ) {
-                                       return $secretKey ?: '';
-                               }
-                       ],
-                       // The config file is likely the most often edited file we know should
-                       // be around so include its stat info into the state.
-                       // The constant with its location will almost always be defined, as
-                       // WebStart.php defines MW_CONFIG_FILE to $IP/LocalSettings.php unless
-                       // being configured with MW_CONFIG_CALLBACK (e.g. the installer).
-                       defined( 'MW_CONFIG_FILE' ) ? [ MW_CONFIG_FILE ] : [],
-                       LoggerFactory::getInstance( 'CryptRand' )
-               );
+       'CryptRand' => function () {
+               return new CryptRand();
        },
 
        'CryptHKDF' => function ( MediaWikiServices $services ) {
@@ -231,9 +210,7 @@ return [
                        $cache = ObjectCache::getLocalClusterInstance();
                }
 
-               return new CryptHKDF( $secret, $config->get( 'HKDFAlgorithm' ),
-                       $cache, $context, $services->getCryptRand()
-               );
+               return new CryptHKDF( $secret, $config->get( 'HKDFAlgorithm' ), $cache, $context );
        },
 
        'MediaHandlerFactory' => function ( MediaWikiServices $services ) {
index 05da0db..0890bc4 100644 (file)
@@ -1597,23 +1597,11 @@ abstract class Installer {
        protected function doGenerateKeys( $keys ) {
                $status = Status::newGood();
 
-               $strong = true;
                foreach ( $keys as $name => $length ) {
-                       $secretKey = MWCryptRand::generateHex( $length, true );
-                       if ( !MWCryptRand::wasStrong() ) {
-                               $strong = false;
-                       }
-
+                       $secretKey = MWCryptRand::generateHex( $length );
                        $this->setVar( $name, $secretKey );
                }
 
-               if ( !$strong ) {
-                       $names = array_keys( $keys );
-                       $names = preg_replace( '/^(.*)$/', '\$$1', $names );
-                       global $wgLang;
-                       $status->warning( 'config-insecure-keys', $wgLang->listToText( $names ), count( $names ) );
-               }
-
                return $status;
        }
 
index 005fef6..dbc1849 100644 (file)
        "config-install-interwiki-exists": "<strong>Warning:</strong> The interwiki table seems to already have entries.\nSkipping default list.",
        "config-install-stats": "Initializing statistics",
        "config-install-keys": "Generating secret keys",
-       "config-insecure-keys": "<strong>Warning:</strong> {{PLURAL:$2|A secure key|Secure keys}} ($1) generated during installation {{PLURAL:$2|is|are}} not completely safe. Consider changing {{PLURAL:$2|it|them}} manually.",
        "config-install-updates": "Prevent running unneeded updates",
        "config-install-updates-failed": "<strong>Error:</strong> Inserting update keys into tables failed with the following error: $1",
        "config-install-sysop": "Creating administrator user account",
index 0779204..d9edde5 100644 (file)
        "config-install-interwiki-exists": "Error notice during the installation saying that one of the database tables is already set up, so it's continuing without taking that step.",
        "config-install-stats": "*{{msg-mw|Config-install-database}}\n*{{msg-mw|Config-install-tables}}\n*{{msg-mw|Config-install-schema}}\n*{{msg-mw|Config-install-user}}\n*{{msg-mw|Config-install-interwiki}}\n*{{msg-mw|Config-install-stats}}\n*{{msg-mw|Config-install-keys}}\n*{{msg-mw|Config-install-sysop}}\n*{{msg-mw|Config-install-mainpage}}",
        "config-install-keys": "*{{msg-mw|Config-install-database}}\n*{{msg-mw|Config-install-tables}}\n*{{msg-mw|Config-install-schema}}\n*{{msg-mw|Config-install-user}}\n*{{msg-mw|Config-install-interwiki}}\n*{{msg-mw|Config-install-stats}}\n*{{msg-mw|Config-install-keys}}\n*{{msg-mw|Config-install-sysop}}\n*{{msg-mw|Config-install-mainpage}}",
-       "config-insecure-keys": "Parameters:\n* $1 - A list of names of the secret keys that were generated.\n* $2 - the number of items in the list $1, to be used with PLURAL.",
        "config-install-updates": "Message indicating that the updatelog table is filled with keys of updates that won't be run when running database updates.\n\nSee also:\n*{{msg-mw|Config-install-database}}\n*{{msg-mw|Config-install-tables}}\n*{{msg-mw|Config-install-interwiki}}\n*{{msg-mw|Config-install-stats}}\n*{{msg-mw|Config-install-keys}}\n*{{msg-mw|Config-install-updates}}\n*{{msg-mw|Config-install-schema}}\n*{{msg-mw|Config-install-user}}\n*{{msg-mw|Config-install-sysop}}\n*{{msg-mw|Config-install-mainpage}}",
        "config-install-updates-failed": "Used as error message. Parameters:\n* $1 - detailed error message",
        "config-install-sysop": "Message indicates that the administrator user account is being created\n\nSee also:\n*{{msg-mw|Config-install-database}}\n*{{msg-mw|Config-install-tables}}\n*{{msg-mw|Config-install-schema}}\n*{{msg-mw|Config-install-user}}\n*{{msg-mw|Config-install-interwiki}}\n*{{msg-mw|Config-install-stats}}\n*{{msg-mw|Config-install-keys}}\n*{{msg-mw|Config-install-sysop}}\n*{{msg-mw|Config-install-mainpage}}",
index c41aab3..0478a33 100644 (file)
@@ -99,22 +99,14 @@ class CryptHKDF {
                'whirlpool' => 64,
        ];
 
-       /**
-        * @var CryptRand
-        */
-       private $cryptRand;
-
        /**
         * @param string $secretKeyMaterial
         * @param string $algorithm Name of hashing algorithm
         * @param BagOStuff $cache
         * @param string|array $context Context to mix into HKDF context
-        * @param CryptRand $cryptRand
         * @throws InvalidArgumentException if secret key material is too short
         */
-       public function __construct( $secretKeyMaterial, $algorithm, BagOStuff $cache, $context,
-               CryptRand $cryptRand
-       ) {
+       public function __construct( $secretKeyMaterial, $algorithm, BagOStuff $cache, $context ) {
                if ( strlen( $secretKeyMaterial ) < 16 ) {
                        throw new InvalidArgumentException( "secret was too short." );
                }
@@ -122,7 +114,6 @@ class CryptHKDF {
                $this->algorithm = $algorithm;
                $this->cache = $cache;
                $this->context = is_array( $context ) ? $context : [ $context ];
-               $this->cryptRand = $cryptRand;
 
                // To prevent every call from hitting the same memcache server, pick
                // from a set of keys to use. mt_rand is only use to pick a random
@@ -150,12 +141,12 @@ class CryptHKDF {
                        $lastSalt = $this->cache->get( $this->cacheKey );
                        if ( $lastSalt === false ) {
                                // If we don't have a previous value to use as our salt, we use
-                               // 16 bytes from CryptRand, which will use a small amount of
+                               // 16 bytes from random_bytes(), which will use a small amount of
                                // entropy from our pool. Note, "XTR may be deterministic or keyed
                                // via an optional “salt value”  (i.e., a non-secret random
                                // value)..." - http://eprint.iacr.org/2010/264.pdf. However, we
                                // use a strongly random value since we can.
-                               $lastSalt = $this->cryptRand->generate( 16 );
+                               $lastSalt = random_bytes( 16 );
                        }
                        // Get a binary string that is hashLen long
                        $this->salt = hash( $this->algorithm, $lastSalt, true );
index f7702dd..a1bbd09 100644 (file)
  * @author Daniel Friesen
  * @file
  */
-use Psr\Log\LoggerInterface;
 
+/**
+ * @deprecated since 1.32, use random_bytes()/random_int()
+ */
 class CryptRand {
        /**
-        * Minimum number of iterations we want to make in our drift calculations.
+        * @deprecated since 1.32, unused
         */
        const MIN_ITERATIONS = 1000;
 
        /**
-        * Number of milliseconds we want to spend generating each separate byte
-        * of the final generated bytes.
-        * This is used in combination with the hash length to determine the duration
-        * we should spend doing drift calculations.
+        * @deprecated since 1.32, unused
         */
        const MSEC_PER_BYTE = 0.5;
 
        /**
-        * A boolean indicating whether the previous random generation was done using
-        * cryptographically strong random number generator or not.
-        */
-       protected $strong = null;
-
-       /**
-        * List of functions to call to generate some random state
+        * Initialize an initial random state based off of whatever we can find
         *
-        * @var callable[]
-        */
-       protected $randomFuncs = [];
-
-       /**
-        * List of files to generate some random state from
+        * @deprecated since 1.32, unused and does nothing
         *
-        * @var string[]
-        */
-       protected $randomFiles = [];
-
-       /**
-        * @var LoggerInterface
-        */
-       protected $logger;
-
-       public function __construct( array $randomFuncs, array $randomFiles, LoggerInterface $logger ) {
-               $this->randomFuncs = $randomFuncs;
-               $this->randomFiles = $randomFiles;
-               $this->logger = $logger;
-       }
-
-       /**
-        * Initialize an initial random state based off of whatever we can find
         * @return string
         */
        protected function initialRandomState() {
-               // $_SERVER contains a variety of unstable user and system specific information
-               // It'll vary a little with each page, and vary even more with separate users
-               // It'll also vary slightly across different machines
-               $state = serialize( $_SERVER );
-
-               // Try to gather a little entropy from the different php rand sources
-               $state .= rand() . uniqid( mt_rand(), true );
-
-               // Include some information about the filesystem's current state in the random state
-               $files = $this->randomFiles;
-
-               // We know this file is here so grab some info about ourselves
-               $files[] = __FILE__;
-
-               // We must also have a parent folder, and with the usual file structure, a grandparent
-               $files[] = __DIR__;
-               $files[] = dirname( __DIR__ );
-
-               foreach ( $files as $file ) {
-                       Wikimedia\suppressWarnings();
-                       $stat = stat( $file );
-                       Wikimedia\restoreWarnings();
-                       if ( $stat ) {
-                               // stat() duplicates data into numeric and string keys so kill off all the numeric ones
-                               foreach ( $stat as $k => $v ) {
-                                       if ( is_numeric( $k ) ) {
-                                               unset( $k );
-                                       }
-                               }
-                               // The absolute filename itself will differ from install to install so don't leave it out
-                               $path = realpath( $file );
-                               if ( $path !== false ) {
-                                       $state .= $path;
-                               } else {
-                                       $state .= $file;
-                               }
-                               $state .= implode( '', $stat );
-                       } else {
-                               // The fact that the file isn't there is worth at least a
-                               // minuscule amount of entropy.
-                               $state .= '0';
-                       }
-               }
-
-               // Try and make this a little more unstable by including the varying process
-               // id of the php process we are running inside of if we are able to access it
-               if ( function_exists( 'getmypid' ) ) {
-                       $state .= getmypid();
-               }
-
-               // If available try to increase the instability of the data by throwing in
-               // the precise amount of memory that we happen to be using at the moment.
-               if ( function_exists( 'memory_get_usage' ) ) {
-                       $state .= memory_get_usage( true );
-               }
-
-               foreach ( $this->randomFuncs as $randomFunc ) {
-                       $state .= call_user_func( $randomFunc );
-               }
-
-               return $state;
+               return '';
        }
 
        /**
         * Randomly hash data while mixing in clock drift data for randomness
         *
+        * @deprecated since 1.32, unused and does nothing
+        *
         * @param string $data The data to randomly hash.
         * @return string The hashed bytes
         * @author Tim Starling
         */
        protected function driftHash( $data ) {
-               // Minimum number of iterations (to avoid slow operations causing the
-               // loop to gather little entropy)
-               $minIterations = self::MIN_ITERATIONS;
-               // Duration of time to spend doing calculations (in seconds)
-               $duration = ( self::MSEC_PER_BYTE / 1000 ) * MWCryptHash::hashLength();
-               // Create a buffer to use to trigger memory operations
-               $bufLength = 10000000;
-               $buffer = str_repeat( ' ', $bufLength );
-               $bufPos = 0;
-
-               // Iterate for $duration seconds or at least $minIterations number of iterations
-               $iterations = 0;
-               $startTime = microtime( true );
-               $currentTime = $startTime;
-               while ( $iterations < $minIterations || $currentTime - $startTime < $duration ) {
-                       // Trigger some memory writing to trigger some bus activity
-                       // This may create variance in the time between iterations
-                       $bufPos = ( $bufPos + 13 ) % $bufLength;
-                       $buffer[$bufPos] = ' ';
-                       // Add the drift between this iteration and the last in as entropy
-                       $nextTime = microtime( true );
-                       $delta = (int)( ( $nextTime - $currentTime ) * 1000000 );
-                       $data .= $delta;
-                       // Every 100 iterations hash the data and entropy
-                       if ( $iterations % 100 === 0 ) {
-                               $data = sha1( $data );
-                       }
-                       $currentTime = $nextTime;
-                       $iterations++;
-               }
-               $timeTaken = $currentTime - $startTime;
-               $data = MWCryptHash::hash( $data );
-
-               $this->logger->debug( "Clock drift calculation " .
-                       "(time-taken=" . ( $timeTaken * 1000 ) . "ms, " .
-                       "iterations=$iterations, " .
-                       "time-per-iteration=" . ( $timeTaken / $iterations * 1e6 ) . "us)" );
-
-               return $data;
+               return '';
        }
 
        /**
         * Return a rolling random state initially build using data from unstable sources
+        *
+        * @deprecated since 1.32, unused and does nothing
+        *
         * @return string A new weak random state
         */
        protected function randomState() {
-               static $state = null;
-               if ( is_null( $state ) ) {
-                       // Initialize the state with whatever unstable data we can find
-                       // It's important that this data is hashed right afterwards to prevent
-                       // it from being leaked into the output stream
-                       $state = MWCryptHash::hash( $this->initialRandomState() );
-               }
-               // Generate a new random state based on the initial random state or previous
-               // random state by combining it with clock drift
-               $state = $this->driftHash( $state );
-
-               return $state;
+               return '';
        }
 
        /**
@@ -211,191 +78,36 @@ class CryptRand {
         * random bytes generation in the previously run generate* call
         * was cryptographically strong.
         *
-        * @return bool Returns true if the source was strong, false if not.
+        * @deprecated since 1.32, always returns true
+        *
+        * @return bool Always true
         */
        public function wasStrong() {
-               if ( is_null( $this->strong ) ) {
-                       throw new RuntimeException( __METHOD__ . ' called before generation of random data' );
-               }
-
-               return $this->strong;
+               return true;
        }
 
        /**
-        * Generate a run of (ideally) cryptographically random data and return
+        * Generate a run of cryptographically random data and return
         * it in raw binary form.
         * You can use CryptRand::wasStrong() if you wish to know if the source used
         * was cryptographically strong.
         *
         * @param int $bytes The number of bytes of random data to generate
-        * @param bool $forceStrong Pass true if you want generate to prefer cryptographically
-        *                          strong sources of entropy even if reading from them may steal
-        *                          more entropy from the system than optimal.
         * @return string Raw binary random data
         */
-       public function generate( $bytes, $forceStrong = false ) {
+       public function generate( $bytes ) {
                $bytes = floor( $bytes );
-               static $buffer = '';
-               if ( is_null( $this->strong ) ) {
-                       // Set strength to false initially until we know what source data is coming from
-                       $this->strong = true;
-               }
-
-               if ( strlen( $buffer ) < $bytes ) {
-                       // If available make use of PHP 7's random_bytes
-                       // On Linux, getrandom syscall will be used if available.
-                       // On Windows CryptGenRandom will always be used
-                       // On other platforms, /dev/urandom will be used.
-                       // Avoids polyfills from before php 7.0
-                       // All error situations will throw Exceptions and or Errors
-                       if ( PHP_VERSION_ID >= 70000
-                               || ( defined( 'HHVM_VERSION_ID' ) && HHVM_VERSION_ID >= 31101 )
-                       ) {
-                               $rem = $bytes - strlen( $buffer );
-                               $buffer .= random_bytes( $rem );
-                       }
-                       if ( strlen( $buffer ) >= $bytes ) {
-                               $this->strong = true;
-                       }
-               }
-
-               if ( strlen( $buffer ) < $bytes && function_exists( 'mcrypt_create_iv' ) ) {
-                       // If available make use of mcrypt_create_iv URANDOM source to generate randomness
-                       // On unix-like systems this reads from /dev/urandom but does it without any buffering
-                       // and bypasses openbasedir restrictions, so it's preferable to reading directly
-                       // On Windows starting in PHP 5.3.0 Windows' native CryptGenRandom is used to generate
-                       // entropy so this is also preferable to just trying to read urandom because it may work
-                       // on Windows systems as well.
-                       $rem = $bytes - strlen( $buffer );
-                       $iv = mcrypt_create_iv( $rem, MCRYPT_DEV_URANDOM );
-                       if ( $iv === false ) {
-                               $this->logger->debug( "mcrypt_create_iv returned false." );
-                       } else {
-                               $buffer .= $iv;
-                               $this->logger->debug( "mcrypt_create_iv generated " . strlen( $iv ) .
-                                       " bytes of randomness." );
-                       }
-               }
-
-               if ( strlen( $buffer ) < $bytes && function_exists( 'openssl_random_pseudo_bytes' ) ) {
-                       $rem = $bytes - strlen( $buffer );
-                       $openssl_strong = false;
-                       $openssl_bytes = openssl_random_pseudo_bytes( $rem, $openssl_strong );
-                       if ( $openssl_bytes === false ) {
-                               $this->logger->debug( "openssl_random_pseudo_bytes returned false." );
-                       } else {
-                               $buffer .= $openssl_bytes;
-                               $this->logger->debug( "openssl_random_pseudo_bytes generated " .
-                                       strlen( $openssl_bytes ) . " bytes of " .
-                                       ( $openssl_strong ? "strong" : "weak" ) . " randomness." );
-                       }
-                       if ( strlen( $buffer ) >= $bytes ) {
-                               // openssl tells us if the random source was strong, if some of our data was generated
-                               // using it use it's say on whether the randomness is strong
-                               $this->strong = !!$openssl_strong;
-                       }
-               }
-
-               // Only read from urandom if we can control the buffer size or were passed forceStrong
-               if ( strlen( $buffer ) < $bytes &&
-                       ( function_exists( 'stream_set_read_buffer' ) || $forceStrong )
-               ) {
-                       $rem = $bytes - strlen( $buffer );
-                       if ( !function_exists( 'stream_set_read_buffer' ) && $forceStrong ) {
-                               $this->logger->debug( "Was forced to read from /dev/urandom " .
-                                       "without control over the buffer size." );
-                       }
-                       // /dev/urandom is generally considered the best possible commonly
-                       // available random source, and is available on most *nix systems.
-                       Wikimedia\suppressWarnings();
-                       $urandom = fopen( "/dev/urandom", "rb" );
-                       Wikimedia\restoreWarnings();
-
-                       // Attempt to read all our random data from urandom
-                       // php's fread always does buffered reads based on the stream's chunk_size
-                       // so in reality it will usually read more than the amount of data we're
-                       // asked for and not storing that risks depleting the system's random pool.
-                       // If stream_set_read_buffer is available set the chunk_size to the amount
-                       // of data we need. Otherwise read 8k, php's default chunk_size.
-                       if ( $urandom ) {
-                               // php's default chunk_size is 8k
-                               $chunk_size = 1024 * 8;
-                               if ( function_exists( 'stream_set_read_buffer' ) ) {
-                                       // If possible set the chunk_size to the amount of data we need
-                                       stream_set_read_buffer( $urandom, $rem );
-                                       $chunk_size = $rem;
-                               }
-                               $random_bytes = fread( $urandom, max( $chunk_size, $rem ) );
-                               $buffer .= $random_bytes;
-                               fclose( $urandom );
-                               $this->logger->debug( "/dev/urandom generated " . strlen( $random_bytes ) .
-                                       " bytes of randomness." );
-
-                               if ( strlen( $buffer ) >= $bytes ) {
-                                       // urandom is always strong, set to true if all our data was generated using it
-                                       $this->strong = true;
-                               }
-                       } else {
-                               $this->logger->debug( "/dev/urandom could not be opened." );
-                       }
-               }
-
-               // If we cannot use or generate enough data from a secure source
-               // use this loop to generate a good set of pseudo random data.
-               // This works by initializing a random state using a pile of unstable data
-               // and continually shoving it through a hash along with a variable salt.
-               // We hash the random state with more salt to avoid the state from leaking
-               // out and being used to predict the /randomness/ that follows.
-               if ( strlen( $buffer ) < $bytes ) {
-                       $this->logger->debug( __METHOD__ .
-                               ": Falling back to using a pseudo random state to generate randomness." );
-               }
-               while ( strlen( $buffer ) < $bytes ) {
-                       $buffer .= MWCryptHash::hmac( $this->randomState(), strval( mt_rand() ) );
-                       // This code is never really cryptographically strong, if we use it
-                       // at all, then set strong to false.
-                       $this->strong = false;
-               }
-
-               // Once the buffer has been filled up with enough random data to fulfill
-               // the request shift off enough data to handle the request and leave the
-               // unused portion left inside the buffer for the next request for random data
-               $generated = substr( $buffer, 0, $bytes );
-               $buffer = substr( $buffer, $bytes );
-
-               $this->logger->debug( strlen( $buffer ) .
-                       " bytes of randomness leftover in the buffer." );
-
-               return $generated;
+               return random_bytes( $bytes );
        }
 
        /**
-        * Generate a run of (ideally) cryptographically random data and return
+        * Generate a run of cryptographically random data and return
         * it in hexadecimal string format.
-        * You can use CryptRand::wasStrong() if you wish to know if the source used
-        * was cryptographically strong.
         *
         * @param int $chars The number of hex chars of random data to generate
-        * @param bool $forceStrong Pass true if you want generate to prefer cryptographically
-        *                          strong sources of entropy even if reading from them may steal
-        *                          more entropy from the system than optimal.
         * @return string Hexadecimal random data
         */
-       public function generateHex( $chars, $forceStrong = false ) {
-               // hex strings are 2x the length of raw binary so we divide the length in half
-               // odd numbers will result in a .5 that leads the generate() being 1 character
-               // short, so we use ceil() to ensure that we always have enough bytes
-               $bytes = ceil( $chars / 2 );
-               // Generate the data and then convert it to a hex string
-               $hex = bin2hex( $this->generate( $bytes, $forceStrong ) );
-
-               // A bit of paranoia here, the caller asked for a specific length of string
-               // here, and it's possible (eg when given an odd number) that we may actually
-               // have at least 1 char more than they asked for. Just in case they made this
-               // call intending to insert it into a database that does truncation we don't
-               // want to give them too much and end up with their database and their live
-               // code having two different values because part of what we gave them is truncated
-               // hence, we strip out any run of characters longer than what we were asked for.
-               return substr( $hex, 0, $chars );
+       public function generateHex( $chars ) {
+               return MWCryptRand::generateHex( $chars );
        }
 }
index f811e3f..4ba34ef 100644 (file)
@@ -65,7 +65,7 @@ class BcryptPassword extends ParameterizedPassword {
                                // Replace + with ., because bcrypt uses a non-MIME base64 format
                                strtr(
                                        // Random base64 encoded string
-                                       base64_encode( MWCryptRand::generate( 16, true ) ),
+                                       base64_encode( random_bytes( 16 ) ),
                                        '+', '.'
                                ),
                                0, 22
index 0ea3c63..d1774ba 100644 (file)
@@ -60,7 +60,7 @@ class EncryptedPassword extends ParameterizedPassword {
                if ( count( $this->args ) ) {
                        $iv = base64_decode( $this->args[0] );
                } else {
-                       $iv = MWCryptRand::generate( openssl_cipher_iv_length( $this->params['cipher'] ), true );
+                       $iv = random_bytes( openssl_cipher_iv_length( $this->params['cipher'] ) );
                }
 
                $this->hash = openssl_encrypt(
@@ -102,7 +102,7 @@ class EncryptedPassword extends ParameterizedPassword {
                $this->params = $this->getDefaultParams();
 
                // Check the key size with the new params
-               $iv = MWCryptRand::generate( openssl_cipher_iv_length( $this->params['cipher'] ), true );
+               $iv = random_bytes( openssl_cipher_iv_length( $this->params['cipher'] ) );
                $this->hash = openssl_encrypt(
                                $underlyingHash,
                                $this->params['cipher'],
index 541fd0e..6065045 100644 (file)
@@ -47,7 +47,7 @@ class Pbkdf2Password extends ParameterizedPassword {
 
        public function crypt( $password ) {
                if ( count( $this->args ) == 0 ) {
-                       $this->args[] = base64_encode( MWCryptRand::generate( 16, true ) );
+                       $this->args[] = base64_encode( random_bytes( 16 ) );
                }
 
                if ( $this->shouldUseHashExtension() ) {
index 024bf9a..e9a03f2 100644 (file)
@@ -481,7 +481,7 @@ final class Session implements \Countable, \Iterator, \ArrayAccess {
 
                // Encrypt
                // @todo: import a pure-PHP library for AES instead of doing $wgSessionInsecureSecrets
-               $iv = \MWCryptRand::generate( 16, true );
+               $iv = random_bytes( 16 );
                $algorithm = self::getEncryptionAlgorithm();
                switch ( $algorithm[0] ) {
                        case 'openssl':
index 5818958..0fc45f7 100644 (file)
@@ -28,6 +28,7 @@ use MediaWiki\MediaWikiServices;
 
 class MWCryptRand {
        /**
+        * @deprecated since 1.32
         * @return CryptRand
         */
        protected static function singleton() {
@@ -39,41 +40,49 @@ class MWCryptRand {
         * random bytes generation in the previously run generate* call
         * was cryptographically strong.
         *
-        * @return bool Returns true if the source was strong, false if not.
+        * @deprecated since 1.32, always returns true
+        *
+        * @return bool Always true
         */
        public static function wasStrong() {
-               return self::singleton()->wasStrong();
+               return true;
        }
 
        /**
-        * Generate a run of (ideally) cryptographically random data and return
+        * Generate a run of cryptographically random data and return
         * it in raw binary form.
-        * You can use MWCryptRand::wasStrong() if you wish to know if the source used
-        * was cryptographically strong.
+        *
+        * @deprecated since 1.32, use random_bytes()
         *
         * @param int $bytes The number of bytes of random data to generate
-        * @param bool $forceStrong Pass true if you want generate to prefer cryptographically
-        *                          strong sources of entropy even if reading from them may steal
-        *                          more entropy from the system than optimal.
         * @return string Raw binary random data
         */
-       public static function generate( $bytes, $forceStrong = false ) {
-               return self::singleton()->generate( $bytes, $forceStrong );
+       public static function generate( $bytes ) {
+               return random_bytes( floor( $bytes ) );
        }
 
        /**
-        * Generate a run of (ideally) cryptographically random data and return
+        * Generate a run of cryptographically random data and return
         * it in hexadecimal string format.
-        * You can use MWCryptRand::wasStrong() if you wish to know if the source used
-        * was cryptographically strong.
         *
         * @param int $chars The number of hex chars of random data to generate
-        * @param bool $forceStrong Pass true if you want generate to prefer cryptographically
-        *                          strong sources of entropy even if reading from them may steal
-        *                          more entropy from the system than optimal.
         * @return string Hexadecimal random data
         */
-       public static function generateHex( $chars, $forceStrong = false ) {
-               return self::singleton()->generateHex( $chars, $forceStrong );
+       public static function generateHex( $chars ) {
+               // hex strings are 2x the length of raw binary so we divide the length in half
+               // odd numbers will result in a .5 that leads the generate() being 1 character
+               // short, so we use ceil() to ensure that we always have enough bytes
+               $bytes = ceil( $chars / 2 );
+               // Generate the data and then convert it to a hex string
+               $hex = bin2hex( random_bytes( $bytes ) );
+
+               // A bit of paranoia here, the caller asked for a specific length of string
+               // here, and it's possible (eg when given an odd number) that we may actually
+               // have at least 1 char more than they asked for. Just in case they made this
+               // call intending to insert it into a database that does truncation we don't
+               // want to give them too much and end up with their database and their live
+               // code having two different values because part of what we gave them is truncated
+               // hence, we strip out any run of characters longer than what we were asked for.
+               return substr( $hex, 0, $chars );
        }
 }
index f84d435..a74056d 100644 (file)
@@ -358,7 +358,7 @@ class SessionTest extends MediaWikiTestCase {
                $logger->clearBuffer();
 
                // Unserializable data
-               $iv = \MWCryptRand::generate( 16, true );
+               $iv = random_bytes( 16 );
                list( $encKey, $hmacKey ) = TestingAccessWrapper::newFromObject( $session )->getSecretKeys();
                $ciphertext = openssl_encrypt( 'foobar', 'aes-256-ctr', $encKey, OPENSSL_RAW_DATA, $iv );
                $sealed = base64_encode( $iv ) . '.' . base64_encode( $ciphertext );