[PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Tue, 31 Jul 2012 14:30:02 +0200

hi,

On Tue, Jul 31, 2012 at 2:02 PM, Jonathan Bond-Caron wrote:
> On Wed Jun 27 12:32 PM, Arvids Godjuks wrote:

> a) In terms of 'effort' to break many passwords, there's a benefit to the salt stored in the hash itself.
> It's not 'more secure' but 'better/recommended' since the attacker would need to create a 'rainbow table' for each password it's trying to crack
> Overall, the technique offers better protection.
>
> b) In terms of 'effort' to break a single password, there's **no** benefit to the salt stored in the hash itself.
>
> If you want a single password to be really secure, don't let the attacker know the salt and keep it long:
>
> // no benefit of short salt, ~ same effort required by the attacker
> $password = '234';
> md5($password);
>
> $salt = '1';
> $password = '234';
> md5($salt . $password);
>
> c) The best of both worlds: long private salt (b) + different for every user (a)
> $saltInpassword = $password[0]; // could be random bytes, stored in password like crypt() does
> $salt = 'my-long-private-value-use-all-bytes'. $saltInPassword;
> $password = '234';
> $hash = md5($salt . $password);
>
> This one requires more effort by the attacker since the long salt forces more 'bits/guesses' to pass into md5()
>
> // require even more effort, iterate
> for($i = 0; $i < 1000; $i++)
> $hash = md5($i . $hash);

This is somehow the 1st implementation (part of) of crypt. See
ext/standard for the full code. And md5 is now known to do not be
secure enough. IIRC.

I would not, but really totally not, begin to try to implement our own
little algorithm but rely on standard well tested implementations.
Crypt support blowfish, for example. Anthony also works on supporting
more algos afair.

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

RE: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Jonathan Bond-Caron — Tue, 31 Jul 2012 14:10:03 +0200

On Wed Jun 27 12:32 PM, Arvids Godjuks wrote:
> because at the moment i do not understand how salt stored in the hash itself makes hash more
> secure than an unsalted one.

a) In terms of 'effort' to break many passwords, there's a benefit to the salt stored in the hash itself.
It's not 'more secure' but 'better/recommended' since the attacker would need to create a 'rainbow table' for each password it's trying to crack
Overall, the technique offers better protection.

b) In terms of 'effort' to break a single password, there's **no** benefit to the salt stored in the hash itself.

If you want a single password to be really secure, don't let the attacker know the salt and keep it long:

// no benefit of short salt, ~ same effort required by the attacker
$password = '234';
md5($password);

$salt = '1';
$password = '234';
md5($salt . $password);

c) The best of both worlds: long private salt (b) + different for every user (a)
$saltInpassword = $password[0]; // could be random bytes, stored in password like crypt() does
$salt = 'my-long-private-value-use-all-bytes'. $saltInPassword;
$password = '234';
$hash = md5($salt . $password);

This one requires more effort by the attacker since the long salt forces more 'bits/guesses' to pass into md5()

// require even more effort, iterate
for($i = 0; $i < 1000; $i++)
$hash = md5($i . $hash);

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Thu, 12 Jul 2012 20:40:02 +0200

Pierre,

> I've added a pair of new functions to the RFC and implementation:
> >
> > password_needs_rehash($hash, $algo, array $options = array())
>
> Not totally convinced about that one.

I'm not either. That's why I added the discussion point around it. I can
see it going either way.

> I would prefer a password_rehash
> instead, then a simple comparison is needed to know if an update is
> necessary or not.
>
> > password_get_info($hash)
>
> Soungs good, to get all the details about the given hash.

The *only* problem with this is what happens if the options array contains
unused variables for an algorithm. Say we implement scrypt, and there's an
R variable in the array. If we check a bcrypt algorithm hash (with
PASSWORD_BCRYPT specifically), if the array contains "R", it could throw a
red herring indicating an un-necessary re-hash. So there's some algorithmic
specific information that needs to be compared. Thereby making a
"needs_rehash" function in userland non-trivial. It's still not complex,
but it's not trivial either. Part of me wants to see it as it would be
maintained over the years as new algorithms are introduced, and hence can
compensate for other things that implementing it in userland can't....

In other words, I'm not sure either way...

Anthony

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Thu, 12 Jul 2012 20:00:04 +0200

hi Anthony!

On Mon, Jul 9, 2012 at 5:19 PM, Anthony Ferrara wrote:
> I've added a pair of new functions to the RFC and implementation:
>
> password_needs_rehash($hash, $algo, array $options = array())

Not totally convinced about that one. I would prefer a password_rehash
instead, then a simple comparison is needed to know if an update is
necessary or not.

> password_get_info($hash)

Soungs good, to get all the details about the given hash.

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Mon, 09 Jul 2012 17:30:02 +0200

I've added a pair of new functions to the RFC and implementation:

password_needs_rehash($hash, $algo, array $options = array())
and
password_get_info($hash)

both are reasonably similar and there's a fair bit of overlap. Considering
that password_needs_rehash can be implemented easily in user-land, I'm not
convinced it's needed. Then again, it's easy to implement and shouldn't
pose a maintenance headache, so I'm not sure if it shouldn't be there
either...

What do you think?

Anthony

On Tue, Jul 3, 2012 at 5:24 PM, Anthony Ferrara wrote:

> Richard,
>
> > There is also the case of an app that simple shouldn't run with the
> > single default, but could pick and choose suitable algorithm from a
> > list of defaults, while still honoring whatever is in the .ini file
> > instead of going rogue with some other algorithm.
>
> I disagree there. I think that's up to the application to decide. A
> list of defaults does nothing but needlessly complicate the
> implementation. How is the hash function supposed to determine which
> of the list of defaults to use? Let the application layer choose, and
> pass it in. The current PASSWORD_DEFAULT lives for the sole reason
> that it auto-updates to indicate the most secure algorithm available.
>
> Anthony
>

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Tue, 03 Jul 2012 23:30:02 +0200

Richard,

> There is also the case of an app that simple shouldn't run with the
> single default, but could pick and choose suitable algorithm from a
> list of defaults, while still honoring whatever is in the .ini file
> instead of going rogue with some other algorithm.

I disagree there. I think that's up to the application to decide. A
list of defaults does nothing but needlessly complicate the
implementation. How is the hash function supposed to determine which
of the list of defaults to use? Let the application layer choose, and
pass it in. The current PASSWORD_DEFAULT lives for the sole reason
that it auto-updates to indicate the most secure algorithm available.

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Tue, 03 Jul 2012 14:40:02 +0200

Pierre,
>> I know you didn't like PASSWORD_MOST_SECURE. So what about keeping
>> PASSWORD_DEFAULT as a moving target, documented, and just making the
>> second parameter (algo) to password_hash required? That way users
>> could choose between PASSWORD_BCRYPT and PASSWORD_DEFAULT.
>>
>> That way, over time, PASSWORD_DEFAULT could be updated, and it would
>> be documented that it would change. But it would require them to
>> understand that it could change...
>>
>> Would that satisfy your issues?
>
> Yes.
>
> Using this constant name and clearly document its changing nature is
> fine. The argument being required fully solves my worry about optional
> argument with changing default value.

I've implemented this in my branch. I've also updated the RFC to
indicate such (we may want to expand it a little bit, but it should
suffice for now).

I've also added a bit to the RFC about a policy for updating the
default constant over time. (Indicating for a non-security release,
changing the default must be done via an RFC, how long the algorithm
must be available before being eligible for default, etc).

> Thanks for your efforts and work!

Absolutely!

Thanks,

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Tue, 03 Jul 2012 14:10:03 +0200

hi Anthony,

On Tue, Jul 3, 2012 at 1:53 PM, Anthony Ferrara wrote:
> Pierre,
>
> Getting back to the PASSWORD_DEFAULT discussion...
>
> I know you didn't like PASSWORD_MOST_SECURE. So what about keeping
> PASSWORD_DEFAULT as a moving target, documented, and just making the
> second parameter (algo) to password_hash required? That way users
> could choose between PASSWORD_BCRYPT and PASSWORD_DEFAULT.
>
> That way, over time, PASSWORD_DEFAULT could be updated, and it would
> be documented that it would change. But it would require them to
> understand that it could change...
>
> Would that satisfy your issues?

Yes.

Using this constant name and clearly document its changing nature is
fine. The argument being required fully solves my worry about optional
argument with changing default value.

Thanks for your efforts and work!

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Tue, 03 Jul 2012 14:00:03 +0200

Pierre,

Getting back to the PASSWORD_DEFAULT discussion...

I know you didn't like PASSWORD_MOST_SECURE. So what about keeping
PASSWORD_DEFAULT as a moving target, documented, and just making the
second parameter (algo) to password_hash required? That way users
could choose between PASSWORD_BCRYPT and PASSWORD_DEFAULT.

That way, over time, PASSWORD_DEFAULT could be updated, and it would
be documented that it would change. But it would require them to
understand that it could change...

Would that satisfy your issues?

Thanks,

Anthony

On Wed, Jun 27, 2012 at 8:12 AM, Pierre Joye wrote:
> hi,
>
> On Wed, Jun 27, 2012 at 1:49 PM, Gustavo Lopes wrote:
>> Em Wed, 27 Jun 2012 13:37:50 +0200, Pierre Joye
>> escreveu:
>>
>>
>>> That's exactly what I meant, having a changing default in this may
>>> force code change during php updates. I'm not in favour of having such
>>> default.
>>>
>>
>> This would not require any code changes after updates.
>>
>> As I understand, hashes computed with the old default method could still be
>> checked without any modification as the hash itself stores information about
>> the method.
>
> That's only about one relatively simple use case where only PHP would
> be involved or crypt-like implemenation. For any other and rather
> common cases, it won't. I do not think a default should be implemented
> and actually let the user knows what he uses and what he is doing.
> That's one argument after all and clears all possible caveats.
>
> Cheers,
> --
> Pierre
>
> @pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Tue, 03 Jul 2012 13:50:02 +0200

Pierre,

> Simply by not allowing to change it. If one does not like it, it can
> pass the option value as he wishes.
>
> An ini setting for that sounds wrong to me.

Alright. I've pulled the ini option from the fork, and have updated
the RFC to the same...

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Nikita Popov — Tue, 03 Jul 2012 11:10:03 +0200

On Tue, Jul 3, 2012 at 10:42 AM, Pierre Joye wrote:
> hi!
>
> On Tue, Jul 3, 2012 at 1:23 AM, Anthony Ferrara wrote:
>
>> Well, if not for an ini parameter, what way would you suggest to alter
>> the default bcrypt cost? (seriously, I'm open to suggestions)...
>
> Simply by not allowing to change it. If one does not like it, it can
> pass the option value as he wishes.
>
> An ini setting for that sounds wrong to me.
Agree on this. We don't need any more ini settings. Having an
ini-setting would just force everyone to explicitly specify the load
parameter.

Nikita

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Tue, 03 Jul 2012 10:50:02 +0200

hi!

On Tue, Jul 3, 2012 at 1:23 AM, Anthony Ferrara wrote:

> Well, if not for an ini parameter, what way would you suggest to alter
> the default bcrypt cost? (seriously, I'm open to suggestions)...

Simply by not allowing to change it. If one does not like it, it can
pass the option value as he wishes.

An ini setting for that sounds wrong to me.

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Tue, 03 Jul 2012 01:30:01 +0200

Chris,

> To be honest, a note next to PASSWORD_DEFAULT would be good too.

Ok, I'll add that in shortly.

>>> The API of password_make_salt() seems restrictive. What if other
>>> options are needed in future?
>>
>>
>> Can you give any examples of what options would be needed in the
>> future, or how you would like to see the API?
>
>
> I only have brainstorm thoughts on this, since I don't have a crystal
> ball. What if characters other than a-zA-Z0-9./ should/can be used
> for some PASSWORD_xxx algorithms? What if some seed is needed? What
> if the salt creation algorithm should be swappable due to resource
> usage reasons, etc?

Actually... What about making the `raw_output` parameter a bitmask.
Then provide:

PASSWORD_SALT_CRYPT = 1
PASSWORD_SALT_RAW = 2

Then, in the future you could add a bunch of others PASSWORD_SALT_SOMETHINGELSE.

And you could combine some: PASSWORD_SALT_DEV_RANDOM, PASSWORD_SALT_WAHTEVER...

> Also, do you really need a php.ini parameter? It's yet another
> potential way to attack a system.

Well, if not for an ini parameter, what way would you suggest to alter
the default bcrypt cost? (seriously, I'm open to suggestions)...

Thanks,

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Christopher Jones — Mon, 02 Jul 2012 23:30:02 +0200

On 07/02/2012 01:55 PM, Anthony Ferrara wrote:
> Chris,
>
>> Can you update the RFC (aka future documentation) and make this obvious
>> to an end user?
>
> I just made an update (in the behavior sections). Let me know if
> additional clarification is needed.

To be honest, a note next to PASSWORD_DEFAULT would be good too.

>> The API of password_make_salt() seems restrictive. What if other
>> options are needed in future?
>
> Can you give any examples of what options would be needed in the
> future, or how you would like to see the API?

I only have brainstorm thoughts on this, since I don't have a crystal
ball. What if characters other than a-zA-Z0-9./ should/can be used
for some PASSWORD_xxx algorithms? What if some seed is needed? What
if the salt creation algorithm should be swappable due to resource
usage reasons, etc?

Also, do you really need a php.ini parameter? It's yet another
potential way to attack a system.

Chris

--
christopher.jones@oracle.com
http://twitter.com/#!/ghrd

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Mon, 02 Jul 2012 23:00:05 +0200

Chris,

> Can you update the RFC (aka future documentation) and make this obvious
> to an end user?

I just made an update (in the behavior sections). Let me know if
additional clarification is needed.

> I think PASSWORD_BCRYPT should be an ordinal value, which the new
> library maps to "2y" when bcrypt is called.

That would be fine. The initial goal for mapping the prefix to the
constant was to provide the ability to map hash prefixes to the
argument. That way, we could add user-supplied algorithms and base
everything off the prefix with no additional mapping needed. But now
that's off the table, I think switching back to an ordinal would be
fine (and would pretty up the code a bit)...

> The API of password_make_salt() seems restrictive. What if other
> options are needed in future?

Can you give any examples of what options would be needed in the
future, or how you would like to see the API?

Thanks for the feedback!

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Christopher Jones — Mon, 02 Jul 2012 22:00:03 +0200

On 06/26/2012 08:25 AM, Anthony Ferrara wrote:
> Hello All,
>
> I've taken the conversation of the previous simplified password
> hashing API, and generated a patch and draft RFC for it. The patch
> isn't ready yet (needs review, cleanup and testing), but it's a start.
>
> https://wiki.php.net/rfc/password_hash
>
> Please have a look and comment away!
>
> Thanks,
>
> Anthony
>

Hi Anthony,

I think PASSWORD_BCRYPT should be an ordinal value, which the new
library maps to "2y" when bcrypt is called.

The API of password_make_salt() seems restrictive. What if other
options are needed in future?

Chris

--
christopher.jones@oracle.com
http://twitter.com/#!/ghrd

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Christopher Jones — Mon, 02 Jul 2012 22:00:03 +0200

On 06/27/2012 07:16 AM, Anthony Ferrara wrote:
> Arvids,
>
> On Wed, Jun 27, 2012 at 9:23 AM, Arvids Godjuks
> wrote:
>> Hello.
>>
>> I personally think that using PASSWORD_DEFAULT for algorythm by default is a
>> bad idea. This should be defined by user in the code. Even worse if it is
>> defined by .ini setting - deploy to a remote server and realize that there
>> is a different .ini default that messes up everything. Lessons learned in
>> the past are forgetten fast?
>
> It wouldn't mess up anything. All it would do is change the algorithm
> used by the library when creating new passwords. Existing ones will
> still validate. The new ones will validate on the old server as long
> as that algorithm is supported (could be an issue in a mixed
> environment where there are servers using an older version without
> support for the new method in crypt())...

Hi Anthony,

Can you update the RFC (aka future documentation) and make this obvious
to an end user?

Chris

--
christopher.jones@oracle.com
http://twitter.com/#!/ghrd

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Fri, 29 Jun 2012 09:20:03 +0200

hi Anthony,

On Thu, Jun 28, 2012 at 9:36 PM, Anthony Ferrara wrote:

>> I haven't looked at your patch. But if it has to call another
>> PHP_FuNCTION then it's not good. crypt implementation should be
>> accessible via C.
>
> I've refactored crypt() slightly to expose a PHP_API crypt_execute()
> function that does just about everything except the argument parsing /
> default randomizing.
> https://github.com/ircmaxell/php-src/blob/hash_password/ext/standard/crypt.c

It looks good. I would name it php_crypt instead. _execute means
there is a prepare (well, there is but it is called on init :).

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Thu, 28 Jun 2012 21:40:02 +0200

Johannes,

> I haven't looked at your patch. But if it has to call another
> PHP_FuNCTION then it's not good. crypt implementation should be
> accessible via C.

I've refactored crypt() slightly to expose a PHP_API crypt_execute()
function that does just about everything except the argument parsing /
default randomizing.
https://github.com/ircmaxell/php-src/blob/hash_password/ext/standard/crypt.c

Now that I did that, I adjusted the implementation to call that instead...

>> I don't like the concept of core functions disappearing if they are
>> not implemented. I think that does nothing but make it harder on the
>> developers (now having to inject a function_exists(), etc).
>
> I think it's rather the opposite. In that case the user has no way to
> check whether the function is there without calling it and reacting to
> errors. If the function "disappears" there is a possibility to check.

I've now based this implementation on HAVE_CRYPT. If that's not
defined, neither are these functions...

>> Additionally, since this is a security issue, I think that always
>> defining the function is the better approach. Otherwise, you can wind
>> up in a situation where someone else comes in and implements function
>> password_verify($password, $hash) { return true; }, which would be all
>> sorts of bad... I can see the static linking to the function (instead
>> of the dynamic call that's there), So in this case, I personally think
>> the warning is appropriate.
>
> No, but a simple zend_parse_parameters with "s" modifier should be
> enough?

Well, that was enough for the main parsing. But I had to do a little
bit of copy/paste for the argument array parsing (not comfortable with
it): https://github.com/ircmaxell/php-src/blob/hash_password/ext/standard/password.c#L262

> I don't see what makes password_verify special. If one wants a typesafe
> check one can go for true === password_verify.

I've changed all other parameter based error returns to NULL (even the
out of range checks). But I left password_verify for now as returning
false on any error. I still think this one case is significant enough
to always return false/true instead of a third parameter. But we can
talk about that more...

Thanks,

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Johannes Schlüter — Thu, 28 Jun 2012 11:20:02 +0200

On Wed, 2012-06-27 at 22:00 -0400, Anthony Ferrara wrote:
> Johannes,
>
> > Some comments on the "error behavior" part:
> >
> > E_WARNING - When CRYPT is not included in core (was disabled
> > compile-time, or is listed in disabled_functions declaration)
> >
> > Disabling a different function should have no effect. This is not
> > intuitive. If crypt is a dependency and is not available this function
> > shouldn't be available either.
>
> Hrm. Well, then I guess I could re-implement against crypt internally.
> That would take either a slight re-implementation of the crypt()
> internals, or slight refactoring of the PHP_FUNCTION(crypt) function
> to enable c calls to it (even if it's disabled).

I haven't looked at your patch. But if it has to call another
PHP_FuNCTION then it's not good. crypt implementation should be
accessible via C.

> I don't like the concept of core functions disappearing if they are
> not implemented. I think that does nothing but make it harder on the
> developers (now having to inject a function_exists(), etc).

I think it's rather the opposite. In that case the user has no way to
check whether the function is there without calling it and reacting to
errors. If the function "disappears" there is a possibility to check.

> Additionally, since this is a security issue, I think that always
> defining the function is the better approach. Otherwise, you can wind
> up in a situation where someone else comes in and implements function
> password_verify($password, $hash) { return true; }, which would be all
> sorts of bad... I can see the static linking to the function (instead
> of the dynamic call that's there), So in this case, I personally think
> the warning is appropriate.

Who should be the bad person in that scenario? The developer himself? I
think even the security goes the other way round - deelopers don't do
error checking, so they store an empty password in case of error. If the
function does not exist and is being called the script temrinates and
nothing further ad happens.

> > E_WARNING - When supplied an incorrect number of arguments.
> > E_WARNING - When supplied a non-string first parameter (password)
> >
> > This should follow common semantics of zend_parse_parameters(... "s").
> > i.e. it has to support objects with __toString(). Also other scalars are
> > fine. (if they can be casted to string)
> >
> > E_WARNING - If a non-string salt option is provided
> >
> > As above.
>
> Yeah, I guess that's fair. Is there a macro or function like
> Z_REPRESENTABLE_AS_STRING_P() or something? To make it easier to
> check?

No, but a simple zend_parse_parameters with "s" modifier should be
enough?

> > If any error is raise, false is returned by the function.
> >
> > In http://de.php.net/functions.internal it is documented that internal
> > functions return NULL on error during parameter parsing. New exceptions
> > for that should have a good reason.
>
> The good reason is consistency. Otherwise there would be three return
> values, `null`, `false` and `true` for password_verify(). Therefore, a
> check of `false === password_verify($foo)` would actually fail
> inadvertantly.
>
> My opinion is that it should do what's appropriate. The other 2 I can
> live with returning null (although I disagree with it significantly),
> but password_verify I think really should only ever return a
> boolean...

I don't see what makes password_verify special. If one wants a typesafe
check one can go for true === password_verify.

johannes

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Thu, 28 Jun 2012 10:50:02 +0200

Hi Anthony!

On Thu, Jun 28, 2012 at 4:00 AM, Anthony Ferrara wrote:

> Hrm. Well, then I guess I could re-implement against crypt internally.
> That would take either a slight re-implementation of the crypt()
> internals, or slight refactoring of the PHP_FUNCTION(crypt) function
> to enable c calls to it (even if it's disabled).

We would have to expose the crypt implementation, it is not the case
right now. But you could still use it using func call.

Let me know if you need to expose it and which parts, we will need to
refactor it a bit (which should be a good thing to do anyway).

> I don't like the concept of core functions disappearing if they are
> not implemented.

Well, that's the case for extensions altogether so... :)

However if I remember correctly, I made crypt always available from
php 5.3 and later, for all algorithms.

>> E_WARNING - When supplied an incorrect number of arguments.
>> E_WARNING - When supplied a non-string first parameter (password)

I would not use warnings at all but return false on error. Maybe add
an argument or a function to fetch the error messages.

In case you call other PHP functions, you can push/pop the errors
easily as well.

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Thu, 28 Jun 2012 08:20:10 +0200

hi,

On Thu, Jun 28, 2012 at 12:03 AM, Ángel González wrote:

> Precisely the point of such constant is to allow the applications to
> magically

Right, but not a default argument, which is bad in this case, for the
reasons explained earlier.

> Obviously, any such bump -which I would expect to happen on major releases-
> would hold an entry in the NEWS file explaining that PASSWORD_DEFAULT_HASH

I have no problem with such constant and let the user uses it instead
of a given algo. But then he will do it on purpose and being well
informed about the implications.

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Thu, 28 Jun 2012 04:10:03 +0200

Pierre,

> No, it is exactly one example out of many where changing values are a
> real pain to deal with over the years. We should not have one.

While I completely see your point (and don't disagree with it in
isolation), I also see the counter point of making it easy for people
to use. Knowing anything about algorithms to force the common
developer to make a choice between bcrypt and scrypt is unreasonable
IMHO. It's an implementation detail that they should know, but most
won't. Knowing the intricacies of the different algorithms is
something even most senior devs (who are not active in security at
least) don't understand. I'd rather present them best-case defaults,
and let them make the decision to diverge from them.

With that said, what about a compromise. What if we made the API:

password_hash($password, $algo, array $options = array())

And instead of just making the users choose which algorithm to use, we
provide a "moving" constant called

PASSWORD_MOST_SECURE

Which will be updated every major release (assuming there's an update
to apply) or in extreme circumstances (a serious flaw is found in the
current most secure algorithm, justifying a concern).

That way, people don't have to worry about moving targets, because the
core moves it for them as needed. But the choice has to be made. They
aren't just relying upon the default. And the documentation
surrounding it must indicate that if cross-platform interoperability
is a concern, pick a standard algorithm such as bcrypt and use just
that.

So then, a basic call would be $hash = password_hash($password,
PASSWORD_MOST_SECURE);

It solves both problems, while still being reasonably easy...

Thoughts?

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Thu, 28 Jun 2012 04:10:02 +0200

Johannes,

> Some comments on the "error behavior" part:
>
> E_WARNING - When CRYPT is not included in core (was disabled
> compile-time, or is listed in disabled_functions declaration)
>
> Disabling a different function should have no effect. This is not
> intuitive. If crypt is a dependency and is not available this function
> shouldn't be available either.

Hrm. Well, then I guess I could re-implement against crypt internally.
That would take either a slight re-implementation of the crypt()
internals, or slight refactoring of the PHP_FUNCTION(crypt) function
to enable c calls to it (even if it's disabled).

I don't like the concept of core functions disappearing if they are
not implemented. I think that does nothing but make it harder on the
developers (now having to inject a function_exists(), etc).
Additionally, since this is a security issue, I think that always
defining the function is the better approach. Otherwise, you can wind
up in a situation where someone else comes in and implements function
password_verify($password, $hash) { return true; }, which would be all
sorts of bad... I can see the static linking to the function (instead
of the dynamic call that's there), So in this case, I personally think
the warning is appropriate.

> E_WARNING - When supplied an incorrect number of arguments.
> E_WARNING - When supplied a non-string first parameter (password)
>
> This should follow common semantics of zend_parse_parameters(... "s").
> i.e. it has to support objects with __toString(). Also other scalars are
> fine. (if they can be casted to string)
>
> E_WARNING - If a non-string salt option is provided
>
> As above.

Yeah, I guess that's fair. Is there a macro or function like
Z_REPRESENTABLE_AS_STRING_P() or something? To make it easier to
check?

> If any error is raise, false is returned by the function.
>
> In http://de.php.net/functions.internal it is documented that internal
> functions return NULL on error during parameter parsing. New exceptions
> for that should have a good reason.

The good reason is consistency. Otherwise there would be three return
values, `null`, `false` and `true` for password_verify(). Therefore, a
check of `false === password_verify($foo)` would actually fail
inadvertantly.

My opinion is that it should do what's appropriate. The other 2 I can
live with returning null (although I disagree with it significantly),
but password_verify I think really should only ever return a
boolean...

> These things are all minor and you might consider them bad, but then
> change it globally, not by adding new inconsistencies.

Fair enough...

Thanks,

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Thu, 28 Jun 2012 04:00:02 +0200

Arvids,

On Wed, Jun 27, 2012 at 12:32 PM, Arvids Godjuks
wrote:
> On that note I have only one request - please point me to the good article
> that describes how this thing works (I would prefer one that at least tries
> to explain in simple words) because at the moment i do not understand how
> salt stored in the hash itself makes hash more secure than an unsalted one.

Here are some articles that are worth while:

http://php.net/manual/en/function.crypt.php
http://www.devshed.com/c/a/PHP/Using-the-PHP-Crypt-Function/
http://stackoverflow.com/a/4808616/338665

If more explanation is needed, I can try to expand the RFC a bit more.
But give the new sections a read and let me know what you think.

Thanks,

Anthony

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Ángel González — Thu, 28 Jun 2012 00:10:04 +0200

On 27/06/12 18:13, Pierre Joye wrote:
> Changing default value forces code change if you have to keep a given
> hash, for one obvious side effect.
>
> If you disagree or does not like the idea, that's all fine, but you
> can't really say that it is not an argument (nothing to justify, this
> is a draft and it is being discussed).
>
> Cheers,
Precisely the point of such constant is to allow the applications to
magically
change to use more secure hashes, without needing to parform a recursive
sed in the codebase to change HASH_SHA2015 with HASH_SHA2048.
If you want to be in precise control of the actual hash used in a newer
version
(such as an hereogeneous deployment), you could set it to the lower
denominator
in php.ini.

Obviously, any such bump -which I would expect to happen on major releases-
would hold an entry in the NEWS file explaining that PASSWORD_DEFAULT_HASH
is now md5 by default instead of crc32 (still, I would expect a new hash
to have
been available on several releases -they could easily be added on minor
ones-
before becoming the PASSWORD_DEFAULT_HASH).

Remember that the goal was to make the next-generation password hasing api.
An (almost) foolproof way to make the applications secure. If you expect
them to
timely realise the problems of md5() and go back to change all their
functions,
you will replace the current function with password_hash('password',
SILLY_HASH, ...).

Developers with higher security knowledge (few of them, you'd almost
need to be a
cryptographer yourself) can use the advanced parameters to tweak it to
their needs.

Regards

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Johannes Schlüter — Wed, 27 Jun 2012 18:50:02 +0200

Hi,

On Tue, 2012-06-26 at 11:25 -0400, Anthony Ferrara wrote:
> https://wiki.php.net/rfc/password_hash

Some comments on the "error behavior" part:

E_WARNING - When CRYPT is not included in core (was disabled
compile-time, or is listed in disabled_functions declaration)

Disabling a different function should have no effect. This is not
intuitive. If crypt is a dependency and is not available this function
shouldn't be available either.

E_WARNING - When supplied an incorrect number of arguments.
E_WARNING - When supplied a non-string first parameter (password)

This should follow common semantics of zend_parse_parameters(... "s").
i.e. it has to support objects with __toString(). Also other scalars are
fine. (if they can be casted to string)

E_WARNING - If a non-string salt option is provided

As above.

If any error is raise, false is returned by the function.

In http://de.php.net/functions.internal it is documented that internal
functions return NULL on error during parameter parsing. New exceptions
for that should have a good reason.

These things are all minor and you might consider them bad, but then
change it globally, not by adding new inconsistencies.

johannes

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Arvids Godjuks — Wed, 27 Jun 2012 18:40:01 +0200

On that note I have only one request - please point me to the good article
that describes how this thing works (I would prefer one that at least tries
to explain in simple words) because at the moment i do not understand how
salt stored in the hash itself makes hash more secure than an unsalted one.

Thank you :-)
27.06.2012 14:16 пользователь "Anthony Ferrara"
написал:

> Arvids,
>
> On Wed, Jun 27, 2012 at 9:23 AM, Arvids Godjuks
> wrote:
> > Hello.
> >
> > I personally think that using PASSWORD_DEFAULT for algorythm by default
> is a
> > bad idea. This should be defined by user in the code. Even worse if it is
> > defined by .ini setting - deploy to a remote server and realize that
> there
> > is a different .ini default that messes up everything. Lessons learned in
> > the past are forgetten fast?
>
> It wouldn't mess up anything. All it would do is change the algorithm
> used by the library when creating new passwords. Existing ones will
> still validate. The new ones will validate on the old server as long
> as that algorithm is supported (could be an issue in a mixed
> environment where there are servers using an older version without
> support for the new method in crypt())...
>
> > And the thing I don't get is how do I verify a salted password? I have
> read
> > throught the RFC and what I know about the salts makes me wonder - how da
> > hell I will verify my salted hash if I can't pass the salt to
> > password_verify?
>
> Ah, I think I see the disconnect. crypt() returns the full salt
> information along with everything necessary to hash it (all settings).
> So the generated hash includes the salt, the method, and the cost
> parameter. For example:
>
> var_dump(crypt("rasmuslerdorf", "$2a$07$usesomesillystringfor"));
>
> string(60) "$2a$07$usesomesillystringfore2uDLvp1Ii2e./U9C8sBjqp8I90dH6hi"
>
> So just storing the hash is enough...
>
> > If there is some trick behind, it should be explained in the RFC (and in
> the
> > docs later, because otherwise it makes people WTF?! who are not into
> > cryptography).
>
> That's completely fair. I'll add a section to the RFC about that...
>
> Thanks,
>
> Anthony
>

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Pierre Joye — Wed, 27 Jun 2012 18:20:05 +0200

hi,

On Wed, Jun 27, 2012 at 2:59 PM, Gustavo Lopes wrote:

> You described why people *may* have to, depending on the circumstances --
> for instance, when interoperability in mixed environments is required. No
> one is saying that relying on a default value is appropriate in those
> circumstances, so this argument misses the mark.

No, it is exactly one example out of many where changing values are a
real pain to deal with over the years. We should not have one.

> If this API existed 10 or more years ago and used MD5 as a default, I don't
> see how it could not be used in a forward compatible manner back then --
> seen from the outside there's nothing different about MD5 or other digest
> method except for different parameters (which can be stored together with
> the salt and the method in the result of password_hash()) and digest size.
> And, unsurprisingly, you have no justification on why it could not be made
> forward compatible.

Changing default value forces code change if you have to keep a given
hash, for one obvious side effect.

If you disagree or does not like the idea, that's all fine, but you
can't really say that it is not an argument (nothing to justify, this
is a draft and it is being discussed).

Cheers,
--
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

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

Re: [PHP-DEV] [DRAFT RFC] Adding Simplified Password Hashing API

Anthony Ferrara — Wed, 27 Jun 2012 16:20:03 +0200

Arvids,

On Wed, Jun 27, 2012 at 9:23 AM, Arvids Godjuks
wrote:
> Hello.
>
> I personally think that using PASSWORD_DEFAULT for algorythm by default is a
> bad idea. This should be defined by user in the code. Even worse if it is
> defined by .ini setting - deploy to a remote server and realize that there
> is a different .ini default that messes up everything. Lessons learned in
> the past are forgetten fast?

It wouldn't mess up anything. All it would do is change the algorithm
used by the library when creating new passwords. Existing ones will
still validate. The new ones will validate on the old server as long
as that algorithm is supported (could be an issue in a mixed
environment where there are servers using an older version without
support for the new method in crypt())...

> And the thing I don't get is how do I verify a salted password? I have read
> throught the RFC and what I know about the salts makes me wonder - how da
> hell I will verify my salted hash if I can't pass the salt to
> password_verify?

Ah, I think I see the disconnect. crypt() returns the full salt
information along with everything necessary to hash it (all settings).
So the generated hash includes the salt, the method, and the cost
parameter. For example:

var_dump(crypt("rasmuslerdorf", "$2a$07$usesomesillystringfor"));

string(60) "$2a$07$usesomesillystringfore2uDLvp1Ii2e./U9C8sBjqp8I90dH6hi"

So just storing the hash is enough...

> If there is some trick behind, it should be explained in the RFC (and in the
> docs later, because otherwise it makes people WTF?! who are not into
> cryptography).

That's completely fair. I'll add a section to the RFC about that...

Thanks,

Anthony

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