duo-labs / android-webauthn-authenticator

A WebAuthn Authenticator for Android leveraging hardware-backed key storage and biometric user verification.
BSD 3-Clause "New" or "Revised" License
110 stars 20 forks source link

Fix keystore exception. #3

Closed thavel closed 5 years ago

thavel commented 5 years ago

Hi,

I got the following exception using your lib. (I've a Pixel 3XL running under Android 9 Pie):

W/KeyStore: KeyStore exception
    android.os.ServiceSpecificException:  (code 7)
        at android.os.Parcel.createException(Parcel.java:1964)
        at android.os.Parcel.readException(Parcel.java:1918)
        at android.os.Parcel.readException(Parcel.java:1868)
        at android.security.IKeystoreService$Stub$Proxy.get(IKeystoreService.java:786)
        at android.security.KeyStore.get(KeyStore.java:195)
        at android.security.keystore.AndroidKeyStoreSpi.engineGetCertificateChain(AndroidKeyStoreSpi.java:118)
        at java.security.KeyStoreSpi.engineGetEntry(KeyStoreSpi.java:484)
        at java.security.KeyStore.getEntry(KeyStore.java:1560)
        at duo.labs.webauthn.util.CredentialSafe.getKeyPairByAlias(CredentialSafe.java:177)
        at duo.labs.webauthn.Authenticator.makeCredential(Authenticator.java:171)
        at test.thavel.webauthn.MainActivity$register$1.run(MainActivity.kt:44)
        at java.lang.Thread.run(Thread.java:764)

Apparently, the proper way to get a private key from the Android keystore (since Android 9) is:

PrivateKey privateKey = (PrivateKey) keyStore.getKey("my-alias", null);

This PR actually fixes the exception.

nickmooney commented 5 years ago

Hey @thavel -- I'm about ready to merge this PR. The tests are all passing in my local repo. I'm curious where you found the recommendation to get the private key this way? All the KeyStore documentation I've read shows the "cast to PrivateKeyEntry, then getPrivateKey" method.

I've noticed the docs tend to be out of date and that calling getKey directly doesn't throw this exception, I'd just love to be able to link out to some official guidance.

Also FWIW, you should be able to use the library as-is -- this is a warning exception, so shouldn't actually stop control flow. Still would rather fix it!

thavel commented 5 years ago

Hey, You're right, this actually doesn't affect the lib. I was struggling to make my app work and I ran into this issue thinking it could be the cause (well, actually, it wasn't). Anyway, I found this answer, I read the official doc about getKey() to make sure it makes sense, then I tried it out and it worked. I only got this warning on Android 9 though.