Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95971
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain giraffy.jp designates 49.212.134.110 as permitted sender)
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: "internals\@lists.php.net" <internals@lists.php.net>
In-Reply-To: <CAGa2bXYJ2=Tn20zy2BkXJCgAxuEq08Bv7ZA85GtYPZND3dTK2g@mail.gmail.com>
	(Yasuo Ohgaki's message of "Tue, 13 Sep 2016 12:07:53 +0900")
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>
	<CAGa2bXZPrarOmRvYiDVar3qw1bUyHXsnMYRsgkPbvwXHO8BLuw@mail.gmail.com>
	<CAGa2bXYJ2=Tn20zy2BkXJCgAxuEq08Bv7ZA85GtYPZND3dTK2g@mail.gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
Date: Tue, 13 Sep 2016 13:20:36 +0900
Message-ID: <87mvjcbfcb.fsf@lil.giraffy.jp>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PHP-DEV] [RFC][DISCUSSION] Improve uniqid() uniqueness
From: kazuo@o-ishi.jp (Kazuo Oishi)

Hi,

> 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.

I think, uniqid should be left as is.


>>> How serious BC is?
>>
>> You should already know that this BC-break breaks existing
>> valid PHP codes in some situation.  (DB error, test failure, etc.)
>>
>> BC-break may be acceptable if the change is clearly great 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.

Misuse is just a misuse.  The function uniqid() will not be a secure
function to be able to use in security sensitive context even if this
RFC is passed, isn't it?

> In short, making PHP be more secure platform (tolerant even for
> mistakes) matter to me.

IMO, THIS change is not important enough and not effective enough to do
in BC break way.


> This BC is nothing compared to mt_rand() everywhere.

It would be worth enough.
(cryptographically secure random > predictive by seed)


> Anyway, let's talk BC with real code.
>
> I didn't look into all, but only briefly.
> https://searchcode.com/?q=uniqid&loc=0&loc2=10000&lan=24

Why didn't you...
You said "BC (BC break) will be minimum"...

> I could only find one code that tests uniqid() return value to test uniqid() (?)

Example:
 https://github.com/BrianPrz/worklist/blob/master/classes/Login.class.php

In this class, output of uniqid() is saved to DB (maybe).
If 'token' field has only 13 length, output length change cause problem.


> Other than that, almost all code does not care about uniqid() return
> value at all.
> Who cares about uniqid() return value? for what purpose? other than testing
> uniqid() itself?

The uniqid() manual explicitly say,

 - default value of more_entropy is false

 - the returned string will be 13 characters long. If more_entropy is
   TRUE, it will be 23 characters.

 - if more_entropy is set to TRUE, uniqid() will add additional entropy
   (using the combined linear congruential generator) at the end of the
   return value

http://php.net/manual/en/function.uniqid.php

So, it is fairly valid to design
 - field length limit to 13 chars in validation code or DB column.
 - acceptable character type limit to "[0-9A-Za-z]+".
 - expect increasing value
in their PHP applications.

-- 
Kazuo Oishi