[Fusionforge-general] Review request: fixing password salts

Sylvain Beucler - Inria sylvain.beucler at inria.fr
Tue Nov 17 14:55:05 CET 2015


Hi Manuel,

Thanks for the feedback :)
At first glance your salt is truncated since you use bin2hex (4 
bits/char) rather than base64 (6 bits/char).
I decided not to use the password_* function (or the compat PHP 
reimplementation) because we don't want to double-hash the password with 
bcrypt, we already have the unix-compatible crypt-* hash.

Also while we're at it, here's what we did at Savannah in 2010:
http://git.savannah.gnu.org/cgit/savane-cleanup.git/tree/frontend/php/include/account.php
with help from Solar Designer for salt generation and password strength 
check:
http://www.openwall.com/lists/announce/2010/12/10/2

Cheers!
Sylvain

Le 17/11/2015 13:55, Manuel Vacelet a écrit :
> Hi Sylvain,
>
> FWIW, we did the same move on Tuleap a couple of months ago to:
> - leverage on std php password hashing methods
> - use a strong hash for unix based auth (not possible to use bcrypt) 
> with secure PRNG.
>
> https://tuleap.net/plugins/git/tuleap/tuleap/stable?p=tuleap%2Fstable.git&a=blob&h=0b3ffd74d1cb99b826aff7b63743fe98efe52f03&hb=8242716702a19f90e4b9f44d6e3811f0455bac2e&f=src/common/user/Password/StandardPasswordHandler.class.php
> https://tuleap.net/plugins/git/tuleap/tuleap/stable?p=tuleap%2Fstable.git&a=blob&h=4fea66ca8059bdda3459e8ea5db57730cc4fd292&hb=8242716702a19f90e4b9f44d6e3811f0455bac2e&f=src/common/user/RandomNumberGenerator.class.php
>
> Hope this helps!
>
>
> On Mon, Nov 16, 2015 at 4:57 PM, Sylvain Beucler - Inria 
> <sylvain.beucler at inria.fr <mailto:sylvain.beucler at inria.fr>> wrote:
>
>     Hi,
>
>     I had a deeper look at the password generation and noticed that:
>
>     - CRYPT-MD5 salt have a length of 2 while they can have up to 8 chars
>       (meaning you get salt duplicates when you have more than 4K users)
>
>     - Blowfish salt is invalid which results in DES3 hashes in the DB
>
>     - crypt(3) now accepts CRYPT-SHA-256/512 but FusionForge doesn't
>     support it
>       (I suggest we default to SHA-512 in FF 6.1, like basically all
>     GNU/Linux distros)
>
>     The attached short patch hopefully addresses these issues.
>     Test-case included :)
>     It is intended for the stable 6.0 branch, and is
>     security-sensitive, hence I'm requesting your review :)
>
>     Notes: the code may not be optimal as is uses genchr() which
>     requires 5-6x more entropy than needed.
>     Also I didn't modify genchr/util_randnum, which look a bit
>     cryptic. AFAICS these come from Evolvis, so,
>     Thorsten: is it still necessary to roll-out our own PRNG, or did
>     PHP improve since ?
>
>     Cheers!
>     Sylvain
>
>
>     _______________________________________________
>     Fusionforge-general mailing list
>     Fusionforge-general at lists.fusionforge.org
>     <mailto:Fusionforge-general at lists.fusionforge.org>
>     http://lists.fusionforge.org/cgi-bin/mailman/listinfo/fusionforge-general
>
>
>
>
> -- 
> Twitter: @vaceletm

-------------- section suivante --------------
Une pièce jointe HTML a été nettoyée...
URL: <http://lists.fusionforge.org/pipermail/fusionforge-general/attachments/20151117/546152ba/attachment.html>


More information about the Fusionforge-general mailing list