Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Pierre,

On Thu, Oct 20, 2016 at 7:12 PM, Pierre Joye  wrote:
>> Application requires unique ID under across multi process/thread
>> tasks, it will have more chance to have collided unique ID.
>
> uniqid fill(s|ed) some needs or maybe still fits for some.
>
> However for modern application with many concurrent requests or nodes,
> it does not fit anymore, since long. Do we need to fix a broken hammer
> to fix the screw? I do not think so.
>

I'm all for bringing UUID to PHP by default, encourage users to use it
for applications requires very unique ID. Let's have UUID module by
default someday!

> I suggested already to simply improve it if we can do it without
> breaking BC and recommend to use something designed for such tasks,
> UUID. ramsey/uuid is one of them, other are available.

I'm aware of that entropy must be enabled by default, too. I guess you
feel uniqid() improvement is impossible due to vote. You could be
right about it.

As far as I searched, there is no code have problem even when entropy
on/off and additional chars/format. (Found one test script that tests
uniqid(). Cannot tell if db has char(13) or like, though) We made
rand() a alias of mt_rand(). IMHO, we are better not to leave _too_
weak unique ID generation function alone. HTML ID attribute and test
data ID is common uniqid() use case. Let's make it reasonably unique
for ID attributes, test database unique values, etc.

Regards,

P.S. If uniqid() is a "Shouldn't use function", we don't have to care
little BC too much. Making it a "Can use function" for proper purposes
is reasonable choice. I'm +1 for deprecating and removing uniqid(),
but I presume it will never happen. That's why I'm trying to improve
it.

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Pierre Joye]

On Thu, Oct 20, 2016 at 4:44 PM, Yasuo Ohgaki  wrote:

> Application requires unique ID under across multi process/thread
> tasks, it will have more chance to have collided unique ID.

uniqid fill(s|ed) some needs or maybe still fits for some.

However for modern application with many concurrent requests or nodes,
it does not fit anymore, since long. Do we need to fix a broken hammer
to fix the screw? I do not think so.

I suggested already to simply improve it if we can do it without
breaking BC and recommend to use something designed for such tasks,
UUID. ramsey/uuid is one of them, other are available.

Cheers,
-- 
Pierre

@pierrejoye | http://www.libgd.org

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Kalle,

I forgot to mention one more thing.

On Thu, Oct 20, 2016 at 6:28 PM, Yasuo Ohgaki  wrote:
> Warnings are based on following facts.
>
> uniqid(); // without entropy
>
> usleep(1) is called to get unique timestamp, but NTP can disturb and
> uniqid() can result in the same ID.
>
> uniqid('', TRUE); // with entropy
>
> It's better, but entropy is based on system timestamp and there is no
> usleep(1), so uniqid() is more sensitive to system clock adjustment by
> NTP, and uniqid() can result in the same ID.
>
> Collision is unlikely, but it not that unlikely with true CSPRNG based
> entropy. Therefore, I made warning a little strong. With CSPRNG, we
> may use more gentle warning. IMO.

Application requires unique ID under across multi process/thread
tasks, it will have more chance to have collided unique ID.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Kalle,

On Thu, Oct 20, 2016 at 5:17 PM, Kalle Sommer Nielsen  wrote:
> 2016-10-20 9:18 GMT+02:00 Yasuo Ohgaki :
>> "Do not make assumption for uniqid() output format, entropy
>> especially. uniqid() output format may be changed to provide
>> reasonably unique ID in future versions."
>
> Sounds reasonable to me; although I would phrase it a little
> differently, something along the lines of:
>
> The uniquid cannot be relied on to be unique and
> there can occur collisions, even with the
> more_entrophy set to .

I added warnings to uniqid() manual recently. It's visible now, could
you check this?

http://php.net/manual/en/function.uniqid.php

Warnings are based on following facts.

uniqid(); // without entropy

usleep(1) is called to get unique timestamp, but NTP can disturb and
uniqid() can result in the same ID.

uniqid('', TRUE); // with entropy

It's better, but entropy is based on system timestamp and there is no
usleep(1), so uniqid() is more sensitive to system clock adjustment by
NTP, and uniqid() can result in the same ID.

Collision is unlikely, but it not that unlikely with true CSPRNG based
entropy. Therefore, I made warning a little strong. With CSPRNG, we
may use more gentle warning. IMO.

> As for the in future version, although we may do that, I don't think
> we should document something that is not in the core yet. What do you
> think?

Entropy is some random value by definition, so we may tell users "Make
no assumption for entropy" at least. IMO.
Is this reasonable to you?

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Kalle Sommer Nielsen]

Hi Yasuo

2016-10-20 9:18 GMT+02:00 Yasuo Ohgaki :
> "Do not make assumption for uniqid() output format, entropy
> especially. uniqid() output format may be changed to provide
> reasonably unique ID in future versions."

Sounds reasonable to me; although I would phrase it a little
differently, something along the lines of:

The uniquid cannot be relied on to be unique and
there can occur collisions, even with the
more_entrophy set to .

As for the in future version, although we may do that, I don't think
we should document something that is not in the core yet. What do you
think?

-- 
regards,

Kalle Sommer Nielsen
ka...@php.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Anatol,

On Wed, Oct 19, 2016 at 8:20 PM, Anatol Belski  wrote:
>> I won't have time to write RFC for this, probably. I have many other things 
>> that I
>> would like to improve, like session error status handling improvement that I
>> recently proposed.
>>
> I see. It's a pity you won't have time to write an RFC. I see one already in 
> place on the wiki though. I see also your several other patches hanging on 
> gihtub. IMHO it is a real waste of time to abandon the work you've done, 
> without really pulling it through. With uniqid(), maybe it'd be even the 
> right decision to return to your original RFC, or just to reduce it to comply 
> with the simple patch variant. I'm sure, that no one wants to lose the good 
> contributions, even though it might take some effort to reach the common 
> ground sometimes.

I meant I wouldn't have time for RFC only replace php_combined_lcg().
I'll address uniqid() improvement sometime in the future.

To all,

We constantly get "uniqid() is not unique" bug reports.
In the meantime, any objection for adding following note to uniqid() manual.

"Do not make assumption for uniqid() output format, entropy
especially. uniqid() output format may be changed to provide
reasonably unique ID in future versions."


Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

RE: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Anatol Belski]

Hi Yasuo,

> -Original Message-
> From: Yasuo Ohgaki [mailto:yohg...@ohgaki.net]
> Sent: Wednesday, October 19, 2016 2:35 AM
> To: Anatol Belski <anatol@belski.net>
> Cc: Joe Watkins <pthre...@pthreads.org>; Niklas Keller <m...@kelunik.com>;
> Leigh <lei...@gmail.com>; PHP Internals <internals@lists.php.net>
> Subject: Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness
> 

> I think you and Joe could not follow the discussion. It's okay, reading them 
> all is
> waste of your time. I read all, but I'm not sure if I understand/remember all 
> of
> them well.
> 
We all have own things, yes. I've checked the early messages in this thread, 
and checked the latest now, and checked the patch as well. Calling it waste of 
time is not correct. I just happen to manage 7.0, so I have to jump into such 
situations. My task in such case is to moderate the process, to hopefully help 
to reach the a common decision.

> IMHO Oppositions for the patch is based on _wrong_ assumption that "new
> uniqid() causes common enough errors to be an issue". This wrong assumption is
> the reason why my commit became an issue, I presume.
> 
> Could you reconsider decision based on _wrong_ assumption?
> 
I don't think the opinions should be ignored. The sense of discussion is to 
clear out all possible issues and to improve the approach, not doing as 
assumptions ping-pong. The actual issue is, that it was commited while the 
concerns was not cleared out and the discussion didn't reach a common opinion. 
Also, where Davey asked you already at some early stage to go by an RFC, the 
merge into 7.0 was somewhat sudden.

> 
> P.S. I'm a bit tired of uniqid() discussion because I expected this is easy 
> one.
> This - unique id (time stamp) + entropy (timestamp based entropy) - is 
> obviously
> wrong for today's PHP.
Speaking about broken - there's always some acceptance criteria. Like you might 
know in physics, uncertainty analysis, some result is always given with +/- 
inaccuracy. It is applicable to anything measurable, thus to uniqid() as well. 
What you say, is that the acceptance criteria is now changed, and the result 
needs to be more accurate. From the other side, there are voices telling the 
result is still acceptable. 

> I won't have time to write RFC for this, probably. I have many other things 
> that I
> would like to improve, like session error status handling improvement that I
> recently proposed.
> 
I see. It's a pity you won't have time to write an RFC. I see one already in 
place on the wiki though. I see also your several other patches hanging on 
gihtub. IMHO it is a real waste of time to abandon the work you've done, 
without really pulling it through. With uniqid(), maybe it'd be even the right 
decision to return to your original RFC, or just to reduce it to comply with 
the simple patch variant. I'm sure, that no one wants to lose the good 
contributions, even though it might take some effort to reach the common ground 
sometimes.

Regards

Anatol




--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

On Wed, Oct 19, 2016 at 8:01 AM, Anatol Belski <anatol@belski.net> wrote:
>> -Original Message-
>> From: Yasuo Ohgaki [mailto:yohg...@ohgaki.net]
>> Sent: Tuesday, October 18, 2016 9:53 PM
>> To: Anatol Belski <anatol@belski.net>
>> Cc: Joe Watkins <pthre...@pthreads.org>; Niklas Keller <m...@kelunik.com>;
>> Leigh <lei...@gmail.com>; PHP Internals <internals@lists.php.net>
>> Subject: Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness
>>
>> Hi Anatol,
>>
>> On Wed, Oct 19, 2016 at 1:41 AM, Anatol Belski <anatol@belski.net> wrote:
>> > AFM the patch is not acceptable for 7.0. It is true that some place was 
>> > moved
>> to the new random int functionality (in password AFAIR). But, it is done at 
>> the
>> place and the way that a BC breach is unlikely. Using the throwing variant 
>> is for
>> sure a BC breach, but also the way pushing while being explicitly asked to go
>> through an RFC, is inappropriate. As the new random_* functions are available
>> and allow to implement the best possible uniqueness in user land, changing 
>> the
>> algorithm of the existing uniqid() doesn't look to have a valid base.
>> >
>>
>> Any additional error could be BC. It's the fact.
>>
>> However, your sentence does not make sense at all.
>> Do we revert any error emitting bug fix? No, not at all.
>>
> As far as I remember, uniqid() was never meant to be cryptographically safe. 
> It is documented. Indeed systems might be too fast for microseconds based 
> function nowadays. In 7.0, my simple exercise -  substr(md5(random_bytes(8)), 
> 0, 13) which does same in the way you want it. We are talking about a 
> oneliner of new code vs. an old function that is guaranteed in use at any 
> possible places.

The original draft RFC proposal aimed to be cryptographically safe
unique ID as much as it can, but the pushed patch is not.

>
>> We do  add errors as normal bug fix process. Many of them are w/o RFC, even
>> w/o discussion.
>>
>> Example: https://bugs.php.net/bug.php?id=73238
>> This bug fix caused WordPress caused 3 additional E_WARNING displayed that
>> can be remove by php.ini or code fix.
>>
> As a reminder - there's no global rule about functions throwing exceptions, 
> so it is not done by default. Except a couple of new places, no function 
> throws an exception. The place in password salt code, that was migrated to 
> the new randomness, did already depend on /dev/urandom and others. However, 
> even it's based on the new functionality, the old behavior is kept and it is 
> done intentionally.

I agree that this apply to cases such as rand(). We do had to keep
rand() behavior even if it produces very bad random on Windows, as you
know well.
Replacing bad entropy that should be "really random" is different story.

Current uniqid('', true)'s result is:

unique id (time stamp) + entropy (timestamp based entropy)

Isn't this a shame of us providing the result as "uniqid()" call?

(I'm not saying original design is bad. Original design was inevitable
due to technical limitation, historical reason, just like Windows
rand()) Entropy is entropy. As long as format is kept, it does not
matter if we use better entropy.

>
>> Which is important?
>>  - uniqid() is not unique
>>  - Really broken system that shouldn't be used may emit error
>>
>> "/dev/urandom cannot read discussion" is FUD and irrelevant to this 
>> discussion.
>> Issues with user script random_bytes() implementation or like does not apply 
>> to
>> uniqid() fix.
>>
> But your implementation indeed uses another API that has other impacts. 
> Php_random_bytes is crossplatform, there can be various errors on various 
> platforms. That's the concern as I'd understand it.
>
>>
>> Anyway, are we going to revert anything emit new errors from now on because
>> it's BC?
>> Are we going to require RFC for this kind of very simple and reasonable fix?
>> I hope not.
>>
>> IMHO my discussion is logical. Please consider revert the revert.
>> Otherwise, we cannot fix even simple bugs.
>>
> No, IMHO you overdo it a bit. Of course it is acceptable with errors, 
> warning, etc. where it makes sense. But it needs a base and a balance also in 
> other areas for usability, performance, BC, language consistency, etc. If one 
> were telling, it's impossible to do it in PHP - but there are functions in 
> PHP 7, that provide the functionality aimed. Yes, there is also some legacy 
> functionality, so should everything be moved to cryptographically safe? The 
> answer is obv

RE: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Anatol Belski]

Yasuo,

> -Original Message-
> From: Yasuo Ohgaki [mailto:yohg...@ohgaki.net]
> Sent: Tuesday, October 18, 2016 9:53 PM
> To: Anatol Belski <anatol@belski.net>
> Cc: Joe Watkins <pthre...@pthreads.org>; Niklas Keller <m...@kelunik.com>;
> Leigh <lei...@gmail.com>; PHP Internals <internals@lists.php.net>
> Subject: Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness
> 
> Hi Anatol,
> 
> On Wed, Oct 19, 2016 at 1:41 AM, Anatol Belski <anatol@belski.net> wrote:
> > AFM the patch is not acceptable for 7.0. It is true that some place was 
> > moved
> to the new random int functionality (in password AFAIR). But, it is done at 
> the
> place and the way that a BC breach is unlikely. Using the throwing variant is 
> for
> sure a BC breach, but also the way pushing while being explicitly asked to go
> through an RFC, is inappropriate. As the new random_* functions are available
> and allow to implement the best possible uniqueness in user land, changing the
> algorithm of the existing uniqid() doesn't look to have a valid base.
> >
> 
> Any additional error could be BC. It's the fact.
> 
> However, your sentence does not make sense at all.
> Do we revert any error emitting bug fix? No, not at all.
> 
As far as I remember, uniqid() was never meant to be cryptographically safe. It 
is documented. Indeed systems might be too fast for microseconds based function 
nowadays. In 7.0, my simple exercise -  substr(md5(random_bytes(8)), 0, 13) 
which does same in the way you want it. We are talking about a oneliner of new 
code vs. an old function that is guaranteed in use at any possible places. 

> We do  add errors as normal bug fix process. Many of them are w/o RFC, even
> w/o discussion.
> 
> Example: https://bugs.php.net/bug.php?id=73238
> This bug fix caused WordPress caused 3 additional E_WARNING displayed that
> can be remove by php.ini or code fix.
> 
As a reminder - there's no global rule about functions throwing exceptions, so 
it is not done by default. Except a couple of new places, no function throws an 
exception. The place in password salt code, that was migrated to the new 
randomness, did already depend on /dev/urandom and others. However, even it's 
based on the new functionality, the old behavior is kept and it is done 
intentionally.

> Which is important?
>  - uniqid() is not unique
>  - Really broken system that shouldn't be used may emit error
> 
> "/dev/urandom cannot read discussion" is FUD and irrelevant to this 
> discussion.
> Issues with user script random_bytes() implementation or like does not apply 
> to
> uniqid() fix.
> 
But your implementation indeed uses another API that has other impacts. 
Php_random_bytes is crossplatform, there can be various errors on various 
platforms. That's the concern as I'd understand it.

> 
> Anyway, are we going to revert anything emit new errors from now on because
> it's BC?
> Are we going to require RFC for this kind of very simple and reasonable fix?
> I hope not.
> 
> IMHO my discussion is logical. Please consider revert the revert.
> Otherwise, we cannot fix even simple bugs.
> 
No, IMHO you overdo it a bit. Of course it is acceptable with errors, warning, 
etc. where it makes sense. But it needs a base and a balance also in other 
areas for usability, performance, BC, language consistency, etc. If one were 
telling, it's impossible to do it in PHP - but there are functions in PHP 7, 
that provide the functionality aimed. Yes, there is also some legacy 
functionality, so should everything be moved to cryptographically safe? The 
answer is obviously - no. For crypto there are dedicated functions and 
extensions there. Besides that, you see many other people opposing this change. 
An RFC were the way to target the PHP version you want, even 7.0. As for me, 
I'd likely vote yes for master, if the throwing part were replaced. 

Regards

Anatol



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Rowan,

On Wed, Oct 19, 2016 at 5:14 AM, Rowan Collins  wrote:
> On 18/10/2016 20:52, Yasuo Ohgaki wrote:
>>
>> Which is important?
>>   - uniqid() is not unique
>>   - Really broken system that shouldn't be used may emit error
>
>
> Frankly, both are pretty rare cases. From the way you talk about it,
> everybody who uses uniqid() will get duplicate values all the time, when in
> fact, it's incredibly unlikely that anyone will even notice.
>
> I know when I've used it, it's for things like avoiding duplicate id
> attributes on an HTML page, or varying the URL of an asset by adding a token
> to the URL. It's perfectly usable as is for those situations, and I use it
> with full knowledge that it has a small chance of generating colliding
> values.
>
> I'm happy to see it improved, but I don't see any hurry, unless I'm
> completely misunderstanding the chances of seeing collisions.

This is reasonable discussion.

I do use uniqid() for HTML id attributes. It is difficult to detect id
collisions
since it's not server side. (Technically it can, but it requires more resources)
While it works almost always, but it should be better than now. Otherwise,
I'm very uncomfortable with uniqid().

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Rowan Collins]

On 18/10/2016 20:52, Yasuo Ohgaki wrote:

Which is important?
  - uniqid() is not unique
  - Really broken system that shouldn't be used may emit error


Frankly, both are pretty rare cases. From the way you talk about it, 
everybody who uses uniqid() will get duplicate values all the time, when 
in fact, it's incredibly unlikely that anyone will even notice.


I know when I've used it, it's for things like avoiding duplicate id 
attributes on an HTML page, or varying the URL of an asset by adding a 
token to the URL. It's perfectly usable as is for those situations, and 
I use it with full knowledge that it has a small chance of generating 
colliding values.


I'm happy to see it improved, but I don't see any hurry, unless I'm 
completely misunderstanding the chances of seeing collisions.


Regards,

--
Rowan Collins
[IMSoP]


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Kalle and all,

On Wed, Oct 19, 2016 at 1:43 AM, Kalle Sommer Nielsen  wrote:
> 2016-10-18 18:41 GMT+02:00 Anatol Belski :
>> AFM the patch is not acceptable for 7.0. It is true that some place was 
>> moved to the new random int functionality (in password AFAIR). But, it is 
>> done at the place and the way that a BC breach is unlikely. Using the 
>> throwing variant is for sure a BC breach, but also the way pushing while 
>> being explicitly asked to go through an RFC, is inappropriate. As the new 
>> random_* functions are available and allow to implement the best possible 
>> uniqueness in user land, changing the algorithm of the existing uniqid() 
>> doesn't look to have a valid base.
>
> I must add, despite not following the discussion entirely, that it
> should also be approved by the two 7.1 RMs to be committed,
> considering we are in RC4 stage at this point and I don't think we
> should just commit things this late without the RM consent to it.

This is usually I do. You'll see my mails discussing which branches to
merge that is not simple. For almost all bug fixes, I do not see
discussion for merging released branchs.

(Following questions are not for Kalle)

Most bug fixes are not discussed at all here.
What is making this simple bug special?

What's wrong with this simple fix?
What makes this a special requires RFC?
---
The patch committed is pure bug fix.

uniqid() is simply _broken_ because it does not provide expected uniqueness due
to timestamp based php_combined_lcg(). (I added large warning to the manual
recently, though)

unique id (time stamp) + entropy (timestamp based entropy)

Who argue result is reasonably unique?
Who don't use NTP to adjust system time?
---

If any new errors cannot be tolerated with bug fix, are we going to
revert any bug fixes with new error?
Besides, "uniqid() will emit error because it uses /dev/urandom" is
FUD, isn't it?

If there is no reasonable / logical answers for these,
The patch should be included PHP 7.0 and up.


BTW, who really think the patch is offending patch to be merged to
released branches?
Please raise your hand now. I don't think there are many.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Anatol,

On Wed, Oct 19, 2016 at 1:41 AM, Anatol Belski  wrote:
> AFM the patch is not acceptable for 7.0. It is true that some place was moved 
> to the new random int functionality (in password AFAIR). But, it is done at 
> the place and the way that a BC breach is unlikely. Using the throwing 
> variant is for sure a BC breach, but also the way pushing while being 
> explicitly asked to go through an RFC, is inappropriate. As the new random_* 
> functions are available and allow to implement the best possible uniqueness 
> in user land, changing the algorithm of the existing uniqid() doesn't look to 
> have a valid base.
>

Any additional error could be BC. It's the fact.

However, your sentence does not make sense at all.
Do we revert any error emitting bug fix? No, not at all.

We do  add errors as normal bug fix process. Many of them are w/o RFC,
even w/o discussion.

Example: https://bugs.php.net/bug.php?id=73238
This bug fix caused WordPress caused 3 additional E_WARNING displayed
that can be remove by php.ini or code fix.

Which is important?
 - uniqid() is not unique
 - Really broken system that shouldn't be used may emit error

"/dev/urandom cannot read discussion" is FUD and irrelevant to this
discussion. Issues with user script random_bytes() implementation or
like does not apply to uniqid() fix.


Anyway, are we going to revert anything emit new errors from now on
because it's BC?
Are we going to require RFC for this kind of very simple and reasonable fix?
I hope not.

IMHO my discussion is logical. Please consider revert the revert.
Otherwise, we cannot fix even simple bugs.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Kalle Sommer Nielsen]

2016-10-18 18:41 GMT+02:00 Anatol Belski :
> AFM the patch is not acceptable for 7.0. It is true that some place was moved 
> to the new random int functionality (in password AFAIR). But, it is done at 
> the place and the way that a BC breach is unlikely. Using the throwing 
> variant is for sure a BC breach, but also the way pushing while being 
> explicitly asked to go through an RFC, is inappropriate. As the new random_* 
> functions are available and allow to implement the best possible uniqueness 
> in user land, changing the algorithm of the existing uniqid() doesn't look to 
> have a valid base.

I must add, despite not following the discussion entirely, that it
should also be approved by the two 7.1 RMs to be committed,
considering we are in RC4 stage at this point and I don't think we
should just commit things this late without the RM consent to it.

-- 
regards,

Kalle Sommer Nielsen
ka...@php.net

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

RE: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Anatol Belski]

Hi Yasuo,

> -Original Message-
> From: Yasuo Ohgaki [mailto:yohg...@ohgaki.net]
> Sent: Tuesday, October 18, 2016 2:03 PM
> To: Joe Watkins <pthre...@pthreads.org>
> Cc: Niklas Keller <m...@kelunik.com>; Leigh <lei...@gmail.com>; PHP Internals
> <internals@lists.php.net>
> Subject: Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness
> 
> Hi Joe,
> 
> On Tue, Oct 18, 2016 at 8:30 PM, Yasuo Ohgaki <yohg...@ohgaki.net> wrote:
> >
> > On Tue, Oct 18, 2016 at 7:32 PM, Joe Watkins <pthre...@pthreads.org>
> wrote:
> >>> This change should go through the standard RFC process and should be
> >>> targeted at 7.2+ (master) *only*.
> >>
> >>> Please check with the RMs before merging functionality changes into
> >>> release branches. All functionality changes need consent and
> >>> consensus. Bug fixes (that don't change functionality or break BC)
> >>> do not.
> >>
> >> You were told very specifically that the kinds of changes you
> >> proposed here require an RFC.
> >
> > This comment is for original proposal that _changes_ output format, isn't 
> > it?
> >
> > It simply switches entropy source which we already relied on.
> 
> As you can see from last minutes discussion.
> 
> "/dev/urandom cannot be read" is FUD.
> It's pure bug fix. (I intentionally made patch easy to extend used chars, 
> though)
> 
> Would you consider revert the revert?
> 
AFM the patch is not acceptable for 7.0. It is true that some place was moved 
to the new random int functionality (in password AFAIR). But, it is done at the 
place and the way that a BC breach is unlikely. Using the throwing variant is 
for sure a BC breach, but also the way pushing while being explicitly asked to 
go through an RFC, is inappropriate. As the new random_* functions are 
available and allow to implement the best possible uniqueness in user land, 
changing the algorithm of the existing uniqid() doesn't look to have a valid 
base.

Thanks.

Anatol


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Niklas Keller]

2016-10-18 14:41 GMT+02:00 Yasuo Ohgaki :

> Hi Niklas,
>
> On Tue, Oct 18, 2016 at 9:33 PM, Niklas Keller  wrote:
> > 2016-10-18 14:12 GMT+02:00 Yasuo Ohgaki :
> >>
> >> Hi Niklas,
> >>
> >> On Tue, Oct 18, 2016 at 9:08 PM, Niklas Keller  wrote:
> >> >>
> >> >> As you can see from last minutes discussion.
> >> >>
> >> >> "/dev/urandom cannot be read" is FUD.
> >> >> It's pure bug fix. (I intentionally made patch easy to extend used
> >> >> chars, though)
> >> >>
> >> >> Would you consider revert the revert?
> >> >
> >> >
> >> > This discussion shows there should be a RFC and a vote. I'd not
> consider
> >> > this a simple bug fix, after all it doesn't really fix it.
> >> >
> >> > If we want to fix it in core, we'd better include an UUID generation
> >> > mechanism than fixing uniq_id.
> >>
> >> UUID like uniqueness is not the subject of uniqid(), isn't it?
> >
> >
> > UUID = Universally Unique Identifier
> > uniqid = Generate a unique ID
> >
> > Where is uniqueness _not_ the subject of uniqid()?
> >
> >>
> >> As I wrote, it's simple bug fix.
> >
> >
> > The issue is that it doesn't fix it. Maybe it band aids. But it doesn't
> fix
> > uniqid.
> >
> > It's exactly why I proposed to better deprecate uniqid. We can do that in
> > 7.2 and provide UUIDs as a standardized and superior alternative.
>
> OK, I understand you prefer to deprecate uniqid(), but I guess
> uniqid() deprecation is less likely to be passed than improving
> uniqid() uniqueness with a little BC.
>
> If you search uniqid() usage, you'll see UUID is too much for many
> usages. uniqid() has it own use cases.
>
> Current uniqid() is not unique at all.


Right, and it's impossible to fix it without breaking BC, because really
fixing it would require more output.


> The patch simply fixes it by
> using proper entropy, no BC basically.
>

It might be fine committing this to master. But as you say, uniqid is
broken and I'd not consider it fixed with just changing the source of
entropy but leaving the output as is.


> What's wrong with this?
>

Committing it directly to a frozen branch is.

Regards, Niklas

---
> The patch committed is pure bug fix.
>
> uniqid() is simply _broken_ because it does not provide expected
> uniqueness due
> to timestamp based php_combined_lcg(). (I added large warning to the manual
> recently, though)
>
> unique id (time stamp) + entropy (timestamp based entropy)
>
> Who argue result is reasonably unique?
> Who don't use NTP to adjust system time?
> ---
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Niklas,

On Tue, Oct 18, 2016 at 9:33 PM, Niklas Keller  wrote:
> 2016-10-18 14:12 GMT+02:00 Yasuo Ohgaki :
>>
>> Hi Niklas,
>>
>> On Tue, Oct 18, 2016 at 9:08 PM, Niklas Keller  wrote:
>> >>
>> >> As you can see from last minutes discussion.
>> >>
>> >> "/dev/urandom cannot be read" is FUD.
>> >> It's pure bug fix. (I intentionally made patch easy to extend used
>> >> chars, though)
>> >>
>> >> Would you consider revert the revert?
>> >
>> >
>> > This discussion shows there should be a RFC and a vote. I'd not consider
>> > this a simple bug fix, after all it doesn't really fix it.
>> >
>> > If we want to fix it in core, we'd better include an UUID generation
>> > mechanism than fixing uniq_id.
>>
>> UUID like uniqueness is not the subject of uniqid(), isn't it?
>
>
> UUID = Universally Unique Identifier
> uniqid = Generate a unique ID
>
> Where is uniqueness _not_ the subject of uniqid()?
>
>>
>> As I wrote, it's simple bug fix.
>
>
> The issue is that it doesn't fix it. Maybe it band aids. But it doesn't fix
> uniqid.
>
> It's exactly why I proposed to better deprecate uniqid. We can do that in
> 7.2 and provide UUIDs as a standardized and superior alternative.

OK, I understand you prefer to deprecate uniqid(), but I guess
uniqid() deprecation is less likely to be passed than improving
uniqid() uniqueness with a little BC.

If you search uniqid() usage, you'll see UUID is too much for many
usages. uniqid() has it own use cases.

Current uniqid() is not unique at all. The patch simply fixes it by
using proper entropy, no BC basically.

What's wrong with this?
---
The patch committed is pure bug fix.

uniqid() is simply _broken_ because it does not provide expected uniqueness due
to timestamp based php_combined_lcg(). (I added large warning to the manual
recently, though)

unique id (time stamp) + entropy (timestamp based entropy)

Who argue result is reasonably unique?
Who don't use NTP to adjust system time?
---

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Niklas Keller]

2016-10-18 14:12 GMT+02:00 Yasuo Ohgaki :

> Hi Niklas,
>
> On Tue, Oct 18, 2016 at 9:08 PM, Niklas Keller  wrote:
> >>
> >> As you can see from last minutes discussion.
> >>
> >> "/dev/urandom cannot be read" is FUD.
> >> It's pure bug fix. (I intentionally made patch easy to extend used
> >> chars, though)
> >>
> >> Would you consider revert the revert?
> >
> >
> > This discussion shows there should be a RFC and a vote. I'd not consider
> > this a simple bug fix, after all it doesn't really fix it.
> >
> > If we want to fix it in core, we'd better include an UUID generation
> > mechanism than fixing uniq_id.
>
> UUID like uniqueness is not the subject of uniqid(), isn't it?
>

UUID = Universally Unique Identifier
uniqid = Generate a unique ID

Where is uniqueness _not_ the subject of uniqid()?


> As I wrote, it's simple bug fix.
>

The issue is that it doesn't fix it. Maybe it band aids. But it doesn't fix
uniqid.

It's exactly why I proposed to better deprecate uniqid. We can do that in
7.2 and provide UUIDs as a standardized and superior alternative.

Regards, Niklas


> ---
> The patch committed is pure bug fix.
>
> uniqid() is simply _broken_ because it does not provide expected
> uniqueness due
> to timestamp based php_combined_lcg(). (I added large warning to the manual
> recently, though)
>
> unique id (time stamp) + entropy (timestamp based entropy)
>
> Who argue result is reasonably unique?
> Who don't use NTP to adjust system time?
> ---
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Niklas,

On Tue, Oct 18, 2016 at 9:08 PM, Niklas Keller  wrote:
>>
>> As you can see from last minutes discussion.
>>
>> "/dev/urandom cannot be read" is FUD.
>> It's pure bug fix. (I intentionally made patch easy to extend used
>> chars, though)
>>
>> Would you consider revert the revert?
>
>
> This discussion shows there should be a RFC and a vote. I'd not consider
> this a simple bug fix, after all it doesn't really fix it.
>
> If we want to fix it in core, we'd better include an UUID generation
> mechanism than fixing uniq_id.

UUID like uniqueness is not the subject of uniqid(), isn't it?

As I wrote, it's simple bug fix.
---
The patch committed is pure bug fix.

uniqid() is simply _broken_ because it does not provide expected uniqueness due
to timestamp based php_combined_lcg(). (I added large warning to the manual
recently, though)

unique id (time stamp) + entropy (timestamp based entropy)

Who argue result is reasonably unique?
Who don't use NTP to adjust system time?
---

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Niklas Keller]

2016-10-18 14:02 GMT+02:00 Yasuo Ohgaki :

> Hi Joe,
>
> On Tue, Oct 18, 2016 at 8:30 PM, Yasuo Ohgaki  wrote:
> >
> > On Tue, Oct 18, 2016 at 7:32 PM, Joe Watkins 
> wrote:
> >>> This change should go through the standard RFC process and should be
> >>> targeted at 7.2+ (master) *only*.
> >>
> >>> Please check with the RMs before merging functionality changes into
> >>> release
> >>> branches. All functionality changes need consent and consensus. Bug
> fixes
> >>> (that don't change functionality or break BC) do not.
> >>
> >> You were told very specifically that the kinds of changes you proposed
> here
> >> require an RFC.
> >
> > This comment is for original proposal that _changes_ output format,
> isn't it?
> >
> > It simply switches entropy source which we already relied on.
>
> As you can see from last minutes discussion.
>
> "/dev/urandom cannot be read" is FUD.
> It's pure bug fix. (I intentionally made patch easy to extend used
> chars, though)
>
> Would you consider revert the revert?
>

This discussion shows there should be a RFC and a vote. I'd not consider
this a simple bug fix, after all it doesn't really fix it.

If we want to fix it in core, we'd better include an UUID generation
mechanism than fixing uniq_id.

Regards, Niklas


> Thanks.
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Joe,

On Tue, Oct 18, 2016 at 8:30 PM, Yasuo Ohgaki  wrote:
>
> On Tue, Oct 18, 2016 at 7:32 PM, Joe Watkins  wrote:
>>> This change should go through the standard RFC process and should be
>>> targeted at 7.2+ (master) *only*.
>>
>>> Please check with the RMs before merging functionality changes into
>>> release
>>> branches. All functionality changes need consent and consensus. Bug fixes
>>> (that don't change functionality or break BC) do not.
>>
>> You were told very specifically that the kinds of changes you proposed here
>> require an RFC.
>
> This comment is for original proposal that _changes_ output format, isn't it?
>
> It simply switches entropy source which we already relied on.

As you can see from last minutes discussion.

"/dev/urandom cannot be read" is FUD.
It's pure bug fix. (I intentionally made patch easy to extend used
chars, though)

Would you consider revert the revert?

Thanks.

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

On Tue, Oct 18, 2016 at 8:47 PM, Lester Caine  wrote:
> On 18/10/16 12:37, Yasuo Ohgaki wrote:
>> The patch committed is pure bug fix.
> https://www.google.co.uk/search?q=%2Fdev%2Furandom+is+not+readable+by+php
>
> Even bug fixes need proper documentation to avoid the WTF !

I'm about to add the doc.

Anyway, this is due to "open_basedir" restriction.
"open_basedir" does not affect php_random_bytes() at all.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Lester Caine]

On 18/10/16 12:37, Yasuo Ohgaki wrote:
> The patch committed is pure bug fix.
https://www.google.co.uk/search?q=%2Fdev%2Furandom+is+not+readable+by+php

Even bug fixes need proper documentation to avoid the WTF !

-- 
Lester Caine - G8HFL
-
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

On Tue, Oct 18, 2016 at 8:00 PM, Lester Caine  wrote:
> On 18/10/16 11:02, Niklas Keller wrote:
>>> 'Suppliers' should perhaps be helped to configure their systems so the
>>> > users can use things, but things like /dev/urandom may need some
>>> > additional notes to help identify problems when frameworks like owncloud
>>> > start throwing errors. As Niklas says it's shared environments where
>>> > this one may bite.
>>> >
>> Just to be clear: I don't argue that those systems are broken, I just say
>> that there is a BC break for those systems and that this has to be
>> documented.
>
> Yes ... and the RFC process is at least part of the documentation.

The patch committed is pure bug fix.

uniqid() is simply _broken_ because it does not provide expected uniqueness due
to timestamp based php_combined_lcg(). (I added large warning to the manual
recently, though)

unique id (time stamp) + entropy (timestamp based entropy)

Who argue result is reasonably unique?
Who don't use NTP to adjust system time?

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Joe,

On Tue, Oct 18, 2016 at 7:32 PM, Joe Watkins  wrote:
>> This change should go through the standard RFC process and should be
>> targeted at 7.2+ (master) *only*.
>
>> Please check with the RMs before merging functionality changes into
>> release
>> branches. All functionality changes need consent and consensus. Bug fixes
>> (that don't change functionality or break BC) do not.
>
> You were told very specifically that the kinds of changes you proposed here
> require an RFC.

This comment is for original proposal that _changes_ output format, isn't it?

It simply switches entropy source which we already relied on.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Lester Caine]

On 18/10/16 11:02, Niklas Keller wrote:
>> 'Suppliers' should perhaps be helped to configure their systems so the
>> > users can use things, but things like /dev/urandom may need some
>> > additional notes to help identify problems when frameworks like owncloud
>> > start throwing errors. As Niklas says it's shared environments where
>> > this one may bite.
>> >
> Just to be clear: I don't argue that those systems are broken, I just say
> that there is a BC break for those systems and that this has to be
> documented.

Yes ... and the RFC process is at least part of the documentation.

-- 
Lester Caine - G8HFL
-
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Joe Watkins]

Morning,

> This change should go through the standard RFC process and should be
> targeted at 7.2+ (master) *only*.

> Please check with the RMs before merging functionality changes into
release
> branches. All functionality changes need consent and consensus. Bug fixes
> (that don't change functionality or break BC) do not.

You were told very specifically that the kinds of changes you proposed here
require an RFC.

You chose to ignore that, and merge an implementation into frozen branches
of PHP.

I have reverted this change.

Do not do that again.

Cheers
Joe

On Tue, Oct 18, 2016 at 8:35 AM, Yasuo Ohgaki  wrote:

> On Tue, Oct 18, 2016 at 4:16 PM, Niklas Keller  wrote:
> > Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016, 08:47:
> >>
> >> Hi Niklas,
> >>
> >> On Tue, Oct 18, 2016 at 3:36 PM, Niklas Keller  wrote:
> >> > Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016,
> 02:21:
> >> >>
> >> >> Hi all,
> >> >>
> >> >> I committed this patch that simply use php_random_bytes() w/o any BC.
> >> >
> >> >
> >> > Doesn't this throw now in some environments where /dev/urandom isn't
> >> > readable?
> >>
> >> It could happen, but such system should not be used now a days.
> >
> >
> > Sure, but it did happen that shared hosts block it, noticed during
> > random_compat adoption.
> >
> > You claimed there isn't any BC break.
>
> The line should be
>
> "There is no BC for usable systems"
>
> Any file permission could disturb PHP script execution, couldn't it?
>
> I think it's nothing special for /dev/urandom. User should set up system
> correctly to use PHP. Then there is no BC at all.
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Niklas Keller]

Lester Caine  schrieb am Di., 18. Okt. 2016, 11:42:

> On 18/10/16 08:35, Yasuo Ohgaki wrote:
> >> Sure, but it did happen that shared hosts block it, noticed during
> >> > random_compat adoption.
> >> >
> >> > You claimed there isn't any BC break.
> > The line should be
> >
> > "There is no BC for usable systems"
> >
> > Any file permission could disturb PHP script execution, couldn't it?
> >
> > I think it's nothing special for /dev/urandom. User should set up system
> > correctly to use PHP. Then there is no BC at all.
>
> 'Suppliers' should perhaps be helped to configure their systems so the
> users can use things, but things like /dev/urandom may need some
> additional notes to help identify problems when frameworks like owncloud
> start throwing errors. As Niklas says it's shared environments where
> this one may bite.
>

Just to be clear: I don't argue that those systems are broken, I just say
that there is a BC break for those systems and that this has to be
documented.

Regards, Niklas

--
> Lester Caine - G8HFL
> -
> Contact - http://lsces.co.uk/wiki/?page=contact
> L.S.Caine Electronic Services - http://lsces.co.uk
> EnquirySolve - http://enquirysolve.com/
> Model Engineers Digital Workshop - http://medw.co.uk
> Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Lester Caine]

On 18/10/16 08:35, Yasuo Ohgaki wrote:
>> Sure, but it did happen that shared hosts block it, noticed during
>> > random_compat adoption.
>> >
>> > You claimed there isn't any BC break.
> The line should be
> 
> "There is no BC for usable systems"
> 
> Any file permission could disturb PHP script execution, couldn't it?
> 
> I think it's nothing special for /dev/urandom. User should set up system
> correctly to use PHP. Then there is no BC at all.

'Suppliers' should perhaps be helped to configure their systems so the
users can use things, but things like /dev/urandom may need some
additional notes to help identify problems when frameworks like owncloud
start throwing errors. As Niklas says it's shared environments where
this one may bite.

-- 
Lester Caine - G8HFL
-
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

On Tue, Oct 18, 2016 at 4:16 PM, Niklas Keller  wrote:
> Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016, 08:47:
>>
>> Hi Niklas,
>>
>> On Tue, Oct 18, 2016 at 3:36 PM, Niklas Keller  wrote:
>> > Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016, 02:21:
>> >>
>> >> Hi all,
>> >>
>> >> I committed this patch that simply use php_random_bytes() w/o any BC.
>> >
>> >
>> > Doesn't this throw now in some environments where /dev/urandom isn't
>> > readable?
>>
>> It could happen, but such system should not be used now a days.
>
>
> Sure, but it did happen that shared hosts block it, noticed during
> random_compat adoption.
>
> You claimed there isn't any BC break.

The line should be

"There is no BC for usable systems"

Any file permission could disturb PHP script execution, couldn't it?

I think it's nothing special for /dev/urandom. User should set up system
correctly to use PHP. Then there is no BC at all.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Niklas Keller]

Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016, 08:47:

> Hi Niklas,
>
> On Tue, Oct 18, 2016 at 3:36 PM, Niklas Keller  wrote:
> > Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016, 02:21:
> >>
> >> Hi all,
> >>
> >> I committed this patch that simply use php_random_bytes() w/o any BC.
> >
> >
> > Doesn't this throw now in some environments where /dev/urandom isn't
> > readable?
>
> It could happen, but such system should not be used now a days.
>

Sure, but it did happen that shared hosts block it, noticed during
random_compat adoption.

You claimed there isn't any BC break.

Regards, Niklas

Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Niklas,

On Tue, Oct 18, 2016 at 3:36 PM, Niklas Keller  wrote:
> Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016, 02:21:
>>
>> Hi all,
>>
>> I committed this patch that simply use php_random_bytes() w/o any BC.
>
>
> Doesn't this throw now in some environments where /dev/urandom isn't
> readable?

It could happen, but such system should not be used now a days.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Niklas Keller]

Yasuo Ohgaki  schrieb am Di., 18. Okt. 2016, 02:21:

> Hi all,
>
> I committed this patch that simply use php_random_bytes() w/o any BC.
>

Doesn't this throw now in some environments where /dev/urandom isn't
readable?

Regards, Niklas

http://git.php.net/?p=php-src.git;a=commitdiff;h=48f1a17886d874dc90867c669481804de90509e8
>
> I thought there is php_random_int(), but it's not.
> So this is one of the best patch for this purpose.
>
> There is bug reports that request stronger uniqueness by default.
> I may address this issue, but I would like to fix other things for the
> time being.
>
> Regards,
>
> --
> Yasuo Ohgaki
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi all,

I committed this patch that simply use php_random_bytes() w/o any BC.

http://git.php.net/?p=php-src.git;a=commitdiff;h=48f1a17886d874dc90867c669481804de90509e8

I thought there is php_random_int(), but it's not.
So this is one of the best patch for this purpose.

There is bug reports that request stronger uniqueness by default.
I may address this issue, but I would like to fix other things for the
time being.

Regards,

--
Yasuo Ohgaki

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Leigh,

On Wed, Oct 5, 2016 at 5:25 PM, Leigh  wrote:
> The list was missed off of Yasuo's replies to me, replying including the
> list

Me too :)

>
> On Wed, 5 Oct 2016 at 01:07 Yasuo Ohgaki  wrote:
>>
>> Hi Leigh,
>>
>> On Tue, Oct 4, 2016 at 7:06 PM, Leigh  wrote:
>> > Since we want to preserve BC
>> >
>> > entropy = random_int(0, );
>> > uniqid = strpprintf(0, "%s%08x%05x.%08d", prefix, sec, usec, entropy);
>>
>> Current entropy is _double_ from php_combined_lcg() and has 10 chars
>> length,
>> has [0-9].[0-9]{8} format.
>>
>> "F"->"d" does not work. It should be something like
>>
>> entropy = (double) random_int(0, 99);
>
>
> No it shouldn't. Don't do this. It is an unnecessary conversion. The fact
> the lcg returns a double is irrelevant. What is relevant is the 8 digits in
> order to maintain BC. The 8 digits you receive from random_int will still be
> higher quality than the 10 you get from the lcg rounded to 8 places.
>
>>
>> uniqid = strpprintf(0, "%s%08x%05x.%08F", prefix, sec, usec,
>> entropy/1);


There is misunderstanding for the format.
The patch is made to be fully compatible with current output.

php_combined_lcg()  produces value between 1 and 0. It is multiplied
by 10, and 8 decimal numbers are used, so additional entropy is
something like

1.23456789 (10 chars)

[yohgaki@dev ~]$ php -v
PHP 5.6.26 (cli) (built: Sep 16 2016 04:36:41)
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies

[yohgaki@dev ~]$ php -r 'var_dump(uniqid(), uniqid("", true));'
string(13) "57f4ce3df2ea5"
string(23) "57f4ce3df2ea81.98781982"

Current uniqid('', true) adds 1 int char + '.' + 8 decimal char.
Tricky format string, but this is what it does.

If we would like to avoid int to double conversion, we may call
php_random_int() twice. Not sure if it's worth or not, though.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Leigh]

The list was missed off of Yasuo's replies to me, replying including the
list

On Wed, 5 Oct 2016 at 01:07 Yasuo Ohgaki  wrote:

> Hi Leigh,
>
> On Tue, Oct 4, 2016 at 7:06 PM, Leigh  wrote:
> > Since we want to preserve BC
> >
> > entropy = random_int(0, );
> > uniqid = strpprintf(0, "%s%08x%05x.%08d", prefix, sec, usec, entropy);
>
> Current entropy is _double_ from php_combined_lcg() and has 10 chars
> length,
> has [0-9].[0-9]{8} format.
>
> "F"->"d" does not work. It should be something like
>
> entropy = (double) random_int(0, 99);
>

No it shouldn't. Don't do this. It is an unnecessary conversion. The fact
the lcg returns a double is irrelevant. What is relevant is the 8 digits in
order to maintain BC. The 8 digits you receive from random_int will still
be higher quality than the 10 you get from the lcg rounded to 8 places.


> uniqid = strpprintf(0, "%s%08x%05x.%08F", prefix, sec, usec,
> entropy/1);
>




On Wed, 5 Oct 2016 at 01:16 Yasuo Ohgaki  wrote:

> On Wed, Oct 5, 2016 at 9:06 AM, Yasuo Ohgaki  wrote:
> > Current entropy is _double_ from php_combined_lcg() and has 10 chars
> length,
> > has [0-9].[0-9]{8} format.
> >
> > "F"->"d" does not work. It should be something like
> >
> > entropy = (double) random_int(0, 99);
> > uniqid = strpprintf(0, "%s%08x%05x.%08F", prefix, sec, usec,
> entropy/1);
>
> Forgot to mention, this code leak more information about PRNG state
> than my patch because php_random_int() copies random binary data into
> long. It's still part of it and exposure of random data shouldn't
> matter, so this is minor issue.
>

I think there is a misunderstanding here. You're using the CSPRNG which is
designed such that the _entire_ output can be made public without you being
able to predict the next result. That is the definition of a CSPRNG. Also
remember this is "output" not "state".

While researching how to implement these CSPRNG functions, I spoke with
real security experts on the subject, they all said the same thing: Use the
system CSPRNG, and yes, it is fine to expose the output directly.

Also if you really are worried (which you shouldn't be), requesting 8
digits from random_int will effectively discard 5 or 37 bits of output
depending on whether you're on a 32 or 64 bit platform. You cannot know the
value of sequential outputs.


> I'll update gist.
> Any more comments?
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Leigh]

On 4 October 2016 at 02:39, Yasuo Ohgaki  wrote:
> Hi Leigh,
>
> On Mon, Oct 3, 2016 at 9:06 PM, Leigh  wrote:
>> I'm curious, did you consider using random_int? It already handles
>> biasing, and you can reduce the repeated calls to random_bytes.
>
> Yes. It seemed it might be slower due to number of retries at first,
> but I realized that it isn't later.
>
> It could be something like
>
> $entropy = random_int(100, 99);
> $entropy[1] = '.';
> $uniqid = timestamp . $entropy;
>
> I don't have particular preference.
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net

Since we want to preserve BC

entropy = random_int(0, );
uniqid = strpprintf(0, "%s%08x%05x.%08d", prefix, sec, usec, entropy);

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi Leigh,

On Mon, Oct 3, 2016 at 9:06 PM, Leigh  wrote:
> I'm curious, did you consider using random_int? It already handles
> biasing, and you can reduce the repeated calls to random_bytes.

Yes. It seemed it might be slower due to number of retries at first,
but I realized that it isn't later.

It could be something like

$entropy = random_int(100, 99);
$entropy[1] = '.';
$uniqid = timestamp . $entropy;

I don't have particular preference.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Leigh]

On 2 October 2016 at 21:03, Yasuo Ohgaki  wrote:
> Hi all,
>
> On Mon, Oct 3, 2016 at 3:56 AM, Yasuo Ohgaki  wrote:
>> Besides improving "more entropy" the default and data, I prepared
>> fully compatible patch to simplify discussion.
>>
>> https://gist.github.com/anonymous/fb615df325d559fa806a265031a06ede
>>
>> I would like to apply this patch from PHP 7.0 branch, then discuss what
>> the default should be.
>>
>> Any comments?
>> If there is no objections, I'll apply this few days later.
>
> Updated patch a little
>
> https://gist.github.com/yohgaki/cbe5431f9d072b57af2883a2b5745195
>
> Exception should not be ignored, but added few lines for this.
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

I'm curious, did you consider using random_int? It already handles
biasing, and you can reduce the repeated calls to random_bytes.

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

[PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi all,

On Mon, Oct 3, 2016 at 3:56 AM, Yasuo Ohgaki  wrote:
> Besides improving "more entropy" the default and data, I prepared
> fully compatible patch to simplify discussion.
>
> https://gist.github.com/anonymous/fb615df325d559fa806a265031a06ede
>
> I would like to apply this patch from PHP 7.0 branch, then discuss what
> the default should be.
>
> Any comments?
> If there is no objections, I'll apply this few days later.

Updated patch a little

https://gist.github.com/yohgaki/cbe5431f9d072b57af2883a2b5745195

Exception should not be ignored, but added few lines for this.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Kalle Sommer Nielsen]

Hi Yasuo

2016-10-02 20:56 GMT+02:00 Yasuo Ohgaki :
> I would like to apply this patch from PHP 7.0 branch, then discuss what
> the default should be.
>
> Any comments?
> If there is no objections, I'll apply this few days later.

If anything this should be considered from 7.1+, I don't think we
should change uniqid() mid life time of a branch, ccing Anatol and
Davey


-- 
regards,

Kalle Sommer Nielsen
ka...@php.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Marco Pivetta]

On Sun, Oct 2, 2016 at 8:56 PM, Yasuo Ohgaki  wrote:

> Besides improving "more entropy" the default and data, I prepared
> fully compatible patch to simplify discussion.
>
> https://gist.github.com/anonymous/fb615df325d559fa806a265031a06ede
>
> I would like to apply this patch from PHP 7.0 branch, then discuss what
> the default should be.
>
> Any comments?
> If there is no objections, I'll apply this few days later.
>

If you need comments on a patch, send a PR?

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

[PHP-DEV] Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

[Yasuo Ohgaki]

Hi all,

On Mon, Sep 12, 2016 at 11:54 AM, Yasuo Ohgaki  wrote:
> This is RFC for improving uniqid() uniqueness.
> https://wiki.php.net/rfc/uniqid
>
> PR
> https://github.com/php/php-src/pull/2123
>
> If there is anything left to discuss, please comment.
>
> Regards,

Besides improving "more entropy" the default and data, I prepared
fully compatible patch to simplify discussion.

https://gist.github.com/anonymous/fb615df325d559fa806a265031a06ede

I would like to apply this patch from PHP 7.0 branch, then discuss what
the default should be.

Any comments?
If there is no objections, I'll apply this few days later.

Regards,

--
Yasuo Ohgaki
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php