Hi all,
Possible patch for mcrypt_create_iv(). I put it in github.
https://github.com/yohgaki/php-src/compare/PHP-5.6-mcrypt_create_iv
- Add /dev/arandom support
- Raise warning for dangerous usage
--
Yasuo Ohgaki
[email protected]
On Mon, Feb 3, 2014 at 7:46 AM, Yasuo Ohgaki <[email protected]> wrote:
> Hi all,
>
> On Mon, Feb 3, 2014 at 7:08 AM, Yasuo Ohgaki <[email protected]> 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
> [email protected]
>