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

hid_open_path fails intermittently on Mac #266

Closed
garrettr opened this issue Mar 26, 2016 · 15 comments
Closed

hid_open_path fails intermittently on Mac #266

garrettr opened this issue Mar 26, 2016 · 15 comments

Comments

@garrettr
Copy link

OS: Mac OS 10.10.5
HIDAPI commit: b5b2e17 (current master)

I'm trying to write some code to connect to a HID U2F authentication device on a Mac. I've been encountering intermittent but frequent failures to connect to the device with hid_open_path. There is no apparent pattern or cause as to whether the connection succeeds or fails.

I did some debugging and found that it was failing in mac/hid.c because IOHIDDeviceOpen was returning kIOReturnNotPrivileged.

I rewrote the same functionality (trying to connect to a specific device) using only the native APIs and it does not have the same problem: connecting to the same device succeeds every time. This implies that this is a problem in HIDAPI, although so far I have not been able to figure out why that might be the case.

To aid debugging, I've forked HIDAPI and created a branch that has some test programs that demonstrate this behavior. I included a README that explains the different programs and how to build them.

@garrettr
Copy link
Author

An interesting observation: when I change the call to IOHIDDeviceOpen in mac/hid.c to use kIOHIDOptionsTypeNone instead of kIOHIDOptionsTypeSeizeDevice, I am able to connect successfully every time.

However, there still appears to be a discrepancy here, because my native example uses kIOHIDOptionsTypeSeizeDevice and is also able to connect successfully every time.

@garrettr
Copy link
Author

Another observation: according to TN2187, you need to call IOHIDManagerOpen before accessing devices enumerated by the HID manager. However, this function is never called in the Mac HID code (well, it is in main, but that's just example code and isn't actually part of the library).

@mrpippy
Copy link
Contributor

mrpippy commented Mar 26, 2016

Unfortunately I don't have a U2F device to try out, but you've given lots of good info. Dumb question first: is there anything else on the system that's opening/using the device?

The difference that sticks out to me between your native and hidapi samples is that the native sample creates a matching dictionary, so the HID Manager can filter and enumerate just the devices you want. Whereas hidapi just gets all devices from the HID Manager, and then filtering happens in the user's code.
In addition, the HID Manager was designed to be used from a GUI app with a running run loop. hidapi spawns a thread when hid_open is called and runs a run loop there to try and emulate this, but it's never seemed to work perfectly.

As for IOHIDManagerOpen, hidapi actually used to call it. The problem is that it essentially calls IOHIDDeviceOpen on every matching device, which for hidapi will be every HID device. This caused the call to fail (with kIOReturnExclusiveAccess) if there was any other HID device already being opened exclusively. (At the time, Logitech's drivers would open their wireless receivers exclusively and trigger this problem). See [https://github.com//issues/26] and 4a229e2 for more details if you want.
The documentation does make it seem like IOHIDManagerOpen needs to be called, but it's only true if you want to manage multiple devices at a time.
The HID Manager is actually open-source if you want to see the code: https://github.com/practicalswift/osx/tree/master/src/iokituser/hid.subproj

I'm out of ideas for now, I'll see if I can come up with anything else

@garrettr
Copy link
Author

Dumb question first: is there anything else on the system that's opening/using the device?

Dumb question in response to your (not so dumb) question: is there a way I can definitively tell if another process on my system is using the same device? (e.g. a diagnostic tool)

As a rough test, I've tried rebooting my machine and running the test on a fresh boot before starting any other processes. The error still occurs.

@garrettr
Copy link
Author

Unfortunately I don't have a U2F device to try out, but you've given lots of good info.

FWIW, I've also tried a Kensington USB keyboard and a Logitech mouse. Interestingly, I could reproduce the same behavior with the keyboard, but could not with the mouse (it always connects successfully).

@garrettr
Copy link
Author

In the context of #266 (comment), why does hid_open_path need to call HIDDeviceOpen with kIOHIDOptionsTypeSeizeDevice?

@garrettr
Copy link
Author

Another interesting observation: if you run the program that uses HIDAPI with root privileges (e.g. sudo ./example_hidapi, using my example program from #266 (comment)), then the connection succeeds every time.

@mrpippy
Copy link
Contributor

mrpippy commented Mar 29, 2016

Unfortunately, I don't know of a way to see whether other processes have a device open.

As for why behavior changes when running as root, from Apple's docs:

Note: As of Leopard, the kIOHIDOptionsTypeSeizeDevice option requires root privileges to be used with keyboard devices.
Does the U2F's HID descriptor describe it as a keyboard?

As for hidapi using kIOHIDOptionsTypeSeizeDevice, I think it's mostly personal preference rather than any hard requirement. I seem to remember the question coming up recently for Windows, and that was the conclusion.
hidapi is intended to be compiled-in and bundled with an app, and if some local modifications make it work right for you (not seizing the device), I would do that.

@garrettr
Copy link
Author

Ok, I found the source of the problem. Fundamentally, the issue is that make_path fails to return a unique identifier for each device on the system. Here's an enumeration of the two interfaces provided by my Yubikey:

Device Found
  type: 1050 0407
  path: USB_1050_0407_14200000
  serial_number: 
  manufacturer: Yubico
  product: Yubikey 4 OTP+U2F+CCID
  usage page: f1d0
  usage: 0001

Device Found
  type: 1050 0407
  path: USB_1050_0407_14200000
  serial_number: 
  manufacturer: Yubico
  product: Yubikey 4 OTP+U2F+CCID
  usage page: 0001
  usage: 0006

The Yubikey is in OTP+U2F+CCID mode, which presents two HID devices: a keyboard (0x01/0x06) for OTP mode and a U2F HID device (0xf1d0/0x01) for U2F mode. However, they have the same path: USB_1050_0407_14200000.

Here's what's causing the intermittent failure to connect in my test program (and would cause the same error in any program that uses HIDAPI on Mac):

  1. hid_enumerate returns a list of devices. We iterate through it and select a target device based on its attributes - in this case, the Yubikey U2F interface (0xf1d0/0x01).
  2. Call hid_open_path with the path of the target device.
    1. hid_open_path reenumerates all available devices, then loops through them and tries to find the matching device by comparing the path.
    2. hid_open_path gets the set (CFSet) of available devices with IOHIDManagerCopyDevices. However, CFSets are unordered (a typical property of set data types across programming languages), so the order of the array of elements returned by CFSetGetValues is non-deterministic.
    3. Since the path of both Yubikey interfaces are the same, with some probability hid_open_path mistakenly tries to open the keyboard interface (0x01/0x06) instead of the U2F interface (hence the intermittent and seemingly nondeterministic pattern of connection successes and failures).
    4. hid_open_path then calls IOHIDDeviceOpen with kIOHIDOptionsTypeSeizeDevice, and Apple restricts non-root users from gaining exclusive access to any device that identifies as a keyboard. Thus, when the keyboard interface of the Yubikey is accidentally and incorrectly chosen, the connection fails.

The correct solution is to ensure that make_path truly returns a unique path for each device on the system. This is an essential invariant. I'm honestly surprised that this problem hasn't been noticed before now, since it affects any device that exposes multiple HID interfaces.

@garrettr
Copy link
Author

Chromium's HID implementation uses IORegistryEntryGetRegistryEntryID to uniquely identify HID devices. HIDAPI should probably do the same, instead of trying to cobble together a unique ID from various attributes of IOHIDDevice.

@mrpippy
Copy link
Contributor

mrpippy commented Mar 29, 2016

Aha, very interesting. I actually have a several-year-old pull request which solves that exact problem (by using the IOKit path instead of fabricating one), I've wanted it to be merged for a while and this is a real problem that hopefully will be solved. Give it a try: #40

@garrettr
Copy link
Author

@mrpippy Can confirm that #40 fixes the problem. It uses IORegistryEntryGetPath instead of IORegistryEntryGetRegistryEntryID, but both functions seem to return a suitably unique per-device identifier.

@garrettr
Copy link
Author

Now that I've diagnosed the problem, this is basically a duplicate of #193, so I'm closing it.

@signal11 Is there any reason not to merge #40 into master?

@mrpippy
Copy link
Contributor

mrpippy commented Mar 29, 2016

That's a good find, Chromium switched to using the entry ID about a year ago because IOKit paths were not unique for Bluetooth HID devices (at least on 10.10):
https://bugs.chromium.org/p/chromium/issues/detail?id=452172
https://chromium.googlesource.com/chromium/src/+/fab70f55e19d3090af7d6d51425abed3c8b3e039

Testing hidapi with Bluetooth devices is something I've always wanted to do but never got around to

@garrettr
Copy link
Author

@mrpippy Ah, that is good to know. Using #40 might then cause problems down the road, because Bluetooth HID U2F devices are currently being developed and we would eventually like to support them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants