KomodoPlatform / komodo-wallet-mobile

KomodoWallet Mobile codebase
https://komodoplatform.com
MIT License
21 stars 31 forks source link

Design Preview - Add/multi account auth #22

Closed CharlVS closed 11 months ago

CharlVS commented 1 year ago

WIP. Watch for regular updates.

Current status: Implementing migration from the old data structure

Warning: Only use with a wallet if you are willing to lose the existing data until migration functions have been validated. Security reviews are pending. Not ready for production use.

Changes still required:

High-level Changelog:

Changelog

TODO

ca333 commented 1 year ago

I am currently not able to test this due to biometrics enforcement. We need to keep the option for those who wish to not use biometrics and instead "only" a password.

CharlVS commented 1 year ago

@ca333 Which of the following two options would be ideal for you?

  1. Pin/password input implemented in our app.
  2. Integrate with the device's secure credential storage, where users would be prompted to enter their lock-screen pin/password to unlock the app.

Authenticating using the options above outputs the user's seed which we use to start up API.

Option 2 has a significant advantage where the app cannot access the seed without the user authenticating. Option 1 would use Flutter secure storage where our app can freely access the storage, and we implement the "guarding" of the seed. As discussed in Opsec discussions, if spoofing our app is a possible attack vector, then the malicious app would have access to this.

I currently have option 2 implemented with a strong authentication method required (fingerprint, iris or face), but I can relax it to allow weak lock screen auth methods (e.g. pin and password).

With option 2, the lock screen password/pin would apply to all apps. Changing the lock screen or device biometrics would clear the stored credentials, and we would require the user to input their seed phrase to regain access.

ca333 commented 1 year ago

I currently have option 2 implemented with a strong authentication method required (fingerprint, iris or face), but I can relax it to allow weak lock screen auth methods (e.g. pin and password)

I wouldn't call it weak since biometrics auth data is actually protected with such a passphrase - just use the device passphrase auth layer and thus native OS specific auth layer, to protect the seed. If its good enough to provide the basis layer for phone auth, should be "good enough" to protect any sensitive data, including the seed. And if this assumption is off, all hope would be lost anyway.

i.e. Option 2 is the way to go however changing device passphrase or biometrics data should not require setting up wallet again (importing seed, etc.) - after all, other apps with sensitive data, don't require this neither.

CharlVS commented 1 year ago

@ca333

I wouldn't call it weak

Agreed. I was referencing the Android API docs term. It's not "weak", but it is objectively weak relative to "strong" biometrics. So it would have been clearer for me to say "weak biometrics". I don't know how a pin is considered as "biometrics".

Android API docs: [BIOMETRIC_WEAK()](https://developer.android.com/reference/androidx/biometric/BiometricManager.Authenticators#BIOMETRIC_WEAK())

i.e. Option 2 is the way to go however changing device passphrase or biometrics data should not require setting up wallet again (importing seed, etc.) - after all, other apps with sensitive data, don't require this neither.

Yeah, that a non-negotiable. Looking at my Samsung Wallet app as inspiration, the user has to enter their wallet pin It's not configurable in the Flutter package, but the native API allows to specify if the credentials get cleared or not. So that's why apps don't always follow this behaviour.

I've designed it in mind that the authentication method is separated from the profile data. This will be useful when we want to do a watch-only mode. User could set up the wallet on an air-gapped machine and then export profile data and pending signed tx to a different device.