-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase key derivation PBKDF2-HMAC-SHA256 iterations to safer value #325
Comments
The way I read the code, Buttercup is currently using PBKDF2-HMAC-SHA1. So it would require an even more considerable increase. Using PBKDF2 has a severe downside: while PBKDF2 computation doesn’t get significantly faster on consumer hardware, performance of GPUs used for bruteforcing is still increasing significantly. This race cannot be won, and the only long-term solution is switching to a better algorithm like Argon2. Of course, that requires either |
Hi! Thanks for reaching out, and for the detailed issue. The low iterations is something we're considering changing again, for security's sake, but we've had an uphill battle with poor vault unlock and save performance on older mobile devices. The current value was a concession for them. We will be changing it soon to both be higher and allow for customisation (users can select a higher/lower value). As for PBKDF2 - we're limited here by the devices we support. Consider that buttercup runs on the desktop, in the browser and on mobile devices; at the time when we started this project PBKDF2 was the only viable option. We'd love to switch to argon2 but it lacks (to my knowledge) support in the browser (must use hardware/native libraries) and I've no idea what support looks like within native Android/iOS. If such an upgrade were possible we'd happily take that path. We want buttercup to be both secure and accessible. |
As for missing UI components: we'd happily accept a PR for them. Currently it's just myself maintaining buttercup on the regular, so I'd appreciate any help I can get :) |
Yes, there is no native support for anything other than PBKDF2 which is an issue. I’ve found https://github.com/antelle/argon2-browser to have okay’ish performance in the browser however. Don’t know whether better implementations exist. Alternatively, one can use scrypt which has the advantage of being better supported. I few years ago I looked into scrypt implementations and found https://www.npmjs.com/package/@stablelib/scrypt to be the fastest – performance is on par with native implementations. Both of these algorithms have the advantage that you no longer have to fight that uphill battle. Bruteforcing hardware improves at the same rate as consumer hardware. |
Haven't looked much into scrypt.. but I'm honestly not sure it's worth the effort right now unless someone would contribute it. If it's a safer and better option when compared to PBKDF2 I'd consider it. Buttercup needs a more healthy arrangement in terms of growth behind it for it to continue to improve.. and that's where my focus is, primarily.. at least until something improves in that department. Changing PBKDF2 behaviour is far preferable in terms of workload than switching it out entirely. Also, we're using SHA256 there - https://github.com/perry-mitchell/iocane/blob/master/source/symbols.ts - not SHA1. |
I see, I misunderstood the default algorithm here. |
Hi! Thanks for looking into it. I am professionally an Infrastructure automation (IaC) guy, a combination of DevOps and SRE with some security knowledge background but my Node.js skills are poor and therefore I cannot offer you much more than a bug report and a user opinion, unfortunately. I can, of course, just change the number of iterations and submit that as a PR, but that won't help you much 😄 . I am looking forward to the changes you've mentioned but they don't seem as trivial as just changing the number. User would have to change the configuration together on all clients in order to avoid having the vault records encrypted using a different iterations num. on different devices. But I guess you know that already or it's not an issue, it could be resolved easily and I just don't see that 😃. Considering the speed on various devices, perhaps you could also consider changing the hashing algorithm from HMAC-SHA-256 to HMAC-SHA-512 which shall perform better on 64-bit CPU's (most of the user devices are 64bit nowadays, aren't they?):
Number of OWASP recommended iterations for PBKDF2-HMAC-SHA512 is significantly lower [3] (210k SHA-512 vs. 600k SHA-256). Here are some old but interesting numbers regarding performance. Regarding your discussion, I think that everything is already said in the OWASP cheatsheet..
... and as neither of the above algorithms are the option due to a lack of support or implementation complexity, the best option for optimal support of various portable devices, performance and security seems to me:
I am just very sorry that I cannot help you more than giving you some links for consideration. Honestly, I will be glad for anything! I know how difficult it is to find some time for your open source projects. So, thank you! [1] https://eprint.iacr.org/2010/548.pdf |
Salt length is irrelevant, as are rainbow tables if a per-user salt is used. Strong master passwords would help however. Personally, I arrived at the conclusion that diceware (randomly generated passphrases) are the only reliable way for the general public to arrive on a strong and rememberable password. I certainly agree with your other conclusions however. I didn’t realize that PBKDF2-HMAC-SHA512 performs better than PBKDF2-HMAC-SHA256 on CPUs – switching to it is a no-brainer then, it’s fairly easy and reduces the performance gap by a factor of five immediately. But the complicated part here is really the migration path for existing users. Not only is it necessary to distinguish between different key derivation configurations (something that wasn’t supported until now I think), it’s also necessary to offer an automatic upgrade for existing databases. |
I can certainly relate. Back when I did this it was quite some effort. |
😄 this is mind-blowing now. I didn't recognize your nickname on the first sight. Your article about Bitwarden design flaws let me thinking about Buttercup and led me to submit this issue. Circle closes 😃 Thanks for correcting me regarding the salt length, you are probably right. I based my recommendation on the fact that Iocane and Buttercup are completely opensource, I didn't find the exact way in the source code how the salt is generated (I didn't try hard to find it, though), so there is a possibility that if there is not enough randomness in the salting mechanism there could have been a pre-computed set for the weakest passwords already due to the relatively low iteration number in the Buttercup. |
If you're going to use Argon2, then please don't use the settings recommended by OWASP, since they are intended to be used for password hashing, not key derivation. |
I don't think it'll necessarily have to be this way. We can simply take the last value used and keep deriving at that count, and ensuring all client versions maintain that behaviour is far easier than perhaps any other option. This would allow us to put the control back into the hands of the user, and set a higher default. Users that want a lower count can lower it and expect it to stay the same.
I'm open to such a change, but it requires significant work in iocane. I may migrate that library in-house under the Buttercup name for better visibility/branding, and upgrading the encryption capabilities might be part of that. It wouldn't be terrible, honestly, switching out some small things like derivation round count and hashing algo, but switching derivation algo or encryption standard is scary.. primarily due to cross platform support and performance. Buttercup has been running on its current config for some time without too much issue, using native crypto everywhere. I'm concerned that moving away from native functions will hurt overall performance, and it's a decision I can't make lightly. I am interested in choosing the best option for everyone, but it will be a careful, slow process.
Yes, I concur with your points. Strengthening the current setup and continually looking for the best option overall. |
Yeah, you're right. It's been on my mind to introduce a required master password strength (in the GUI only, not the core library).
Yes, this is the painful part. We need to release support for the new option, let it roll out to all clients and perhaps sit there for some arbitrary period of time before exposing it to migrate to. At some stage it should become the default. But doing this without breaking clients will be tricky. It might just be safest to at best use the new crypto for new vaults and not convert old ones in the case that they're using the vault in some old client. We have to be realistic but the user spread over old versions is still somewhat high. -- Nice to see this issue getting to much attention, and I really appreciate the attention to detail and all the great suggestions and advice. I think for the new year one of my first areas of review will be the derivation increase and option to customise via the desktop app. That'll get us back on track in terms of being somewhat considerate strength-wise. That and a password strength requirement. If any of you happen to have a recommendation on a library/methodology for strength measurements/requirements I'm all ears. Used Dropbox's zxcvbn library some years ago but I'm sure it doesn't make the cut these days. |
Hello,
After the recent breaches to some cloud password managers, I was wondering how strong security against brute-forcing master password is applied in Buttercup and noticed that it wasn't bad until you've weakened it in favor of better performance few years ago (https://github.com/buttercup/buttercup-core/blob/master/source/env/core/constants.ts)
I'd like to request changing the value back to some safer values or making it user configurable.
According to the latest OWASP recommendations it should be set to 600k iterations, compromise number 200-300k should bring enough protection even to weak passwords. OWASP reference: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2
Until the change is effective user shall be encouraged to use longer and more complex passwords or use Diceware approach to the master password.
Thank you
PS: As the current number of iterations is low, I wanted to at least change the master password to something stronger but there
is missing the master password change option in the UI :) buttercup/buttercup-desktop#1046
The text was updated successfully, but these errors were encountered: