Hi all,
On Mon, Feb 3, 2014 at 7:08 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> What's wrong with mcrypt_create_iv() which exists specifically for the
>> purpose of generating secure random string?
>>
>
> User may use it. IV should be random bytes and it can be used as
> secure source for hash. I does almost the same thing that I would
> like to do. Issues are
>
> - it does not auto detect RNG and use /dev/random by default
> - it does not support /dev/arandom
> - it uses php_rand() to create random bytes if source option is not
> RANDOM or URANDOM
> - it is not an available function by default...
>
> 1st issue is not a issue actually. I think this is good that it uses
> /dev/random by default
> even if it may block script. As a crypt module, it should use most secure
> source by default. We may improve mcrypt_create_iv() a little by raising
> E_NOTICE
> error when user set source other than RANDOM or URANDOM, and add ARANDOM
> as a source.
>
> Even though mcrypt_create_iv() good enough for it's original purpose, it's
> not good as
> a general (fool proof) method for generating random bytes as it can block
> script execution.
>
I'm about to write patch improve mcrypt_create_iv(), but there is problem
IV source is defined as enum and user has to specify it as number
typedef enum {
RANDOM = 0,
URANDOM,
RAND
} iv_source;
To maintain compatibility, it would be
typedef enum {
RANDOM = 0,
URANDOM,
RAND,
ARANDOM
} iv_source;
Which seems a little odd to me. Anyway, possible patch would be something
like
diff --git a/ext/mcrypt/mcrypt.c b/ext/mcrypt/mcrypt.c
index 89ad83f..98b1848 100644
--- a/ext/mcrypt/mcrypt.c
+++ b/ext/mcrypt/mcrypt.c
@@ -539,7 +539,8 @@ PHP_MINFO_FUNCTION(mcrypt) /* {{{ */
typedef enum {
RANDOM = 0,
URANDOM,
- RAND
+ RAND,
+ ARANDOM
} iv_source;
/* {{{ proto resource mcrypt_module_open(string cipher, string
cipher_directory, string mode, string mode_directory)
@@ -1387,8 +1388,23 @@ PHP_FUNCTION(mcrypt_create_iv)
}
iv = ecalloc(size + 1, 1);
-
- if (source == RANDOM || source == URANDOM) {
+
+ switch(source) {
+ case RAND:
+ source = NULL;
+ break;
+ case URANDOM:
+ source = "/dev/urandom";
+ break;
+ case ARANDOM:
+ source = "/dev/arandom";
+ break;
+ case RANDOM:
+ default:
+ source = "/dev/random";
+ }
+
+ if (source) {
#if PHP_WIN32
/* random/urandom equivalent on Windows */
BYTE *iv_b = (BYTE *) iv;
@@ -1402,7 +1418,7 @@ PHP_FUNCTION(mcrypt_create_iv)
int fd;
size_t read_bytes = 0;
- fd = open(source == RANDOM ? "/dev/random" : "/dev/urandom", O_RDONLY);
+ fd = open(source, O_RDONLY);
if (fd < 0) {
efree(iv);
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot open source device");
@@ -1428,6 +1444,7 @@ PHP_FUNCTION(mcrypt_create_iv)
while (size) {
iv[--size] = (char) (255.0 * php_rand(TSRMLS_C) / RAND_MAX);
}
+ php_error_docref(NULL TSRMLS_CC, E_NOTICE, "RAND is not safe");
}
RETURN_STRINGL(iv, n, 0);
}
--
Yasuo Ohgaki
yohgaki@ohgaki.net