Skip to content

CTAPHID report size incorrectly limited #335

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

Closed
tmthrgd opened this issue Jun 15, 2021 · 3 comments
Closed

CTAPHID report size incorrectly limited #335

tmthrgd opened this issue Jun 15, 2021 · 3 comments

Comments

@tmthrgd
Copy link

tmthrgd commented Jun 15, 2021

libfido2 imposes a maximum HID report size of CTAP_MAX_REPORT_LEN = 64 bytes. This limit isn't part of either the CTAP 2.0 or CTAP 2.1 specs, although the spec is a tad vague about this. (All references will be to the CTAP 2.0 spec as 2.1 is still a draft).

USB 1.x Full Speed devices are limited to just 64-byte report sizes, but USB 2.0 (from 2001) introduced High Speed devices which can have report sizes up 1024-bytes (from what I've been able to find). The Linux kernel can support HID reports up to 16KiB in size since v5.11.

§ 8.1.8. says (emphasis mine):

The description further assumes (but is not limited to) a full-speed USB device (12 Mbit/s). Although not excluded per se, USB low-speed devices are not practical to use given the 8-byte report size limitation together with the protocol overhead.

§ 8.1.8.1. says (emphasis mine):

The device implements two endpoints (except the control endpoint 0), one for IN and one for OUT transfers. The packet size is vendor defined, but the reference implementation assumes a full-speed device with two 64-byte endpoints.

The actual endpoint order, intervals, endpoint numbers and endpoint packet size may be defined freely by the vendor and the host application is responsible for querying these values and handle these accordingly. For the sake of clarity, the values listed above are used in the following examples.

§ 8.1.8.2. says:

The length values specified by the HID_INPUT_REPORT_BYTES and the HID_OUTPUT_REPORT_BYTES should typically match the respective endpoint sizes defined in the endpoint descriptors.

While § 8.1.4. does say:

With a packet size of 64 bytes (max for full-speed devices), this means that the maximum message payload length is 64 - 7 + 128 * (64 - 5) = 7609 bytes.

This is clearly only an example of how to calculate the maximum message size. There's nothing else in the spec (from what I can see) that would impose a 64-byte maximum report size.

This implementation shouldn't be imposing a limit on the maximum HID report size (except where USB might have a limit) and certainly not one as small as 64-bytes.

@kongeo
Copy link
Member

kongeo commented Jun 16, 2021

Hi, thanks for your report! In our opinion supporting HID reports of variable sizes would increase code complexity for no real benefit. Moreover, we are not aware of any authenticators using HID report sizes other than 64 bytes, which makes "testability" of this feature even more difficult.

@kongeo kongeo closed this as completed Jun 16, 2021
@tmthrgd
Copy link
Author

tmthrgd commented Jun 16, 2021

Respectfully, I have to strongly disagree.

"Moreover, we are not aware of any authenticators using HID report sizes other than 64 bytes"

That's entirely a chicken or the egg problem. It's currently impossible to deploy larger report sizes because non-spec-compliant client libraries are ossifying the protocol. It's what TLS GREASE was designed to solve. At the same time #165 indicates that authenticators with non-64 byte report sizes are in fact being deployed, they just all happen to be less than at this point in time.

"for no real benefit"

The benefits of larger report sizes are potentially significant. Using 64-byte report sizes puts an unnecessary 7609-byte limit on all CTAPHID messages, where as using larger report sizes allows up to the full 64KiB to be used. It's also much slower and requires far more system calls to send the same message. For example basing it off my YubiKey 5 NFC's maxMsgSize of 1200, it would take 20 separate frames to send a message of that size, while taking just 2 if using the 1024 USB High Speed limit while also using a faster USB protocol. That means 4 system calls rather than 40.

That's not to mention the potential desire to use vendor extensions for other things that might require larger message sizes. Things like firmware updating could benefit greatly from larger report sizes. Or future evolutions in the protocol that might benefit from larger, less fragmented messages.

I opened this bug report after noticing the same bug in a client library I'm writing. While I'm no C developer, the fix actually turned out simple and made the code easier to reason about. When the Solo v2 ships one thing I'm planning on experimenting with is larger report sizes and other applications, neither I nor anyone else can do that while this bug exists in widely deployed software.

@mcuee
Copy link

mcuee commented May 14, 2023

Moreover, we are not aware of any authenticators using HID report sizes other than 64 bytes, which makes "testability" of this feature even more difficult.
That's entirely a chicken or the egg problem. It's currently impossible to deploy larger report sizes because non-spec-compliant client libraries are ossifying the protocol. It's what TLS GREASE was designed to solve. At the same time #165 indicates that authenticators with non-64 byte report sizes are in fact being deployed, they just all happen to be less than at this point in time.

@tmthrgd
This is kind of interesting to know.

We have a relevant discussion here in hidapi project. I am trying to understand why High Speed USB HID device developers would not choose to have the Interrupt IN endpoint's wMaxPacketSize to be higher value (up to 1024 Bytes). From what I read the HID report size is usually less than 1024 Bytes. In that case they can simplify their Firmware development efforts (send the report in one packet and no need to deal with multiple packets on the device side).

The following is my suspect.

[To be confimred]
It seems to me because Windows inbox HID driver most likely does not really support Interrupt IN/OUT Endpoint with wMaxPacketSize greater than 64 bytes, so for the High Speed USB HID device out in the market, the Interrupt IN/OUT Endpoint will still have wMaxPacketSize at 64Bytes.

Later version of Windows may or may not have such limitations but the device out in the market have to support older version of Windows.
[/To be confirmed]

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

No branches or pull requests

3 participants