Yubico / Yubico.NET.SDK

A YubiKey SDK for .NET developers
Apache License 2.0
96 stars 48 forks source link

FIDO2 command fail with older yubikeys. #51

Closed barnabas90 closed 1 year ago

barnabas90 commented 1 year ago

Hi, we have a simple .net 6 command line app that accepts a pin code, then can register / authenticate with fido2. It works well with our new yubikey with firmware version 5.4.3, but fails with an older yubikey with firmware version 5.2.7. We've tried two different older yubikeys so it's not a hardware failure. We checked with yubikey manager, all capabilities were enabled with all yubikeys, including fido2. We ran the app/visual studio with admin privileges. We observed that the yubikey with older firmware uses protocol one, the newer uses protocol two. The lib accepts the pin code with both old and new yubikeys, but throws an exception with the older yubikey when we call MakeCredential on the Fido2Session object. The exception says command failed, no inner exceptions or additional details.

Is there something we miss here, or is this a bug?

burnett86 commented 1 year ago

First, I'm assuming you're using the latest SDK available, version 1.7.

Second, I'm assuming the exception thrown is Fido2Exception and the message is "The comand failed to complete." Notice the typo in there, the word command is misspelled. We fix that in the next version of the SDK. This exception/message combo is thrown when the error does not match our list of expected errors for an operation. That is, we send a command into the YubiKey, and it comes back with an error response for which we had not planned.

Third, it does not sound like the problem is with the PIN. If that were a problem, either the VerifyPin call would fail or the exception thrown by MakeCredential would be something such as InvalidOperationException, or the message would be, "...the PIN and/or UV was not verified". And because your get the exception when calling Fido2Session.MakeCredential, then I'm guessing the problem is in the MakeCredentialCommand.

Finally, I have not been able to replicate this. I use the same code to make a credential on my 5.4.3 and 5.2.7 YubiKeys, and it always works. I have tried different MakeCredentialParameters to see if there is something supported in 5.4.3 but not 5.2.7, but I can't find anything. If I can make a credential on 5.4.3, then I can make a credential on 5.2.7.

There are two obvious things supported in 5.4.3 not supported in 5.2.7. One is the AuthenticatorInfo.FirmwareVersion property and the second is PinProtocolTwo. However, neither of those appears to play a role in your error.

I would like to know what your MakeCredentialParameters looks like. For example, here is a very simple call to MakeCredential:

byte[] clientDataHash = new byte[] {
    0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38,
    0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38
};
var rp = new RelyingParty("relyingparty1");
var userId = new UserEntity(new byte[] { 1, 2, 3, 4 })
{
    Name = "TestUser1",
    DisplayName = "Test User"
};
var mcParams = new MakeCredentialParameters(rp, userId)
{
    ClientDataHash = clientDataHash
};
mcParams.AddOption(AuthenticatorOptions.rk, true);

MakeCredentialData mcData = fido2.MakeCredential(mcParams);

So if you can provide me a little more information, that would be great. I really want to get to the bottom of this.

barnabas90 commented 1 year ago

Thank you, we haven't set the Name and DisplayName props of the UserEntity here. I added those and now it works with the older yubikey too.

Yes, we're on 1.7, and the exception message was "The comand failed to complete.".

burnett86 commented 1 year ago

I confirmed that the YubiKey 5.2.7 failed to make a credential if the User (the UserEntity class) did not contain a Name. Building a UserEntity without a DisplayName is fine.

That is, in order to make a credential on "older" YubiKeys, the User must contain an ID and a Name. On "later" YubiKeys, it is possible to make a credential for a User that has only an ID.

For the upcoming release, I will update the message in the exception for this case. Unfortunately, the CTAP error is "MissingParameter", so there is no guarantee that this specific error means that the missing parameter is "missing Name". Hence, the message will indicate a missing parameter, but not necessarily "missing Name".

I'll also try to figure out the version of YubiKey before which the Name is necessary.

I will also update the documentation to indicate that the Name is necessary for some versions of YubiKey.

I will leave this issue open for now. However, if I don't hear anything from anyone for a couple days, I will close it. In other words, if you are OK with this solution, don't do anything. But if you would like further information or clarification, or if you have any more comments, go ahead and add them.