From 6e10bdf3611723dd2f2021ab766c6eb0d097b879 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 5 Nov 2018 09:37:03 +0100 Subject: [PATCH 1/5] Move random Digits to Crypto class --- boot.php | 10 --- include/conversation.php | 3 +- mod/editpost.php | 3 +- mod/photos.php | 7 +- src/Core/System.php | 1 - src/Object/Post.php | 3 +- src/Util/Crypto.php | 26 +++++++ tests/src/Util/CryptoTest.php | 131 ++++++++++++++++++++++++++++++++++ 8 files changed, 167 insertions(+), 17 deletions(-) create mode 100644 tests/src/Util/CryptoTest.php diff --git a/boot.php b/boot.php index a94b380387..6373b2c64b 100644 --- a/boot.php +++ b/boot.php @@ -673,16 +673,6 @@ function curPageURL() return $pageURL; } -function random_digits($digits) -{ - $rn = ''; - for ($i = 0; $i < $digits; $i++) { - /// @TODO Avoid rand/mt_rand, when it comes to cryptography, they are generating predictable (seedable) numbers. - $rn .= rand(0, 9); - } - return $rn; -} - function get_server() { $server = Config::get("system", "directory"); diff --git a/include/conversation.php b/include/conversation.php index c10a7bec73..cd635521e5 100644 --- a/include/conversation.php +++ b/include/conversation.php @@ -27,6 +27,7 @@ use Friendica\Util\DateTimeFormat; use Friendica\Util\Proxy as ProxyUtils; use Friendica\Util\Temporal; use Friendica\Util\XML; +use Friendica\Util\Crypto; function item_extract_images($body) { @@ -1166,7 +1167,7 @@ function status_editor(App $a, $x, $notes_cid = 0, $popup = false) '$notes_cid' => $notes_cid, '$sourceapp' => L10n::t($a->sourcename), '$cancel' => L10n::t('Cancel'), - '$rand_num' => random_digits(12), + '$rand_num' => Crypto::randomDigits(12), // ACL permissions box '$acl' => $x['acl'], diff --git a/mod/editpost.php b/mod/editpost.php index 0023e35edd..b518588a59 100644 --- a/mod/editpost.php +++ b/mod/editpost.php @@ -12,6 +12,7 @@ use Friendica\Core\System; use Friendica\Model\FileTag; use Friendica\Model\Item; use Friendica\Database\DBA; +use Friendica\Util\Crypto; function editpost_content(App $a) { @@ -131,7 +132,7 @@ function editpost_content(App $a) '$jotplugins' => $jotplugins, '$sourceapp' => L10n::t($a->sourcename), '$cancel' => L10n::t('Cancel'), - '$rand_num' => random_digits(12), + '$rand_num' => Crypto::randomDigits(12), //jot nav tab (used in some themes) '$message' => L10n::t('Message'), diff --git a/mod/photos.php b/mod/photos.php index 69b1972d4c..7a49f061a7 100644 --- a/mod/photos.php +++ b/mod/photos.php @@ -26,6 +26,7 @@ use Friendica\Model\User; use Friendica\Network\Probe; use Friendica\Object\Image; use Friendica\Protocol\DFRN; +use Friendica\Util\Crypto; use Friendica\Util\DateTimeFormat; use Friendica\Util\Map; use Friendica\Util\Security; @@ -1500,7 +1501,7 @@ function photos_content(App $a) '$preview' => L10n::t('Preview'), '$sourceapp' => L10n::t($a->sourcename), '$ww' => '', - '$rand_num' => random_digits(12) + '$rand_num' => Crypto::randomDigits(12) ]); } } @@ -1539,7 +1540,7 @@ function photos_content(App $a) '$preview' => L10n::t('Preview'), '$sourceapp' => L10n::t($a->sourcename), '$ww' => '', - '$rand_num' => random_digits(12) + '$rand_num' => Crypto::randomDigits(12) ]); } @@ -1599,7 +1600,7 @@ function photos_content(App $a) '$preview' => L10n::t('Preview'), '$sourceapp' => L10n::t($a->sourcename), '$ww' => '', - '$rand_num' => random_digits(12) + '$rand_num' => Crypto::randomDigits(12) ]); } } diff --git a/src/Core/System.php b/src/Core/System.php index 0f9a611ab9..d24581e996 100644 --- a/src/Core/System.php +++ b/src/Core/System.php @@ -265,7 +265,6 @@ class System extends BaseObject function notice($s) function info($s) function is_site_admin() - function random_digits($digits) function get_server() function get_temppath() function get_cachefile($file, $writemode = true) diff --git a/src/Object/Post.php b/src/Object/Post.php index e2c1ea03c3..644d53e25d 100644 --- a/src/Object/Post.php +++ b/src/Object/Post.php @@ -18,6 +18,7 @@ use Friendica\Database\DBA; use Friendica\Model\Contact; use Friendica\Model\Item; use Friendica\Model\Term; +use Friendica\Util\Crypto; use Friendica\Util\DateTimeFormat; use Friendica\Util\Proxy as ProxyUtils; use Friendica\Util\Temporal; @@ -815,7 +816,7 @@ class Post extends BaseObject '$indent' => $indent, '$sourceapp' => L10n::t($a->sourcename), '$ww' => $conv->getMode() === 'network' ? $ww : '', - '$rand_num' => random_digits(12) + '$rand_num' => Crypto::randomDigits(12) ]); } diff --git a/src/Util/Crypto.php b/src/Util/Crypto.php index 2b1ce0f02b..785860c182 100644 --- a/src/Util/Crypto.php +++ b/src/Util/Crypto.php @@ -476,4 +476,30 @@ class Crypto return self::decryptAES256CBC(base64url_decode($data['data']), $k, $i); } + + + /** + * Creates cryptographic secure random digits + * + * @param string $digits The count of digits + * @return int The random Digits + */ + public static function randomDigits($digits) + { + $rn = ''; + + if (!function_exists('random_int')) { + // using rand() function for PHP 5.x compatibility + for ($i = 0; $i < $digits; $i++) { + $rn .= rand(0, 9); + } + } else { + // generating cryptographically secure pseudo-random integers + for ($i = 0; $i < $digits; $i++) { + $rn .= random_int(0, 9); + } + } + + return $rn; + } } diff --git a/tests/src/Util/CryptoTest.php b/tests/src/Util/CryptoTest.php new file mode 100644 index 0000000000..d014d28182 --- /dev/null +++ b/tests/src/Util/CryptoTest.php @@ -0,0 +1,131 @@ + $value) { + if ($function == $name) { + return $value; + } + } + return '__phpunit_continue__'; + }; + } + + /** + * Replaces rand results with given mocks + * + */ + private function assertRand($min, $max) + { + global $phpMock; + $phpMock['rand'] = function($mMin, $mMax) use ($min, $max) { + $this->assertEquals($min, $mMin); + $this->assertEquals($max, $mMax); + return 1; + }; + } + + /** + * Replaces rand results with given mocks + * + */ + private function assertRandomInt($min, $max) + { + global $phpMock; + $phpMock['random_int'] = function($mMin, $mMax) use ($min, $max) { + $this->assertEquals($min, $mMin); + $this->assertEquals($max, $mMax); + return 1; + }; + } + + public function testRandomDigitsRand() + { + $this->setFunctions(['random_int' => false]); + $this->assertRand(0, 9); + + $test = Crypto::randomDigits(1); + $this->assertEquals(1, strlen($test)); + $this->assertEquals(1, $test); + + $test = Crypto::randomDigits(8); + $this->assertEquals(8, strlen($test)); + $this->assertEquals(11111111, $test); + } + + + public function testRandomDigitsRandomInt() + { + $this->setFunctions(['random_int' => true]); + $this->assertRandomInt(0, 9); + + $test = Crypto::randomDigits(1); + $this->assertEquals(1, strlen($test)); + $this->assertEquals(1, $test); + + $test = Crypto::randomDigits(8); + $this->assertEquals(8, strlen($test)); + $this->assertEquals(11111111, $test); + } +} + +/** + * A workaround to replace the PHP native function_exists() with a mocked function + * + * @param string $function_name the Name of the function + * + * @return bool true or false + */ +function function_exists($function_name) +{ + global $phpMock; + if (isset($phpMock['function_exists'])) { + $result = call_user_func_array($phpMock['function_exists'], func_get_args()); + if ($result !== '__phpunit_continue__') { + return $result; + } + } + return call_user_func_array('\function_exists', func_get_args()); +} + +/** + * A workaround to replace the PHP native rand() (< 7.0) with a mocked function + * + * @return int + */ +function rand($min, $max) +{ + global $phpMock; + if (isset($phpMock['rand'])) { + $result = call_user_func_array($phpMock['rand'], func_get_args()); + return $result; + } +} + +/** + * A workaround to replace the PHP native random_int() (>= 7.0) with a mocked function + * + * @return int + */ +function random_int($min, $max) +{ + global $phpMock; + if (isset($phpMock['random_int'])) { + $result = call_user_func_array($phpMock['random_int'], func_get_args()); + return $result; + } +} From c94936a1496b4ff11e05720888c0961fa2c35575 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 5 Nov 2018 21:03:55 +0100 Subject: [PATCH 2/5] copy/paste error --- tests/src/Util/CryptoTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Util/CryptoTest.php b/tests/src/Util/CryptoTest.php index d014d28182..7c6af0070c 100644 --- a/tests/src/Util/CryptoTest.php +++ b/tests/src/Util/CryptoTest.php @@ -1,6 +1,6 @@ Date: Mon, 5 Nov 2018 21:15:30 +0100 Subject: [PATCH 3/5] Using random_int directly --- src/Util/Crypto.php | 15 +++----- tests/src/Util/CryptoTest.php | 67 ----------------------------------- 2 files changed, 5 insertions(+), 77 deletions(-) diff --git a/src/Util/Crypto.php b/src/Util/Crypto.php index 785860c182..7dd0dee5c0 100644 --- a/src/Util/Crypto.php +++ b/src/Util/Crypto.php @@ -483,21 +483,16 @@ class Crypto * * @param string $digits The count of digits * @return int The random Digits + * + * @throws \Exception In case 'random_int' isn't usable */ public static function randomDigits($digits) { $rn = ''; - if (!function_exists('random_int')) { - // using rand() function for PHP 5.x compatibility - for ($i = 0; $i < $digits; $i++) { - $rn .= rand(0, 9); - } - } else { - // generating cryptographically secure pseudo-random integers - for ($i = 0; $i < $digits; $i++) { - $rn .= random_int(0, 9); - } + // generating cryptographically secure pseudo-random integers + for ($i = 0; $i < $digits; $i++) { + $rn .= random_int(0, 9); } return $rn; diff --git a/tests/src/Util/CryptoTest.php b/tests/src/Util/CryptoTest.php index 7c6af0070c..b976a09838 100644 --- a/tests/src/Util/CryptoTest.php +++ b/tests/src/Util/CryptoTest.php @@ -7,24 +7,6 @@ use PHPUnit\Framework\TestCase; class CryptoTest extends TestCase { - /** - * Replaces function_exists results with given mocks - * - * @param array $functions a list from function names and their result - */ - private function setFunctions($functions) - { - global $phpMock; - $phpMock['function_exists'] = function($function) use ($functions) { - foreach ($functions as $name => $value) { - if ($function == $name) { - return $value; - } - } - return '__phpunit_continue__'; - }; - } - /** * Replaces rand results with given mocks * @@ -53,24 +35,8 @@ class CryptoTest extends TestCase }; } - public function testRandomDigitsRand() - { - $this->setFunctions(['random_int' => false]); - $this->assertRand(0, 9); - - $test = Crypto::randomDigits(1); - $this->assertEquals(1, strlen($test)); - $this->assertEquals(1, $test); - - $test = Crypto::randomDigits(8); - $this->assertEquals(8, strlen($test)); - $this->assertEquals(11111111, $test); - } - - public function testRandomDigitsRandomInt() { - $this->setFunctions(['random_int' => true]); $this->assertRandomInt(0, 9); $test = Crypto::randomDigits(1); @@ -83,39 +49,6 @@ class CryptoTest extends TestCase } } -/** - * A workaround to replace the PHP native function_exists() with a mocked function - * - * @param string $function_name the Name of the function - * - * @return bool true or false - */ -function function_exists($function_name) -{ - global $phpMock; - if (isset($phpMock['function_exists'])) { - $result = call_user_func_array($phpMock['function_exists'], func_get_args()); - if ($result !== '__phpunit_continue__') { - return $result; - } - } - return call_user_func_array('\function_exists', func_get_args()); -} - -/** - * A workaround to replace the PHP native rand() (< 7.0) with a mocked function - * - * @return int - */ -function rand($min, $max) -{ - global $phpMock; - if (isset($phpMock['rand'])) { - $result = call_user_func_array($phpMock['rand'], func_get_args()); - return $result; - } -} - /** * A workaround to replace the PHP native random_int() (>= 7.0) with a mocked function * From 47095d70a9a8378cc14ebd515bcb2477e2710153 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 5 Nov 2018 21:16:18 +0100 Subject: [PATCH 4/5] Using random_int directly --- tests/src/Util/CryptoTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Util/CryptoTest.php b/tests/src/Util/CryptoTest.php index b976a09838..f3047effa1 100644 --- a/tests/src/Util/CryptoTest.php +++ b/tests/src/Util/CryptoTest.php @@ -22,7 +22,7 @@ class CryptoTest extends TestCase } /** - * Replaces rand results with given mocks + * Replaces random_int results with given mocks * */ private function assertRandomInt($min, $max) From 6949841947483b2b22bd8760c9ab30e70ee32294 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Mon, 5 Nov 2018 21:17:46 +0100 Subject: [PATCH 5/5] Removed unnecessary function --- tests/src/Util/CryptoTest.php | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/src/Util/CryptoTest.php b/tests/src/Util/CryptoTest.php index f3047effa1..76e3076f0f 100644 --- a/tests/src/Util/CryptoTest.php +++ b/tests/src/Util/CryptoTest.php @@ -7,20 +7,6 @@ use PHPUnit\Framework\TestCase; class CryptoTest extends TestCase { - /** - * Replaces rand results with given mocks - * - */ - private function assertRand($min, $max) - { - global $phpMock; - $phpMock['rand'] = function($mMin, $mMax) use ($min, $max) { - $this->assertEquals($min, $mMin); - $this->assertEquals($max, $mMax); - return 1; - }; - } - /** * Replaces random_int results with given mocks *