Hi Anthony, On Tue, Jul 22, 2014 at 11:27 PM, Anthony Ferrara <ircmax...@gmail.com> wrote:
> > However, I don't mind at all to write RFC raising E_NOTICE for > > password_hash() > > with PASSWORD_BCRYPT. > > Awesome, thanks! > I'll write it later. > Although cryptgraphic hash functions are supposed work cryptgraphic way, > but > > many of them are failing. This was in real world and I aware of the risk. > > It's less about being aware of the risk, and more showing that the > example you cited of combining hash functions (to show that's it's OK > to do) was actually changed to stop doing that because it's known to > be insecure. So if anything, by citing that example you're actually > demonstrating what shouldn't be done, rather than giving an example of > "it's ok to do in the real world". > I agree that the SSL implementation was bad for the reason you described it. In fact, it was awful because it reduced hash value(key) space effectively. In practical security (i.e. not science), it would be acceptable as you described "It's ok for stackoverflow, etc". Users choose weak passwords anyway. Even some specialists suggest to use weak passwords. http://www.ibtimes.com/microsoft-research-study-suggests-reusing-passwords-unimportant-accounts-1634380 > > I agree with your discussion and have no objection. I understand that > it's > > correct in > > theory and real world. > > > > Even if I understand the risk, it's acceptable for me to use SHA512 to > > workaround > > password length limit as long as SHA512 and/or blowfish is considered > safe > > by > > specialists. As I wrote, password length limit is decided by app > developers > > and we > > are not the app developer. > > > > (BTW, I do code audit/web security check/education for living, but I'm > not a > > cryptographer) > > For your code, that's fine. For you **personally** to advocate for it, > that's fine. Write blog posts, answer stack-overflow questions, etc, > that's absolutely fine! > > To have a programming language and associated community (which is seen > as a significant authority) to advocate for it is not. To document it > in an official manner in an official context (PHP's documentation), > "understanding the risk" isn't good enough. We shouldn't be > recommending anything that "probably is fine", instead we should be > documenting known good method. Methods that have proofs. > I understand point of your view. Re-invention of crypt feature has history of failures and re-invention/ tweak of crypt feature should not be done in general. I agree. > > >> And a E_NOTICE will prevent nothing. If you *really* want to prevent > >> it, raise a warning and return false from the hash function. But > >> that's not possible without a MASSIVE BC break. Hence why I'm > >> recommending that you open an RFC on it. > > > > > > I'll strongly object returning FALSE for faulty usage. IMHO, it is as > bad as > > using too long > > prefix for PASSWORD_BCRYPT. It could disable authentication completely. > > And that's why this discussion needs to happen. A notice isn't going > to change anything. A significant number of developers turn them off. > Especially in production (which is where long passwords are likely to > occur). It may help a few people, but the majority it wouldn't. And it > would make some people start calling `@password_hash()` to suppress > the notice (because why bother fixing bugs, when we can ignore them!). > Which is **really** bad. > > If you truly want to prevent people doing silly things like > password_hash(SHA512($secret) . $password), then returning false and > raising a warning is the only way that will actually do so. > > How to do so is tricky, as it's a pretty significant BC break, and > will break some running software. But it would prevent the issue that > you've stated you want to prevent. > > So that leaves 2 practical options (IMHO): > > 1. Document the behavior, and don't change code functionality. > 2. Add warning, returning false on overlong password (with documentation). > > I don't like 2 (for a similar reason as you), so that leaves 1. Which > is where we are today, so nothing left to do. > OK. I'll add option keep current behavior. (No errors) > > >> > If we would like to recommend "Just use it", we may consider adding > >> > SHA512 > >> > to password_hash(). > >> > >> As I pointed out above, no. > > > > I'm lost here. Are you going to discuss use of SHA512 for password > hashing > > is > > insecure? Blowfish is suitable for password hashing because of its > > "slowness". > > Isn't it matter of rounds? > > I assumed that you meant bcrypt(sha512(password)). If you mean > crypt-sha512 (aka $6$), why would we want to add a demonstrably less > secure algorithm? > I meant "crypt-sha512 (aka $6$)". Sorry that it wasn't clear. It's less secure for sure. However, it's considered good enough for practical security. Since it is less secure, it would not be PASSWORD_DEFAULT and should be used only when users require it to satisfy application requirement. The reason why I would like to have "crypt-sha512" is to educate average PHP users "Just use password_*()". "If your password is less than 72 bytes, use PASSWORD_BCRYPT. If it is longer than 72 bytes for reasons, use PASSWORD_SHA512 until next standard crypt() algorithm that removes length limit." Although, you would not like to have it, if we have this what would be the suggested rounds? (for now. It would be changed as time goes by) > > If anything, I'd add PBKDF2-SHA512, but considering there are no > public crypt bindings to PBKDF2 (aka a crypt format), I'm **really** > hesitant to do that. > > Yes, Python has one (a crypt format): > https://www.dlitz.net/software/python-pbkdf2/ > Yes, PERL has one: > http://search.cpan.org/~arodland/Crypt-PBKDF2-0.101170/lib/Crypt/PBKDF2.pm > Yes, Ruby has one: https://github.com/nomoon/pbkdf2-crypt > > But note none of those are in core distributions. And all of them are > different (incompatible with one another). > Adding PBKDF2-SHA512 to crypt()/password_*() wouldn't be a good idea for reasons you describe. We may recommend(document) alternative, when it is needed by users/developers. http://jp1.php.net/manual/en/function.openssl-pbkdf2.php http://jp1.php.net/manual/en/function.hash-pbkdf2.php Problem is it's not "password_*()", but recommending possible choice would be acceptable. Suggesting users to use strongest hash algo and proper iterations with these function would be enough. It seems current document is missing "User should use hash stronger than SHA-2 to be considered secure", "$iterations must be at least 2000" (IIRC, it was 2000 in NIST document. A few years have passed since I read it and my memory is vague. Is this valid still in their document? 2000 would be too small now a days.) 1password is using PBKDF2 SHA512 with 10 thousands rounds. http://blog.agilebits.com/2014/03/10/crackers-report-great-news-for-1password-4/ LastPass is using PBKDF2 SHA-1 with 5 thousands rounds. (Weak to GPU crack...) https://helpdesk.lastpass.com/security-options/password-iterations-pbkdf2/ Even if user uses them, they may migrate their password database to password_*() or other new standards with timestamping and few lines of additional codes in the future. Baseline for recommendation would be SHA-512 with at least 10,000 rounds. This might be too slow for embedded environment, though. Do you have suggestion? > We need 72 bytes limit workaround/solution some how, but it seems you are > > objecting possible choices. Most developers need adequate level of > security > > to > > build applications, not perfect security. Not all developers have right > to > > decide > > maximum password length and how password is hashed. (e.g. Adding secret > salt > > is > > still valid protection against stolen password database via SQL > > injection,etc. > > Some organizations require to add custom static slat to user defined > > password.) > > I am objecting to the possible choices, because none of them are good > enough to offset the risk. The risk associated with overlong passwords > is trivially small. The risk to "inventing crypto" (such as with > `crypt(sha512(password))` ) or weakening the case for the rest of > passwords (such as with crypt-sha512) is significantly greater IMHO > then the case where a user's password exceeds 72 characters. Yes, we > could add a notice, but it wouldn't really help the class of users > you're intending it to. And it may wind up hurting some by masking > more serious errors (when the suppress with @). > > So I stick with the only good option that IMHO doesn't hurt users, > which is documentation. > But again, this is why I asked for the RFC. My opinion isn't the only > one that matters. I care about it significantly, and I don't want to > see users hurt. But ultimately there are far more people involved than > just you and me. Putting together an RFC will let the discussion > happen, each of us make our case, and let the community decide. > Even if I prefer to have E_NOTICE error (since I think it's our responsibility for PHP users), resolving this issue as documentation problem is acceptable choice for me also as long as it is precise and users could find out what they should do to adopt their application requirements. > I'm not sure your opinions for > > - password_hash() truncation behavior > > - What to do to accept password longer than 72 bytes > > > > Could you give your opinion for these? > > My opinions (which I've stated above, but will summarize here): > > Truncation behavior: document it. That should be all that's needed. > > What to do to accept passwords longer than 72 bytes: Either put an > upper cap on password length in the application, or don't worry about > it. The chances the first 72 bytes can be guessed, but a further one > is different is EXTREMELY small (you're more likely to win a lottery's > jackpot 5 times, than to do so, based on a quick back-of-envelope > estimation). > > Yes, ideally a hash function would allow for infinitely long > (practically, 2^32-1 would be enough, 2^64-1 would be better) > passwords. But it's not worth weakening a system to defend against it > (by moving to crypt-sha512). And it's not worth suggesting invented > crypto to defend against it (recommending crypt(sha512(password))). > > My recommendation: deal with it today. When scrypt (or the winner of > the current PHC) gets crypt(3) bindings, then we'll pull it in and > make it available to password_hash(). Until then, it's not a > significant enough risk to worry about... I understood your point of view. I'll write it in upcoming RFC. Please feel free to edit it when it is available. My point of view is "Crypt related document/feature should not be cryptic. It should be easy to find out what is happening(i.e. truncation), what users should do for their requirements." Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
