From e7a888ad11f8eabd505e3b8c9b6cf41c78b39390 Mon Sep 17 00:00:00 2001 From: Tom Worster Date: Sun, 20 Dec 2015 11:48:36 -0500 Subject: [PATCH] use file_get_contents and not magic numbers --- framework/base/Security.php | 70 +++++++++------------------ tests/framework/base/SecurityTest.php | 22 +++++++++ 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/framework/base/Security.php b/framework/base/Security.php index bffe138d94..b7169bf1cf 100644 --- a/framework/base/Security.php +++ b/framework/base/Security.php @@ -30,6 +30,11 @@ use Yii; */ class Security extends Component { + const DEV_URANDOM = '/dev/urandom'; + const SOURCE_MCRYPT = 'mcrypt'; + const SOURCE_URANDOM = 'urandom'; + const SOURCE_OPEN_SSL = 'OpenSSL'; + const SOURCE_LIBRE_SSL = 'LibreSSL'; /** * @var string The cipher to use for encryption and decryption. */ @@ -455,7 +460,7 @@ class Security extends Component // The recent LibreSSL RNGs are faster and better than /dev/urandom. // Parse OPENSSL_VERSION_TEXT because OPENSSL_VERSION_NUMBER is no use for LibreSSL. // https://bugs.php.net/bug.php?id=71143 - if ($this->_randomSource === 'LibreSSL' + if ($this->_randomSource === self::SOURCE_LIBRE_SSL || ($this->_randomSource === null && defined('OPENSSL_VERSION_TEXT') && preg_match('{^LibreSSL (\d\d?)\.(\d\d?)\.(\d\d?)$}', OPENSSL_VERSION_TEXT, $matches) @@ -468,7 +473,7 @@ class Security extends Component ); } if ($key !== false && StringHelper::byteLength($key) === $length) { - $this->_randomSource = 'LibreSSL'; + $this->_randomSource = self::SOURCE_LIBRE_SSL; return $key; } @@ -478,14 +483,14 @@ class Security extends Component // mcrypt_create_iv() does not use libmcrypt. Since PHP 5.3.7 it directly reads // CrypGenRandom on Windows. Elsewhere it directly reads /dev/urandom. - if ($this->_randomSource === 'mcrypt' + if ($this->_randomSource === self::SOURCE_MCRYPT || ($this->_randomSource === null && PHP_VERSION_ID >= 50307 && function_exists('mcrypt_create_iv')) ) { $key = mcrypt_create_iv($length, MCRYPT_DEV_URANDOM); if (StringHelper::byteLength($key) === $length) { - $this->_randomSource = 'mcrypt'; + $this->_randomSource = self::SOURCE_MCRYPT; return $key; } @@ -493,48 +498,21 @@ class Security extends Component $this->_randomSource = null; } - // If not on Windows, try reading from /dev/urandom device. If successful, cache - // the file pointer in $this->_randomSource. - if (is_resource($this->_randomSource) - || ($this->_randomSource === null - && DIRECTORY_SEPARATOR === '/' - && @is_readable('/dev/urandom')) - ) { - // Either open /dev/urandom or get the cached file pointer. - if (is_resource($this->_randomSource)) { - $urandomFile = $this->_randomSource; - } else { - $urandomFile = fopen('/dev/urandom', 'rb'); - if ($urandomFile) { - // Check the file's inode protection mode is 'character special'. - // NOTE: octal integer literals! - $fstat = fstat($urandomFile); - if (($fstat['mode'] & 0170000) !== 020000) { - fclose($urandomFile); - $urandomFile = null; - } - } else { - $urandomFile = null; - } - } + // If not on Windows, test for a /dev/urandom device. + if ($this->_randomSource === null && DIRECTORY_SEPARATOR === '/') { + // Check it for speacial character device protection mode. + $lstat = @lstat(self::DEV_URANDOM); + $urandomDevice = $lstat !== false && ($lstat['mode'] & 0170000) === 020000; + } else { + $urandomDevice = false; + } + if ($this->_randomSource === self::SOURCE_URANDOM || $urandomDevice) { + $key = @file_get_contents(self::DEV_URANDOM, false, null, 0, $length); - if ($urandomFile !== null) { - // $length could be large so read using a loop. - $key = ''; - do { - $buffer = fread($urandomFile, $length); - if (!$buffer) { - $key = null; - break; - } - $key .= $buffer; - } while (StringHelper::byteLength($key) < $length); + if ($key !== false && StringHelper::byteLength($key) === $length) { + $this->_randomSource = self::SOURCE_URANDOM; - if ($key !== null && StringHelper::byteLength($key) === $length) { - $this->_randomSource = $urandomFile; - - return $key; - } + return $key; } $this->_randomSource = null; @@ -542,7 +520,7 @@ class Security extends Component // Since 5.4.0, openssl_random_pseudo_bytes() reads from CryptGenRandom on Windows instead // of using OpenSSL library. Don't use OpenSSL on other platforms. - if ($this->_randomSource === 'OpenSSL' + if ($this->_randomSource === self::SOURCE_OPEN_SSL || (DIRECTORY_SEPARATOR !== '/' && PHP_VERSION_ID >= 50400) ) { $key = openssl_random_pseudo_bytes($length, $cryptoStrong); @@ -552,7 +530,7 @@ class Security extends Component ); } if ($key !== false && StringHelper::byteLength($key) === $length) { - $this->_randomSource = 'OpenSSL'; + $this->_randomSource = self::SOURCE_OPEN_SSL; return $key; } diff --git a/tests/framework/base/SecurityTest.php b/tests/framework/base/SecurityTest.php index 13fa64f899..006d3b76c7 100644 --- a/tests/framework/base/SecurityTest.php +++ b/tests/framework/base/SecurityTest.php @@ -810,6 +810,28 @@ TEXT; $this->assertTrue($key1 != $key2); } + public function testGenerateRandomKeyURandom() + { + if (function_exists('random_bytes')) { + $this->markTestSkipped('This test can only work on platforms where random_bytes() function does not exist. You may disable it in php.ini for testing. http://php.net/manual/en/ini.core.php#ini.disable-functions'); + } + if (PHP_VERSION_ID >= 50307 && function_exists('mcrypt_create_iv')) { + $this->markTestSkipped('This test can only work on platforms where mcrypt_create_iv() function does not exist. You may disable it in php.ini for testing. http://php.net/manual/en/ini.core.php#ini.disable-functions'); + } + if (!@is_readable('/dev/urandom')) { + $this->markTestSkipped('/dev/urandom does not seem to exist on your system.'); + } + + $length = 1024 * 1024; + $key1 = $this->security->generateRandomKey($length); + $this->assertInternalType('string', $key1); + $this->assertEquals($length, strlen($key1)); + $key2 = $this->security->generateRandomKey($length); + $this->assertInternalType('string', $key2); + $this->assertEquals($length, strlen($key2)); + $this->assertTrue($key1 != $key2); + } + public function testGenerateRandomString() { $length = 21;