Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f8d7aec
Got initial API added
SammyK Feb 13, 2015
26e4ed2
Got mostly working. I think Hex is broken. Need to add int.
SammyK Feb 13, 2015
3c5fcac
Updated to use zend_string but getting memory leaks. Doh!
SammyK Feb 20, 2015
e96b077
Got random_bytes() working again.
SammyK Feb 20, 2015
aa0ca69
Fix random_int() checking size of wrong var.
SammyK Feb 20, 2015
b32e0d0
Got random_int() seemingly working thanks to @ircmaxell
SammyK Feb 20, 2015
2c659ed
Make maximum argument to random_int() optional with default to INT_MAX.
SammyK Feb 20, 2015
a1e6229
Remove random_hex(). *sadface*
SammyK Feb 20, 2015
bbc9198
Detect presence of /dev/arandom
lt Feb 21, 2015
7a99db6
Tidy up `php_random_bytes` and add /dev/arandom
lt Feb 21, 2015
3d413ad
Ensure random_int() uses a uniform distribution
lt Feb 22, 2015
513d5c9
Allow full integer range from random_int()
lt Feb 24, 2015
77f99cc
Use arc4random where present
lt Feb 24, 2015
7ef5754
Merge pull request #1 from lt/rand-bytes
SammyK Feb 24, 2015
766ce0c
Add tests
SammyK Feb 24, 2015
99e36d6
Fix wording in error message. Add check for max value.
SammyK Feb 24, 2015
c6fc391
Fix return types on error. Avoid a warning on BSD systems.
SammyK Feb 25, 2015
ab02b7b
Add fd caching
lt Mar 4, 2015
fd0570b
Merge remote-tracking branch 'leigh/rand-bytes' into rand-bytes
SammyK Mar 13, 2015
7ae4917
Fixes based on PR feedback
lt Mar 27, 2015
a67e42f
Changes based on feedback
lt Apr 9, 2015
f8a6d38
Normalized the return value for errors & updated tests.
SammyK Apr 10, 2015
2990341
Fix merge conflicts
SammyK Apr 10, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Got initial API added
  • Loading branch information
SammyK committed Feb 13, 2015
commit f8d7aec431397e008fb63302e361444b0e0ea737
18 changes: 18 additions & 0 deletions ext/standard/basic_functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -1897,6 +1897,20 @@ ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_INFO(arginfo_mt_getrandmax, 0)
ZEND_END_ARG_INFO()
/* }}} */
/* {{{ random.c */
ZEND_BEGIN_ARG_INFO_EX(arginfo_random_bytes, 0, 0, 0)
ZEND_ARG_INFO(0, bytes)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_random_hex, 0, 0, 0)
ZEND_ARG_INFO(0, bytes)
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()
/* }}} */
/* {{{ sha1.c */
ZEND_BEGIN_ARG_INFO_EX(arginfo_sha1, 0, 0, 1)
ZEND_ARG_INFO(0, str)
Expand Down Expand Up @@ -2820,6 +2834,10 @@ const zend_function_entry basic_functions[] = { /* {{{ */
PHP_FE(mt_srand, arginfo_mt_srand)
PHP_FE(mt_getrandmax, arginfo_mt_getrandmax)

PHP_FE(random_bytes, arginfo_random_bytes)
PHP_FE(random_hex, arginfo_random_hex)
PHP_FE(random_int, arginfo_random_int)

#if HAVE_GETSERVBYNAME
PHP_FE(getservbyname, arginfo_getservbyname)
#endif
Expand Down
3 changes: 2 additions & 1 deletion ext/standard/config.m4
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,8 @@ PHP_NEW_EXTENSION(standard, array.c base64.c basic_functions.c browscap.c crc32.
incomplete_class.c url_scanner_ex.c ftp_fopen_wrapper.c \
http_fopen_wrapper.c php_fopen_wrapper.c credits.c css.c \
var_unserializer.c ftok.c sha1.c user_filters.c uuencode.c \
filters.c proc_open.c streamsfuncs.c http.c password.c,,,
filters.c proc_open.c streamsfuncs.c http.c password.c \
random.c,,,
-DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)

PHP_ADD_MAKEFILE_FRAGMENT
Expand Down
2 changes: 1 addition & 1 deletion ext/standard/config.w32
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ EXTENSION("standard", "array.c base64.c basic_functions.c browscap.c \
url_scanner_ex.c ftp_fopen_wrapper.c http_fopen_wrapper.c \
php_fopen_wrapper.c credits.c css.c var_unserializer.c ftok.c sha1.c \
user_filters.c uuencode.c filters.c proc_open.c password.c \
streamsfuncs.c http.c flock_compat.c", false /* never shared */,
streamsfuncs.c http.c flock_compat.c random.c", false /* never shared */,
'/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1');
PHP_INSTALL_HEADERS("", "ext/standard");
if (PHP_MBREGEX != "no") {
Expand Down
34 changes: 34 additions & 0 deletions ext/standard/php_random.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
+----------------------------------------------------------------------+
| PHP Version 7 |
+----------------------------------------------------------------------+
| Copyright (c) 1997-2015 The PHP Group |
+----------------------------------------------------------------------+
| This source file is subject to version 3.01 of the PHP license, |
| that is bundled with this package in the file LICENSE, and is |
| available through the world-wide-web at the following url: |
| http://www.php.net/license/3_01.txt |
| If you did not receive a copy of the PHP license and are unable to |
| obtain it through the world-wide-web, please send a note to |
| license@php.net so we can mail you a copy immediately. |
+----------------------------------------------------------------------+
| Authors: Sammy Kaye Powers <me@sammyk.me> |
+----------------------------------------------------------------------+
*/

/* $Id$ */

#ifndef PHP_RANDOM_H
#define PHP_RANDOM_H

PHP_FUNCTION(random_bytes);
PHP_FUNCTION(random_hex);
PHP_FUNCTION(random_int);
#endif

/*
* Local variables:
* tab-width: 4
* c-basic-offset: 4
* End:
*/
1 change: 1 addition & 0 deletions ext/standard/php_standard.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include "php_ftok.h"
#include "php_type.h"
#include "php_password.h"
#include "php_random.h"

#define phpext_standard_ptr basic_functions_module_ptr
PHP_MINIT_FUNCTION(standard_filters);
Expand Down
158 changes: 158 additions & 0 deletions ext/standard/random.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
+----------------------------------------------------------------------+
| PHP Version 7 |
+----------------------------------------------------------------------+
| Copyright (c) 1997-2015 The PHP Group |
+----------------------------------------------------------------------+
| This source file is subject to version 3.01 of the PHP license, |
| that is bundled with this package in the file LICENSE, and is |
| available through the world-wide-web at the following url: |
| http://www.php.net/license/3_01.txt |
| If you did not receive a copy of the PHP license and are unable to |
| obtain it through the world-wide-web, please send a note to |
| license@php.net so we can mail you a copy immediately. |
+----------------------------------------------------------------------+
| Authors: Sammy Kaye Powers <me@sammyk.me> |
+----------------------------------------------------------------------+
*/

/* $Id$ */

#include <stdlib.h>

#include "php.h"

// Copy/pasted from string.c
static char hexconvtab[] = "0123456789abcdef";

// Copy/pasted from string.c
static zend_string *php_bin_to_hex(const unsigned char *old, const size_t oldlen)
{
zend_string *result;
size_t i, j;

result = zend_string_safe_alloc(oldlen, 2 * sizeof(char), 0, 0);

for (i = j = 0; i < oldlen; i++) {
result->val[j++] = hexconvtab[old[i] >> 4];
result->val[j++] = hexconvtab[old[i] & 15];
}
result->val[j] = '\0';

return result;
}

static int *php_random_bytes(zend_string *bytes, size_t length)
{
FILE *fp;

#ifdef PHP_WIN32
// @todo Need to add Windows support
fp = NULL;
#else
fp = fopen("/dev/urandom" , "rb");
#endif

if (fp == NULL) {
php_error_docref(NULL, E_WARNING, "Unable to open /dev/urandom");
return FAILURE;
}

// @todo I think this has to be char & int!
if (fgets(bytes, 10, fp) == NULL) {
fclose(fp);
php_error_docref(NULL, E_WARNING, "Unable to read from /dev/urandom");
return FAILURE;
}
fclose(fp);

return bytes;
}

/* {{{ proto string random_bytes(int bytes)
Return an arbitrary length of pseudo-random bytes as binary string */
PHP_FUNCTION(random_bytes)
{
size_t length;
zend_string *bytes;

int argc = ZEND_NUM_ARGS();

if (argc != 0) {
if (zend_parse_parameters(argc, "l", &length) == FAILURE) {
return;
} else if (length <= 0) {
php_error_docref(NULL, E_WARNING, "length(" ZEND_LONG_FMT ") must be greater than 0", length);
RETURN_FALSE;
}
}

if (php_random_bytes(bytes, length) == FAILURE) {
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably that should be RETURN_FALSE as well? Same in random_int.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! I'll fix.

}

RETURN_STR(bytes);
}
/* }}} */

/* {{{ proto string random_hex(int bytes)
Return an arbitrary length of pseudo-random bytes as hexadecimal string */
PHP_FUNCTION(random_hex)
{
size_t length;
zend_string *bytes;
zend_string *hex;

int argc = ZEND_NUM_ARGS();

if (argc != 0) {
if (zend_parse_parameters(argc, "l", &length) == FAILURE) {
return;
} else if (length <= 0) {
php_error_docref(NULL, E_WARNING, "length(" ZEND_LONG_FMT ") must be greater than 0", length);
RETURN_FALSE;
}
}

if (php_random_bytes(bytes, length) == FAILURE) {
return;
}

hex = php_bin_to_hex(bytes, length);

RETURN_STR(hex);
}
/* }}} */

/* {{{ proto int random_int(int min, int max)
Return an arbitrary pseudo-random integer */
PHP_FUNCTION(random_int)
{
zend_long min;
zend_long max;
zend_long number;
int argc = ZEND_NUM_ARGS();

if (argc != 0) {
if (zend_parse_parameters(argc, "ll", &min, &max) == FAILURE) {
return;
} else if (max < min) {
php_error_docref(NULL, E_WARNING, "max(" ZEND_LONG_FMT ") is smaller than min(" ZEND_LONG_FMT ")", max, min);
RETURN_FALSE;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the random_int() function:

  • has a non-deterministic execution time (actually, it's quite random)
  • that doesn't have any limit
  • so the function isn't guaranteed to return at all, ever.

Naturally I understand that the likelihood of these characteristics causing problems in PHP applications is small but I think reviewers should be aware of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let bw = sizeof(zend_ulong) * 8. ZEND_ULONG_MAX % umax will take its largest value for umax = 2 ** (bw - 1), in which case limit = 2 ** (bw - 1) - 1. This means that all results with top bit 1 will be discarded here. For every result the probability for a set top bit is 1/2. As such the worst case probability that this loop has not stopped after generating n random numbers is 1 / 2**n.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic That looks correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just use the RAND_RANGE macro (or something similar) for this? It seems to me that fighting against the modulo problem is a wasted effort.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@datibbaw Because RAND_RANGE is highly biased. It's not an option for crypto-quality randomness. I'm not aware of algorithms for this that don't use a form of rejection sampling.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that fighting against the modulo problem is a wasted effort.

Get out! :)

In rand() or mt_rand() I completely agree, it would be wasted effort. In a function that is advertised as crypto-quality it's important.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's fair enough; I shall read more into this, thanks :)

}

number = min + max;

RETURN_LONG(number);
}
/* }}} */

/*
* Local variables:
* tab-width: 4
* c-basic-offset: 4
* End:
* vim600: sw=4 ts=4 fdm=marker
* vim<600: sw=4 ts=4
*/