Date: Sun, 27 Mar 2016 09:26:35 -0400 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Re: [PATCH] crypt_blowfish: allow short salt strings On Sun, Mar 27, 2016 at 06:30:07AM +0300, Solar Designer wrote: > On Sun, Mar 27, 2016 at 05:11:21AM +0300, Solar Designer wrote: > > 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. > > I just discussed this with Rich. > > Yes, musl's modified crypt_blowfish returns "*" on error. No, this > isn't a security bug in musl as a whole, because that code path is not > reached when the setting argument is "*" as well. So no match. > > However, there's risk for code reuse from musl by other projects, and > for potential cut-down revisions of musl (with only bcrypt left, invoked > unconditionally). So I think a change is needed, either reintroducing > the "*0" / "*1" behavior (my preference) or returning NULL (Rich's > preference) on error like glibc does (and unfortunately crashing older > programs that don't expect this). The reason I prefer returning null is that applications get a specified diagnostic that the provided setting string failed to produce a matchable hash; this allows correct applications to avoid storing an unmatchable hash that would render password authentication impossible (always-failing) and instead retry with different settings. This also seems to be the only reasonable way to runtime-probe for which hashes are supported. Solar has pointed out to me that you can use strlen(result)<13 as a failure condition, and that robust programs may already be doing this, but it seems hackish and it's not documented/specified anywhere. Rich
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.