Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72027
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.54 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAGa2bXamnJ6ZgjaKJ-qnwnD8oEuqJ5MTZ1e1ratKSWo8kVTpUw@mail.gmail.com>
References: <CAGa2bXbq1ttBz+hSk20-CkKWvDdHr5+8bBmc04Q22i7K2VSs+A@mail.gmail.com>
 <52EE1EDC.2010309@sugarcrm.com> <CAGa2bXZZ3ug3oWg_im5bHUDjo2EHVeBMqU0fvB4WU-4OZTMCuw@mail.gmail.com>
 <CAGa2bXamnJ6ZgjaKJ-qnwnD8oEuqJ5MTZ1e1ratKSWo8kVTpUw@mail.gmail.com>
Date: Mon, 3 Feb 2014 10:26:17 +0900
Message-ID: <CAGa2bXYzXmzdzQ65XZLON-yUzyx4y-h2HJwF5TxPo2b2XhMfMw@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1136dfc674e4f704f1766bd5
Subject: Re: [PHP-DEV] Extending uniqid() or not?
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a1136dfc674e4f704f1766bd5
Content-Type: text/plain; charset=UTF-8

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
yohgaki@ohgaki.net


On Mon, Feb 3, 2014 at 7:46 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

> 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
>

--001a1136dfc674e4f704f1766bd5--