Skip to content

[WIP] Easy User-land CSPRNG #1119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SammyK wants to merge 23 commits into from
Closed

[WIP] Easy User-land CSPRNG #1119

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
Prev Previous commit
Next Next commit
Add fd caching
  • Loading branch information
@lt
lt committed Mar 4, 2015
commit ab02b7bb0391624371ca6c1374ae9740b7fc66ec
2 changes: 2 additions & 0 deletions ext/standard/basic_functions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3659,6 +3659,8 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */
# endif
#endif

BASIC_MINIT_SUBMODULE(random)
Copy link
Contributor

@weltling weltling Mar 26, 2015

Choose a reason for hiding this comment

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

The prototype to this needs to be somewhere in the header, otherwise there'll be visibility issue.


return SUCCESS;
}
/* }}} */
Expand Down
13 changes: 13 additions & 0 deletions ext/standard/php_random.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@

PHP_FUNCTION(random_bytes);
PHP_FUNCTION(random_int);

PHP_MINIT_FUNCTION(lcg);

ZEND_BEGIN_MODULE_GLOBALS(random)
int fd;
ZEND_END_MODULE_GLOBALS(random)

#ifdef ZTS
# define RANDOM_G(v) TSRMG(random_globals_id, zend_random_globals *, v);
Copy link
Contributor

@weltling weltling Mar 26, 2015

Choose a reason for hiding this comment

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

there should be no semicolon at the end

Copy link
Contributor

@weltling weltling Mar 26, 2015

Choose a reason for hiding this comment

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

Also ZEND_TSRMG should be used to avoid abundant function calls. But even without it I guess the TS build is broken as there's no code allocating/deallocating globals. You can lookup how it is done in some other submodule, fe browser cap.

I'd also like to ask - shouldn't the secure device actually be reopened on each request rather than once on the start? Maybe it were better to introduce the request handlers, if it should. Or even not putting it into globals, but reopening every time it's required - then it's guaranteed to be valid every time it's used.

#else
# define RANDOM_G(v) random_globals.v
#endif

#endif

/*
Expand Down
33 changes: 25 additions & 8 deletions ext/standard/random.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,23 @@
#include <math.h>

#include "php.h"
#include "php_random.h"

#if PHP_WIN32
# include "win32/winutil.h"
#endif

ZEND_DECLARE_MODULE_GLOBALS(random);

/* {{{ */
PHP_MINIT_FUNCTION(random)
{
RANDOM_G(fd) = -1;
Copy link
Contributor

@weltling weltling Mar 26, 2015

Choose a reason for hiding this comment

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

this descriptor should be closed somewhen

return SUCCESS;
}
/* }}} */

/* {{{ */
static int php_random_bytes(void *bytes, size_t size)
{
int n = 0;
Copy link
Member

@nikic nikic Feb 24, 2015

Choose a reason for hiding this comment

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

Better would be ssize_t.

Copy link
Member

@nikic nikic Feb 24, 2015

Choose a reason for hiding this comment

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

The declaration should also be moved into the while loop, otherwise this will likely generate a warning on BSD systems with arc4random.

Expand All @@ -43,18 +55,23 @@ static int php_random_bytes(void *bytes, size_t size)
#if HAVE_DECL_ARC4RANDOM_BUF
arc4random_buf(bytes, size);
#else
int fd = -1;
int fd = RANDOM_G(fd);
size_t read_bytes = 0;

if (fd < 0) {
#if HAVE_DEV_ARANDOM
fd = open("/dev/arandom", O_RDONLY);
fd = open("/dev/arandom", O_RDONLY);
#else
#if HAVE_DEV_URANDOM
fd = open("/dev/urandom", O_RDONLY);
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;
if (fd < 0) {
php_error_docref(NULL, E_WARNING, "Cannot open source device");
return FAILURE;
}

RANDOM_G(fd) = fd;
}

while (read_bytes < size) {
Expand All @@ -65,7 +82,6 @@ static int php_random_bytes(void *bytes, size_t size)
read_bytes += n;
}

close(fd);
if (read_bytes < size) {
php_error_docref(NULL, E_WARNING, "Could not gather sufficient random data");
return FAILURE;
Expand All @@ -75,6 +91,7 @@ static int php_random_bytes(void *bytes, size_t size)

return SUCCESS;
}
/* }}} */

/* {{{ proto string random_bytes(int length)
Return an arbitrary length of pseudo-random bytes as binary string */
Expand Down Expand Up @@ -139,7 +156,7 @@ PHP_FUNCTION(random_int)
umax++;

// Powers of two are not biased
if (umax & ~umax != umax) {
if ((umax & ~umax) != umax) {
// Ceiling under which ZEND_LONG_MAX % max == 0
limit = ZEND_ULONG_MAX - (ZEND_ULONG_MAX % umax) - 1;

Expand Down