hi,
On Sun, Jun 7, 2009 at 6:00 PM, Solar Designer<so...@openwall.com> wrote:
> Hi,
>
> First of all, thank you for getting this functionality into PHP proper.
Thank for your work, I'm the one who merged your implementation to PHP
(we had a discussion about it btw, per email if you remember :).
> It appears that the file was very slightly out of date. crypt_blowfish
> 1.0.2 additionally made this change:
>
> -#elif defined(__alpha__) || defined(__hppa__)
> +#elif defined(__x86_64__) || defined(__alpha__) || defined(__hppa__)
>
> which improved performance on x86_64. I recommend that you apply the
> change to the copy in PHP as well.
>
> The function php_crypt_gensalt_blowfish_rn() appears to be unused.
> If so, I suggest #if 0'ing it for now.
Thanks for the review and the notice, I'll update the code tomorrow
(5.3+ have it).
> Finally, I'd like to ask whoever made the following change about the
> rationale behind it:
Backward compatibility was the reason. I like to drop this thing in
php6 but it was not possible to do it in a minor release (I discussed
it with Stefan Esser last year). I'm not sure yet how but certainly by
providing another function or changing the API to allow one to disable
the old behavior.
> --- ../crypt_blowfish-1.0.2/crypt_blowfish.c 2006-05-22 23:52:41 +0000
> +++ ext/standard/crypt_blowfish.c 2008-08-14 01:13:18 +0000
> [...]
> @@ -380,6 +387,7 @@
> #define BF_safe_atoi64(dst, src) \
> { \
> tmp = (unsigned char)(src); \
> + if (tmp == '$') break; \
> if ((unsigned int)(tmp -= 0x20) >= 0x60) return -1; \
> tmp = BF_atoi64[tmp]; \
> if (tmp > 63) return -1; \
> @@ -407,6 +415,9 @@ static int BF_decode(BF_word *dst, __CON
> *dptr++ = ((c3 & 0x03) << 6) | c4;
> } while (dptr < end);
>
> + while (dptr < end)
> + *dptr++ = 0;
> +
> return 0;
> }
>
> My understanding is that this "adds support" for salt strings shorter
> than those bcrypt (the password hashing method we're talking about)
> normally requires, but only as long as they're terminated with a dollar
> sign. Why is this needed, and is it? Do we really want to encourage
> sloppy programming? I don't think this may support any extra existing
> bcrypt-like hashes, which might have been generated by sloppy
> implementations, because the encodings for newly computed hashes (during
> authentication) would be full-length anyway. Am I missing something?
>
> Thanks again,
>
> Alexander
>
> P.S. For those who don't know yet, my "upstream" version of the code is
> available here:
>
> http://www.openwall.com/crypt/
> http://cvsweb.openwall.com/crypt
Cheers,
--
Pierre
http://blog.thepimp.net | http://www.libgd.org
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
