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 KeyStore backed fingerprint verification #897

Merged
merged 1 commit into from Jun 2, 2022

Conversation

hjubb
Copy link
Collaborator

@hjubb hjubb commented May 26, 2022

No description provided.

@hjubb hjubb requested a review from KeeJef May 26, 2022 07:26
@Retr02332
Copy link

Retr02332 commented May 31, 2022

Hello Session team

I will test your solution once implemented to verify that everything works correctly.

However, doing an analysis of the code, it seems to me that you should take into account step 4, which I will show you below:

Link: https://labs.f-secure.com/blog/how-secure-is-your-android-keystore-authentication

image
image

I mention this, because it seems that the onAuthenticationSucceeded method still depends on a boolean.

If the cryptographic verification works well, it returns true. However, the correct thing would be that it will retrieve the encryption object from the parameter and USE this encryption object to decrypt some other crucial data, such as the session key (by "session key" I do not mean the private key of the session app. I simply mean a unique identifier of the user's session) or a secondary symmetric key that will be used to decrypt the application data.

In this way, an attacker could no longer simply hook into the process so that the function returns true, because now we are dealing with cryptographically sound processes, which if we do not have the corresponding key to decrypt the data, then we will not be able to enter the app.

If you want some guidance with this, I invite you to see the following project, which is an app that implements biometric authentication in a secure way: https://github.com/FSecureLABS/android-keystore-audit/tree/master/keystorecrypto-app

Finally, you can check this link where it is explained in even more detail how to solve this problem so that the fingerprint protections cannot be bypassed definitively: https://mobile-security.gitbook.io/mobile-security-testing-guide/android-testing-guide/0x05f-testing-local-authentication#biometric-library

image

Remember not to reuse keys and not to leave them exposed in RAM for a long time !!

Regards, @Retr02332

@hjubb
Copy link
Collaborator Author

hjubb commented May 31, 2022

@Retr02332

I think the issue presented with that approach is that it would require refactoring the application's keystore which provides the ability to decrypt the key information for the user's profile to depend on the biometric-backed key. In the event that a user entrolls a new fingerprint, or turns off biometric security on their device, this key would no longer work and lock the user out of Session, requiring them to restore their account again. Currently the refactored AuthenticationCallback uses a signature and certificate mechanism, which generates a signature from the CryptoObject parameter (unlocked after biometric authentication), then verifies that signing random data with that key matches what is expected, otherwise preventing someone from opening the application. It prevents the original approach to attack the process returning a successful authentication without requiring a major refactor or depending on a key which may be erased external to the application process for crucial data.

Does this make sense to you and sync up with your understanding?

@Retr02332
Copy link

Retr02332 commented May 31, 2022

Hi @hjubb

You may perform cryptographic procedures with the cryptobject object. However, the proposed check results in a boolean:

image

With this, nothing would prevent an attacker from hooking into the process so that this function always returns true. This is why in this OWASP document we can see the following statement:

Link: https://mobile-security.gitbook.io/mobile-security-testing-guide/android-testing-guide/0x05f-testing-local-authentication

image

It is necessary to adopt the approach I have mentioned to avoid bypassing biometric authentication, even on rooted devices. This is because if the approach I mentioned to you is adopted, an attacker no matter how much he hooks into the process, would not be able to bypass authentication, since for the user's session to be unlocked a symmetric key must first be recovered with the help of the cryptobject after a successful biometric authentication.

As you see, it is no longer a matter of reimplementing a function in memory so that it returns true forever (the bypass), but we are now facing solid cryptographic process that if we do not have the keys that will help us decrypt the information needed to continue (keys that will be obtained after a successful biometric authentication with the help of the cryptobject), then we will not be able to enter the application.

Finally, you tell me that "it would require refactoring the application keystore that provides the ability to decrypt the user's profile key information to trust the biometric-backed key". This is true, but only if the user has configured biometric fingerprint authentication as the system protection method.

Regards, @Retr02332

@Retr02332
Copy link

Retr02332 commented May 31, 2022

Hi @hjubb

We have assigned the CVE-2022-1955 to refer to the issue. The details of the CVE will be published once the corresponding patch has been implemented and verified.

Disclosure policy: https://fluidattacks.com/advisories/policy/

Regards, @Retr02332

@hjubb
Copy link
Collaborator Author

hjubb commented May 31, 2022

Hi @Retr02332 could you please confirm if this patch still applies to your original issue

@Retr02332
Copy link

Retr02332 commented May 31, 2022

@hjubb The original problem was solved. That is, now a Cryptobject is used, so the validation of the fingerprint no longer depends on the result of the event handler.

However, please note the recommendations given to improve the patch.

@hjubb
Copy link
Collaborator Author

hjubb commented May 31, 2022

Ok thanks for that info

@hjubb
Copy link
Collaborator Author

hjubb commented May 31, 2022

Regarding the further recommendations, I am currently trying to understand in which context the signature verification boolean return statement - after using the crypto object which is only unlocked after biometric authentication by the KeyStore - would be an attack surface. I believe the CryptoObject returned via the biometric authentication callback will throw a security exception if it is used in a locked state before this check is performed, and I would assume the threat model of an unlocked device (is a rooted device required?) would have different opportunities to circumvent even the requirement of a fingerprint in the first place. Please let me know if I'm on the right track here or if I'm missing something and if you could elaborate on the requirements of the state of the device and OS (root, physically unlocked etc) to perform such a procedure.

@Retr02332
Copy link

@hjubb

I understand. I would like to explain all these points in a practical way. So could you approve this patch that you have made and then I could test the patch in a practical way.

In case I find the bypass in the way I raised you above, then I will send you a mail as I did with this finding.

Regards.

@hjubb hjubb merged commit db92034 into oxen-io:dev Jun 2, 2022
@Retr02332
Copy link

Hi @hjubb

I just sent an email with the following subject line: "Bypass of the biometric fingerprint authentication patch"

As I had told you before, once I had the functional exploit I would send you a poc so you could see in detail what I was referring to. In the email are all the complementary details you need.

Regards, @Retr02332

@Retr02332
Copy link

Hi @hjubb

Any update?

@hjubb
Copy link
Collaborator Author

hjubb commented Jun 21, 2022

Hey @Retr02332 at the moment the further points you have raised aren't practical to implement from an application perspective. It is not really practical to take into consideration an attack relying on full root control of an unlocked device which has been configured with appropriate screen locks or to completely re-architect the application in a way that would rely on such defences which would leave a user's application unusable by turning off or adding additional fingerprints on a device from the settings menu forcing them to re-sync their account. If there are further practical implementations we can look at discussing them and implementing them further but for now I think it is sufficient 👍

@Retr02332
Copy link

Hi @hjubb

In compliance with our disclosure policy: https://fluidattacks.com/advisories/policy/ previously shared with you. We proceed to make responsible disclosure of the finding: https://fluidattacks.com/advisories/tempest/.

Regards,
@Retr02332

@hjubb
Copy link
Collaborator Author

hjubb commented Jun 30, 2022

Thanks @Retr02332, it might be helpful in describing the context and state of the devices, I can only see the OS level, not any indication of whether the device was rooted as you had previously mentioned.

@Retr02332
Copy link

Retr02332 commented Jun 30, 2022

Hi @hjubb

Thanks for the feedback, in a few moments I will update the advisory to include those details.

Regards.

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.

None yet

2 participants