Skip to content

Commit

Permalink
Made the screenlock authenticator try to use StrongBox and attestatio…
Browse files Browse the repository at this point in the history
…n if available, by try again without them if it doesn't work
  • Loading branch information
TheMartinizer authored and mar-v-in committed Aug 16, 2024
1 parent 667ac95 commit 8e0aa42
Showing 1 changed file with 41 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
package org.microg.gms.fido.core.transport.screenlock

import android.content.Context
import android.content.pm.PackageManager
import android.database.sqlite.SQLiteDatabase
import android.database.sqlite.SQLiteOpenHelper
import android.os.Build.VERSION.SDK_INT
import android.security.keystore.KeyGenParameterSpec
import android.security.keystore.KeyPermanentlyInvalidatedException
import android.security.keystore.KeyProperties
import android.security.keystore.StrongBoxUnavailableException
import android.util.Base64
import android.util.Log
import androidx.annotation.RequiresApi
Expand All @@ -32,6 +34,8 @@ class ScreenLockCredentialStore(val context: Context) {

@RequiresApi(23)
fun createKey(rpId: String, challenge: ByteArray): ByteArray {
var useStrongbox = false
if (SDK_INT >= 28) useStrongbox = context.packageManager.hasSystemFeature(PackageManager.FEATURE_STRONGBOX_KEYSTORE)
val keyId = Random.nextBytes(32)
val identifier = getAlias(rpId, keyId)
Log.d(TAG, "Creating key for $identifier")
Expand All @@ -40,9 +44,44 @@ class ScreenLockCredentialStore(val context: Context) {
.setDigests(KeyProperties.DIGEST_SHA256)
.setAlgorithmParameterSpec(ECGenParameterSpec("secp256r1"))
.setUserAuthenticationRequired(true)
if (SDK_INT >= 28) builder.setIsStrongBoxBacked(useStrongbox)
if (SDK_INT >= 24) builder.setAttestationChallenge(challenge)
generator.initialize(builder.build())
generator.generateKeyPair()

var generatedKeypair = false
val exceptionClassesCaught = HashSet<Class<Exception>>()
while (!generatedKeypair) {
try {
generator.initialize(builder.build())
generator.generateKeyPair()
generatedKeypair = true
} catch (e: Exception) {
// Catch each exception class at most once.
// If we've caught the exception before, tried to correct it, and still catch the
// same exception, then we can't fix it and the exception should be thrown further
if (exceptionClassesCaught.contains(e.javaClass)) {
throw e
}
exceptionClassesCaught.add(e.javaClass)

if (SDK_INT >= 28 && e is StrongBoxUnavailableException) {
Log.w(TAG, "Failed with StrongBox, retrying without it...")
// Not all algorithms are backed by the Strongbox. If the Strongbox doesn't
// support this keypair, fall back to TEE
builder.setIsStrongBoxBacked(false)
} else if (SDK_INT >= 24 && e is ProviderException) {
Log.w(TAG, "Failed with attestation challenge, retrying without it...")
// This ProviderException is often thrown if the TEE or Strongbox doesn't have
// a built-in key to attest the new key pair with. If this happens, remove the
// attestation challenge and create an unattested key
builder.setAttestationChallenge(null)
} else {
// We don't know how to handle other errors, so they should be thrown up the
// system
throw e
}
}
}

return keyId
}

Expand Down

3 comments on commit 8e0aa42

@dylangerdaly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should allow easier use of Passkeys? As in microG actually managing the keys themselves.

If microG starts to be trusted for FIDO2 Keys, it should definitely attempt to use Strongbox, failing that a TEE.

Does this logging come up to the UI?

@mar-v-in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently don't fully support passkeys, as we don't have support for discoverable credentials. Adding discoverable credentials will need some more thoughts on UI, Backup and Sync (as it makes microG effectively a password manager). Once we have it, we definitely will show information about the security of that password manager (Strongbox or TEE, w/ or w/o attestation) in the corresponding UI.

@dylangerdaly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's fair enough, cheers

Please sign in to comment.