Giter Site home page Giter Site logo

Comments (34)

jpgoldberg avatar jpgoldberg commented on May 17, 2024 91

I work for a competitor of Bitwarden's (1Password), and I hate PBDKF2 as much as anyone (See my PasswordsCon talk from 2013). But I will outline why we are still using PBKDF2-SHA256 despite being fully aware of its limitations, and why we don't put the number of iterations under user control

  1. Highly optimized implementations for all platforms.

    Lots of crypto libraries have highly tuned implementations of PBKDF2-HMAC-SHA256, but few do that for SHA512. One of the annoyances of PBKDF2 is exactly the fact that there are optimizations that mean that you can't just drop one hash algorithm in for another without a major performance cut that goes beyond the mere differences between the hash algorithms themselves. Trying to build that optimizations yourself definitely falls into the "rolling your own crypto"

    What is crucial here is that we don't want to have the defender stuck with a slow implementation of something that the attacker will have a highly optimized implementation for. Attackers will have some advantages in this regard, but we don't want to hand them a bunch of extra advantages over the defender on a platter.

  2. WebASM may be the answer, but not quite yet.

    WebASM appears to be the best route for to get Argon2 in the most troublesome environments. But wasm breaks some of the really nice security guaranties that browsers have built against unsafe evaluation. And it's performance is still spotty. I obviously won't and can't speak for Bitwarden, but we won't be deploying argon2 in wasm until we know it is ready and understand all the gotchas. (Again, PBKDF2 has lots of gotchas that you need to work around, but those are all well known by now.)

  3. Same parameters on different devices.

    If you tune, say, your KDF memory and time hardness for your desktop device, what happens when you need to unlock the same data on an older, cheaper Android device? Sure, you might tolerate a longer unlock time, but is the security gain really worth the cost to your battery?

  4. Diminishing marginal security gain

    Going from 10,000 to 100,000 rounds increases the strength by a factor of 10 for the additional cost of 90,000 rounds. But what happens when we do that again. Increasing your cost by 90,000 rounds a second time brings you to 190,000 rounds (let me round to 200,000). So the same cost increase merely doubles your strength. Do that again by adding 100,000 rounds taking you to 300,000. You are getting less than 1 bit of additional strength. (I talk more about this is in a blog post from 2015.)

    Suppose we start at 100,000 rounds. Adding a single randomly chosen digit to the end of a password will increase its strength by 10x. To get the same effect through increasing rounds you would need to go from 100,000 rounds to 1,000.000 rounds. Ad ding a second randomly chosen digit would have the effect of bumping to 10,000,000 rounds.

    Using a slow/memory-hard hash is crucial, but over-using it is silly, and beyond a certain point it is security theater for geeks who want to feel that they are taking on some pain for extra security when there are far less costly ways to get better security gains.

  5. The people who most want control over the parameters are the ones who least understand the previous point.

With all that said, where I work, we continue to explore successors to PBKDF2-SHA256. I thought we would be able to roll one out before now, but I was wrong. And because I know I can be wrong about such guesses, I am not going to make my guesses of a time line public.

And while I won't speak for Bitwarden, there are two over-all points from the above that may affect their security thinking

  • Dropping in a new KDF is not nearly as easy as it might appear at first
  • The security gain from doing so is not nearly as large it might appear at first

Those two facts should play into decisions about development priorities.

from jslib.

michaelsmoody avatar michaelsmoody commented on May 17, 2024 14

Argon2 is strongly recommended. It is highly resistant to side-channel attacks, and continues to be well regarded. If the choice to continue to use PBKDF2 relating to FIPS, could we at least get the option? Argon2id with t/p=1, and a reasonable M setting (since we're also talking mobile devices here) should be a really good choice. This is the one security-specific change keeping me from switching over a large number of users to Bitwarden from Dashlane (which uses Argon2). Can we help the process?

from jslib.

sneak avatar sneak commented on May 17, 2024 13

Using a slow/memory-hard hash is crucial,

This is really all you needed to write.

from jslib.

sneak avatar sneak commented on May 17, 2024 10

Three important things to note:

  1. Bitwarden is already set up for pluggable KDFs.

  2. I offered to do all the work for free if they'd give me a spec and a guarantee that it would be merged if done to the provided spec.

  3. The server limits the max kdf iterations (even for the current kdf) to an insecure/low value.

I myself switched to using bitwarden_rs, which is compatible with the bitwarden clients. It's in rust and is easy to patch to permit a higher kdf max iteration count, and has the added benefit of not costing anything for use of the server.

from jslib.

sneak avatar sneak commented on May 17, 2024 9

Screen Shot 2019-11-01 at 06 38 40

If we're stuck with SHA256 as you senselessly claim, it would be nice to be able to specify a reasonable number of iterations so that the weaknesses in SHA256 as a PBKDF can be mitigated.

However, that workaround is specifically prohibited by Bitwarden, restricting it to 2M iterations. Modern SHA256 hardware1 can do 22,200,000,000 hashes per watt-second, so a single unit operating at 1000W can bruteforce 11,100,000 passwords per second with the maximum iteration count allowed. The default iteration count is much less, allowing for practical and efficient dictionary attacks with hardware.

Looks like this was previously discussed2, but this isn't a feature request, this is security vulnerability report.

If you're going to insist on SHA256, the iteration count should default to at least 10M, and should permit up to 100M as a configuration option. Really, though, using an appropriate password-to-key derivation function is the correct solution here. Failing that, at least remove the 2M limit which forces SHA256 to an insecure mode.

from jslib.

sneak avatar sneak commented on May 17, 2024 8

Ping @kspearrin? I’m still willing to do this work for free if you specify it so I know my efforts won’t be wasted if I work to the spec.

from jslib.

cscharf avatar cscharf commented on May 17, 2024 7

@sneak (@michaelsmoody, et. all.), we would absolutely accept an update to Bitwarden clients that added an option of using Argon2 to the supported list of algorithms. Please move comments, design discussion topics, package (npm, etc.) decisions/questions, etc. to the community forums topic for the same. Kyle and I will ensure this is on our radar so we can help provide PR reviews, feedback, etc. (updating there as well)

from jslib.

tarcieri avatar tarcieri commented on May 17, 2024 7

I'm not a fan of PBKDF2 (although I do maintain implementations of it alongside Argon2, scrypt, etc). That said, this statement is misleading:

SHA256 is perhaps practically the fastest hash function on earth due to optimizations made for Bitcoin (e.g. cheap ASICs and suchlike).

Bitcoin PoW uses SHA256d, i.e. SHA256(SHA256(x)). ASICs built for solving SHA256d are not useful for solving HMAC-SHA-256 which is the core PRF used by PBKDF2-SHA-256.

Furthermore, ASICs optimized for Bitcoin usage further integrate the structure of Bitcoin block headers, and are very much not generally reusable for anything else other than solving Bitcoin PoW.

Edit: haha wow, @chjj just posted a nearly identical comment at the exact same time I did. Serendipity! 😅

from jslib.

cscharf avatar cscharf commented on May 17, 2024 6

@sneak ,

The discussion forum indicates that as of three days ago, nobody inside of Bitwarden is working on this security issue.

You're absolutely correct, we are not working on this feature request. We are open, available and willing to answer questions, provide feedback and support the community, including you, who have offered to work on this, however. The Bitwarden team is a small (but growing) team with a lot of competing priorities and as a primarily open source organization we are supported in requests such as this by our community involvement which we are exceptionally grateful for. Security is a priority, but we currently already offer the industry standard means of cryptography within our product today, so more advanced or hyper-secure algorithms like Argon2 are considered a feature request/enhancement and not a mission-critical the "world will end" without it kind of thing. We also actively support, remediate and disclose items that come through our HackerOne program (and are starting to evaluate a private bug bounty program with HackerOne as well).

I offered to do the work for free if you'd just specify the project precisely to ensure that the work I do will not be wasted, but that's not happened in a year, neither here nor on the forum.

I'm afraid I'm not clear on your needs in order to get started or to assist in contributing to the Argon2 effort. There are other community members that have also offered and to my knowledge have already started an implementation, I would highly encourage you get with them (such as @michaelsmoody ) to see where they are and how you can help.

If there are specific needs, such as technical direction, specifications, assistance with your development environment, location of certain constructs within the code, application or solution, we have a Gitter channel for developers, we have community forums (which you have also posted to) as well as other resources for getting the assistance you need in developing against the Bitwarden platform.

If you are looking for detailed specifications, requirements or other documents from the Bitwarden team on exactly what to build and how to build it, we honestly don't even do that for our internal development team as we use an agile approach to PoC, test, revise and review cycles. I would highly recommend some rapid prototyping with a draft PR to be open for comments, feedback, etc.

from jslib.

sneak avatar sneak commented on May 17, 2024 5

from @kspearrin in the above linked issue:

I am aware of Argon2 and it's benefits over PBKDF2, however, since bitwarden is a cross-platform application that is written on many different languages/frameworks we must use an algorithm that is a standard. PBKDF2 has native implementations on all platforms that we deal with. Argon2 is relatively young and is not yet widely implemented. If we were to use Argon2 we would have to rely on unproven third-party libraries.

Argon2 is fully specified, and is a standard.

How old does it need to be to be used?

On which platforms does it need to be implemented, and which are as yet “unproven”? What is “proven” in your view: a full test suite?

Please tell us the specific hoops that must be jumped through, and we will jump through them. I am happy to write some comprehensive test suites for all languages/platforms required by Bitwarden, although I am reasonably certain they already exist.

How old must it be? What platforms are required? Are full test vectors “proven” enough for you?

from jslib.

sneak avatar sneak commented on May 17, 2024 5

This is really just a band-aid, though. I am more than happy to do whatever legwork you deem necessary to polish libraries and test suites for whatever languages and platforms you deem necessary to switch to using a modern KDF.

from jslib.

michaelsmoody avatar michaelsmoody commented on May 17, 2024 5

I've created a (planned to be temporary) fork at https://github.com/michaelsmoody/bitwarden-jslib-argon2. If you would like access to help move to Argon2, let me know. The goal is simply to do the work, and create a PR for upstream, rather than being a permanent fork.

from jslib.

chjj avatar chjj commented on May 17, 2024 5

SHA256 is perhaps practically the fastest hash function on earth due to optimizations made for Bitcoin (e.g. cheap ASICs and suchlike)

I think this is slightly misleading. The ASICs used for bitcoin mining are optimized for hashing bitcoin block headers (80 bytes of data / 2 sha256 blocks). It's unlikely they could retrofitted by an end-user to crack KDFs.

There's a ton of esoteric optimizations baked into them specifically for bitcoin: e.g. the block header's nonce is in the second sha256 block, meaning asics are optimized for hashing the first block, saving that state, and incrementing the nonce in the second block (which is pre-padded to 64 bytes), and then hashing it to reach the final state. The first state can be re-used as long as nonce space is available. There's also another round of hashing that needs to be done once this process is completed. This is all implemented in hardware and not really changeable.

Perhaps the actual threat is that a mining hardware vendor may use their expertise to create some kind of chip to crack PBKDF2 (though, I suspect that is not worth their time or energy).

from jslib.

sneak avatar sneak commented on May 17, 2024 4

The discussion forum indicates that as of three days ago, nobody inside of Bitwarden is working on this security issue.

https://community.bitwarden.com/t/switch-to-argon2/350/32?u=sneak

I offered to do the work for free if you'd just specify the project precisely to ensure that the work I do will not be wasted, but that's not happened in a year, neither here nor on the forum.

Could you please give us some hint that Bitwarden-the-company cares about the security of its product? This is a major issue. The current KDF's use of an extremely low iteration count (with the server actively prohibiting a high enough value) is actually a vulnerability that has existed for over a year since I reported it:

bitwarden/server#589

Switching to Argon2 (this issue) would fix it, but so would fixing 589 to just not use an insecure low iteration count for the current KDF.

from jslib.

tycho avatar tycho commented on May 17, 2024 3

I think Argon2 would indeed be a preferred alternative. As mentioned before, KeePassXC and other KeePass variants use it to great effect. It also has some solid tunable parameters to add complexity rules.

from jslib.

michaelsmoody avatar michaelsmoody commented on May 17, 2024 3

@sneak Would you be interested in speaking with me about my efforts to implement libsodium? I would also love to sit and work on a design document for the planned change (I too have asked for some guidance as to what would be accepted, but it basically was: "whatever you create as long as it's properly licensed and coded the same way"). To that end, I'm working on integrating the excellent cross-platform libsodium, and I'm working my way through the flow to determine what would be necessary to allow configurable hashing, password hash conversion (en masse), sync'ing, etc. If you're interested, let me know. The "step one" problem of swapping out the current function calls is straightforward enough, it's the "everything else" that's taking some time to map out.

from jslib.

gstkenpo avatar gstkenpo commented on May 17, 2024 2

https://github.com/bitwarden/jslib/blob/57e49207e9ad57c71576fc487a38513a4d0fe120/src/services/auth.service.ts

const hash = await this.cryptoFunctionService.pbkdf2(key.key, password, 'sha256', 1);

if you look at the last attribute of the pbkdf function, you shell find the hash iterations is ONE and the random salt is the user password.....

from jslib.

kspearrin avatar kspearrin commented on May 17, 2024 2

@sneak This is incorrect. The clients enforce a minimum iteration count. See https://github.com/bitwarden/jslib/blob/master/src/services/crypto.service.ts#L311

from jslib.

richardnpaul avatar richardnpaul commented on May 17, 2024 2

I'm going to quote my post from the forum feature request about this here:

As already noted there are performant variants in all the platforms, except maybe JS for the browser plugin.

Except that I then looked into the WASM performance and it actually seemed reasonable, that is < 1s at the defaults and apparently this can be faster still under LLVM compiled WASM rather than using the older wasm.js, according to the author of the argon2-browser implementation. https://awesomeopensource.com/project/antelle/argon2-browser

Talking about performance, if you’re concerned enough about SHA256 that you have increased the number of iterations to the maximum with the known and warned about impact on performance on slower devices, does it really make sense to cry off about Argon being slow when the users are already taking that penalty with the existing solution?

It’s always worth coming back and reviewing stuff like this, and there seems to be genuine community support to at least implement this change as an option, even if it isn’t the default.

from jslib.

bitwarden-bot avatar bitwarden-bot commented on May 17, 2024 2

Hi @sneak,
We're cleaning up our repositories in preparation for a major reorganization. Issues from last year will be marked as stale and closed after two weeks. If you still need help, comment to let us know and we'll look into it.
Thanks!

from jslib.

orangesunny avatar orangesunny commented on May 17, 2024 2

Just switch to Argon2 🤷

from jslib.

tmikaeld avatar tmikaeld commented on May 17, 2024 1

Maybe it's possible to switch to this library:

https://www.npmjs.com/package/argon2-browser

Since it's already used by another Vault (KeeWeb).

from jslib.

sneak avatar sneak commented on May 17, 2024 1

@cscharf

What about the vulnerability I reported last October?

bitwarden/server#589

Fixing this issue would resolve that one as well. Even if you’re not interested in adding this feature, when do you plan on fixing the vulnerability (CVE-2019-19766)?

from jslib.

antemasqued avatar antemasqued commented on May 17, 2024 1

Using a slow/memory-hard hash is crucial,

This is really all you needed to write.

Hey, please forgive me if this next paragraph is known to you (likely is), but I would hate for people to misread your comment and unknowingly write off KDFs that use computationally intensive hashes. One of the child comments have been linked on HN and is likely to attract people unfamiliar with this area of cryptography.

Onlookers, PDKDF2 and it's variants work with inefficiency as a chosen variable. An excellent writeup and intro to the topic (along with a foray into optimizations) can be found by reading @ctz's writeup here.

from jslib.

kspearrin avatar kspearrin commented on May 17, 2024

This has been discussed before. See:

  1. bitwarden/server#37
  2. https://community.bitwarden.com/t/switch-to-argon2/350

from jslib.

dbalikhin avatar dbalikhin commented on May 17, 2024

Is there any ASIC mining hardware that can be used for password cracking?
Defaulting to 10M may kill Availability mitigating non existent threat?

from jslib.

sneak avatar sneak commented on May 17, 2024

It's making a (poorly named) "password hash" from the prelogin key (which is generated from the master password with the proper iteration count), so it's okay on its face.

HOWEVER, the iteration count is provided by the server!

If the server were to tell the client the iteration count is supposed to be 1 (regardless of the real value), the prelogin key would be brute-forceable, and also the password hash derived therefrom would further be brute-forceable to recover the master password.

If the server were to then log this hash value on login attempt, the server admin could straightforwardly bruteforce the master key and decrypt the entire vault.

The client blindly trusts the kdf iteration count provided by the server. EDIT: Inaccurate. See below.

const preloginResponse = await this.apiService.postPrelogin(new PreloginRequest(email));
if (preloginResponse != null) {
this.kdf = preloginResponse.kdf;
this.kdfIterations = preloginResponse.kdfIterations;

This means that if the server responds to the Prelogin request with an iteration count of 1, the client will provide a representation to the server to log-in that can be easily brute-forced.

const response = await this.apiService.postIdentityToken(request);

At the very minimum, the client needs to be sanity-checking the iteration count in the server's prelogin response here: EDIT: Inaccurate. The sanity check happens inside of makeKey().

@kspearrin Please advise re: security issue.

from jslib.

sneak avatar sneak commented on May 17, 2024

Ahh, I see, you're right - I missed it inside of makeKey(). Still, the minimum of 5000 iterations still means 4.4 million bruteforce attempts per watt-second, or 4.4B per 1000W per second, meaning the entire alphanumeric a-zA-Z0-9 space for an 8 character password can be bruteforced in half a day.

Please increase the minimum by 10-100x. (You will of course have to have the clients opportunistically update first.) Please also increase the server-imposed maximum limit by at least 100x as well.

from jslib.

michaelsmoody avatar michaelsmoody commented on May 17, 2024

As mentioned, I also agree. I'm open to PRs (merge requests) for the fork I created. I've started work to integrate the well-supported libsodium into the Bitwarden source, allowing Argon2 to be used. I'm open to other suggestions if there's a superior alternative implementation (well supported, properly licensed, existing library, etc).

from jslib.

cscharf avatar cscharf commented on May 17, 2024

As far as allowing a higher max iterations for KDF, I'll respond in the other item @sneak , thanks.

from jslib.

indolering avatar indolering commented on May 17, 2024

@jpgoldberg, may I ask what sort of gotcha's you are concerned about with WASM? Perhaps we could talk offline or you could direct me to a post somewhere?

from jslib.

 avatar commented on May 17, 2024

Hello!
Excuse me, i've been interested in this issue for months but I never knew how to code/modify Bitwarden so the PBKDF is more secure.
@sneak @michaelsmoody has anyone been interested still on merging something?

If i look in PRs here https://github.com/bitwarden/jslib/pulls - i don't see any activity on this exactly?

Excuse me if I bothered you, but I have been interested for months now,
and I prefer the default Bitwarden server/client myself.

Edit: Ok so the Bitwarden team isn't working on this, but i still pinged other users maybe?

Last Edit: I checked out BW forums https://community.bitwarden.com/t/switch-to-argon2/350 - only 127 votes as of ~ Aug 2021. It might take some time.

from jslib.

michaelsmoody avatar michaelsmoody commented on May 17, 2024

Ultimately, that's still the thing that I intend to do. I got seriously sidetracked by other things, and bitwarden reorganized their repositories as well. With that said, I haven't forgotten about it, and while @jpgoldberg made entirely valid points about PBKDF2-SHA256 and the iterations, those don't apply to Argon2 at all.

WASM is quite well tested cross-platform, and with the recent deprecation of older platforms for many many reasons (RE: SSL certificates, such as CA/B and their multitudinous changes to lifetimes, LetsEncrypt and their changes, expirations of root certificates, Apple moving older devices to legacy/EOL status, and Microsoft doing the same, etc, etc), the fact is that there are few reasons why not to use it, given that it is now nearly a decade old, with many well-supported, and well-tested implementations.

I'm certainly open to opposition, however.

from jslib.

cscharf avatar cscharf commented on May 17, 2024

While we would normally wait for 2 weeks, on this issue in Github I'm going to go ahead and close it so all future conversation moves to our community contribution forums here. Thanks @orangesunny & @michaelsmoody. My understanding is there would still be interest in this, however the Bitwarden team won't be focusing on the implementation ourselves any time real soon, but are happy to work with any community contributions of the same.

from jslib.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.