End to end encryption, insufficient iterations when deriving master key

RocketChat / Rocket.Chat

The communications platform that puts data protection first.

https://rocket.chat/

Other

40.38k stars 10.49k forks source link

End to end encryption, insufficient iterations when deriving master key #12993

Open larixer opened 5 years ago

larixer commented 5 years ago

Hi There,

Right now according to this code, 1000 iterations is used for PBKDF2 to derive master key for encrypting private RSA key of the user and storing it to the DB: https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-e2e/client/helper.js#L86 Please correct me if I am looking to the wrong place. This weakens protection from offline attacks when adversary has stolen the database and tries to guess user passwords. He can guess password, do 1000 iterations of PBKDF2 to have probable masterkey and then try to decrypt user RSA private key and check if it corresponds to user RSA PublicKey.

According to section 5.1.1.2 of https://pages.nist.gov/800-63-3/sp800-63b.html#sec5 the NIST recommends at least 10000 iterations: https://pages.nist.gov/800-63-3/sp800-63b.html#sec5

geekgonecrazy commented 5 years ago

@rocketchat/core what do you guys think? Any reason not to increase iterations?

rodrigok commented 5 years ago

@vlasenko thanks for your report.

But that is not the case, the password is not stored in anyplace, even encrypted, we only store the private key encrypted using the encrypted password, in that case an attacker don't have access to the encrypted password to try to discover the password.

Does this makes sense?

larixer commented 5 years ago

@rodrigok Yes, and iterations count matters for this case. I have described the attack for this case, when you store private key encrypted on the derived master key in the first post of this issue

rodrigok commented 5 years ago

@vlasenko I missed some parts. Ok, I'll take this in consideration.

larixer commented 5 years ago

@rodrigok Thank you!

rodrigok commented 5 years ago

Just describing a possible solution to migrate the ones using 1k iterations:

Start saving all the keys using 10k
Try to decrypt the key using 10k when loading from database
- If not possible try to decrypt using 1k and if successful encrypt it again using 10k and save to the database

larixer commented 5 years ago

@rodrigok Yes, this migration path looks good to me too.

sampaiodiego commented 5 years ago

looks good.. I'll schedule this to the next release.

GoetheG commented 5 years ago

Are thereany news regarding this matter? Istill don't know whether to use the end-to-end-encryption or not. The apps for desktop and mobile say that it's still in alpha status. When will therebe a beta version? Doesanybody havefurther information?

geekgonecrazy commented 4 years ago

@rodrigok With mobile getting e2e support I imagine we are wanting to move e2e forward in status maybe to beta? Should this be addressed?

rodrigok commented 4 years ago

@geekgonecrazy we will not change the status yet, we need to reevaluate everything and make some changes to promote it to a more stable version. The mobile implementation is just a missing part getting support.