Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement fingerprint login method #244

Merged
merged 15 commits into from
Apr 24, 2018
Merged

Conversation

0140454
Copy link
Contributor

@0140454 0140454 commented Apr 21, 2018

Related issue: #229

In the current implementation, we have to enter password to open wallet.
To implement fingerprint login method, i think we need to save user password in a secure method for future use.

Therefore, in this commit, if user allows to open wallet using fingerprint, app will encrypt user password (not CrAzYpass) using RSA algorithm and save ciphertext in SharedPreferences.

screenshot_20180422-002112

On access to the wallet, user can touch fingerprint sensor to use saved password to unlock the wallet.

20180422

@0140454 0140454 force-pushed the feature/fingerprint branch from 0ea5017 to 50c99dc Compare April 21, 2018 17:52
@Codivorous
Copy link
Contributor

I feel like we should implement CrAzYpass on this as well, before it gets merged. But really good work!

@0140454 0140454 force-pushed the feature/fingerprint branch from e321ae9 to ed6f611 Compare April 21, 2018 18:25
@m2049r
Copy link
Owner

m2049r commented Apr 22, 2018

this is really nice!!

  • please delete the keys/prefs for removed wallets - since wallets can be removed manually this should be done somewhere around LognFragment.loadList() (after filterList)
  • please add the fingerprint switch to the "change password" code (prompt_changepw layout) to enable to turn this on/off after creation.
  • from reading the code I think that a wallet named "MonerujoRSA" would use the CrAzYpass key. and so removing the wallet would also destroy all wallets' passwords. we should maybe rename this key to something like "wallet" and use "wallet-[Wallet Name]" for the wallet keys?
  • we want to avoid someone using your finger to access your funds. it may be a good idea to allow fingerprint access only to open the wallet or "Receive Menu" in the wallet list. and require the passphrase for sending & viewing details (details in open wallet should need passphrase only if wallet was opened with fingerprint or - maybe easier - is openable by fingerprint). but a sub-average hacker can get around this software restriction easily (modify the github code to not request passphrase) which they can install since they have access to the device (otherwise they can't use the finger). so maybe we just add a security warning to the help.

@0140454
Copy link
Contributor Author

0140454 commented Apr 22, 2018

About last item,
maybe we can pop up a security warning while user switches on "Allow to open wallet using fingerprint" ?

If you allow to open wallet using fingerprint, anyone who can get your fingerprint will have access to your funds.
For example, malicious user around you can unlock your wallet when you are asleep.

Are you sure to enable this function?

@m2049r
Copy link
Owner

m2049r commented Apr 22, 2018

Security warning is a good idea. Two small changes to your text:

  • ... will have full access to your funds.
  • For example, a malicious ...

@0140454
Copy link
Contributor Author

0140454 commented Apr 22, 2018

Thanks for your correction.

Now, app will pop up security warning when user enables this function.

@m2049r
Copy link
Owner

m2049r commented Apr 22, 2018

actually, it's not totally true about replacing the app with a hacked one because the app signatures have to match. but I think the security warning is the way to go nonetheless.

@m2049r m2049r self-requested a review April 22, 2018 17:53
@0140454
Copy link
Contributor Author

0140454 commented Apr 23, 2018

You're right.
Let me implement the above-mentioned to provide best information security?

  • If user opens wallet using fingerprint
    It has to verify the identity with password again when sending funds or viewing details.
    Other operations, such as receiving or just viewing wallet balance, don't need to verify the identity.
  • If user opens wallet using password
    All is same as before.

@m2049r m2049r merged commit a2d6ca0 into m2049r:master Apr 24, 2018
@m2049r
Copy link
Owner

m2049r commented Apr 24, 2018

@erciccione this adds some new strings:

  • prompt_fingerprint_auth
  • bad_fingerprint
  • generate_fingerprint_hint
  • generate_fingerprint_warn

chinese is already done

This was referenced Apr 25, 2018
el00ruobuob added a commit to el00ruobuob/xmrwallet that referenced this pull request Apr 25, 2018
@ghost ghost mentioned this pull request May 7, 2018
m2049r pushed a commit that referenced this pull request Jul 27, 2018
* Update strings.xml

* Update about.xml

* Update help.xml
m2049r pushed a commit that referenced this pull request Jul 27, 2018
* Update strings.xml

* Update about.xml

* Update help.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants