Skip to content
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

Open
rastakajakwanna opened this issue Mar 6, 2023 · 13 comments

Comments

@rastakajakwanna
Copy link

rastakajakwanna commented Mar 6, 2023

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

@rastakajakwanna rastakajakwanna changed the title Raise PBKDF2-HMAC-SHA256 iterations to safer value Increase key derivation PBKDF2-HMAC-SHA256 iterations to safer value Mar 6, 2023
@palant
Copy link

palant commented Mar 25, 2023

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 iocane to improve or going without this dependency here.

@perry-mitchell
Copy link
Member

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.

@perry-mitchell
Copy link
Member

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 :)

@palant
Copy link

palant commented Mar 26, 2023

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.

@perry-mitchell
Copy link
Member

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.

@palant
Copy link

palant commented Mar 26, 2023

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.

@rastakajakwanna
Copy link
Author

rastakajakwanna commented Mar 26, 2023

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?):

[1] The reason why SHA-512 is faster than SHA-256 on 64-bit machines is that has 37.5% less rounds per byte (80 rounds operating on 128 byte blocks) compared to SHA-256 (64 rounds operating on 64 byte blocks), where the operations use 64-bit integer arithmetic.

[2] SHA-512 was selected due to its 64-bit word size, which is friendly to CPU defenders but hinders GPU attackers.

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..

Argon2 is the winner of the 2015 Password Hashing Competition. There are three different versions of the algorithm, and the Argon2id variant should be used, as it provides a balanced approach to resisting both side-channel and GPU-based attacks.

The bcrypt password hashing function should be the second choice for password storage if Argon2id is not available or PBKDF2 is required to achieve FIPS-140 compliance.

scrypt is a password-based key derivation function created by Colin Percival. While new systems should consider Argon2id for password hashing, scrypt should be configured properly when used in legacy systems.

... 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:

  • Short-term: focus on hardening the current implementation by raising the iterations number (prolonging the time of successful match when brute-forcing master passwords), using salt length above standard recommendation (reducing possibility of rainbow tables match), allowing users to change the master password (weak or leaked passwords shall be changed ASAP)
  • Mid-term: switching to PBKDF2-HMAC-SHA512
  • Ideal future: switching to Argon2id when wider devices support is available

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
[2] https://github.com/epixoip/hmac-bcrypt
[3] https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2

@palant
Copy link

palant commented Mar 26, 2023

salt length above standard recommendation

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.

@palant
Copy link

palant commented Mar 26, 2023

but I'm honestly not sure it's worth the effort right now unless someone would contribute it.

I can certainly relate. Back when I did this it was quite some effort.

@rastakajakwanna
Copy link
Author

😄 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.

@ghost
Copy link

ghost commented Dec 15, 2023

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.

@perry-mitchell
Copy link
Member

@rastakajakwanna

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

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.

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

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.

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..

Yes, I concur with your points. Strengthening the current setup and continually looking for the best option overall.

@perry-mitchell
Copy link
Member

@palant

Strong master passwords would help however.

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).

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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants