Re: [PHP-DEV] Password_hash salt generation refactor

[Tom Worster]

On 10/30/15 2:36 PM, Anatol Belski wrote:

Hi Anthony,


-Original Message-
From: Anthony Ferrara [mailto:ircmax...@gmail.com]
Sent: Friday, October 30, 2015 11:58 AM
All,

On Tue, Oct 20, 2015 at 11:35 PM, Anatol Belski 
wrote:

Could php_random_bytes() be extended with a flag that would tell exceptions

to pass? Or maybe exceptions could be moved out from the
php_random_bytes() and thrown directly in the new functions that was
undergoing the RFC but not password_hash() ? I'd be actually for the second
variant.


That ways FAILURE would be returned, but a decision about what to do about

it would be made outside. IMHO it were also useful as the API is intended to be
exported. So for the cases where php_random_bytes() came to use as a library
function outside immediate PHP userspace (for example in SAPI), it should not
throw exceptions from itself.


Do this suggestions sound eligible?



I have created a PR for this: https://github.com/php/php-src/pull/1614

It changes the public API of the internal php_random_bytes function to have a
third "should_throw" parameter, as well as defining two macros:
php_random_bytes_throw(dest, len) and php_random_bytes_silent(dest, len).

If this looks acceptable, it can be pulled into 7.0, and then the move for
password_hash can be done at a later date.


IMHO this looks fine for 7.0. The API is new and was not exported, so no ABI 
breach and no behavior change when used for internal refactoring.


This change to php_random_bytes() should make it easier to migrate 
various old stuff (e.g. Leigh proposed sessions) to the new source, and 
I like it for that.


But silenced failure should probably not be used for anything new and it 
would be good if a developer refactoring an old API would comment how 
the new code processes FAILURE together with a justification. Could 
comments be included in this PR to encourage it? As it stands, this 
change allows hazardous use without mentioning it.


Tom

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

RE: [PHP-DEV] Password_hash salt generation refactor

[Anatol Belski]

Hi Anthony,

> -Original Message-
> From: Anthony Ferrara [mailto:ircmax...@gmail.com]
> Sent: Friday, October 30, 2015 11:58 AM
> To: Anatol Belski 
> Cc: internals@lists.php.net; Kalle Sommer Nielsen 
> Subject: Re: [PHP-DEV] Password_hash salt generation refactor
> 
> All,
> 
> On Tue, Oct 20, 2015 at 11:35 PM, Anatol Belski 
> wrote:
> > Hi Anthony,
> >
> >> -Original Message-
> >> From: Anthony Ferrara [mailto:ircmax...@gmail.com]
> >> Sent: Monday, October 19, 2015 1:00 AM
> >> To: internals@lists.php.net
> >> Subject: [PHP-DEV] Password_hash salt generation refactor
> >>
> >> All,
> >>
> >> With PHP 7 comes random_bytes and random_int. This duplicates some of
> >> the logic internally that password_hash uses to generate its salt.
> >>
> >> I would like to refactor this to unify generation. I've opened a PR
> >> against
> >> master: https://github.com/php/php-src/pull/1585
> >>
> >> I don't feel comfortable pulling against 7 this far into RC status.
> >> Perhaps wait until after it goes gold? Or should this target 7.1?
> >> It's not a big deal in either direction. Though it does add a
> >> side-effect, where if it can't gather enough entropy it will throw an
> >> exception and return failure (where prior it would simply make a "best
> effort".
> >>
> >> Thoughts?
> >>
> > As much as it could be an improvement of the password_hash(), one would
> better avoid any behavior change at this stage. I was thinking about it and 
> came
> to an idea that maybe could work for 7.0 - one should just make
> php_random_bytes() that side effect free.
> >
> > Could php_random_bytes() be extended with a flag that would tell exceptions
> to pass? Or maybe exceptions could be moved out from the
> php_random_bytes() and thrown directly in the new functions that was
> undergoing the RFC but not password_hash() ? I'd be actually for the second
> variant.
> >
> > That ways FAILURE would be returned, but a decision about what to do about
> it would be made outside. IMHO it were also useful as the API is intended to 
> be
> exported. So for the cases where php_random_bytes() came to use as a library
> function outside immediate PHP userspace (for example in SAPI), it should not
> throw exceptions from itself.
> >
> > Do this suggestions sound eligible?
> 
> 
> I have created a PR for this: https://github.com/php/php-src/pull/1614
> 
> It changes the public API of the internal php_random_bytes function to have a
> third "should_throw" parameter, as well as defining two macros:
> php_random_bytes_throw(dest, len) and php_random_bytes_silent(dest, len).
> 
> If this looks acceptable, it can be pulled into 7.0, and then the move for
> password_hash can be done at a later date.
> 
IMHO this looks fine for 7.0. The API is new and was not exported, so no ABI 
breach and no behavior change when used for internal refactoring.

Regards

Anatol


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

Re: [PHP-DEV] Password_hash salt generation refactor

[Anthony Ferrara]

All,

On Tue, Oct 20, 2015 at 11:35 PM, Anatol Belski  wrote:
> Hi Anthony,
>
>> -Original Message-
>> From: Anthony Ferrara [mailto:ircmax...@gmail.com]
>> Sent: Monday, October 19, 2015 1:00 AM
>> To: internals@lists.php.net
>> Subject: [PHP-DEV] Password_hash salt generation refactor
>>
>> All,
>>
>> With PHP 7 comes random_bytes and random_int. This duplicates some of
>> the logic internally that password_hash uses to generate its salt.
>>
>> I would like to refactor this to unify generation. I've opened a PR against
>> master: https://github.com/php/php-src/pull/1585
>>
>> I don't feel comfortable pulling against 7 this far into RC status.
>> Perhaps wait until after it goes gold? Or should this target 7.1? It's not a 
>> big
>> deal in either direction. Though it does add a side-effect, where if it can't
>> gather enough entropy it will throw an exception and return failure (where
>> prior it would simply make a "best effort".
>>
>> Thoughts?
>>
> As much as it could be an improvement of the password_hash(), one would 
> better avoid any behavior change at this stage. I was thinking about it and 
> came to an idea that maybe could work for 7.0 - one should just make 
> php_random_bytes() that side effect free.
>
> Could php_random_bytes() be extended with a flag that would tell exceptions 
> to pass? Or maybe exceptions could be moved out from the php_random_bytes() 
> and thrown directly in the new functions that was undergoing the RFC but not 
> password_hash() ? I'd be actually for the second variant.
>
> That ways FAILURE would be returned, but a decision about what to do about it 
> would be made outside. IMHO it were also useful as the API is intended to be 
> exported. So for the cases where php_random_bytes() came to use as a library 
> function outside immediate PHP userspace (for example in SAPI), it should not 
> throw exceptions from itself.
>
> Do this suggestions sound eligible?


I have created a PR for this: https://github.com/php/php-src/pull/1614

It changes the public API of the internal php_random_bytes function to
have a third "should_throw" parameter, as well as defining two macros:
php_random_bytes_throw(dest, len) and php_random_bytes_silent(dest,
len).

If this looks acceptable, it can be pulled into 7.0, and then the move
for password_hash can be done at a later date.

Thanks

Anthony

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

RE: [PHP-DEV] Password_hash salt generation refactor

[Anatol Belski]

Hi Anthony,

> -Original Message-
> From: Anthony Ferrara [mailto:ircmax...@gmail.com]
> Sent: Monday, October 19, 2015 1:00 AM
> To: internals@lists.php.net
> Subject: [PHP-DEV] Password_hash salt generation refactor
> 
> All,
> 
> With PHP 7 comes random_bytes and random_int. This duplicates some of
> the logic internally that password_hash uses to generate its salt.
> 
> I would like to refactor this to unify generation. I've opened a PR against
> master: https://github.com/php/php-src/pull/1585
> 
> I don't feel comfortable pulling against 7 this far into RC status.
> Perhaps wait until after it goes gold? Or should this target 7.1? It's not a 
> big
> deal in either direction. Though it does add a side-effect, where if it can't
> gather enough entropy it will throw an exception and return failure (where
> prior it would simply make a "best effort".
> 
> Thoughts?
> 
As much as it could be an improvement of the password_hash(), one would better 
avoid any behavior change at this stage. I was thinking about it and came to an 
idea that maybe could work for 7.0 - one should just make php_random_bytes() 
that side effect free.

Could php_random_bytes() be extended with a flag that would tell exceptions to 
pass? Or maybe exceptions could be moved out from the php_random_bytes() and 
thrown directly in the new functions that was undergoing the RFC but not 
password_hash() ? I'd be actually for the second variant. 

That ways FAILURE would be returned, but a decision about what to do about it 
would be made outside. IMHO it were also useful as the API is intended to be 
exported. So for the cases where php_random_bytes() came to use as a library 
function outside immediate PHP userspace (for example in SAPI), it should not 
throw exceptions from itself.

Do this suggestions sound eligible?

Regards

Anatol


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

Re: [PHP-DEV] Password_hash salt generation refactor

[Tom Worster]

On 10/19/15, 6:43 PM, "Ángel González"  wrote:
>Tom Worster wrote:
>
>  I've verified that password_hash() without /dev/urandom can
>  produce systematically predictable salts, repeating a sequence of
>  just two salts. There's nothing statistical involved. Reported in
>  bug 70743.
>  
>  
>  Seems crypt() is similarly afflicted.
>
>
>Only providing two hashes *would* be very worrisome. Note however
>that you are crippling rand() by always resetting the seed (those

My point was to show that password_hash()'s promises of security are
nullified by use of an unrelated API. The docs do not explain how
password_hash() can depend on the use of other functions, like the
old rand() LCG, and there's no reason the user should suspect it.


>two different outputs appear because it is xoring with the same
>sequence). It's also reading unitialized memory in line 155, which
>is probably accounting for the salts being different every time. And
>a change in allocations for the first run being different.
>
>Whereas on a real world usage, even if a new php instance was used
>every time, you would need a GENERATE_SEED() collision. It
>initializes the seed using the timestamp, pid, microseconds and
>thread id (plus any state change caused if different php scripts
>were run).

Security BCPs do not allow arguments of this nature. Security-related
APIs need to be robust in all reasonable circumstances, not just in what
you consider typical circumstances.


>You have a very good point in that it would fail only if
>/dev/urandom does not exist at all, my initial concerns came from
>thinking that it could fail on a low-entropy case, but that won't
>happen on Linux (urandom always works), and even a urandom blocking
>OS (eg. on FreeBSD) wouldn't make it use the fallback code. Seems it
>would only be used if (a) you don't have a /dev/urandom at all or
>(b) it isn't the special file it is supposed to be (eg. it's a
>regular file). In which case you are running a broken OS :)
>So, changing my previous opinion, it'd be acceptable to drop it (I'd
>prefer it being done in the point release, though).

There's no such thing as "a low-entropy case" for our purposes. Once the
system's CSPRNG is seeded it will produce an endless supply of random
bytes. PHP has no alternative but to trust this. Anthony recently made
this point in a humorously obscene manner on Twitter.

/dev/random and /dev/urandom are the same thing on FreeBSD. The situation
on OpenBSD, NetBSD and OS X is similar.

FreeBSD's random device does not block except during a reseeding event.
This happens immediately after boot for a short period but PHP can safely
ignore that. The other causes for a reseeding event are very unlikely (and
have nothing to do with "how much entropy is consumed" or any such
nonsense) 
but if they do happen then the best thing PHP can do is hang and wait for
the result. I believe PHP can take the same attitude on OpenBSD, NetBSD
and 
OS X, although I admit I know less about these.

We do not need to consider low-entropy cases. If we read from urandom and
allow it to block, we should be safe. There is danger in libc arc4random
but that's for another thread #70744.

Tom



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

Re: [PHP-DEV] Password_hash salt generation refactor

[Ángel González]

On 19/10/15 21:43, Scott Arciszewski wrote:

(...)

If you have a keyspace of 2^32 possible output sequences like we do
from rand(), we can say that after 65,536 there is a 50% probability
of finding at least one collision.

It should go without saying, but if users have weak/common password
choices and their salts collide, they will end up with duplicate
bcrypt hashes.

My conclusion is thus: Yes, we do need a CSPRNG here, especially if we
want to encourage the use of the password_* API for any large system.

That assumes that:
(a) rand() has the same internal state on each password generation. Given
that it is seeded once per minit, that suggests they are using something 
like cgi.

Which is not too suitable for large systems.

(b) You have people sharing the same password *and* colliding in the 
same salt bucket.


Supposing that you have an equiprobable 4-byte salt (which is actually 
the case to which
we fall if (a) holds), and 152 million users (the amount of the adobe 
leak, biggest to date)
we would only have collisions on the top-13 passwords… not even 
colliding on 1234 (maybe
relax it to the top-16 or top-25 to account for not being completely 
fairy distributed).Which
are passwords in the really really bad zone, and would be cracked even 
with no help of collisions.
And that's another point, a salt used by two users (ie. finding one 
collision) won't speed you

much in terms of cracking their respective passwords.

Undesirable? Sure. Broken? Not really.


Tom Worster wrote:
I don't follow the logic of this hypothetical. php_rand() is *only* 
relevant to this discussion if PHP can't read /dev/random or 
CryptGenRandom, in which case we know that the state of php_rand()'s 
LCG is not randomized (it's open to tampering) so this statistical 
analysis is not possible.


I've verified that password_hash() without /dev/urandom can produce 
systematically predictable salts, repeating a sequence of just two 
salts. There's nothing statistical involved. Reported in bug 70743.


Seems crypt() is similarly afflicted. 
Only providing two hashes *would* be very worrisome. Note however that 
you are crippling rand() by always resetting the seed (those two 
different outputs appear because it is xoring with the same sequence). 
It's also reading unitialized memory in line 155, which is probably 
accounting for the salts being different every time. And a change in 
allocations for the first run being different.


Whereas on a real world usage, even if a new php instance was used every 
time, you would need a GENERATE_SEED() collision. It initializes the 
seed using the timestamp, pid, microseconds and thread id (plus any 
state change caused if different php scripts were run).



You have a very good point in that it would fail only if /dev/urandom 
does not exist at all, my initial concerns came from thinking that it 
could fail on a low-entropy case, but that won't happen on Linux 
(urandom always works), and even a urandom blocking OS (eg. on FreeBSD) 
wouldn't make it use the fallback code. Seems it would only be used if 
(a) you don't have a /dev/urandom at all or (b) it isn't the special 
file it is supposed to be (eg. it's a regular file). In which case you 
are running a broken OS :)
So, changing my previous opinion, it'd be acceptable to drop it (I'd 
prefer it being done in the point release, though).


Best regards

Re: [PHP-DEV] Password_hash salt generation refactor

[Tom Worster]

On 10/19/15 3:43 PM, Scott Arciszewski wrote:

On Mon, Oct 19, 2015 at 1:00 PM, Chris Riley  wrote:

On 19 October 2015 at 16:22, Tom Worster  wrote:


On 10/18/15 7:39 PM, Ángel González wrote:


Korvin wrote:


+1 for 7.0.x security patch release, best effort sounds scary.


This is a salt. It doesn't need to be cryptographically secure. Using
php_rand()
there should pose no problem.
I would actually include that into the patch (move old lines 154-156
into the
FAILURE if).



A password salt needs to be unique. It does not need to be drawn from a
CSPRNG but that is one of the few ways we can be reasonably confident of
uniqueness (since, as usual, we assume the platform RNG is properly seeded).




A password salt should not be predictable, allowing a salt to potentially
become predictable is a bad idea. Solution is to use a CSPRNG for
generation of salts.


Hi all,

I'd like to weigh in by quantifying the predictability of rand(). If
you're well aware of statistics or versed in cryptography, you might
be able to skip this email. (You might still want to read it to help
verify or challenge my conclusion.)

First, for the sake of generosity, let's assume that rand() is
properly seeded, e.g. by 4 bytes from /dev/urandom, on each invocation
so that the pid + time() issue doesn't come into play. (In reality,
this is very important.)

After seeding rand, there are 2^32 possible outputs. That's a little
over 4 billion, and it's highly unlikely that your website has 4
billion user accounts with a hashed password, so that's good enough
right? Well, not quite.

For a moment, imagine you are in a room with 22 other people born in
the same year as you. What is the probability that any two people in
the room share the exact same birthday? Well, it turns out that the
answer is 50%. We call this the Birthday Problem.

 From studying this problem, we can estimate the number of random
samples to generate a collision from an n-bit keyspace by a simple
formula: G = 2^(n/2). This is called the Birthday Bound.

If you have a keyspace of 2^32 possible output sequences like we do
from rand(), we can say that after 65,536 there is a 50% probability
of finding at least one collision.

It should go without saying, but if users have weak/common password
choices and their salts collide, they will end up with duplicate
bcrypt hashes.

My conclusion is thus: Yes, we do need a CSPRNG here, especially if we
want to encourage the use of the password_* API for any large system.


I don't follow the logic of this hypothetical. php_rand() is *only* 
relevant to this discussion if PHP can't read /dev/random or 
CryptGenRandom, in which case we know that the state of php_rand()'s LCG 
is not randomized (it's open to tampering) so this statistical analysis 
is not possible.


I've verified that password_hash() without /dev/urandom can produce 
systematically predictable salts, repeating a sequence of just two 
salts. There's nothing statistical involved. Reported in bug 70743.


Seems crypt() is similarly afflicted.

Tom


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

Re: [PHP-DEV] Password_hash salt generation refactor

[Scott Arciszewski]

On Mon, Oct 19, 2015 at 1:00 PM, Chris Riley  wrote:
> On 19 October 2015 at 16:22, Tom Worster  wrote:
>
>> On 10/18/15 7:39 PM, Ángel González wrote:
>>
>>> Korvin wrote:
>>>
 +1 for 7.0.x security patch release, best effort sounds scary.

>>> This is a salt. It doesn't need to be cryptographically secure. Using
>>> php_rand()
>>> there should pose no problem.
>>> I would actually include that into the patch (move old lines 154-156
>>> into the
>>> FAILURE if).
>>>
>>
>> A password salt needs to be unique. It does not need to be drawn from a
>> CSPRNG but that is one of the few ways we can be reasonably confident of
>> uniqueness (since, as usual, we assume the platform RNG is properly seeded).
>>
>
>
> A password salt should not be predictable, allowing a salt to potentially
> become predictable is a bad idea. Solution is to use a CSPRNG for
> generation of salts.

Hi all,

I'd like to weigh in by quantifying the predictability of rand(). If
you're well aware of statistics or versed in cryptography, you might
be able to skip this email. (You might still want to read it to help
verify or challenge my conclusion.)

First, for the sake of generosity, let's assume that rand() is
properly seeded, e.g. by 4 bytes from /dev/urandom, on each invocation
so that the pid + time() issue doesn't come into play. (In reality,
this is very important.)

After seeding rand, there are 2^32 possible outputs. That's a little
over 4 billion, and it's highly unlikely that your website has 4
billion user accounts with a hashed password, so that's good enough
right? Well, not quite.

For a moment, imagine you are in a room with 22 other people born in
the same year as you. What is the probability that any two people in
the room share the exact same birthday? Well, it turns out that the
answer is 50%. We call this the Birthday Problem.

>From studying this problem, we can estimate the number of random
samples to generate a collision from an n-bit keyspace by a simple
formula: G = 2^(n/2). This is called the Birthday Bound.

If you have a keyspace of 2^32 possible output sequences like we do
from rand(), we can say that after 65,536 there is a 50% probability
of finding at least one collision.

It should go without saying, but if users have weak/common password
choices and their salts collide, they will end up with duplicate
bcrypt hashes.

My conclusion is thus: Yes, we do need a CSPRNG here, especially if we
want to encourage the use of the password_* API for any large system.

https://en.wikipedia.org/wiki/Birthday_problem
https://en.wikipedia.org/wiki/Birthday_attack
http://eprint.iacr.org/2005/004

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises 

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

Re: [PHP-DEV] Password_hash salt generation refactor

[Chris Riley]

On 19 October 2015 at 16:22, Tom Worster  wrote:

> On 10/18/15 7:39 PM, Ángel González wrote:
>
>> Korvin wrote:
>>
>>> +1 for 7.0.x security patch release, best effort sounds scary.
>>>
>> This is a salt. It doesn't need to be cryptographically secure. Using
>> php_rand()
>> there should pose no problem.
>> I would actually include that into the patch (move old lines 154-156
>> into the
>> FAILURE if).
>>
>
> A password salt needs to be unique. It does not need to be drawn from a
> CSPRNG but that is one of the few ways we can be reasonably confident of
> uniqueness (since, as usual, we assume the platform RNG is properly seeded).
>


A password salt should not be predictable, allowing a salt to potentially
become predictable is a bad idea. Solution is to use a CSPRNG for
generation of salts.

Re: [PHP-DEV] Password_hash salt generation refactor

[Tom Worster]

On 10/18/15 7:39 PM, Ángel González wrote:

Korvin wrote:

+1 for 7.0.x security patch release, best effort sounds scary.

This is a salt. It doesn't need to be cryptographically secure. Using
php_rand()
there should pose no problem.
I would actually include that into the patch (move old lines 154-156
into the
FAILURE if).


A password salt needs to be unique. It does not need to be drawn from a 
CSPRNG but that is one of the few ways we can be reasonably confident of 
uniqueness (since, as usual, we assume the platform RNG is properly seeded).


I can seed php_rand() from my script but, other than using the platform 
RNG, I have no idea how. Or I can let PHP seed it but its algorithm, a 
function of time and PID, shows PHP doesn't know how either.


As PHP's version numbers increase, so should it's rigor in using best 
practices. I've no problem with apps breaking in the 5 -> 7 upgrade if 
they have no access to platform RNG. So doing Anthony's proposed change 
as early as possible in 7.0.x is best.


Tom


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

Re: [PHP-DEV] Password_hash salt generation refactor

[Stanislav Malyshev]

Hi!

> With PHP 7 comes random_bytes and random_int. This duplicates some of
> the logic internally that password_hash uses to generate its salt.
> 
> I would like to refactor this to unify generation. I've opened a PR
> against master: https://github.com/php/php-src/pull/1585
> 
> I don't feel comfortable pulling against 7 this far into RC status.
> Perhaps wait until after it goes gold? Or should this target 7.1? It's

If functionality does not change and it's just internal refactoring not
breaking BC (both language and binary) then it can go into 7.0.x. From
what I can see, it is pretty unintrusive, so I wouldn't mind too much
even getting it into 7.0 but that's on RM to decide. In fact, at least
making php_random_bytes() public API should be in 7.0 as that makes for
much less compatibility problems for extensions later.
Generally speaking, having public random generating function sounds like
a very prudent thing, even if we end up not merging the rest of the
patch into 7.0.

> not a big deal in either direction. Though it does add a side-effect,
> where if it can't gather enough entropy it will throw an exception and
> return failure (where prior it would simply make a "best effort".

>From what I can see, the system that can't return enough random bytes
for what php_random_bytes() wants is deeply fubar, so on this scenario
failing fast is the best option.

-- 
Stas Malyshev
smalys...@gmail.com

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

Re: [PHP-DEV] Password_hash salt generation refactor

[Ángel González]

On 19/10/15 00:59, Anthony Ferrara wrote:

I don't feel comfortable pulling against 7 this far into RC status.
Perhaps wait until after it goes gold? Or should this target 7.1? It's
not a big deal in either direction. Though it does add a side-effect,
where if it can't gather enough entropy it will throw an exception and
return failure (where prior it would simply make a "best effort".

Thoughts?

Anthony

It's a clean patch. It doesn't really seem like a problem pulling it.


Korvin wrote:

+1 for 7.0.x security patch release, best effort sounds scary.
This is a salt. It doesn't need to be cryptographically secure. Using 
php_rand()

there should pose no problem.
I would actually include that into the patch (move old lines 154-156 
into the

FAILURE if).



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

Re: [PHP-DEV] Password_hash salt generation refactor

[Sherif Ramadan]

If I'm understanding this correctly, this change doesn't effect actual
behavior, right? It's just taking advantage of reusing code for
random_bytes / random_int ?

If that is true I don't think it much matters whether the change goes
through 7.0 or 7.1 since it has no real end-user impact.

On Sun, Oct 18, 2015 at 6:59 PM, Anthony Ferrara 
wrote:

> All,
>
> With PHP 7 comes random_bytes and random_int. This duplicates some of
> the logic internally that password_hash uses to generate its salt.
>
> I would like to refactor this to unify generation. I've opened a PR
> against master: https://github.com/php/php-src/pull/1585
>
> I don't feel comfortable pulling against 7 this far into RC status.
> Perhaps wait until after it goes gold? Or should this target 7.1? It's
> not a big deal in either direction. Though it does add a side-effect,
> where if it can't gather enough entropy it will throw an exception and
> return failure (where prior it would simply make a "best effort".
>
> Thoughts?
>
> Anthony
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

Re: [PHP-DEV] Password_hash salt generation refactor

[Korvin Szanto]

+1 for 7.0.x security patch release, best effort sounds scary.
On Sun, Oct 18, 2015 at 4:01 PM Anthony Ferrara  wrote:

> All,
>
> With PHP 7 comes random_bytes and random_int. This duplicates some of
> the logic internally that password_hash uses to generate its salt.
>
> I would like to refactor this to unify generation. I've opened a PR
> against master: https://github.com/php/php-src/pull/1585
>
> I don't feel comfortable pulling against 7 this far into RC status.
> Perhaps wait until after it goes gold? Or should this target 7.1? It's
> not a big deal in either direction. Though it does add a side-effect,
> where if it can't gather enough entropy it will throw an exception and
> return failure (where prior it would simply make a "best effort".
>
> Thoughts?
>
> Anthony
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>