From cc06b6cebea04757e8f3c785813871b651b22d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 27 Jul 2022 20:13:44 +0200 Subject: [PATCH 1/3] Improve error messages in php_random_bytes() --- ext/random/random.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/ext/random/random.c b/ext/random/random.c index c7250f404fc9a..2b7ce8b5d3d45 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -470,7 +470,7 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) /* Defer to CryptGenRandom on Windows */ if (php_win32_get_random_bytes(bytes, size) == FAILURE) { if (should_throw) { - zend_throw_exception(zend_ce_exception, "Could not gather sufficient random data", 0); + zend_throw_exception(zend_ce_exception, "BCryptGenRandom failed", 0); } return FAILURE; } @@ -483,7 +483,7 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) */ if (CCRandomGenerateBytes(bytes, size) != kCCSuccess) { if (should_throw) { - zend_throw_exception(zend_ce_exception, "Error generating bytes", 0); + zend_throw_exception(zend_ce_exception, "CCRandomGenerateBytes failed", 0); } return FAILURE; } @@ -522,7 +522,7 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) /* Try again */ continue; } else { - /* If the syscall fails, fall back to reading from /dev/urandom */ + /* If the syscall fails, fall back to reading from /dev/urandom */ break; } } @@ -539,12 +539,17 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) struct stat st; if (fd < 0) { + errno = 0; # if HAVE_DEV_URANDOM fd = open("/dev/urandom", O_RDONLY); # endif if (fd < 0) { if (should_throw) { - zend_throw_exception(zend_ce_exception, "Cannot open source device", 0); + if (errno != 0) { + zend_throw_exception_ex(zend_ce_exception, 0, "Cannot open /dev/urandom: %s", strerror(errno)); + } else { + zend_throw_exception_ex(zend_ce_exception, 0, "Cannot open /dev/urandom"); + } } return FAILURE; } @@ -558,7 +563,7 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) ) { close(fd); if (should_throw) { - zend_throw_exception(zend_ce_exception, "Error reading from source device", 0); + zend_throw_exception(zend_ce_exception, "Error reading from /dev/urandom", 0); } return FAILURE; } @@ -566,6 +571,7 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) } for (read_bytes = 0; read_bytes < size; read_bytes += (size_t) n) { + errno = 0; n = read(fd, bytes + read_bytes, size - read_bytes); if (n <= 0) { break; @@ -574,7 +580,11 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) if (read_bytes < size) { if (should_throw) { - zend_throw_exception(zend_ce_exception, "Could not gather sufficient random data", 0); + if (errno != 0) { + zend_throw_exception_ex(zend_ce_exception, 0, "Could not gather sufficient random data: %s", strerror(errno)); + } else { + zend_throw_exception_ex(zend_ce_exception, 0, "Could not gather sufficient random data"); + } } return FAILURE; } From 76f2bca946aeb8f1f5cd2f116e51f99eaae0b28f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 27 Jul 2022 21:54:49 +0200 Subject: [PATCH 2/3] Show strerror for `fstat` failures --- ext/random/random.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ext/random/random.c b/ext/random/random.c index 2b7ce8b5d3d45..5dab9d9ac9663 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -496,6 +496,8 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) /* Linux getrandom(2) syscall or FreeBSD/DragonFlyBSD getrandom(2) function*/ /* Keep reading until we get enough entropy */ while (read_bytes < size) { + errno = 0; + /* Below, (bytes + read_bytes) is pointer arithmetic. bytes read_bytes size @@ -553,6 +555,8 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) } return FAILURE; } + + errno = 0; /* Does the file exist and is it a character device? */ if (fstat(fd, &st) != 0 || # ifdef S_ISNAM @@ -563,7 +567,11 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) ) { close(fd); if (should_throw) { - zend_throw_exception(zend_ce_exception, "Error reading from /dev/urandom", 0); + if (errno != 0) { + zend_throw_exception_ex(zend_ce_exception, 0, "Error reading from /dev/urandom: %s", strerror(errno)); + } else { + zend_throw_exception_ex(zend_ce_exception, 0, "Error reading from /dev/urandom"); + } } return FAILURE; } From 941335844fc473590664d5f674593c81379ad289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 28 Jul 2022 18:13:25 +0200 Subject: [PATCH 3/3] Improve messages when Windows/macOS CSPRNGs fail --- ext/random/random.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/random/random.c b/ext/random/random.c index 5dab9d9ac9663..ec9877544844e 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -470,7 +470,7 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) /* Defer to CryptGenRandom on Windows */ if (php_win32_get_random_bytes(bytes, size) == FAILURE) { if (should_throw) { - zend_throw_exception(zend_ce_exception, "BCryptGenRandom failed", 0); + zend_throw_exception(zend_ce_exception, "Failed to retrieve randomness from the operating system (BCryptGenRandom)", 0); } return FAILURE; } @@ -483,7 +483,7 @@ PHPAPI int php_random_bytes(void *bytes, size_t size, bool should_throw) */ if (CCRandomGenerateBytes(bytes, size) != kCCSuccess) { if (should_throw) { - zend_throw_exception(zend_ce_exception, "CCRandomGenerateBytes failed", 0); + zend_throw_exception(zend_ce_exception, "Failed to retrieve randomness from the operating system (CCRandomGenerateBytes)", 0); } return FAILURE; }