All, On Tue, Oct 20, 2015 at 11:35 PM, Anatol Belski <anatol....@belski.net> 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
