Decrease number of kdf iterations in pfs database

[rasom]

PFS database key is derived via pbkdf which runs 64000 iterations of sha1. This makes Login call quite slow on android devices (probably on iOS too, but it didn't work on iOS).

This PR changes the number of iterations to 3200, and, well, makes it faster. The problem is that from security pov key becomes kind of weaker( currently, that's probably not the case as the same key is used for keystore file and it can be encrypted way faster, so no point in brut-forcing pfs database key at all).

Migration is done using sqlcipher_export command.

[status-github-bot]

Pull Request Checklist

• Have you updated the documentation, if impacted (e.g. docs.status.im)?

[rasom]

On the other hand, we also have realm db on the client side, and this one is encrypted via sha256(random_key + password), where random_key is some random bytes generated for each account and stored in devices keychain/keystore. In case if an attacker has access to db files but doesn't have access to keychain/keystore brut-forcing is kind of useless.

So the question is what should be done here

a. let's decrease the number of iterations to 3200, that's fine for now (still doesn't make too much sense IMO because of account's keystore, the attacker isn't going to brute-force this db now anyway)
b. use the same key as for realm db, or the key which is derived in a similar way, thus skip pbkdf
c. use some completely different scheme for key derivation, but again skipping pbkdf

[mandrigin]

@rasom @cammellos are the messages from PFS-protected chats also stored in the realm DB?

[rasom]

@mandrigin yep i think so, not 100% sure though

[mandrigin]

If the messages plaintexts are stored in Realm anyway, is there a point to protect the SQLite DB better?

[cammellos]

They are, unless the user removes them.

Personally I don't find the argument "other part of the app have lower security" convincing, as argument to reduce current security of the app. if anything we should improve security in the rest of the app, not lowering it.

Performance of slower devices is a fair trade-off so lowering the value it's a choice, removing it completely does not make much sense.

To go into the specifics, there are quite a few cases where having a stronger encryption on this db might just be what prevents a successful attack, even if the rest is compromised.

Mind that this keys are used for data transferred on the wire, locally you always have the option to delete the deciphered text (messages), while ciphertext transferred on the wire anyone can intercept and keep indefinitely.

Say you deleted compromising messages on your phone, and someone has physical access to your phone. If they can break in to realm database, they won't be able to retrieve the compromising messages. But if they have been collecting previous messages (trivial) and they bruteforce the sqlcipher db then they have access to the compromising texts.

Anyone using status-go as well without using status-react would also be affected.

It's not clear to me what the argument to remove is, are we still not happy with the performance on slower devices?

[cammellos]

Basically the assumption that anything confidential is stored in realm, is just not true currently (messages can be deleted), and likely won't hold in the future (confidential info might be sent but not stored).

[cammellos]

@corpetty probably you might want to join the conversation

[rasom]

It's not clear to me what the argument to remove is, are we still not happy with the performance on slower devices?

arguments to remove:

1. it is slow. Not only on slower devices, but it is also noticeably slow on faster Android devices (on galaxy s9 Login call takes 2.2-2.3s and most of that time is key derivation) and we just figured that it hasn't been applied on iOS at all. So it might be slow there as well.
2. if it's not slow it is kind of useless, that's the whole point of pbkdf2. Here it's 3200 iterations when in the latest version of sqlcipher that kdf_iter is 256000 (80x) by default. It is recommended as i understand. So if we use the recommended value on, let's say, galaxy s9 it's not going to be 2 seconds anymore, but 7-8 seconds to derive the key from a passphrase.
3. If we can bring randomness into key derivation we can defend against brute-force attacks in a different manner. And the cost of key derivation will be much lower. So well, if we can do this (and do this properly), why not?

Anyone using status-go as well without using status-react would also be affected.

easy to solve in API, we can explicitly tell users when the key will be derived or when a raw key will be used. No problem to use pbkdf on a desktop, for instance.

[cammellos]

Fair enough, my understanding was that only slow devices were affected.

If we can bring randomness into key derivation we can defend against brute-force attacks in a different manner. And the cost of key derivation will be much lower. So well, if we can do this (and do this properly), why not?

That sounds reasonable, but then let's just do that and we can remove it, I'd say

[rasom]

Statusgo.Login duration

galaxy S9
64000 2.1s (~1.694s for pbkdf)
6400 0.575s (~0.169s)
3200 0.5055s (~0.09s)
256000 kdf iterations will cost approximately 6.7s (assuming the same algorithm is used), then Statusgo.Login will take ~7.1s

samsung a500h
64000 8.72s (~7.37s for pbkdf)
6400 2.08s (~0.74s)
3200 1.75s (~0.4s)
256000 kdf iterations will cost approximately 29.5s (assuming the same algorithm is used)then Statusgo.Login will take ~31.8s

[corpetty]

As this doesn't directly affect the user's password or key derivation outside of this DB, and this is only hardening the DB from other content unlocked by user's password. I am ok with this. Furthermore, the password being iterated is not the user's password, but it's hash.

I am not ok with the removal of the iterations.

[rasom]

https://notes.status.im/i8Y_l7ccTiOYq09HVgoFwA