[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <72488216-a847-4b41-b184-c699faec6b14@oss.cyber.gouv.fr>
Date: Mon, 30 Jun 2025 14:25:47 +0200
From: Nicolas Bouchinet <nicolas.bouchinet@....cyber.gouv.fr>
To: Sabyrzhan Tasbolatov <snovitoll@...il.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Alan Stern <stern@...land.harvard.edu>, Kannappan R <r.kannappan@...el.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Stefan Eichenberger <stefan.eichenberger@...adex.com>,
Thomas Gleixner <tglx@...utronix.de>, Pawel Laszczak <pawell@...ence.com>,
Ma Ke <make_ruc2021@....com>, Jeff Johnson <jeff.johnson@....qualcomm.com>,
Luc Bonnafoux <luc.bonnafoux@....gouv.fr>,
Luc Bonnafoux <luc.bonnafoux@....cyber.gouv.fr>,
Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>,
linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [RFC PATCH 3/4] usb: core: Plug the usb authentication capability
On 6/21/25 13:09, Sabyrzhan Tasbolatov wrote:
> On Fri, Jun 20, 2025 at 7:27 PM <nicolas.bouchinet@....cyber.gouv.fr> wrote:
>> From: Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>
>>
>> Plugs the usb authentication implementation in the usb stack and more
>> particularly in the usb_parse_configuration function after the BOS has
>> been parsed and the usb authentication capacity has been controlled.
>>
>> The authentication bulk is implemented by the usb_authenticate_device
>> function.
>>
>> The authorization decision enforcement is done via the authorized field of
>> the usb_device and the associated authorization and deauthorization functions.
>> The usb_device also contains an authenticated field that could be used to track
>> the result of the authentication process and allow for more complex security
>> policy: the user could manually authorize a device that failed the
>> authentication or manually deauthorize a device that was previously
>> authenticated.
>>
>> The usb_authenticate_device returns 0 or an error code. If 0 is
>> returned, the authorized and authenticated fields of the usb_device are
>> updated with the result of the authentication.
>>
>> Co-developed-by: Luc Bonnafoux <luc.bonnafoux@....gouv.fr>
>> Signed-off-by: Luc Bonnafoux <luc.bonnafoux@....gouv.fr>
>> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>
>> ---
>> drivers/usb/core/config.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
>> drivers/usb/core/hub.c | 6 ++++++
>> drivers/usb/core/usb.c | 5 +++++
>> include/linux/usb.h | 2 ++
>> 4 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
>> index 13bd4ec4ea5f7a6fef615b6f50b1acb3bbe44ba4..45ee8e93e263c41f1bf4271be4e69ccfcac3f650 100644
>> --- a/drivers/usb/core/config.c
>> +++ b/drivers/usb/core/config.c
>> @@ -14,6 +14,7 @@
>> #include <asm/byteorder.h>
>> #include "usb.h"
>>
>> +#include "authent.h"
>>
>> #define USB_MAXALTSETTING 128 /* Hard limit */
>>
>> @@ -824,7 +825,50 @@ static int usb_parse_configuration(struct usb_device *dev, int cfgidx,
>> kref_init(&intfc->ref);
>> }
>>
>> - /* FIXME: parse the BOS descriptor */
>> + /* If device USB version is above 2.0, get BOS descriptor */
>> + /*
>> + * Requirement for bcdUSB >= 2.10 is defined in USB 3.2 §9.2.6.6
>> + * "Devices with a value of at least 0210H in the bcdUSB field of their
>> + * device descriptor shall support GetDescriptor (BOS Descriptor) requests."
>> + *
>> + * To discuss, BOS request could be also sent for bcdUSB >= 0x0201
>> + */
>> + // Set a default value for authenticated at true in order not to block devices
>> + // that do not support the authentication
>> + dev->authenticated = 1;
>> +
>> + if (le16_to_cpu(dev->descriptor.bcdUSB) >= 0x0201) {
>> + pr_notice("bcdUSB >= 0x0201\n");
>> + retval = usb_get_bos_descriptor(dev);
>> + if (!retval) {
>> + pr_notice("found BOS\n");
>> +#ifdef CONFIG_USB_AUTHENTICATION
>> + if (dev->bos->authent_cap) {
>> + /* If authentication cap is present, start device authent */
>> + pr_notice("found Authent BOS\n");
>> + retval = usb_authenticate_device(dev);
>> + if (retval != 0) {
>> + pr_err("failed to authenticate the device: %d\n",
>> + retval);
>> + } else if (!dev->authenticated) {
>> + pr_notice("device has been rejected\n");
>> + // return early from the configuration process
>> + return 0;
> Returning 0 after rejecting a device leaves udev half-initialised and still
> linked in usb_bus_type (the hub driver only aborts after this call).
> The next hot-plug may oops on dangling pointers.
> Please consider returning -EPERM instead of 0.
We have changed the way we handle errors and
authentication/authorization. It
will appear in the next version of the patch.
We have commented and explained how we want to proceed in reply to Alan [1].
[1]:
https://lore.kernel.org/linux-usb/8cc10112-23a7-41af-b81f-7fc0c097d34d@oss.cyber.gouv.fr/
>
>> + } else {
>> + pr_notice("device has been authorized\n");
>> + }
>> + } else {
>> + // USB authentication unsupported
>> + // Apply security policy on failed devices
>> + pr_notice("no authentication capability\n");
>> + }
>> +#endif
>> + } else {
>> + // Older USB version, authentication not supported
>> + // Apply security policy on failed devices
>> + pr_notice("device does not have a BOS descriptor\n");
>> + }
>> + }
>>
>> /* Skip over any Class Specific or Vendor Specific descriptors;
>> * find the first interface descriptor */
>> @@ -1051,6 +1095,7 @@ int usb_get_bos_descriptor(struct usb_device *dev)
>> length = bos->bLength;
>> total_len = le16_to_cpu(bos->wTotalLength);
>> num = bos->bNumDeviceCaps;
>> +
>> kfree(bos);
>> if (total_len < length)
>> return -EINVAL;
>> @@ -1122,6 +1167,10 @@ int usb_get_bos_descriptor(struct usb_device *dev)
>> dev->bos->ptm_cap =
>> (struct usb_ptm_cap_descriptor *)buffer;
>> break;
>> + case USB_AUTHENT_CAP_TYPE:
>> + dev->bos->authent_cap =
>> + (struct usb_authent_cap_descriptor *)buffer;
>> + break;
>> default:
>> break;
>> }
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index 0e1dd6ef60a719f59a22d86caeb20f86991b5b29..753e55155ea34a7739b5f530dad429534e60842d 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -2640,6 +2640,12 @@ int usb_new_device(struct usb_device *udev)
>> udev->dev.devt = MKDEV(USB_DEVICE_MAJOR,
>> (((udev->bus->busnum-1) * 128) + (udev->devnum-1)));
>>
>> + // TODO: Check the device state, we want to avoid semi-initialized device to userspace.
>> + if (!udev->authenticated) {
>> + // If the device is not authenticated, abort the procedure
>> + goto fail;
>> + }
>> +
>> /* Tell the world! */
>> announce_device(udev);
>>
>> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> index 0b4685aad2d50337f3cacb2198c95a68ae8eff86..76847c01d3493e2527992a3bb927422519d9a974 100644
>> --- a/drivers/usb/core/usb.c
>> +++ b/drivers/usb/core/usb.c
>> @@ -46,6 +46,7 @@
>> #include <linux/dma-mapping.h>
>>
>> #include "hub.h"
>> +#include "authent_netlink.h"
>>
>> const char *usbcore_name = "usbcore";
>>
>> @@ -1080,6 +1081,10 @@ static int __init usb_init(void)
>> usb_debugfs_init();
>>
>> usb_acpi_register();
>> +
>> + // TODO : check error case
>> + usb_auth_init_netlink();
>> +
>> retval = bus_register(&usb_bus_type);
>> if (retval)
>> goto bus_register_failed;
>> diff --git a/include/linux/usb.h b/include/linux/usb.h
>> index b46738701f8dc46085f251379873b6a8a008d99d..e9037c8120b43556f8610f9acb3ad4129e847b98 100644
>> --- a/include/linux/usb.h
>> +++ b/include/linux/usb.h
>> @@ -431,6 +431,8 @@ struct usb_host_bos {
>> struct usb_ssp_cap_descriptor *ssp_cap;
>> struct usb_ss_container_id_descriptor *ss_id;
>> struct usb_ptm_cap_descriptor *ptm_cap;
>> + /* Authentication capability */
>> + struct usb_authent_cap_descriptor *authent_cap;
>> };
>>
>> int __usb_get_extra_descriptor(char *buffer, unsigned size,
>>
>> --
>> 2.50.0
>>
Powered by blists - more mailing lists