Merge pull request #1 from lt/rand-bytes

SammyK · SammyK · commit 7ef5754cbfa5 · 2015-02-23T22:49:33.000-06:00
Changes from my fork of rand-bytes
diff --git a/Zend/Zend.m4 b/Zend/Zend.m4
@@ -425,11 +425,13 @@ if test -r "/dev/urandom" && test -c "/dev/urandom"; then
   AC_MSG_RESULT(yes) 
 else 
   AC_MSG_RESULT(no) 
-  AC_MSG_CHECKING(whether /dev/arandom exists) 
-  if test -r "/dev/arandom" && test -c "/dev/arandom"; then 
-    AC_DEFINE([HAVE_DEV_ARANDOM], 1, [Define if the target system has /dev/arandom device])
-    AC_MSG_RESULT(yes) 
-  else 
-    AC_MSG_RESULT(no) 
-  fi 
+fi
+
+AC_MSG_CHECKING(whether /dev/arandom exists) 
+if test -r "/dev/arandom" && test -c "/dev/arandom"; then 
+  AC_DEFINE([HAVE_DEV_ARANDOM], 1, [Define if the target system has /dev/arandom device])
+  AC_MSG_RESULT(yes) 
+else 
+  AC_MSG_RESULT(no) 
 fi 
+
diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c
@@ -1899,10 +1899,11 @@ ZEND_END_ARG_INFO()
 /* }}} */
 /* {{{ random.c */
 ZEND_BEGIN_ARG_INFO_EX(arginfo_random_bytes, 0, 0, 0)
-	ZEND_ARG_INFO(0, bytes)
+	ZEND_ARG_INFO(0, length)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_random_int, 0, 0, 0)
+	ZEND_ARG_INFO(0, min)
 	ZEND_ARG_INFO(0, max)
 ZEND_END_ARG_INFO()
 /* }}} */
diff --git a/ext/standard/config.m4 b/ext/standard/config.m4
@@ -592,6 +592,11 @@ dnl Check for atomic operation API availability in Solaris
 dnl
 AC_CHECK_HEADERS([atomic.h])
 
+dnl
+dnl Check for arc4random on BSD systems
+dnl
+AC_CHECK_DECLS([arc4random_buf])
+
 dnl
 dnl Setup extension sources
 dnl
diff --git a/ext/standard/random.c b/ext/standard/random.c
@@ -29,58 +29,54 @@
 # include "win32/winutil.h"
 #endif
 
-// Big thanks to @ircmaxell for the help on this bit
-union rand_long_buffer {
-	char buffer[8];
-	long number;
-};
-
-// Copy/pasted from mcrypt.c
-static int php_random_bytes(char *bytes, zend_long size)
+static int php_random_bytes(void *bytes, size_t size)
 {
 	int n = 0;
 
 #if PHP_WIN32
-	/* random/urandom equivalent on Windows */
-	BYTE *win_bytes = (BYTE *) bytes;
-	if (php_win32_get_random_bytes(win_bytes, (size_t) size) == FAILURE) {
+	/* Defer to CryptGenRandom on Windows */
+	if (php_win32_get_random_bytes(bytes, size) == FAILURE) {
 		php_error_docref(NULL, E_WARNING, "Could not gather sufficient random data");
 		return FAILURE;
 	}
-	n = (int)size;
 #else
-	// @todo Need to cache the fd for random_int() call within loop
-	int    fd;
+#if HAVE_DECL_ARC4RANDOM_BUF
+	arc4random_buf(bytes, size);
+#else
+	int    fd = -1;
 	size_t read_bytes = 0;
-
+#if HAVE_DEV_ARANDOM
+	fd = open("/dev/arandom", O_RDONLY);
+#else
+#if HAVE_DEV_URANDOM
 	fd = open("/dev/urandom", O_RDONLY);
+#endif // URANDOM
+#endif // ARANDOM
 	if (fd < 0) {
 		php_error_docref(NULL, E_WARNING, "Cannot open source device");
 		return FAILURE;
 	}
+
 	while (read_bytes < size) {
 		n = read(fd, bytes + read_bytes, size - read_bytes);
 		if (n < 0) {
 			break;
 		}
 		read_bytes += n;
 	}
-	n = read_bytes;
 
 	close(fd);
-	if (n < size) {
+	if (read_bytes < size) {
 		php_error_docref(NULL, E_WARNING, "Could not gather sufficient random data");
 		return FAILURE;
 	}
-#endif
-
-	// @todo - Do we need to do this?
-	bytes[size] = '\0';
+#endif // !ARC4RANDOM_BUF
+#endif // !WIN32
 
 	return SUCCESS;
 }
 
-/* {{{ proto string random_bytes(int bytes)
+/* {{{ proto string random_bytes(int length)
 Return an arbitrary length of pseudo-random bytes as binary string */
 PHP_FUNCTION(random_bytes)
 {
@@ -91,8 +87,8 @@ PHP_FUNCTION(random_bytes)
 		return;
 	}
 
-	if (size <= 0 || size >= INT_MAX) {
-		php_error_docref(NULL, E_WARNING, "Cannot genrate a random string with a size of less than 1 or greater than %d", INT_MAX);
+	if (size < 1) {
+		php_error_docref(NULL, E_WARNING, "Length must be greater than 0");
 		RETURN_FALSE;
 	}
 
@@ -103,48 +99,59 @@ PHP_FUNCTION(random_bytes)
 		return;
 	}
 
+	bytes->val[size] = '\0';
+
 	RETURN_STR(bytes);
 }
 /* }}} */
 
-/* {{{ proto int random_int(int maximum)
+/* {{{ proto int random_int(int max)
 Return an arbitrary pseudo-random integer */
 PHP_FUNCTION(random_int)
 {
-	zend_long maximum;
-	zend_long size;
-	size_t i;
+	zend_long min = ZEND_LONG_MIN;
+	zend_long max = ZEND_LONG_MAX;
+	zend_ulong limit;
+	zend_ulong umax;
+	zend_ulong result;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "|l", &maximum) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "|ll", &min, &max) == FAILURE) {
 		return;
 	}
 
-	if (ZEND_NUM_ARGS() == 0) {
-		maximum = INT_MAX;
-	}
-
-	if (maximum <= 0 || maximum > INT_MAX) {
-		php_error_docref(NULL, E_WARNING, "Cannot use maximum less than 1 or greater than %d", INT_MAX);
+	if (min >= max) {
+		php_error_docref(NULL, E_WARNING, "Minimum value must be greater than the maximum value");
 		RETURN_FALSE;
 	}
 
-	long range = (long) maximum; // @todo Support min?
+	umax = max - min;
 
-	// Big thanks to @ircmaxell for the help on this bit
-	union rand_long_buffer value;
-	long result;
-	int bits = (int) (log((double) range) / log(2.0)) + 1;
-	int bytes = MAX(ceil(bits / 8), 1);
-	long mask = (long) pow(2.0, (double) bits) - 1;
+	if (php_random_bytes(&result, sizeof(result)) == FAILURE) {
+		return;
+	}
 
-	do {
-		if (php_random_bytes(&value.buffer, 8) == FAILURE) {
-			return;
+	// Special case where no modulus is required
+	if (umax == ZEND_ULONG_MAX) {
+		RETURN_LONG((zend_long)result);
+	}
+
+	// Increment the max so the range is inclusive of max
+	umax++;
+
+	// Powers of two are not biased
+	if (umax & ~umax != umax) {
+		// Ceiling under which ZEND_LONG_MAX % max == 0
+		limit = ZEND_ULONG_MAX - (ZEND_ULONG_MAX % umax) - 1;
+	
+		// Discard numbers over the limit to avoid modulo bias
+		while (result > limit) {
+			if (php_random_bytes(&result, sizeof(result)) == FAILURE) {
+				return;
+			}
 		}
-		result = value.number & mask;
-	} while (result > maximum);
+	}
 
-	RETURN_LONG(result);
+	RETURN_LONG((zend_long)((result % umax) + min));
 }
 /* }}} */
 
