search

smstuebe / xamarin-fingerprint

Xamarin and MvvMCross plugin for authenticate a user via fingerprint sensor
Microsoft Public License
490 stars 115 forks source link

Added CryptoObject to Android #229

Open DarkIrata opened 1 year ago

DarkIrata commented 1 year ago

Mentioned in #225 there is a security vulnerability in the current android implementation. I fixed it with examples from Microsoft and Googles Android documentation.

The current state is tested against the Frida Script (as of writing) https://codeshare.frida.re/@Saket-taneja/biometricauthenticationbypassnullcryptoobject/

It is not required to make use of the new CryptoSettings class. The default configuration should work for every current implementation of this library.

DarkIrata commented 1 year ago

After way more testing and reading into it, i further improved it a little bit. This should catch most cases. A More secure implementation would be a breaking change or be a separate implementation. To improve security it would be need to change to have one Method to "Register" which would encrypt the given data (for example the Login Data), one Method to check if "IsRegisteredAndValid" and one Method to "Authenticate" which would decrypt the data. While registering we would need to save the IV generated from Authentication and pass it to the Decryption CryptoObject.

If i find time, i will implement this but in another branch and do a new pull request. The current implementation just secure the current state without any breaking changes.

hig-dev commented 1 year ago

@smstuebe Can you merge this for the .NET Maui prereleases?

eblis commented 1 year ago

If you have time could you also look in #231 ? I tried to build the code myself but have lots of missing dependencies and not sure when i'll have the time to look into it :(

SelminBiop commented 1 year ago

Out of scope for this PR I guess, but are there any kind of tests that are run?

Should we look into adding some? I'm curious to know the rationale.

DarkIrata commented 1 year ago

The only tests i could run where some Frida scripts. I tried to adjust few to test more cases but thats it. I'm not into frida too much to say how hard it would be to make automated tests, since i already had to build a specific environment to test it. But for other parts i would be happy if someone wants to write tests! :) Never bad to have some

SelminBiop commented 1 year ago

Is there something blocking this PR from entering main? I'm using this NuGet and the mentioned security vulnerability is a concern I'd rather see fixed.

Also, any plans on fixing this same vulnerability on iOS's side?

DarkIrata commented 1 year ago

Not sure if there is a similar vulnerability in iOS since the CryptoObject is an android specific component. But even if there is a same vulnerability, it would need someone with an iOS Dev environment because i don't have any apple devices.

If you dont want to wait for the pr, download and recompile it yourself. Did the same until the pr is merged.

aschuhardt commented 1 year ago

Let's get this merged!

jvillaro commented 8 months ago

Great! Thanks. Please merge this!

HavenDV commented 5 months ago

We are using this as part of the net8 MAUI project and decided to update this project and release the new version as Open-Source: https://github.com/oscoreio/Maui.Biometric I renamed the project, removed some legacy code, got rid of the use of obsolete api platforms, and introduced the AuthenticationStrength abstraction.

I will be glad if you can transfer this PR to a new project. I can guarantee a quick review and acceptance of changes. I don't have enough encryption skills to do this myself.