From 3fd957d8367849c0d23d44eca9ed6adbd773fb3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 21 Jan 2023 01:19:12 +0100 Subject: [PATCH 1/2] password: Use `php_random_bytes_throw` in `php_password_make_salt` The CSPRNG failing should be rare nowadays, but it *might* happen and without this patch it's hard for the user to find out why the salt generation failed: The error message is not actionable. This patch will automatically set the CSPRNG exception to the `$previous` exception of the ValueError that is thrown, allowing the developer to determine the cause of the salt generation failure. Before: Fatal error: Uncaught ValueError: Unable to generate salt in php-src/test3.php:3 Stack trace: #0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y') #1 {main} thrown in php-src/test3.php on line 3 After: Fatal error: Uncaught Random\RandomException: Cannot open /dev/urandom: No such file or directory in php-src/test3.php:3 Stack trace: #0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y') #1 {main} Next ValueError: Unable to generate salt in php-src/test3.php:3 Stack trace: #0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y') #1 {main} thrown in php-src/test3.php on line 3 --- ext/standard/password.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/password.c b/ext/standard/password.c index 503e72fbbf366..30e524dafbb18 100644 --- a/ext/standard/password.c +++ b/ext/standard/password.c @@ -83,7 +83,7 @@ static zend_string* php_password_make_salt(size_t length) /* {{{ */ } buffer = zend_string_alloc(length * 3 / 4 + 1, 0); - if (FAILURE == php_random_bytes_silent(ZSTR_VAL(buffer), ZSTR_LEN(buffer))) { + if (FAILURE == php_random_bytes_throw(ZSTR_VAL(buffer), ZSTR_LEN(buffer))) { zend_value_error("Unable to generate salt"); zend_string_release_ex(buffer, 0); return NULL; From e847b24dcd6b39f77cb35ae0bad619a715a8cac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 23 Jan 2023 18:32:40 +0100 Subject: [PATCH 2/2] [ci skip] NEWS/UPGRADING --- NEWS | 2 ++ UPGRADING | 2 ++ 2 files changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 203ba7f3b108a..6d55388b4eeda 100644 --- a/NEWS +++ b/NEWS @@ -95,6 +95,8 @@ PHP NEWS - Standard: . E_NOTICEs emitted by unserialize() have been promoted to E_WARNING. (timwolla) . Make array_pad's $length warning less confusing. (nielsdos) + . password_hash() will now chain the original RandomException to the ValueError + on salt generation failure. (timwolla) - Streams: . Fixed bug #51056: blocking fread() will block even if data is available. diff --git a/UPGRADING b/UPGRADING index ead75f216c023..50d9314366a0b 100644 --- a/UPGRADING +++ b/UPGRADING @@ -68,6 +68,8 @@ PHP 8.3 UPGRADE NOTES . array_pad() is now only limited by the maximum number of elements an array can have. Before, it was only possible to add at most 1048576 elements at a time. + . password_hash() will now chain the underlying Random\RandomException + as the ValueError’s $previous Exception when salt generation fails. ======================================== 6. New Functions