search

Yubico / yubikit-android

Yubico Mobile Android SDK - YubiKit
Apache License 2.0
108 stars 40 forks source link

FIDO makeCredential withot PIN #115

Closed kysosk closed 8 months ago

kysosk commented 8 months ago

Hello,

We are using Yubikit SDK for Android (and planning to use for iOS). I have a question about using FIDO modul without PIN being used. We have sample application working with Yubi Key based on samples.

Everything works fine as long as we have FIDO2 PIN set and use it. Without PIN being set the credentials are not created. Is this by design or there needs to be something set up?

PublicKeyCredentialCreationOptions.AuthenticatorSelectionCriteria.userVerification is set to UserVerificationRequirement.DISCOURAGED

Here you state:

The YubiKey, in contrast, will only work with a PIN. If you want to make a credential, the YubiKey must have a PIN set, and the PIN must be entered.

Method com.yubico.yubikit.fido.client.makeCredential has PIN attribute as nullable @Nullable char[] pin

So to me it is confusing whether it should work without PIN in the SDK or not (quoted text is not directly linked to the SDK). And if so, how should we set up parameters for the method?

And my second question is, do you have any knowledge that your SDK works (even with some changes) with Idem Keys from GoTrust? I am aware that we are talking about different products and is not your responsibility to make it work with other products.

AdamVe commented 8 months ago

Hi, thanks for your interest in yubikit-android and for the question. I will do some investigations here and reply ASAP.

AdamVe commented 8 months ago

Hi @kysosk, please find my answers in this comment.

Everything works fine as long as we have FIDO2 PIN set and use it. Without PIN being set the credentials are not created. Is this by design or there needs to be something set up?

There are several conditions for using PIN verification in FIDO, which I will summarize here, refer to CTAP2.0 and CTAP2.1 specifications for details, please.

yubikit-android 2.4.0 has an issue, where the first option (not using PIN on authenticator which does not have PIN set) is not available. I have fixed this in a branch, and added tests which can be run against authenticators and demonstrate makeCredential/getAssertion with no pin and discouraged user verification.

Creating credentials without providing a PIN (passing null to makeCredential/getAssertion) on a device which has PIN set is not supported by CTAP2 and not supported by yubikit-android.

And my second question is, do you have any knowledge that your SDK works (even with some changes) with Idem Keys from GoTrust?

The yubikit-android SDK can communicate over NFC and USB transports. As per the CTAP specification, NFC uses the SmartCard protocol and USB uses the HID FIDO protocol. Currently the SDK can work with any NFC FIDO2 key, but only Yubico USB keys - the reason is that the SDK filters the USB device by the USB vendor. It should be possible to make the SDK work with any USB HID FIDO2 device, if there is interest, I have already experimented with that.

You can use the SDK’s testing framework to get information about what is going on, including FIDO errors. The tests work with NFC and USB keys so you can see the differences. Every connection will provide CTAP2 information which is great start to understand what are the authenticator's capabilities:

Ctap2.InfoData: Ctap2Session.InfoData{versions=[FIDO_2_0, FIDO_2_1_PRE, FIDO_2_1], extensions=[credProtect, hmac-secret, largeBlobKey, credBlob, minPinLength], aaguid=redacted , options={uv=false, alwaysUv=true, largeBlobs=true, setMinPINLength=true, ep=false, clientPin=false, bioEnroll=false, credentialMgmtPreview=true, makeCredUvNotRqd=false, userVerificationMgmtPreview=false, credMgmt=true, pinUvAuthToken=true, rk=true, up=true, plat=false, authnrCfg=true}, maxMsgSize=1280, pinUvAuthProtocols=[2, 1], maxCredentialCountInList=8, maxCredentialIdLength=128, transports=[nfc, usb], algorithms=[{type=public-key, alg=-7}, {type=public-key, alg=-8}, {type=public-key, alg=-35}], maxSerializedLargeBlobArray=1024, forcePinChange=false, minPinLength=4, firmwareVersion=2051, maxCredBlobLength=32, maxRPIDsForSetMinPinLength=1, preferredPlatformUvAttempts=3, uvModality=2, certifications={}, remainingDiscoverableCredentials=25, vendorPrototypeConfigCommands=null}

Let me know if you have more questions, I will be happy to answer.

kysosk commented 8 months ago

Thank you for the response. It is good to hear that you have already fixed the issue in branch. This was only case in question. Having yubi key without PIN being set and having parameter UserVerificationRequirement.DISCOURAGED

Regarding the Idem Key from GoTrust: It would be great to have support, even unofficial, for different vendors. We are planning to support different tokens in our Android and iOS app. For the start we went for Yubi Key and Idem Key. I have your Demo application up and running and I have implemented FIDO use case where I test stuff. I believe I was even able to create some credentials for Idem Key. As I was playing around I have changed timeout for PublicKeyCredentialCreationOptions from 9000 to 19000 (just in case as I was getting Tag was lost). And then the call did not fail. Strange enough, as I was trying to add some more logging what is going on and cleaning up the code a bit, after 3rd or 4th attempt it stopped working. So just in case, I reset the key - message said successful - and since I am getting error "Can't read the touch key" (in any application and OS). Resetting seems to work, but reading the key is not working. So I guess it is bricked now. I was still calling makeCredential so I did not play with commands themselves. At the end I was able to read credentialId from the makeCredential response. I did not make it to read publicKey or something else and did not make attestation (bricked it when trying to write some code for this). So I cannot fully confirm it is working with IdemKey. And it was surprise to me to read that there is a vendor block somewhere. Nevertheless, this is more to let you know what I ended up with. If you are willing to add support for other vendors as well, that would be great news for us.

AdamVe commented 8 months ago

Good info, thanks! We will merge the branch after internal review and it will be part of next release. Regarding the support for other vendors, I will bring that up with our team/project leadership.

I have changed timeout for PublicKeyCredentialCreationOptions from 9000 to 19000

There is a recommendation for the timeout values in the WebAuthn specification, based on the value of options.authenticatorSelection.userVerification (see here, point 4). 9000 seems too low.

Related to the key you bricked, could you share more details how that happened, please? Was it over USB or NFC? Did it happen after calling makeCredential from the sdk?

kysosk commented 8 months ago

I was testing Idem Key on Android over NFC only. I did not get to USB but that is planned too. Basically the code was copied from tests and bit rewritten to kotlin to use in frontend. After successful attempt I was trying to work with result - read values - and change hardcoded parameters for some inputs. No serious changes to the function and strictly calling just makeCredential. After 3 || 4 calls I was getting error. So I thought I have made some mistakes during code rewriting. I have reverted the changes to last known working state and still getting the errors. Here I thought that maybe there is user name already registered (or something) on the Key and it might return something your SDK does not expect. Resetting is quick enough so I did it and that was it. I think after last call of makeCredential something happened to the key and was already in unreadable state and resetting the key had no effect in this - I have used standard Windows Account setting functionality where key can be reset. I am also in contact with Idem Key support whether I can use or read the Key. All changes I did was basically changes to challenge, user data and domain data prior to calling makeCredential. To me, so far no explanation to this behaviour other than Idem Key is not really compatible with your SDK at the moment.

The code that worked:

private fun makeCredentials(session: Ctap2Session) : PublicKeyCredential {
        var challenge = "myPwd".toByteArray()
        var rpId = "example.com"
        var userName = "john.doe@example.com"

        var clientData = ClientData(
            "webauthn.create",
            "https://"+ rpId,
            Base64.getEncoder().encodeToString(challenge),
            "TestPackage"
        )

        val json = Gson().toJson(clientData)

        var rp = PublicKeyCredentialRpEntity("Example Company", rpId)
        var user = PublicKeyCredentialUserEntity(userName, userName.toByteArray(), "John Doe")

        var criteria = AuthenticatorSelectionCriteria(
            null,
            ResidentKeyRequirement.DISCOURAGED,
            UserVerificationRequirement.DISCOURAGED
        )

        var credentialParameter = PublicKeyCredentialParameters(PublicKeyCredentialType.PUBLIC_KEY, -7)
        var credParams = listOf<PublicKeyCredentialParameters>(credentialParameter)

        var credOptions = PublicKeyCredentialCreationOptions(
            rp,
            user,
            challenge,
            credParams,
            19000L,
            null,
            criteria,
            null,
            null
        )

        val pin = "11111111".toCharArray()
        //val pin = null

        val client = BasicWebAuthnClient(session)
        return client.makeCredential(
            json.toByteArray(),
            credOptions,
            "example.com",
            pin,
            null,
            null
        )
    }
AdamVe commented 8 months ago

I have just merged #116 which fixes handling of discouraged user verification.