|
Message-ID: <20160327021121.GA28942@openwall.com> Date: Sun, 27 Mar 2016 05:11:21 +0300 From: Solar Designer <solar@...nwall.com> To: musl@...ts.openwall.com Cc: Timo Teras <timo.teras@....fi> Subject: Re: [PATCH] crypt_blowfish: allow short salt strings Hi Timo, On Fri, Mar 25, 2016 at 02:12:35PM +0200, Timo Ter??s wrote: > assume the remainder of the salt to be zero bits. > --- > src/crypt/crypt_blowfish.c | 2 ++ > 1 file changed, 2 insertions(+) > > Any comments if this makes sense? Sort of, but we shouldn't do it. crypt_blowfish is deliberately strict, and recent versions of it (after the inclusion in musl, though) were cross-tested against OpenBSD's original implementation resulting in the latter becoming stricter as well, including on wrong inputs like this. Prior to that cross-testing, IIRC it was possible for OpenBSD's implementation to leak uninitialized stack space on too-short salts like this. This confirms that too-short salts were never meant to be supported. OpenBSD is upstream for bcrypt. > There seems to be some test suites that even verify that short > salt strings should succeed. > > See: http://bugs.alpinelinux.org/issues/5141 This looks like a script testing PHP's behavior. I vaguely recall PHP relaxing the PHP-embedded crypt_blowfish code like this. I think they shouldn't have. Especially they shouldn't have done that when at the same time (apparently) continuing to detect and prefer the underlying system's bcrypt support whenever that is available. The behavior you're observing on a system with glibc is most likely actually that of PHP's embedded copy of crypt_blowfish, since upstream glibc lacks bcrypt support (or has this changed?) Only some Linux distros with glibc have bcrypt support patched into their glibc. There is one maybe-bug seen in your test results: the "*" return on error. Normally, it would be "*0" or "*1" for crypt_blowfish, and it would never match the salt input. Rich, did musl retain this behavior? Where does the bare "*" come from? The concern here is that a "*" might also be returned when crypt() is called with "*" for the salt input. Then its output would match what may be a stored hash placeholder, where "*" means locked account or an error having occurred when the password was set. We must deny access in such cases, but returning "*" on all errors would grant access. This could be a musl or PHP security bug, if it's indeed as bad as it appears from that test. Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.