Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] KeyNotFoundException in CmDevice causes application to quit when plugging / unplugging Yubikey #144

Open
1 task done
canton7 opened this issue Aug 27, 2024 · 4 comments
Labels
awaiting yubico action When we've got the ball bug Something isn't working

Comments

@canton7
Copy link

canton7 commented Aug 27, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

If you plug / unplug a Yubikey a lot (and YubiKeyDeviceListener has been instantiated), it's possible to occasionally hit a KeyNotFoundException in CmDevice.

This exception is thrown on a background thread, meaning that it crashes the user's application, and there is no way for the user to prevent this.

The stack trace is:

Yubico.Core.dll!Yubico.PlatformInterop.CmDevice.GetProperty<string>(Yubico.PlatformInterop.CmDeviceProperty property) Line 112
Yubico.Core.dll!Yubico.PlatformInterop.CmDevice.CmDevice(string devicePath) Line 41
Yubico.Core.dll!Yubico.Core.Devices.Hid.WindowsHidDeviceListener.OnEventReceived(nint hNotify, nint context, Yubico.PlatformInterop.NativeMethods.CM_NOTIFY_ACTION action, nint eventDataPtr, int eventDataSize) Line 97

The line in question is this one, called with CmDeviceProperty.PdoName from here.

image

Expected Behavior

The expected behaviour is that Yubikey does not crash the user's application in an unrecoverable way.

Steps To Reproduce

  1. In a Windows project, subscribe to YubiKeyDeviceListener.Instance.Arrived. I don't think the subscription is actually necessary (just accessing YubiKeyDeviceListener.Instance might be enough), since the YuibKeyDeviceListener ctor calls HidDeviceListener.Create(), which is what instantiates WindowsHidDeviceListener, which is what has the unsafe event handler.
  2. Plug and unplug the Yubikey a lot (might take a number of minutes). Vary the amount of time that the Yubikey is plugged in for, but no need to have it plugged in for more than 2s. Having Yubico Authenticator open at the same time might help reproduce, but I have seen it with nothing else open
  3. Watch your application crash with the above exception

I have seen this pop up two or three times in normal usage.

Version

1.9.0

Version

5.4.3

Anything else?

No response

@canton7 canton7 added the bug Something isn't working label Aug 27, 2024
@Yubico Yubico deleted a comment Aug 27, 2024
@Yubico Yubico deleted a comment Aug 27, 2024
@DennisDyallo
Copy link
Collaborator

Adding this to our internal issue tracker. Thanks.

@DennisDyallo DennisDyallo added the awaiting yubico action When we've got the ball label Aug 27, 2024
@DennisDyallo
Copy link
Collaborator

Does this happen in the latest 1.11 release as well?

@DennisDyallo DennisDyallo added awaiting reply When we are waiting for response from user and removed awaiting yubico action When we've got the ball labels Sep 25, 2024
@canton7
Copy link
Author

canton7 commented Sep 25, 2024

I'll take a look next time I get the chance, thanks!

@canton7
Copy link
Author

canton7 commented Sep 26, 2024

Yes, I can still replicate this in 1.11.

It doesn't look like any of the files in the stack trace have been changed in any meaningful way? Is there a PR which you think should fix it?

@DennisDyallo DennisDyallo added awaiting yubico action When we've got the ball and removed awaiting reply When we are waiting for response from user labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting yubico action When we've got the ball bug Something isn't working
Development

No branches or pull requests

2 participants