search

helloworld1 / FreeOTPPlus

Enhanced fork of FreeOTP-Android providing a feature-rich 2FA authenticator
Apache License 2.0
648 stars 74 forks source link

[Feature request] Encrypt secret key on local storage #182

Open gelavat opened 2 years ago

gelavat commented 2 years ago

It is not clear to me if the secret keys are encrypted on the device local storage and decrypted when the apps starts, can you please state on this?

If not done, I strongly suggest that all TOTP secret keys are encrypted, even without entering a password or checking 'authentication required' (like Signal Messenger does for example). This way, no hacker is able to read the configuration file and retrieve the secret keys. I think it is very important for a security app.

I put this separate of issue #128 because that one is done for exports encryption which is something else, although still important.

mo-rijndael commented 2 years ago

Android isolates app's private storage, so no app can see other app filles. Except in case there is root access ofc, but it's generally impossible to protect something from root Also, Android has OS-level disk encryption (force enabled by default in modern versions)

I think there is no need to encrypt already encrypted and strongly isolated app's private storage

helloworld1 commented 2 years ago

Agreed that app storage is more or less secure for a reputable device manufacturer. I think at least the exported token should be encrypted since those are in shared storage.

gelavat commented 3 weeks ago

Thank you @PhilKes for this link to issue #214 and new feature, this is exactly what I meant and wanted to have. Great job done, thanks a lot! If possible, I would still like to see a password authentication when I open FreeOTPPlus, still also keeping the PIN authentication which is very nice, and that we can choose the method when we enable the authentication. Maybe I will open another feature request for this as most of the work is now done and it is probably a quite little thing to add.

I saw that there are two comments from @ShellCode33 on ticket #214 that seem to be very pertinent ("your AES implementation is flawed" and 600000 iterations), but they have been added after the commits of the solution for #214. Do you know if those comments can be addressed ? It seems important security wise.