See http://seclists.org/oss-sec/2011/q2/632
We are using this code in etc/standard/crypt_blowfish.c
End of the day here for me, so if someone could go through that and
apply the patch plus figure out the BC issues with the $2x$ stuff
discussed here: http://seclists.org/oss-sec/2011/q2/636
I would appreciate it.
-Rasmus
As far as I remember Stas was working on that, Stas?
See http://seclists.org/oss-sec/2011/q2/632
We are using this code in etc/standard/crypt_blowfish.cEnd of the day here for me, so if someone could go through that and
apply the patch plus figure out the BC issues with the $2x$ stuff
discussed here: http://seclists.org/oss-sec/2011/q2/636
I would appreciate it.-Rasmus
--
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
Hi!
As far as I remember Stas was working on that, Stas?
I wasn't yet doing anything as I was waiting for this matter to come to
official resolution (on the list there, it looks like Solar Designer has
not yet decided which road to take) and then have our solution match
whatever is done there. I'll see what is going on there and if there's
an official solution I'll add the patch to match it.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
See http://seclists.org/oss-sec/2011/q2/632
We are using this code in etc/standard/crypt_blowfish.c
I've committed the patch for 5.4/trunk, not sure what to do about 5.3
since there's some BC breakage in the fix for old hashes. See the ML
thread for more details. Any thoughts about if we want this in 5.3?
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
hi!
I did not read the report, do you have the details about the breakage?
It could be acceptable in 5.3.
Hi!
See http://seclists.org/oss-sec/2011/q2/632
We are using this code in etc/standard/crypt_blowfish.cI've committed the patch for 5.4/trunk, not sure what to do about 5.3 since
there's some BC breakage in the fix for old hashes. See the ML thread for
more details. Any thoughts about if we want this in 5.3?Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227--
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
hi!
I did not read the report, do you have the details about the breakage?
It could be acceptable in 5.3.
If the hash changes everybody who stored encrypted passwords or such
using the old format can't verify them anymore.
My suggestion without looking really deep into these things: Change the
default, and an "old_blowfish" for compatibility and advertise it ...
not sure it's the best thing.
johannes
Hi!
See http://seclists.org/oss-sec/2011/q2/632
We are using this code in etc/standard/crypt_blowfish.cI've committed the patch for 5.4/trunk, not sure what to do about 5.3 since
there's some BC breakage in the fix for old hashes. See the ML thread for
more details. Any thoughts about if we want this in 5.3?Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227--
--
Pierre@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
Hi!
If the hash changes everybody who stored encrypted passwords or such
using the old format can't verify them anymore.
The change will be only for 8-bit data though.
My suggestion without looking really deep into these things: Change the
default, and an "old_blowfish" for compatibility and advertise it ...
not sure it's the best thing.
We could add some flag, etc. to it to trigger the old behavior - it
needs to pass 1 as the last parameter to BF_set_key then. That would
probably require some modifications to crypt_blowfish.c as currently
php_crypt_blowfish_rn uses the salt to determine the algorithm.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
hi!
I did not read the report, do you have the details about the breakage?
It could be acceptable in 5.3.
The problem is that if the string has 8-bit chars in it, the hash
ignores certain characters, making the password much less secure and
generating high quantity of collisions. It also makes it incompatible
with OpenBSD hashing, which may be lesser problem, depending on the use,
and likely is, since nobody reported it beforehand.
The solution implemented in the original patch and 5.4 is to fix the new
hash and introduce a new hash type (starting with $2x and not with $2a
as regular blowfish hash) that would use old, buggy hash type. This
allows people that need to have password compatibility to have it at the
cost of modifications on their data (which is possible without knowing
the original passwords since only the prefix changes), while keeping the
algorithm secure.
However, it still has a chance somebody's data won't work after the
update if he had 8-bit data hashed with old crypt(). He would need
either to re-hash or to change prefix from $2a to $2x.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
However, it still has a chance somebody's data won't work after the update if he had 8-bit data hashed with old
crypt(). He would need either to re-hash or to change prefix from $2a to $2x.
IMO that's a fair trade-off; people could even implement this in their app code by replacing "$2a" with "$2x" for a transitional period in the hash if the comparison fails (and then simply re-hash the password again with $2a so it's secure). I'm volunteering to write the necessary code sample for the upgrading notes :p
David
However, it still has a chance somebody's data won't work after the
update if he had 8-bit data hashed with oldcrypt(). He would need
either to re-hash or to change prefix from $2a to $2x.IMO that's a fair trade-off; people could even implement this in their
app code by replacing "$2a" with "$2x" for a transitional period in
the hash if the comparison fails (and then simply re-hash the password
again with $2a so it's secure). I'm volunteering to write the
necessary code sample for the upgrading notes :p
if people read it ... what might happen is that people test when
upgrading (yay!) all tests and all work and then 1% of the users or so
can't login anymore (with an european site for instance where 8bit
characters might happen ...)
johannes
David
However, it still has a chance somebody's data won't work after the
update if he had 8-bit data hashed with oldcrypt(). He would need
either to re-hash or to change prefix from $2a to $2x.IMO that's a fair trade-off; people could even implement this in their
app code by replacing "$2a" with "$2x" for a transitional period in
the hash if the comparison fails (and then simply re-hash the password
again with $2a so it's secure). I'm volunteering to write the
necessary code sample for the upgrading notes :pif people read it ... what might happen is that people test when
upgrading (yay!) all tests and all work and then 1% of the users or so
can't login anymore (with an european site for instance where 8bit
characters might happen ...)
That might happen, but it isn't a critical issue I think since the change does not produce unconsumable hashes or silently corrupt data in some other way. I think you're also overestimating the amount of people using bcrypt for password storage; most people unfortunately still use SHA1s (with or without a salt).
As Stas said though, whatever the upstream implementation uses as a solution should be mirrored by PHP. The alternative would be to introduce a new hash algorithm code that only works in newer versions of PHP, which hurts portability (which is the major selling point of crypt()). Simply "breaking" old hashes (there's not gonna be many of them out there) with the ability to easily and transparently fix it without user interaction in userland code seems like a much better idea to me.
David
2011/6/28 David Zülke david.zuelke@bitextender.com:
However, it still has a chance somebody's data won't work after the
update if he had 8-bit data hashed with oldcrypt(). He would need
either to re-hash or to change prefix from $2a to $2x.IMO that's a fair trade-off; people could even implement this in their
app code by replacing "$2a" with "$2x" for a transitional period in
the hash if the comparison fails (and then simply re-hash the password
again with $2a so it's secure). I'm volunteering to write the
necessary code sample for the upgrading notes :pif people read it ... what might happen is that people test when
upgrading (yay!) all tests and all work and then 1% of the users or so
can't login anymore (with an european site for instance where 8bit
characters might happen ...)That might happen, but it isn't a critical issue I think since the change does not produce unconsumable hashes or silently corrupt data in some other way. I think you're also overestimating the amount of people using bcrypt for password storage; most people unfortunately still use SHA1s (with or without a salt).
As Stas said though, whatever the upstream implementation uses as a solution should be mirrored by PHP. The alternative would be to introduce a new hash algorithm code that only works in newer versions of PHP, which hurts portability (which is the major selling point of
crypt()). Simply "breaking" old hashes (there's not gonna be many of them out there) with the ability to easily and transparently fix it without user interaction in userland code seems like a much better idea to me.David
it would be good if we could communicate this change more than
mentioning in the changelog.
Tyrael