Add support for getrandom(2), add type check on file descriptor

Fix to_read, throw exception if syscall fails

Fixes thanks to feedback from sarnold at ##crypto on freenode

Correction on error conditions

Remove dead code (thanks @defuse)

It turns out getrandom can take >256, getentropy refuses.

Better semantics

Thanks @defuse for catching my silly mistake here

Cast to size_t to be explicit

Let's simplify the logic a bit

Let's be consistent; define everything before we do any logic

Continuously check that the file descriptor is still a valid one

Add device type check on fd initialization

diff --git a/ext/standard/config.m4 b/ext/standard/config.m4
index c75b141c80dcc0363c90f49201cd73726675dddd..c435f9655407e2467537c131c28b59c502373c38 100644 (file)
--- a/ext/standard/config.m4
+++ b/ext/standard/config.m4
@@ -597,6 +597,11 @@ dnl Check for arc4random on BSD systems
 dnl
 AC_CHECK_DECLS([arc4random_buf])
 
+dnl
+dnl Check for getrandom on newer Linux kernels
+dnl
+AC_CHECK_DECLS([getrandom])
+
 dnl
 dnl Setup extension sources
 dnl

diff --git a/ext/standard/random.c b/ext/standard/random.c
index e2e48951cc0be1b730e33635e304696c1b39de13..6bee9d31d5151433d255b202ceb9ad3bdb7436b8 100644 (file)
--- a/ext/standard/random.c
+++ b/ext/standard/random.c
@@ -30,6 +30,9 @@
 #if PHP_WIN32
 # include "win32/winutil.h"
 #endif
+#ifdef __linux__
+# include <linux/random.h>
+#endif
 
 #ifdef ZTS
 int random_globals_id;
@@ -75,6 +78,7 @@ PHP_MSHUTDOWN_FUNCTION(random)
 /* }}} */
 
 /* {{{ */
+
 static int php_random_bytes(void *bytes, size_t size)
 {
 #if PHP_WIN32
@@ -85,26 +89,75 @@ static int php_random_bytes(void *bytes, size_t size)
        }
 #elif HAVE_DECL_ARC4RANDOM_BUF
        arc4random_buf(bytes, size);
+#elif HAVE_DECL_GETRANDOM
+       /* Linux getrandom(2) syscall */
+       size_t read_bytes = 0;
+       size_t amount_to_read = 0;
+       ssize_t n;
+
+       /* Keep reading until we get enough entropy */
+       do {
+               amount_to_read = size - read_bytes;
+               /* Below, (bytes + read_bytes)  is pointer arithmetic.
+
+                  bytes   read_bytes  size
+                    |      |           |
+                   [#######=============] (we're going to write over the = region)
+                            \\\\\\\\\\\\\
+                             amount_to_read
+
+               */
+               n = getrandom(bytes + read_bytes, amount_to_read, 0);
+
+               if (n == -1) {
+                       if (errno == EINTR || errno == EAGAIN) {
+                               /* Try again */
+                               continue;
+                       }
+                       /*
+                               If the syscall fails, we are doomed. The loop that calls
+                               php_random_bytes should be terminated by the exception instead
+                               of proceeding to demand more entropy.
+                       */
+                       zend_throw_exception(zend_ce_exception, "Could not gather sufficient random data", errno);
+                       return FAILURE;
+               }
+
+               read_bytes += (size_t) n;
+       } while (read_bytes < size);
 #else
        int    fd = RANDOM_G(fd);
+       struct stat st;
        size_t read_bytes = 0;
+       ssize_t n;
 
        if (fd < 0) {
-#if HAVE_DEV_ARANDOM
-               fd = open("/dev/arandom", O_RDONLY);
-#elif HAVE_DEV_URANDOM
+#if HAVE_DEV_URANDOM
                fd = open("/dev/urandom", O_RDONLY);
 #endif
                if (fd < 0) {
                        zend_throw_exception(zend_ce_exception, "Cannot open source device", 0);
                        return FAILURE;
                }
-
+               /* Does the file exist and is it a character device? */
+               if (fstat(fd, &st) != 0 || !S_ISCHR(st.st_mode)) {
+                       close(fd);
+                       zend_throw_exception(zend_ce_exception, "Error reading from source device", 0);
+                       return FAILURE;
+               }
+#ifdef __linux__
+               // Make sure that /dev/urandom is the proper urandom device on Linux
+               if (st.st_rdev != makedev(1, 9)) {
+                       close(fd);
+                       zend_throw_exception(zend_ce_exception, "Error reading from source device", 0);
+                       return FAILURE;
+               }
+#endif
                RANDOM_G(fd) = fd;
        }
 
        while (read_bytes < size) {
-               ssize_t n = read(fd, bytes + read_bytes, size - read_bytes);
+               n = read(fd, bytes + read_bytes, size - read_bytes);
                if (n <= 0) {
                        break;
                }

diff --git a/ext/standard/tests/random/random_bytes.phpt b/ext/standard/tests/random/random_bytes.phpt
index 86391383e443ff5d757b4b2cd376993e9b45e88d..6a7d438835f95198910a00136ae2433230179c5e 100644 (file)
--- a/ext/standard/tests/random/random_bytes.phpt
+++ b/ext/standard/tests/random/random_bytes.phpt
@@ -8,7 +8,10 @@ var_dump(strlen(bin2hex(random_bytes(16))));
 
 var_dump(is_string(random_bytes(10)));
 
+var_dump(is_string(random_bytes(257)));
+
 ?>
 --EXPECT--
 int(32)
 bool(true)
+bool(true)