Hi,
First of all, thank you for getting this functionality into PHP proper.
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.
Finally, I'd like to ask whoever made the following change about the
rationale behind it:
--- ../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
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
