[PATCH 4/6] usb: kbd: Ignore Yubikeys
Marek Vasut
marex at denx.de
Sun Feb 25 22:57:23 CET 2024
On 2/25/24 5:07 PM, Janne Grunau wrote:
> Hej,
>
> On Wed, Feb 21, 2024, at 13:41, Marek Vasut wrote:
>> On 2/21/24 08:25, Janne Grunau via B4 Relay wrote:
>>> From: Hector Martin <marcan at marcan.st>
>>>
>>> We currently only support one USB keyboard device, but some devices
>>> emulate keyboards for other purposes. Most commonly, people run into
>>> this with Yubikeys, so let's ignore those.
>>>
>>> Even if we end up supporting multiple keyboards in the future, it's
>>> safer to ignore known non-keyboard devices.
>>>
>>> Signed-off-by: Hector Martin <marcan at marcan.st>
>>> ---
>>> common/usb_kbd.c | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>>> index 4cbc9acb73..774d3555d9 100644
>>> --- a/common/usb_kbd.c
>>> +++ b/common/usb_kbd.c
>>> @@ -120,6 +120,15 @@ struct usb_kbd_pdata {
>>>
>>> extern int __maybe_unused net_busy_flag;
>>>
>>> +/*
>>> + * Since we only support one usbkbd device in the iomux,
>>> + * ignore common keyboard-emulating devices that aren't
>>> + * real keyboards.
>>> + */
>>> +const uint16_t vid_blocklist[] = {
>>> + 0x1050, /* Yubico */
>>> +};
>>> +
>>> /* The period of time between two calls of usb_kbd_testc(). */
>>> static unsigned long kbd_testc_tms;
>>>
>>> @@ -465,6 +474,7 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum)
>>> struct usb_endpoint_descriptor *ep;
>>> struct usb_kbd_pdata *data;
>>> int epNum;
>>> + int i;
>>>
>>> if (dev->descriptor.bNumConfigurations != 1)
>>> return 0;
>>> @@ -480,6 +490,15 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum)
>>> if (iface->desc.bInterfaceProtocol != USB_PROT_HID_KEYBOARD)
>>> return 0;
>>>
>>> + for (i = 0; i < ARRAY_SIZE(vid_blocklist); i++) {
>>> + if (dev->descriptor.idVendor == vid_blocklist[i]) {
>>> + printf("Ignoring keyboard device 0x%x:0x%x\n",
>>> + dev->descriptor.idVendor,
>>> + dev->descriptor.idProduct);
>>> + return 0;
>>> + }
>>> + }
>>
>> I vaguely recall a discussion about previous version of this, I think
>> the suggestion was to make the list of ignored devices configurable via
>> environment variable, so users can add to that list from U-Boot shell.
>> Would it be possible to make it work this way ?
>
> oh, I completely forgot that this patch was already submitted. I briefly looked through asahi tree for related patches and did not check whether this was previously submitted.
> I've added environment based blocking as separate patch with blocking either complete vendor IDs or vendor, product ID combinations. A separate patch to simplify authorship tracking and the implementation doesn't share any code.
It would be better to have only one patch which does not hard-code any
USB IDs, and then add those blocked IDs via U-Boot default environment
for this specific machine. We cannot predict what yubico will do in the
future, whether they might make a device that shouldn't be blocked for
example. If they do, the user should be able to unblock their device by
running e.g. '=> setenv usb_blocklist' instead of updating their bootloader.
I think a simple list of blocked VID:PID pairs, maybe with wildcards,
would be nice, i.e. something like 'usb_blocklist=1234:5678,1050:*' to
block device 0x1234:0x5678 and all devices with VID 0x1050 . That should
be easy to parse with strtok()/strtol() or some such and the code should
not be too complex.
More information about the U-Boot
mailing list