Skip to content

Commit fa6cf1f

Browse files
committed
Require biometrics to delete key. Add logging to cleanup functions
1 parent e4faeb3 commit fa6cf1f

File tree

4 files changed

+29
-16
lines changed

4 files changed

+29
-16
lines changed

play-services-fido/core/src/main/kotlin/org/microg/gms/fido/core/RequestHandling.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class UserInfo(
3939
@Parcelize
4040
class AuthenticatorResponseWrapper (
4141
val responseChoices: List<Pair<UserInfo?, suspend () -> AuthenticatorResponse>>,
42-
val deleteFunctions: List<() -> Unit> = listOf()
42+
val deleteFunctions: List<suspend () -> Boolean> = ArrayList()
4343
) : Parcelable
4444

4545
val RequestOptions.registerOptions: PublicKeyCredentialCreationOptions

play-services-fido/core/src/main/kotlin/org/microg/gms/fido/core/transport/screenlock/ScreenLockCredentialStore.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@ class ScreenLockCredentialStore(val context: Context) : SQLiteOpenHelper(context
112112
}
113113

114114
fun deleteKey(rpId:String, keyId: ByteArray) {
115-
keyStore.deleteEntry(getAlias(rpId, keyId))
115+
val keyAlias = getAlias(rpId, keyId)
116+
Log.w(TAG, "Deleting key with alias $keyAlias")
117+
keyStore.deleteEntry(keyAlias)
116118
}
117119

118120
fun clearInvalidatedKeys() {
@@ -129,6 +131,7 @@ class ScreenLockCredentialStore(val context: Context) : SQLiteOpenHelper(context
129131
val signature = Signature.getInstance("SHA256withECDSA")
130132
signature.initSign(key)
131133
} catch (e: KeyPermanentlyInvalidatedException) {
134+
Log.w(TAG, "Removing permanently invalidated key with alias $alias")
132135
keysToDelete.add(alias)
133136
} catch (e: Exception) {
134137
// Any other exception, we just continue
@@ -248,6 +251,7 @@ class ScreenLockCredentialStore(val context: Context) : SQLiteOpenHelper(context
248251
// Use prepared statements to avoid SQL injections
249252
val preparedDeleteStatement = db.compileStatement("DELETE FROM $TABLE_DISPLAY_NAMES WHERE $COLUMN_KEY_ALIAS = ?")
250253
for (aliasToDelete in aliasesToDelete) {
254+
Log.w(TAG, "Removing userinfo for key with alias $aliasToDelete, since key no longer exists")
251255
preparedDeleteStatement.bindString(1, aliasToDelete)
252256
preparedDeleteStatement.executeUpdateDelete()
253257
}

play-services-fido/core/src/main/kotlin/org/microg/gms/fido/core/transport/screenlock/ScreenLockTransportHandler.kt

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ class ScreenLockTransportHandler(private val activity: FragmentActivity, callbac
220220
suspend fun sign(
221221
options: RequestOptions,
222222
callerPackage: String
223-
): Pair<List<Pair<UserInfo?,suspend () -> AuthenticatorAssertionResponse>>, List<() -> Unit>> {
223+
): Pair<List<Pair<UserInfo?,suspend () -> AuthenticatorAssertionResponse>>, List<suspend () -> Boolean>> {
224224
if (options.type != RequestOptionsType.SIGN) throw RequestHandlingException(ErrorCode.INVALID_STATE_ERR)
225225
val candidates = mutableListOf<CredentialId>()
226226
for (descriptor in options.signOptions.allowList.orEmpty()) {
@@ -266,7 +266,7 @@ class ScreenLockTransportHandler(private val activity: FragmentActivity, callbac
266266
val (clientData, clientDataHash) = getClientDataAndHash(activity, options, callerPackage)
267267

268268
val credentialList = ArrayList<Pair<UserInfo?, suspend () -> AuthenticatorAssertionResponse>>()
269-
val deleteFunctions = ArrayList<() -> Unit>()
269+
val deleteFunctions = ArrayList<suspend () -> Boolean>()
270270

271271
for (credentialId in candidates) {
272272
val keyId = credentialId.data
@@ -287,8 +287,14 @@ class ScreenLockTransportHandler(private val activity: FragmentActivity, callbac
287287
)
288288
}
289289

290-
val deleteFunction = {
291-
store.deleteKey(options.rpId, keyId)
290+
val deleteFunction = suspend {
291+
try {
292+
showBiometricPrompt(getApplicationName(activity, options, callerPackage), null)
293+
store.deleteKey(options.rpId, keyId)
294+
true
295+
} catch (e: RequestHandlingException) {
296+
false
297+
}
292298
}
293299

294300
credentialList.add(userInfo to responseCallable)

play-services-fido/core/src/main/kotlin/org/microg/gms/fido/core/ui/CredentialSelectorFragment.kt

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import org.microg.gms.fido.core.transport.Transport
2525
class CredentialListAdapter(
2626
private val responseWrapper: AuthenticatorResponseWrapper,
2727
private val listSelectionFunction: (suspend () -> AuthenticatorResponse) -> Unit,
28-
private val deleteCredentialFunction: (() -> Unit) -> Unit
29-
) : BaseAdapter() {
28+
private val deleteCredentialFunction: (suspend () -> Boolean) -> Unit
29+
) : BaseAdapter() {
3030
override fun getCount(): Int {
3131
return responseWrapper.responseChoices.size
3232
}
@@ -70,7 +70,6 @@ class CredentialListAdapter(
7070

7171
return view
7272
}
73-
7473
}
7574

7675
class CredentialSelectorFragment : AuthenticatorActivityFragment() {
@@ -110,13 +109,17 @@ class CredentialSelectorFragment : AuthenticatorActivityFragment() {
110109
}
111110
}
112111

113-
fun credentialDeletion(deleteFunction: () -> Unit) {
114-
deleteFunction.invoke()
115-
if (!findNavController().navigateUp()) {
116-
findNavController().navigate(
117-
R.id.transportSelectionFragment,
118-
arguments,
119-
navOptions { popUpTo(R.id.usbFragment) { inclusive = true } })
112+
fun credentialDeletion(deleteFunction: suspend () -> Boolean) {
113+
binding.fidoCredentialListView.isEnabled = false
114+
authenticatorActivity?.lifecycleScope?.launch {
115+
val deletionSucceeded = deleteFunction.invoke()
116+
117+
// If credential is deleted, make leave the fragment, since the list is no longer valid
118+
// There is probably some way to update the list, but it doesn't seem to work from inside
119+
// the authenticatorActivity's lifecycleScope
120+
if (deletionSucceeded) {
121+
findNavController().navigateUp()
122+
}
120123
}
121124
}
122125
}

0 commit comments

Comments
 (0)