search

oblador / react-native-keychain

:key: Keychain Access for React Native
MIT License
3.16k stars 520 forks source link

Android: Fingerprint authentication not implemented yet #116

Open alexanderjarvis opened 6 years ago

alexanderjarvis commented 6 years ago

It's possible to get if the device supports fingerprint authentication with getSupportedBiometryType() but the keychain item doesn't use the fingerprint authentication (or provide options for doing so).

Looking at https://github.com/oblador/react-native-keychain/blob/c4b2b4dcfe4e36d2cfc86bf25d3a5af461779464/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageKeystoreAESCBC.java#L71

The code is commented out. I've tried playing with this feature and uncommenting the code and even passing if the device supports fingerprint auth as an extra argument to encrypt.

However I can't get it working and there's nothing in the logs to suggest what is wrong.

oblador commented 6 years ago

@alexanderjarvis It's on the todo, but I'm afraid my Android skills are somewhat lacking and the scope of work is fairly big. Storing it with fingerprint encryption isn't that hard, but unlocking it is a pretty big task as it involves a fair bit of UI and logic on how to handle retries etc which iOS neatly just handles for you. I'd be eternally grateful for help with this and could brief anybody willing to take a stab at it of the complexities involved.

alexanderjarvis commented 6 years ago

@oblador thanks for your reply. It's a shame that unlocking it is not so straightforward. If I get some time I may look at doing it and create a PR.

pcoltau commented 6 years ago

I would recommend that we don't add any UI to this library, but simply notify (through an event?) the app when authentication is needed and then wait for the app to tell us if that was successful or not and then proceed.

FlaviooLima commented 6 years ago

Hope to see this implemented soon :) With this implemented this package can go to the 🌔 🚀 🚀 🚀 🚀

oblador commented 6 years ago

Any takers to help out with this? 🙏

jacksontbryan commented 6 years ago

React native touch id has done some work around fingerprint.

https://github.com/naoufal/react-native-touch-id/tree/master/android

Could this work be leveraged to accelerate this feature?

cladjules commented 6 years ago

Let me try to add some native code to Android. See how we could link that up easily and PR.

LinusU commented 6 years ago

API Level 28 introduces a complete dialog for this: https://developer.android.com/reference/android/hardware/biometrics/BiometricPrompt

Here is a compat lib: https://github.com/fython/BiometricPromptCompat

I of course found all of this just after implementing it myself 😭 😂

@oblador what do you think about shipping with this dialog? I could possibly whip up a PR, need this in an app I'm working on...

cladjules commented 6 years ago

@oblador @LinusU I have started some work around it, using the current API (FingerPrintManager), which doesn't offer a native UI for it, you have to implement your own.

At least the logic is there, the only problem, for now, is, you have to put your fingerprint to Encode as well as to Decode the keychain. I have seen some scenario, where the encoding doesn't require your fingerprint. https://github.com/cladjules/react-native-keychain

I believe it's a matter of using a different algorithm.

LinusU commented 6 years ago

I have a working implementation of UI + logic here: https://github.com/LinusU/react-native-biopass

It's using RSA so that it doesn't require a fingerprint to encode, only to decode.

I'd be interested in PRing that up to this repo if we want to include the UI part here. That way I would get iOS support without implementing it myself ☺️

cladjules commented 6 years ago

@LinusU That's really good.

I managed to merge some of your code into my PR. You can now encrypt without using fingerprint and decrypt using it. https://github.com/cladjules/react-native-keychain

At least, there is now a separate cipherStorage so it's easier to manage and in a different file.

I will try and add the BiometricPromptCompat so it uses both Compat and BiometricPrompt code.

LinusU commented 6 years ago

I will try and add the BiometricPromptCompat so it uses both Compat and BiometricPrompt code.

That's awesome!

Let me know if you need any help ☺️

Jyrno42 commented 5 years ago

Since I needed this asap I took the modifications by @cladjules and rebased them to master. Then I did a naive integration with BiometricPromptCompat. Feel free to use it (or not use it).

Note: I also bumped the ENCRYPTION_KEY_SIZE to 512 for CipherStorageKeystoreRSAECB since I need to store more than 53 characters.

E: added note

LinusU commented 5 years ago

Great to see things happening here! I would love to see this work upstreamed as soon as possible, and would be happy to help with this.

@oblador what are your thoughts on this? How do we best get this merged back?

cladjules commented 5 years ago

I'll try to do a more official implementation of BiometricPrompt, anything I could re-use @Jyrno42 would be great.

Will try to do this weekend.

vonovak commented 5 years ago

@LinusU I will be able to do code review for android

oblador commented 5 years ago

@LinusU @Jyrno42: I'm sorry that my time is somewhat lacking, so without researching much or even reading this thread attentively I have some thoughts/pointers:

LinusU commented 5 years ago

Will this BiometricPrompt enable us to encrypt/decrypt using the fingerprint?

Yes. My implementation in BioPass only allows reading the value if the fingerprint is successfully scanned, otherwise the Android key system won't hand out the private key.

oblador commented 5 years ago

@LinusU That's great, would prefer to embed that functionality into this package rather than requiring users to integrate another. Do you think @Jyrno42's work is on the right track? Would you like to own this? What help aside from reviewing do you need from us?

LinusU commented 5 years ago

That's great, would prefer to embed that functionality into this package rather than requiring users to integrate another

Absolutely, that's what I want too 😄

Do you think @Jyrno42's work is on the right track?

Yes! I'm looking thru the changes now, and it looks good.

Would you like to own this?

Would love to 🙌

What help aside from reviewing do you need from us?

Just reviewing ☺️

LinusU commented 5 years ago

@Jyrno42 5c00a5d9e08772a2e28bb14255b95adcbd256218 doesn't have anything to do with fingerprint, right? Can you submit that as a separate pull request?

cladjules commented 5 years ago

@oblador Ideally we would be using the fingerprint to encrypt the secret like iOS does. Nothing is currently preventing anyone from doing regular fingerprint auth and then retrieving the secret in your app with mentioned libraries, but it's less secure. Will this BiometricPrompt enable us to encrypt/decrypt using the fingerprint?

The implementation of FingerprintManager, does encrypt directly the Keychain entry, so it does hardware encryption.

I suggest we modify BiometricPromptCompat which uses both BiometricPrompt and FingerprintManager and directly encrypt the Cipher. It doesn't look like the default BiometricPromptCompat allows you to pass a Cipher to it. So that wouldn't really do a hardware encryption.

oblador commented 5 years ago

@cladjules Cool, maybe collaborate with @LinusU on his PR?

@LinusU Amazing 💯 😄

cladjules commented 5 years ago

@LinusU I think we miss an important point and we would need to pass the Cipher to BiometricPrompt and FingerprintManager using the FingerprintCrypto.CryptoObject or BiometricPrompt.CryptoObject when calling authenticate (which is always null with the Compat) library.

That would require using the same Cipher to both encrypt that we can retrieve in the Fingerprint callback.

I will update the PR, but we definitely need to add some bits to the Compat one too.

Jyrno42 commented 5 years ago

@Jyrno42 5c00a5d doesn't have anything to do with fingerprint, right? Can you submit that as a separate pull request?

Will do

E: https://github.com/oblador/react-native-keychain/pull/150 is out

Timm0 commented 5 years ago

Is there any progress on this?

jordanfloyd commented 5 years ago

Also interested in this. It would be nice to use a single library, but I can use another Fingerprint auth library if needed.

vonovak commented 5 years ago

please track progress in https://github.com/oblador/react-native-keychain/pull/148

OleksandrKucherenko commented 4 years ago

https://github.com/oblador/react-native-keychain/pull/260 - PR with implemented and tested biometric. Enjoy