Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95969
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <877fagczci.fsf@lil.giraffy.jp>
References: <CAGa2bXYg1kkoYGQNr8T2oTshuqrvweefYO+EdY41w1vwUM32iQ@mail.gmail.com>
 <878tuxenl4.fsf@lil.giraffy.jp> <CAGa2bXaTydTSZniX4dUA9h_FcNaymyqfDXQJO0zx1ZDfLmvWdg@mail.gmail.com>
 <87twdlcs2j.fsf@lil.giraffy.jp> <CAGa2bXaziNMtNAgsJgX-BCgLL9z2WkoSaP8aNVrAgwo9MJ4WEQ@mail.gmail.com>
 <87h99lch13.fsf@lil.giraffy.jp> <CAGa2bXYeWZSx2vOYPhnCJ_O7ry2jPNNPcm=mpNuVJAwYAY5sMA@mail.gmail.com>
 <877fagczci.fsf@lil.giraffy.jp>
Date: Tue, 13 Sep 2016 11:48:18 +0900
Message-ID: <CAGa2bXZPrarOmRvYiDVar3qw1bUyHXsnMYRsgkPbvwXHO8BLuw@mail.gmail.com>
To: Kazuo Oishi <kazuo@o-ishi.jp>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC][DISCUSSION] Improve uniqid() uniqueness
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

On Tue, Sep 13, 2016 at 11:23 AM, Kazuo Oishi <kazuo@o-ishi.jp> wrote:
>>>>> IMO, improving it (generate better semi-unique ID) is not important
>>>>> enoungh to introduce unnecessary BC break.  (Why returning string length
>>>>> is changed?)
>>>>
>>>> It cannot not produce unique ID by default as name "uniqid()" implies.
>>>> Reason is described in the RFC. Please read RFC because it's the
>>>> official proposal.
>>>
>>> I had read it, of course.  But I could not understand why you chose BC
>>> break way.
>>
>> IMHO, 10 bits (about a million) entropy is not considered enough
>> entropy, do you?
>
> Do you say about extra part which is added by "more_entropy" option?
>
> Current "more_entropy" part (10 bytes) pattern is "n.nnnnnnnn" and its
> variation is 10^9 (1 billion) as written in your RFC.  (about 30bits?)
>
> I think it is enough to avoid collision in the same usec, for
> non-security purpose.

Oops. Thank you for the correction :) I'll fix the RFC.

Actually not with current implementation. If time is rewined, the same ID
could be generated by chance. Because, both ID part and entropy part
is generated based on the current system time. Collision would be rare,
but it's not rare as it should be at all.

I think minimum would be 64 bits, 128 at least, 256 bits recommended.

How about use 2nd parameter for entropy length? 0 for none, 1 for
default, 10 to 255 chars.

Then user can have choice for prefered number of entropy chars.

>
>> How serious BC is?
>
> You should already know that this BC-breack breaks existing
> valid PHP codes in some situation.  (DB error, test failure, etc.)
>
> BC-breack may be acceptable if the change is clearly greate improvement
> or obviously necessary.  But this change is not, I think.

I do think this is needed.

Let's not please security audit companies. Use of current uniqid() in
security sensitive context is fatal because it is too easy to predict
generated ID even with "more_entropy". Letting make such mistake
moderate is worth the change.

>
>> It's much less impact than using mt_rand() all over
>> the code. i.e. rand() and mt_rand() is predictable random generator.
>
> Sorry, I cannot understand what you say...  (Why mt_rand?)

The change have been done.
https://wiki.php.net/rfc/rng_fixes

IMHO, this RFC's BC is nothing compare to this.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net