Skip to content

Commit f40ea15

Browse files
committed
U2F: Fix on safari/macos
U2F is typically a stateless protocol, so between two transactions (request+response) there is not supposed to be any lock held. It was assumed that the "FIDO Client" (typically web browser) would keep sending only U2F `register` or `authenticate` messages when it had started doing so. But it is also legal to send for example a new U2FHID `init` message in between. (the U2F reference test cases do this for example). Before this commit we held a "lock" while the confirm screen was active until the user had confirmed and didn't allow any other messages in between. This breaks the U2F protocol as explained above. After this commit we instead only hold the "lock" for the transaction (request + response), which is the correct way according to the u2f reference: The application channel that manages to get through the first initialization packet when the device is in idle state will keep the device locked for other channels until the last packet of the response message has been received. The device then returns to idle state, ready to perform another transaction for the same or a different channel. Between two transactions, no state is maintained in the device and a host application must assume that any other process may execute other transactions at any time.
1 parent 96d2313 commit f40ea15

File tree

12 files changed

+111
-150
lines changed

12 files changed

+111
-150
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
88

99
### [Unreleased]
1010
- EVM: add HyperEVM (HYPE) and SONIC (S) to known networks
11+
- U2F: fix macos/safari macos/firefox support
1112

1213
### 9.23.0
1314
- Ethereum: add confirmation screen for known networks, change base unit to ETH for Arbitrum and Optimism

src/firmware.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,13 @@
2525
#include "screen.h"
2626
#include "ui/screen_stack.h"
2727
#include "usb/usb_processing.h"
28+
#include <hww.h>
2829
#include <memory/memory_spi.h>
2930

31+
#if APP_U2F == 1
32+
#include <u2f.h>
33+
#endif
34+
3035
uint32_t __stack_chk_guard = 0;
3136

3237
int main(void)
@@ -44,6 +49,11 @@ int main(void)
4449
da14531_protocol_init();
4550
}
4651
usb_processing_init();
52+
// Setup usb_processing handlers
53+
hww_setup();
54+
#if APP_U2F == 1
55+
u2f_device_setup();
56+
#endif
4757
firmware_main_loop();
4858
return 0;
4959
}

src/firmware_main_loop.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ void firmware_main_loop(void)
103103
}
104104
if (!u2f_data) {
105105
u2f_data = queue_pull(queue_u2f_queue());
106+
// If USB stack was locked and there is no more messages to send out, time to
107+
// unlock it.
108+
if (!u2f_data && usb_processing_locked(usb_processing_u2f())) {
109+
usb_processing_unlock();
110+
}
106111
}
107112
#endif
108113
// Do USB Input
@@ -120,6 +125,7 @@ void firmware_main_loop(void)
120125
}
121126
#if APP_U2F == 1
122127
if (!u2f_data && hid_u2f_read(&u2f_frame[0])) {
128+
util_log("u2f data %s", util_dbg_hex((void*)u2f_frame, 16));
123129
u2f_packet_process((const USB_FRAME*)u2f_frame);
124130
if (communication_mode_ble_enabled()) {
125131
// Enqueue a power down command to the da14531
@@ -152,6 +158,7 @@ void firmware_main_loop(void)
152158
#if APP_U2F == 1
153159
if (!communication_mode_ble_enabled() && u2f_data) {
154160
if (hid_u2f_write_poll(u2f_data)) {
161+
util_log("u2f wrote %s", util_dbg_hex(u2f_data, 16));
155162
u2f_data = NULL;
156163
}
157164
}

0 commit comments

Comments
 (0)