Add Biometric data encryption and decryption #229
Conversation
|
Hi @jayfunk! Could you have a look at this? |
| ); | ||
|
|
||
| PromptInfo promptInfo = new PromptInfo.Builder() | ||
| .setDeviceCredentialAllowed(false) |
There was a problem hiding this comment.
Allowing device credentials is a new aspect being added into this library. I would like to either implement that capability or note in the documentation that it is not supported.
There was a problem hiding this comment.
Is there a technical reason not to allow device credentials fallback here?
There was a problem hiding this comment.
Ah, I'm guessing it's this:
Calling this method invokes crypto-based authentication, which is incompatible with Class 2 (formerly Weak) biometrics and (prior to Android 11) device credential.
There was a problem hiding this comment.
Yes, the customer who contracted us for the feature needed it paired with strong biometrics, so we didn't bother with a fallback path, esp. since all the design work was done pre Android 11. (And 11 did have a bunch of bugs in it too, iirc. It's been a while.)
I think these days it'd be more reasonable to support them, since the bugs should hopefully be fixed…?
@f-23 Could you look into supporting allowDeviceCredentials here?
|
I have updated the README to include a note for the unsupported feature. |
Can you re-request review? |
|
Any update on this? |
|
@jayfunk Could you have another look at this? |
|
Would love to see this merged - what is it that's blocking it? |
There was a problem hiding this comment.
Thanks for sharing this code. It's a very welcome addition to this library.
I've left lots of comments but they're mostly questions and text changes.
One general question I have is: do we need a separate encryption key? Couldn't we use the existing private key to encrypt data? It would simplify the API and avoid confusion between methods like deleteKeys and deleteEncryptionKey.
android/src/main/java/com/rnbiometrics/DecryptDataCallback.java
Outdated
Show resolved
Hide resolved
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) { | ||
| throw new Exception("Cannot generate keys on android versions below 6.0"); | ||
| } | ||
| deleteBiometricEncryptionKey(); |
There was a problem hiding this comment.
I think it's worth documenting in the Readme that calling createEncryptionKey will delete any existing key, rendering any data encrypted with that key forever undecipherable.
I see that this is also the behaviour of createKeys but that's a little different because you get a public key in return and you are expected to do something with it. Here the key is not returned so it's a bit more magical and I, at least, hadn't thought about it invalidating existing data until I saw this.
There was a problem hiding this comment.
Yes. We only ever used the functionality to guard authentication tokens, so it wasn't really a limitation to us.
For now I'll just document it in the readme, I'm not sure what's the best way to handle this. Should createEncryptionKeys just fail if a key exists, forcing devs to manually wipe existing keys first? Should multiple keys be supported? Both options break symmetry with the signing methods, and the latter adds another layer of complexity.
android/src/main/java/com/rnbiometrics/ReactNativeBiometrics.java
Outdated
Show resolved
Hide resolved
| ); | ||
|
|
||
| PromptInfo promptInfo = new PromptInfo.Builder() | ||
| .setDeviceCredentialAllowed(false) |
There was a problem hiding this comment.
Is there a technical reason not to allow device credentials fallback here?
android/src/main/java/com/rnbiometrics/ReactNativeBiometrics.java
Outdated
Show resolved
Hide resolved
| ); | ||
|
|
||
| PromptInfo promptInfo = new PromptInfo.Builder() | ||
| .setDeviceCredentialAllowed(false) |
There was a problem hiding this comment.
Ah, I'm guessing it's this:
Calling this method invokes crypto-based authentication, which is incompatible with Class 2 (formerly Weak) biometrics and (prior to Android 11) device credential.
Fix typo Co-authored-by: Tamlyn Rhodes <[email protected]>
Fix typos Co-authored-by: Tamlyn Rhodes <[email protected]>
No, you need to specify the if you want to use the key pair for signatures or encryption. See https://developer.android.com/training/sign-in/biometric-auth#crypto Even if you could use the same key pair (which you typically can't) for signing and encryption, there's no guarantee that works across systems and versions. |
|
It's usually a Very Bad Idea to reuse one key for multiple purposes (e.g.), so even if Android or iOS were unsafe enough to allow it, we would not want to support it. |
Co-authored-by: Tamlyn Rhodes <[email protected]>
…ometrics into dataEncrypt
|
We'll look into |
This PR adds a compact API for data encryption and decryption using the devices Biometric features. Related to #41.
We've already updated the README, please refer to it for the usage details.