search

Yubico / Yubico.NET.SDK

A YubiKey SDK for .NET developers
Apache License 2.0
99 stars 47 forks source link

The require touch property is not supported on YubiKeys with firmware version less than 4.2.4. #41

Closed mjfara closed 1 year ago

mjfara commented 1 year ago

The require touch property is not supported on YubiKeys with firmware version less than 4.2.4.

Getting this error when trying to enroll a YubiKey with firmware version 4.2.8.

Looking at the code, it seems the actual minimum version is 4.3.1.

https://github.com/Yubico/Yubico.NET.SDK/blob/66eb8f5069a87f380f5980c3af66c76a34138484/Yubico.YubiKey/src/Yubico/YubiKey/YubiKeyFeatureExtensions.cs#L212-L214

mjfara commented 1 year ago

Another thing to note is "Require Touch" can be selected when adding an account manually with the Yubico Authenticator app in Windows. So seems like the issue may be in the actual version comparison, rather than a typo in the error message?

It would also be nice if there was a custom exception being thrown that included all of the features the current key is missing. FeatureNotSupportedException or similar.

The InvalidOperationException makes it difficult to catch and handle, we can compare the message, but relying on a string that can potentially change, or even be localized down the road, is not ideal.

GregDomzalski commented 1 year ago

Thanks for the report. Yes - I have confirmed the message is correct (4.2.4 is when this feature was introduced) and the code is incorrect. We'll get this fix out in our upcoming March release.

I agree that the exceptions could be better. We have definitely learned a lot over the creation of this SDK and if we get the chance to make breaking changes (in a v2.0, perhaps) this is one of the things I'd love to see addressed.

For this particular case and in the 1.x SDK, the best way to avoid exceptions based on missing YubiKey features is to use the YubiKeyDevice.HasFeature method to check up front for the features that you require.

GregDomzalski commented 1 year ago

Hi @mjfara - this should be addressed in the latest 1.7.0 release. I am closing the ticket as "fixed".

If upgrading to the latest version does not fix the issue, please feel free to reactivate and let me know.