From a3c62f442981e39186ce78c140921c4bedfd7b16 Mon Sep 17 00:00:00 2001 From: Carlos Garnacho Date: Fri, 17 May 2024 11:35:41 +0200 Subject: [PATCH] usb-protection: Treat hubs and HID devices like any other USB gadget The checks on the classes offered by a USB device are pretty lax and uninformative from the kernel and UsbGuard levels, so our attempt at user friendliness with USB hubs and HID devices may result in everyone (lastly us) allowing maliciously crafted devices that present themselves as one of these devices, but implement other classes (e.g. mass storage). We believe this is ultimately an issue in the lower layers if this may go through as good up to us and we cannot truly believe UsbGuard information, but it is definitely us being the front face of this issue. Avoid treating USB hubs and HID devices different to any other USB gadget, this will require users to "enroll" them the same ways. --- .../gsd-usb-protection-manager.c | 58 +++++-------------- 1 file changed, 15 insertions(+), 43 deletions(-) diff --git a/plugins/usb-protection/gsd-usb-protection-manager.c b/plugins/usb-protection/gsd-usb-protection-manager.c index 43644408..538b6b1f 100644 --- a/plugins/usb-protection/gsd-usb-protection-manager.c +++ b/plugins/usb-protection/gsd-usb-protection-manager.c @@ -678,56 +678,28 @@ on_usbguard_signal (GDBusProxy *proxy, * If this device advertises also interfaces outside the HID class, or the * HUB class, it is suspect. It could be a false positive because this could * be a "smart" keyboard for example, but at this stage is better be safe. */ - if (hid_or_hub && !has_other_classes) { - guint device_id; + if (protection_level == G_DESKTOP_USB_PROTECTION_LOCKSCREEN) { show_notification (manager, - _("New device detected"), - _("Either one of your existing devices has been reconnected or a new one has been plugged in. " - "If you did not do it, check your system for any suspicious device.")); - g_variant_get_child (parameters, POLICY_APPLIED_DEVICE_ID, "u", &device_id); - authorize_device (manager, device_id); + _("Reconnect USB device"), + _("New device has been detected while you were away. " + "Please disconnect and reconnect the device to start using it.")); } else { - if (protection_level == G_DESKTOP_USB_PROTECTION_LOCKSCREEN) { - show_notification (manager, - _("Reconnect USB device"), - _("New device has been detected while you were away. " - "Please disconnect and reconnect the device to start using it.")); - } else { - const char* name_for_notification = device_name ? device_name : "unknown name"; - g_debug ("Showing notification for %s", name_for_notification); - show_notification (manager, - _("USB device blocked"), - _("New device has been detected while you were away. " - "It has been blocked because the USB protection is active.")); - } + const char* name_for_notification = device_name ? device_name : "unknown name"; + g_debug ("Showing notification for %s", name_for_notification); + show_notification (manager, + _("USB device blocked"), + _("New device has been detected while you were away. " + "It has been blocked because the USB protection is active.")); } } else { /* If the protection level is "lockscreen" the device will be automatically * authorized by usbguard. */ if (protection_level == G_DESKTOP_USB_PROTECTION_ALWAYS) { - /* We authorize the device if this is a HID, - * e.g. a keyboard or a mouse, or an HUB. - * We also lock the screen to prevent an attacker to plug malicious - * devices if the legitimate user forgot to lock his session. - * - * If this device advertises also interfaces outside the HID class, or the - * HUB class, it is suspect. It could be a false positive because this could - * be a "smart" keyboard for example, but at this stage is better be safe. */ - if (hid_or_hub && !has_other_classes) { - ManagerDeviceId* manager_devid = g_malloc ( sizeof (ManagerDeviceId) ); - manager_devid->manager = manager; - g_variant_get_child (parameters, POLICY_APPLIED_DEVICE_ID, "u", &(manager_devid->device_id)); - gsd_screen_saver_call_lock (manager->screensaver_proxy, - manager->cancellable, - (GAsyncReadyCallback) on_screen_locked, - manager_devid); - } else { - show_notification (manager, - _("USB device blocked"), - _("The new inserted device has been blocked because the USB protection is active. " - "If you want to activate the device, disable the USB protection and re-plug " - "the device.")); - } + show_notification (manager, + _("USB device blocked"), + _("The new inserted device has been blocked because the USB protection is active. " + "If you want to activate the device, disable the USB protection and re-plug " + "the device.")); } else { /* This is protection level == Lockscreen, so we allow everything when the session is unlocked. There should be a USBGuard rule that automatically allows all devices, -- 2.44.0