-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
FIDO2 screenlock authentication fixes #2199
Conversation
Quote from https://developer.android.com/privacy-and-security/keystore:
It seems that it may be available but still fail so the StrongBoxUnavailableException need to be handled. |
You are right! It really doesn't hurt to do some extra error checking. I think that according to the documentation, all StrongBoxes should be able to handle the key parameters that that the code currently chooses for the screenlock key, but that doesn't mean that all phones will have implemented it correctly. I don't have a phone with StrongBox to test on myself, but I have tested the error handling, and it seems to work. |
Not only that. To support requests without an allowed credential list, we'd need to support storing of user entities (including handle) on the device when registering, as the result must include the corresponding user handle. Returning the user handle is optional when the allowed credential list is provided (as it is assumed that the user handle is already known as the authenticating entity already knows which credentials are valid), but is needed as soon as we do support resident/discoverable credentials. It is also required to have the user entity (which includes name, icon and display name) to make the credential chooser any reasonable (we can't ask users to select from random IDs or public keys). Another complexity comes from the question if and how we want to support backups of keys used for WebAuthN/Passkeys. Google now stores a backup of all passkeys with the Google account instead of storing it exclusively in the secure element of the device. This is so that user's don't lose access to their account when losing their phone or moving to another phone. A backup functionality certainly would be desirable for microG's Passkeys implementation as well. Independent of all of this, your work is a good start and thanks for that. |
@TheMartinizer I suggest also to log warnings to make things clearer in the logcat, like this:
|
If you want to reuse, I have done the separate changes in my fork here: micro5k@6b334ef |
…n if available, by try again without them if it doesn't work
8e8e67a
to
d08ca98
Compare
Thank you! I used your patch, and now the pull request only contains the exception fixes. I might start to look into credential selection, but that might take a while, and will be in a different pull request! |
Thanks. |
@mar-v-in |
I was still doing a few tests. But yes, all good and thanks @TheMartinizer for the work! |
When registering a FIDO2 key using the ScreenLockCredentialStore, the process currently throws an exception if the Android Keystore does not have a default attestation key to sign the newly generated key pair with (as is the case for both phones I've tested on). I've added code to catch this exception and try generating a key pair again without any attestation. Also added code to use Strongbox if available, although this hasn't been tested since none of my phones have Strongbox.
Also added code that allows signing in with screenlock even if the requesting party has not supplied an allowed credential list. Currently, the code looks through the keystore for all keys that match the requesting party ID, and returns the first one it finds. In the future, the user should probably be allowed to choose a credential, as specified in the Web Authentication standard: https://www.w3.org/TR/webauthn-2/#sctn-op-get-assertion