Require biometrics to delete key. Add logging to cleanup functions

TheMartinizer · Feb 29, 2024 · 2863d8d · 2863d8d
1 parent 19fb4f2
commit 2863d8d
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 16 deletions.
diff --git a/play-services-fido/core/src/main/kotlin/org/microg/gms/fido/core/RequestHandling.kt b/play-services-fido/core/src/main/kotlin/org/microg/gms/fido/core/RequestHandling.kt
@@ -39,7 +39,7 @@ class UserInfo(
 @Parcelize
 class AuthenticatorResponseWrapper (
  val responseChoices: List<Pair<UserInfo?, suspend () -> AuthenticatorResponse>>,
- val deleteFunctions: List<() -> Unit> = listOf()
+ val deleteFunctions: List<suspend () -> Boolean> = ArrayList()
 ) : Parcelable
 
 val RequestOptions.registerOptions: PublicKeyCredentialCreationOptions

diff --git a/...rc/main/kotlin/org/microg/gms/fido/core/transport/screenlock/ScreenLockCredentialStore.kt b/...rc/main/kotlin/org/microg/gms/fido/core/transport/screenlock/ScreenLockCredentialStore.kt
@@ -112,7 +112,9 @@ class ScreenLockCredentialStore(val context: Context) : SQLiteOpenHelper(context
  }
 
  fun deleteKey(rpId:String, keyId: ByteArray) {
- keyStore.deleteEntry(getAlias(rpId, keyId))
+ val keyAlias = getAlias(rpId, keyId)
+ Log.w(TAG, "Deleting key with alias $keyAlias")
+ keyStore.deleteEntry(keyAlias)
  }
 
  fun clearInvalidatedKeys() {
@@ -129,6 +131,7 @@ class ScreenLockCredentialStore(val context: Context) : SQLiteOpenHelper(context
  val signature = Signature.getInstance("SHA256withECDSA")
  signature.initSign(key)
  } catch (e: KeyPermanentlyInvalidatedException) {
+ Log.w(TAG, "Removing permanently invalidated key with alias $alias")
  keysToDelete.add(alias)
  } catch (e: Exception) {
  // Any other exception, we just continue
@@ -248,6 +251,7 @@ class ScreenLockCredentialStore(val context: Context) : SQLiteOpenHelper(context
  // Use prepared statements to avoid SQL injections
  val preparedDeleteStatement = db.compileStatement("DELETE FROM $TABLE_DISPLAY_NAMES WHERE $COLUMN_KEY_ALIAS = ?")
  for (aliasToDelete in aliasesToDelete) {
+ Log.w(TAG, "Removing userinfo for key with alias $aliasToDelete, since key no longer exists")
  preparedDeleteStatement.bindString(1, aliasToDelete)
  preparedDeleteStatement.executeUpdateDelete()
  }

diff --git a/...c/main/kotlin/org/microg/gms/fido/core/transport/screenlock/ScreenLockTransportHandler.kt b/...c/main/kotlin/org/microg/gms/fido/core/transport/screenlock/ScreenLockTransportHandler.kt
@@ -220,7 +220,7 @@ class ScreenLockTransportHandler(private val activity: FragmentActivity, callbac
  suspend fun sign(
  options: RequestOptions,
  callerPackage: String
- ): Pair<List<Pair<UserInfo?,suspend () -> AuthenticatorAssertionResponse>>, List<() -> Unit>> {
+ ): Pair<List<Pair<UserInfo?,suspend () -> AuthenticatorAssertionResponse>>, List<suspend () -> Boolean>> {
  if (options.type != RequestOptionsType.SIGN) throw RequestHandlingException(ErrorCode.INVALID_STATE_ERR)
  val candidates = mutableListOf<CredentialId>()
  for (descriptor in options.signOptions.allowList.orEmpty()) {
@@ -266,7 +266,7 @@ class ScreenLockTransportHandler(private val activity: FragmentActivity, callbac
  val (clientData, clientDataHash) = getClientDataAndHash(activity, options, callerPackage)
 
  val credentialList = ArrayList<Pair<UserInfo?, suspend () -> AuthenticatorAssertionResponse>>()
- val deleteFunctions = ArrayList<() -> Unit>()
+ val deleteFunctions = ArrayList<suspend () -> Boolean>()
 
  for (credentialId in candidates) {
  val keyId = credentialId.data
@@ -287,8 +287,14 @@ class ScreenLockTransportHandler(private val activity: FragmentActivity, callbac
  )
  }
 
- val deleteFunction = {
- store.deleteKey(options.rpId, keyId)
+ val deleteFunction = suspend {
+ try {
+ showBiometricPrompt(getApplicationName(activity, options, callerPackage), null)
+ store.deleteKey(options.rpId, keyId)
+ true
+ } catch (e: RequestHandlingException) {
+ false
+ }
  }
 
  credentialList.add(userInfo to responseCallable)

diff --git a/...vices-fido/core/src/main/kotlin/org/microg/gms/fido/core/ui/CredentialSelectorFragment.kt b/...vices-fido/core/src/main/kotlin/org/microg/gms/fido/core/ui/CredentialSelectorFragment.kt
@@ -25,8 +25,8 @@ import org.microg.gms.fido.core.transport.Transport
 class CredentialListAdapter(
  private val responseWrapper: AuthenticatorResponseWrapper,
  private val listSelectionFunction: (suspend () -> AuthenticatorResponse) -> Unit,
- private val deleteCredentialFunction: (() -> Unit) -> Unit
- ) : BaseAdapter() {
+ private val deleteCredentialFunction: (suspend () -> Boolean) -> Unit
+) : BaseAdapter() {
  override fun getCount(): Int {
  return responseWrapper.responseChoices.size
  }
@@ -70,7 +70,6 @@ class CredentialListAdapter(
 
  return view
  }
-
 }
 
 class CredentialSelectorFragment : AuthenticatorActivityFragment() {
@@ -110,13 +109,17 @@ class CredentialSelectorFragment : AuthenticatorActivityFragment() {
  }
  }
 
- fun credentialDeletion(deleteFunction: () -> Unit) {
- deleteFunction.invoke()
- if (!findNavController().navigateUp()) {
- findNavController().navigate(
- R.id.transportSelectionFragment,
- arguments,
- navOptions { popUpTo(R.id.usbFragment) { inclusive = true } })
+ fun credentialDeletion(deleteFunction: suspend () -> Boolean) {
+ binding.fidoCredentialListView.isEnabled = false
+ authenticatorActivity?.lifecycleScope?.launch {
+ val deletionSucceeded = deleteFunction.invoke()
+
+ // If credential is deleted, make leave the fragment, since the list is no longer valid
+ // There is probably some way to update the list, but it doesn't seem to work from inside
+ // the authenticatorActivity's lifecycleScope
+ if (deletionSucceeded) {
+ findNavController().navigateUp()
+ }
  }
  }
 }