[Feature] Database encryption with User Password-Authentication

[PhilKes]

This is a work-in-progress pull-request. Potentially fixes #182 , #128

I am in no means an expert in Encryption/Security, therefore please feel free to give feedback on any security concerns/bugs you might find in this PR.

This is supposed to be a proposition to add actual authentication via password or biometric scan with encrypted token secrets stored in the database using AES.

Propose workflow (screenshots emulator device with no biometric auth setup):

1. Initial Setup with 3 Tokens, no authentication required: In the database:

2. Setup of Authentication with setting password: When saving the password, all tokens are encrypted with the user-set password. This is done by generating a PasswordHash using PBKDF2WithHmacSHA1 (Source: https://howtodoinjava.com/java/java-security/how-to-generate-secure-password-hash-md5-sha-pbkdf2-bcrypt-examples/) and using this PasswordHash as a secretKey for AES/CBC/PKCS5Padding Encryption. Since my knowledge of AES encryption is quite limited I would appreciate thoughts on generating/storing the salt and initialization vector, since how I understand, these values have to be the same on every device to later keep the possibility to import/export encrypted tokens from one device to another.

In the database: At the moment only the Token's secret is encrypted, but in theory the other columns could be encrypted too. The PasswordHash is stored in the SharedPreferences, I am not yet sure what the best way to securely store the hash on the device (Android Keystore?), so I am open for proposals on this.

3. Access to decrypted Tokens once the user unlocked the db:

Once all tokens are encrpyted with the PasswordHash the user has to unlock the database by either entering the correct password, or if the device has setup biometric authentication, the user can simply authenticate by e.g. fingerprint which would be the default authentication method in that case. Once the user successfully unlocked the db, he can access the MainActivity as usual with the decrypted tokens.

In the menu you also have the option to change the password, when that happens all tokens are re-encrypted with the new PasswordHash and the User is presented with the UnlockActivity again. If the User turns off "Require Authentication" he will be warned that all the tokens will then be stored unencrypted again.

Please note that since this is merely a proposal yet, I did not clean up hardcoded strings or any actual test cases for the encryption part. I first want to get some general feedback before finishing everything up.

[helloworld1]

Thanks a lot for the contribution. I haven't looked closely yet but I think the AES symmetry encryption is OK on the database layer. The user experience is not ideal to always let the user enter the password. We can use biometrics to encrypt the password when authentication is turned on and use biometrics to unlock and cache the password in memory until timeout or lock.

[helloworld1]

I really like how you put common logic in the common module. And in general the implementation of AES encryption looks great. One comment is that about the performance of the encryption / decryption and whether it warrant to use a coroutine with Dispatchers.DEFAULT.

For the UI part, I would like to integrate the fingerprint / biometrics to make it more convenient to use. It would be great if you can split out the backend part and UI part as separate PR. I can merge the backend part rather quickly as long as it is compatible with current UI. We can work on the UI part to make it easy to use. Thanks a lot!

[PhilKes]

Thanks for your comments. I can try to split the UI/Backend parts into 2 PRs. As for fingerprint it is already integrated, if the Unlock Activity is shown, the Activity checks if the Device has Fingerprint enabled and shows the Fingerprint prompt automatically. If the User presses the back button the Password input field is shown. The User can re-enter the Fingerprint prompt via a Button on the lower left. My Screenshots are from a Virtual Device, which does not have fingerprint/biometrics enabled, therefore the Fingerprint prompt/button is not shown.

[jugendhacker]

Could you maybe explain a bit more how you actually store the key used for decryption if biometrics are used? From what I could read, you actually store the password inside shared preferences?

[helloworld1]

SharedPreference is not designed to store secret securely. If you can access /data/data/pkg, you can get the password in clear text. Adb backup can also dump it.

I would prefer to use biometric and store in the trusted space.

See this article about it https://medium.com/androiddevelopers/using-biometricprompt-with-cryptoobject-how-and-why-aace500ccdb7

[PhilKes]

Could you maybe explain a bit more how you actually store the key used for decryption if biometrics are used? From what I could read, you actually store the password inside shared preferences?

Thats one of the topics where I wanted to get some propositions, since I was not sure what the best practice concerning storing Secrets on Android is.

SharedPreference is not designed to store secret securely. If you can access /data/data/pkg, you can get the password in clear text. Adb backup can also dump it.

I would prefer to use biometric and store in the trusted space.

See this article about it https://medium.com/androiddevelopers/using-biometricprompt-with-cryptoobject-how-and-why-aace500ccdb7

Thanks, I will have a look at the article and see how I can integrate that in the PR.

[ShellCode33]

Hey, thanks for your work !

A few comments if you don't mind :

• PBKDF2 is ok-ish but Argon2id is state of the art, you should probably use that to make FreeOTP+ more future-proof. LUKS2 which is used for disk encryption switched over to Argon2 instead of PBKDF2 a few years ago. If you still want to stick with PBKDF2, please use SHA256 instead of SHA1 and increase the iterations count to 600000.
• var salt: ByteArray = "12345678".toByteArray() this is fine, but please use a more unique salt so that there are less chances that rainbow tables have been built around that salt. You could use something like var salt: ByteArray = "FreeOTP+S4lt".toByteArray()
• Now the most important stuff: your AES implementation is flawed, you cannot encrypt twice using the same IV with AES CBC. I'm no crypto expert but I believe AES GCM would be a better fit. What you could do is generate a random IV (96 bits long) and store it in the database next to what you encrypted (one unique IV for every encrypted message, never reuse an IV). Moreover the GCM mode provides data integrity, CBC does not. You could also check the integrity of other things (issuer and label for example ?) thanks to the additional data feature provided by GCM. It doesn't encrypt the additional data, it just checks its integrity

If you have any question let me know

[helloworld1]

Thanks a lot @ShellCode33 for the insight. Argon2 is not supported in Android's SecretKeyFactory, so it will be harder to use. AES GCM is only supported on Android API level 26+ (Android 8.0). Probably it is time to bump version to use AES GCM.

[ShellCode33]

Use PBKDF2-HMAC-SHA256 with 600,000 iterations then, it's more than fine !

Same for AES CBC, if you don't want to bump the API level you can keep using it, it's safe. Even though GCM integrity would be nice to have, I don't think it is that important in this context security-wise. Worst case scenario: you restore a corrupted backup and your OTPs are off. The only difference GCM will make is that you will be able to warn the user that his backup is corrupted and not usable.

[callegar]

Is this same as https://github.com/freeotp/freeotp-android/issues/6? Can the solution there be ported over?