status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.85k stars 984 forks source link

[Epic] Implement "Save password" functionality for Android #5793

Closed lukaszfryc closed 5 years ago

lukaszfryc commented 5 years ago

Description

Type: Feature

Summary: Recently "Save password until logout" (https://github.com/status-im/status-react/pull/5617) was implemented for iOS. This issue is about doing the same for Android. More details can be found in https://github.com/status-im/status-react/pull/5617.

TODO

mandrigin commented 5 years ago

We use react-native-keychain library for saving passwords.

Next steps:

  1. fork this library and switch to our fork;
  2. use the same library to implement this on Android (essentially, enable Android code-paths);
  3. disable or remove whatsoever their "unsafe" option of storing in the facebook DB

Some gotchas

  1. passwords shouldn't be accessible to any other app on the OS
  2. this feature should not be enabled if the device is rooted
  3. there might be an OS version limitation, I would say let's enable it for Android 8.0+

When that is implemented, please ping me and @corpetty for additional security review.

chadyj commented 5 years ago

@rasom Interested in taking this one?

We have iOS and Android parity except for this feature, and about half our users are on Android and they would love this.

cheatfate commented 5 years ago

I'm just curious, because i'm actually very interested in this feature. You said you are using react-native-keychain, and i see this library is actually supports Keychain.BIOMETRY_TYPE::FINGERPRINT. Is it tough task to implement fingerprint usage on Android?

mandrigin commented 5 years ago

@cheatfate for biometrics there are https://github.com/status-im/status-react/issues/5556 and https://github.com/status-im/status-react/issues/5557.

It is not so hard to just use some feature of a lib, but it is much trickier to figure out if this library is actually as safe as it claims. The last thing you want in crypto is to have an adversary to be able to decrypt your private key. We need to make sure that it is safe first, on different devices with different OS versions and flavours from different vendors.

mandrigin commented 5 years ago

[Research] Use an Android Backround Service to hold password in RAM

As a first step, I tried to use a background service to keep password in RAM only. That wouldn't have solved the whole issue, but it would have a much less of the attack surface than storing it somewhere on the device. (Afaik, there is an idea to do a similar thing with Desktop and Keycard).

Essentially, the idea was to create a service that will run even when the app has been killed by the OS, that would hold the password in a variable. When the app is re-started, it can re-connect to the service and request the password from there. It is pretty safe, because the operating system protects access to the service.

Unfortunately, I hit a roadblock. Any service that is bound to the app is stopped on force-quitting or due to the lack of RAM. There are certain flags like START_STICKY, but they are only helping to restart the service after it was killed. Hence, the RAM contents would be wiped out anyways.

There are ways on keeping services "running forever" (like this one), but what they all mean is restarting the service when it is killed in a more or less intrusive ways.

Resolution

Background services aren't suitable to keep the password in RAM.

Next steps

Research key storage methods available on Android and jailbreak detection.

mandrigin commented 5 years ago

[Research] Using Android Keystore System

https://developer.android.com/training/articles/keystore

It looks like Android Keystore System is something we can use to store the password. It provides a tamper-proof (see Extraction Prevention).

We need to check for [KeyInfo.isInsideSecurityHardware()](https://developer.android.com/reference/android/security/keystore/KeyInfo.html#isInsideSecureHardware()) for the keys and not allow using it otherwise.

We need to check for rooted Android and not allow to use this tech if it is rooted.

We shouldn't store plaintext password anywhere, but we can create a key inside the KeyStore that is hardware-backed and use it to encrypt the password, which then we can store to any local storage.

If possible, we should use StrongBox Security Module — a Secure Enclave's analogue with hardware separation.

The difference between isInsideSecurityHardware() and isStrongBoxBacked() are described in this article. TL;DR is that

Resolution

We can have this option to store user's password using this key. Security audit and threat analysis of the implementation is needed though.

Implementation notes

Android Developers
Android keystore system  |  Android Developers
corpetty commented 5 years ago

As these are HSMs, I would further push to have the private keys stored this way as well if the user is not using the keycard.

mandrigin commented 5 years ago

Yep, 100%, I will add it to our backlog.

mandrigin commented 5 years ago

@corpetty there are some flowcharts of the proposal.

With the proposal like the least amount of changes are necessary to our codebase (because it works very similarly to what we do on iOS). Almost all of the changes will be in our fork of react-native-keychain (we will have to fork it). The major downside is that the plaintext password is exposed to JS. Though, it is exposed to JS right now anyway, when user types the password in. The attack on generic react-native-keychain is more probable though, than on our own codebase.

Flowchart 1. How the password is used now (no pwd saving)

img_0008

Flowchart 2. Password Saving Proposal -- Storing the password

img_0009

Note: steps 9 and 10. The encryption key is never exposed to the app and never leaves the TEE/SE. Encryption happens inside TEE/SE.

Flowchart 3. Password Saving Proposal -- Retrieving the password

img_0007

Note: steps 3, 4 and 5. The encryption key is never exposed to the app and never leaves the TEE/SE. Decryption happens inside TEE/SE.

mandrigin commented 5 years ago

Today I started working to add an additional parameter to react-native-keychain that allows a user to set a minimum encryption key guarantee (ANY/SECURE_SOFTWARE/TEE) to encryption operations. That way we can be sure that the app password is always encrypted with a key that is at least inside the security hardware (TEE guarantees).

mandrigin commented 5 years ago

Hashing of the password needs to be added.

mandrigin commented 5 years ago

https://github.com/status-im/react-native-keychain/pull/6 — minimum security guarantees https://github.com/status-im/react-native-keychain/releases/tag/v.3.0.0-status — our fork with this functionality https://github.com/status-im/status-react/pull/6616 — status-react PR

GitHub
status-im/react-native-keychain
:key: Keychain Access for React Native. Contribute to status-im/react-native-keychain development by creating an account on GitHub.
mandrigin commented 5 years ago

Android is done and released, further improvements will be done in: https://www.pivotaltracker.com/epic/show/4246970