-
Notifications
You must be signed in to change notification settings - Fork 72
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
Extract in/out HID report sizes from descriptors #112
Conversation
The U2F/CTAP specifications for the HID transport allow arbitrary sizes for the HID input/output reports, not just the standard 64 bytes. This commit adds the general logic needed to deal with varying report sizes and implements the relevant descriptor parsing for Linux.
eb6c6ad
to
76b1369
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this, @FabianHenneke! Can you give me an example of a device that provides variable size HID reports?
@@ -5,17 +5,17 @@ | |||
// Allow dead code in this module, since it's all packet consts anyways. | |||
#![allow(dead_code)] | |||
|
|||
pub const HID_RPT_SIZE: usize = 64; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used by all the other platforms as well, so we'll have to find/replace across the repo.
Dynamic or at least non-standard HID report sizes are used by authenticators that implement the HID protocol via Bluetooth. I am the developer of such an authenticator, WearAuthn, and I know that @rstoica is working on such an authenticator due to #32. While Bluetooth as a HID transport is not part of the FIDO specs, it is supported by the Windows WebAuthn API and by Chrome on Linux, macOS and Chrome OS. It would be great if Firefox could also become compatible on non-Windows platforms. I will take another look at the PR and take care of the HID_RPT_SIZE issue. |
That seems very doable. I just want to be able to test if I can. I'll see about getting something running WearOS if I can.
Thanks. I put a skeleton fix in 5d28484 - I spent a short while this morning trying to figure out how to get the same data out of iokit but it needs more research than I could pull off at the time. |
If Android OS would be easier to get a hang on for the necessary testing you are welcome to try WioKey (Early Access) as well. Source code is not yet out, but it uses the same underlying Bluetooth HID within the Android Bluetooth stack.
|
Thanks - a good idea. My general Android test device (a Samsung S7 edge) is apparently incompatible, but I'll give it a try on my other devices tomorrow. I don't have to have running code to finish this review and accept. Would just be nice to be able to work it out for MacOS and Windows, too! I should have this patch into the next release of |
Yes, unfortunately not all of the devices have the Bluetooth HID profile enabled, but most of the Android 9+ should support it. The other Bluetooth LE GATT solution is currently blacklisted by Google.
Current code version (without this PR) works out of the box on Windows machines as it goes through Windows Hello and the drivers pick up the Bluetooth automatically as an input device anyway. I don't have now a MacOS to check particularly the Mozilla browser, but I can do it maybe in the next days. I know though that it works on Mac with other browser clients as another colleague tested it already, but don't recall exactly the state for Firefox and don't want to pass misinformation. I will update here this comment once I find out. |
It also doesn't work with OnePlus and (most) Nokia phones. More recent Samsung phones should work, but Pixels are ideal.
Unfortunately, it is even blacklisted in the Android Open Source Project directly. Without a custom ROM, it will thus remain disabled on most phones indefinitely. @jcjones This is why Bluetooth HID is the only workable solution for third-party FIDO apps on phones.
I have added support for dynamic HID sizes on macOS to libfido2 in Yubico/libfido2#180 (and Windows in Yubico/libfido2#182, but as @rstoica said, Windows Hello makes this rather uninteresting). Maybe that code can be helpful, as I may not have time to work on it myself. |
As an update, I should be able to take this for the next Firefox cycle, starting end-of-month. |
Major changes: * Move to Rust 2018 edition #125 * Remove dependency on boxfnonce #121 * Reworked error handling #130 * Added a higher-level AuthenticatorService that can use multiple backends #123 * Changed the C API to use the new AuthenticatorService #123 * Added a Status channel for backends #122 * Now obtaining HID report sizes from the descriptors #112 * Add authenticator USB and Firmware details to the C API #93
The U2F/CTAP specifications for the HID transport allow arbitrary
sizes for the HID input/output reports, not just the standard 64 bytes.
This commit adds the general logic needed to deal with varying report
sizes and implements the relevant descriptor parsing for Linux.