search

trezor / trezor-firmware

:lock: Trezor Firmware Monorepo
https://trezor.io
Other
1.36k stars 661 forks source link

U2F: enhance u2fhid_wink behavior #6

Open prusnak opened 5 years ago

prusnak commented 5 years ago

reposted from @invd:

In the U2F protocol, the Trezor explicitly specifies that it supports the U2F_WINK mechanism This is meant as

The wink command performs a vendor-defined action that provides some visual- or audible identification a particular U2F device. A typical implementation will do a short burst of flashes with a LED or something similar. This is useful when more than one device is attached to a computer and there is confusion which device is paired with which connection.

However, as far as I can see, u2fhid_wink() does not perform such a visual indication and mainly treats it as a sort of keepalive message instead: https://github.com/trezor/trezor-mcu/blob/b1725e7264aae72b0ad3c6f4e829f7fc48629a46/firmware/u2f.c#L262

If I recall correctly, @jhoenicke has also said in personal conversation that we should likely improve this.

To be clear, this is a low priority topic and a pure UI / UX enhancement. As far as I can see, there are no security implications or spec violations in terms of protocol handling with the current implementation.

prusnak commented 5 years ago

I think we don't need to react with "winking" because Trezor is already showing U2F dialog on the display when this happens. This means there is no confusion which device is linked to the current U2F process.

I'll keep this open for now, but let's not fix this in 1.7.2.