search

patrickfav / armadillo

A shared preference implementation for confidential data in Android. Per default uses AES-GCM, BCrypt and HKDF as cryptographic primitives. Uses the concept of device fingerprinting combined with optional user provided passwords and strong password hashes.
https://favr.dev/opensource/armadillo
Apache License 2.0
280 stars 52 forks source link

Password is not being used for derivating encryption key #11

Closed davidmigloz closed 6 years ago

davidmigloz commented 6 years ago

The user provided password is supposed to be used to derivate the encryption key. However, it seems that currently is not being used.

How to reproduce:

  1. Instantiate Armadillo with password A.
  2. Save some data
  3. Instantiate Armadillo with password B (without deleting data).
  4. Try to retrieved data stored with password A. -> You are able to retrieve the plain data
davidmigloz commented 6 years ago

The stretched password was not being added to the input key material array. image I've made a pull request with the fix.

davidmigloz commented 6 years ago

But the fix implies that if someone was using password is not going to be able to decrypt the data anymore :$. Can we migrate the data somehow?

patrickfav commented 6 years ago

Hi David thx for the PR.

That looks bad :( I will think about if migration is feasible, otherwise we will have to bump the major version and maybe provide a workaround.

davidmigloz commented 6 years ago

Maybe for future migrations, we can start storing the protocol version in the same way the storage salt is stored. So then we can easily check when the protocol version has been bumped and apply the proper data migration. Because currently I have no idea how we can detect it. What do you think?

patrickfav commented 6 years ago

I fear the app developer would need to track the migration (e.g. the have a migration flag for all the key/value pairs with user PW, the first time the value is accessed read it with password = null, then write it with the correct pw)

I will release a new version today with a note on the issue and probable migration issues.

patrickfav commented 6 years ago

For today there is a 0.5.0 release. Will think about a solution regarding migration the next couple of days.

davidmigloz commented 6 years ago

Perfect. Thanks Patrick.

davidmigloz commented 6 years ago

We could use the changing password feature #13 to make the migration easier. It would be changing from empty password to the user password.

patrickfav commented 6 years ago

Good idea, this would make it more simple.