Timeout too short for multiple HID packets to be sent in `send_transport_response`

[svareille]

Hi!

I'm currently playing with PGP key signature and decryption using my OnlyKey. I dialogue directly with the key, sending the OKSIGN and OKDECRYPT messages over HID. While testing with a 4096 bits RSA key, signing gives a wrong signature:

Signatures with a 4096 bits RSA key should be 512 bytes long (8 HID packets). However I only get 448 bytes (7 HID packets). I think I understand why: the usb_rawhid_send2 function will send a packet only if there is less than 4 packets processed at the moment. If there is 4 packets currently processed, the function will "sleep" a little then try again until the packet can be sent or a timeout is reached. That's the problem: the send_transport_response function has to send 8 packets, but a timeout of 0 ms is specified (RawHID.send2(resp_buffer, 0);):

libraries/onlykey/okcore.cpp

Lines 2552 to 2566 in a133bea

	{ //USB
	for (int i = 0; i < len; i += 64)
	{
	if (len-i>=64) {
	memcpy(resp_buffer, data+i, 64);
	}
	else {
	memcpy(resp_buffer, data+i, len-i);
	}
	#ifdef DEBUG
	byteprint(resp_buffer, 64);
	#endif
	RawHID.send2(resp_buffer, 0);
	}
	}

I guess the timeout is too short for the packet to be sent, thus the loss.

Increasing this timeout to 100 ms (as for FIDO) should be enough I think

libraries/fido2/device.cpp

Lines 181 to 194 in a133bea

	void usbhid_send(uint8_t * msg)
	{
	printf1(TAG_GREEN, "Sending FIDO response block");
	#ifdef DEBUG
	byteprint(msg, 64);
	#endif
	extern uint8_t useinterface;
	if (useinterface == 2) {
	RawHID.send2(msg, 100);
	} else {
	RawHID.send(msg, 100);
	}
	}

[onlykey]

We can certainly try increasing timeout value, however there are others that use the OnlyKey Agent with RSA 4096 keys. Is it possible there is another issue on the system you are using? Are you able to check on another workstation to see if the issue persists there?

Here is the documentation on the RawHID function https://pjrc.com/teensy/rawhid.html

[onlykey]

@svareille I am working on the next OnlyKey firmware release, if I send you firmware with this increased timeout value can you test to see if this corrects the issue you are seeing?

[svareille]

Sure, I would gladly try it!
I have trouble making a MWE which works on multiple OS, so I couldn't rigorously analyze the problem yet. Although Wireshark only receive 7 packets and further inspection shows that the 7th or 8th one is lost.
So yeah, trying with the increased timeout could be a good reference.

[svareille]

Okay, so I finally got a MWE and it gives me the same result: only 7 packets are received. I have tried it on Windows and Debian, and on multiple hardware. The code can be found here: https://github.com/svareille/test-rsa4096

[onlykey]

@svareille Thanks for putting together the test script, I have used this to test but I am seeing all 8 packets returned.

cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/test-rsa4096`
Found Onlykey device at 7504:24828
Sending payload: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32]
Wrote: 39 byte(s)
Read n°1: [74, 127, 225, 147, 83, 34, 57, 233, 57, 163, 149, 162, 0, 24, 34, 175, 243, 224, 55, 203, 154, 210, 30, 50, 246, 223, 54, 109, 132, 169, 238, 228, 185, 235, 85, 144, 19, 209, 3, 234, 73, 221, 185, 31, 216, 85, 134, 117, 131, 100, 230, 180, 114, 155, 252, 182, 194, 218, 88, 172, 225, 241, 62, 124], len = 64
Read n°2: [63, 143, 29, 162, 201, 176, 205, 128, 236, 86, 24, 29, 106, 247, 34, 57, 211, 214, 244, 124, 183, 72, 255, 65, 196, 195, 187, 10, 88, 33, 185, 139, 88, 228, 5, 200, 6, 76, 160, 153, 41, 217, 77, 149, 18, 193, 51, 134, 206, 216, 82, 255, 14, 234, 144, 155, 25, 123, 222, 16, 110, 249, 32, 76], len = 64
Read n°3: [192, 153, 156, 31, 192, 0, 227, 106, 20, 57, 24, 66, 216, 172, 160, 24, 7, 150, 233, 43, 55, 168, 124, 88, 34, 132, 57, 188, 80, 195, 184, 169, 89, 107, 253, 27, 52, 221, 214, 170, 69, 30, 45, 169, 24, 241, 82, 64, 195, 102, 253, 105, 121, 200, 160, 199, 49, 78, 27, 107, 95, 148, 241, 60], len = 64
Read n°4: [147, 182, 25, 139, 16, 184, 118, 83, 140, 241, 29, 151, 75, 43, 234, 20, 209, 5, 105, 251, 0, 195, 246, 20, 220, 254, 24, 53, 89, 87, 76, 248, 88, 198, 153, 178, 5, 198, 24, 199, 112, 113, 82, 152, 215, 137, 153, 75, 229, 54, 210, 149, 77, 19, 208, 135, 203, 45, 69, 191, 42, 196, 0, 65], len = 64
Read n°5: [253, 109, 155, 182, 179, 175, 18, 75, 5, 223, 247, 134, 80, 221, 236, 189, 89, 210, 77, 151, 161, 101, 213, 163, 173, 212, 63, 70, 205, 139, 247, 130, 47, 128, 166, 61, 247, 187, 235, 101, 188, 61, 111, 119, 176, 186, 222, 237, 56, 179, 48, 142, 132, 184, 193, 19, 34, 44, 59, 22, 95, 138, 229, 56], len = 64
Read n°6: [34, 231, 97, 147, 101, 187, 37, 77, 213, 89, 35, 121, 8, 23, 224, 31, 171, 112, 253, 254, 117, 224, 209, 244, 98, 121, 123, 191, 232, 143, 191, 24, 156, 130, 3, 130, 99, 127, 9, 10, 11, 208, 231, 85, 103, 84, 147, 143, 132, 197, 249, 44, 146, 227, 99, 150, 28, 166, 68, 198, 163, 15, 61, 171], len = 64
Read n°7: [135, 159, 7, 31, 14, 29, 165, 95, 246, 177, 145, 147, 11, 177, 65, 41, 201, 3, 114, 213, 156, 18, 235, 154, 180, 134, 31, 110, 248, 75, 1, 105, 204, 76, 227, 67, 129, 55, 56, 87, 139, 188, 190, 132, 199, 243, 52, 0, 61, 220, 134, 110, 105, 157, 211, 244, 107, 199, 93, 81, 5, 183, 116, 16], len = 64
Read n°8: [148, 140, 54, 144, 102, 141, 21, 146, 14, 9, 128, 65, 164, 104, 218, 127, 42, 201, 168, 210, 116, 51, 173, 123, 120, 78, 155, 127, 21, 225, 127, 74, 116, 24, 236, 194, 42, 88, 75, 159, 224, 240, 137, 43, 76, 129, 5, 188, 225, 17, 163, 153, 8, 208, 14, 211, 137, 148, 116, 232, 125, 212, 187, 161], len = 64

USB devices like OnlyKey have send buffers so RawHID.send doesn't really send the message to your computer it just places message in a buffer and then next time the computer polls to read it reads out the messages in the buffer if that makes sense. The buffer size is larger than 8 so all 8 messages are stored in the USB buffer to be read first in first out.

So I don't think changing the timeout value will have any affect, but we can try it if you would like to test.

Another cause of this issue might be if there is another app on the computer polling OnlyKey and it gets the first message 1 of 8. Then the agent polls and there are only 7 messages left. Can you try with the OnlyKey app closed and make sure there isn't something else running that might do this?

[svareille]

Thanks for your feedback. The result is indeed strange: the same script gives me only 7 packets on
3 different computers under Debian, Windows and both. Two of these computers never saw any OnlyKey
before and thus have no OnlyKey App or onlykey-cli installed.

On the computer I currently use (Windows), with no OnlyKey App running, the script gives me 7 packets
and Wireshark (USBpcap to be precise) only sees 7 interrupt pairs. So, if I correctly understand how
USBpcap works, there is only 7 packets sent by my OnlyKey.

As this seems to work fine for you and there is no obvious oddity on my computers, I would tend to
incriminate my key (maybe a little bit slower than yours? Not something clearly visible anyway).
Thus I would still try the increased timeout.

---

The buffer size is larger than 8 so all 8 messages are stored in the USB buffer to be read first in first out.

Looking at usb_rawhid_send2, which is used to send the relevant data,
https://github.com/trustcrypto/OnlyKey-Firmware/blob/bbb910a13eb7483e44a5adec72947d0b3fce2ba1/usb_rawhid.c#L180-L188
we see that the packet will be allocated (sent to the underlying buffer) only if usb_tx_packet_count(RAWHID_TX_ENDPOINT2) < TX_PACKET_LIMIT, i.e. only if there is less than TX_PACKET_LIMIT in the underlying buffer. TX_PACKET_LIMIT is #define a few lines above to be 4
https://github.com/trustcrypto/OnlyKey-Firmware/blob/bbb910a13eb7483e44a5adec72947d0b3fce2ba1/usb_rawhid.c#L153

So I guess there can be only 4 packets waiting in the buffer...

Anyway, trying with the increased timeout of 100ms as for FIDO should definitely show if the issue is this or not.