[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e028a659-9535-4cf9-92c1-373f72fae3cf@suse.com>
Date: Wed, 25 Jun 2025 11:59:21 +0200
From: Oliver Neukum <oneukum@...e.com>
To: nicolas.bouchinet@....cyber.gouv.fr,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Alan Stern <stern@...land.harvard.edu>,
Kannappan R <r.kannappan@...el.com>,
Sabyrzhan Tasbolatov <snovitoll@...il.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 2/4] usb: core: Introduce usb authentication feature
On 20.06.25 16:27, nicolas.bouchinet@....cyber.gouv.fr wrote:
> +/**
> + * usb_authent_req_digest - Check if device is known via its digest
> + * @dev: [in] pointer to the usb device to query
> + * @buffer: [inout] buffer to hold request data
> + * @digest: [out] device digest
> + *
> + * Context: task context, might sleep.
> + *
> + * This function sends a digest request to the usb device.
> + *
> + * Possible errors:
> + * - ECOMM : failed to send or received a message to the device
> + * - EINVAL : if buffer or mask is NULL
> + *
> + * Return: If successful, zero. Otherwise, a negative error number.
> + */
> +static int usb_authent_req_digest(struct usb_device *dev, uint8_t *const buffer,
How can buffer be const if it is used for output?
[..]
> +struct usb_auth_cert_req {
> + uint16_t offset;
> + uint16_t length;
> +} __packed;
Endianness?
> +/**
> + * usb_authent_read_certificate - Read a device certificate
> + * @dev: [in] pointer to the usb device to query
> + * @buffer: [inout] buffer to hold request data, caller allocated
> + * @slot: [in] certificate chain to be read
> + * @cert_der: [out] buffer to hold received certificate chain
> + * @cert_len: [out] length of received certificate
> + *
> + * Context: task context, might sleep.
> + *
> + * Possible errors:
> + * - EINVAL : NULL pointer or invalid slot value
> + * - ECOMM : failed to send request to device
> + * - ENOMEM : failed to allocate memory for certificate
> + *
> + * Return: If successful, zero. Otherwise, a negative error number.
> + */
> +static int usb_authent_read_certificate(struct usb_device *dev, uint8_t *const buffer,
> + uint8_t slot, uint8_t **cert_der, size_t *cert_len)
> +{
> + uint16_t read_offset = 0;
> + uint16_t read_length = 0;
> + uint8_t chain_part[64] = { 0 };
> +
> + if (unlikely(slot >= 8 || buffer == NULL || cert_der == NULL || cert_len == NULL)) {
> + pr_err("invalid arguments\n");
> + return -EINVAL;
> + }
> +
> + // First request to get certificate chain length
> + if (usb_auth_read_cert_part(dev, buffer, slot, 0,
> + USB_AUTH_CHAIN_HEADER_SIZE,
> + chain_part) != 0) {
> + pr_err("Failed to get first certificate part\n");
> + return -ECOMM;
> + }
> +
> + // Extract total length
> + *cert_len = ((uint16_t *)chain_part)[0];
Endianness
> +
> +/**
> + * usb_authent_challenge_dev - Challenge a device
> + * @dev: [in] pointer to the usb device to query
> + * @buffer: [in] pointer to the buffer allocated for USB query
> + * @slot: [in] certificate chain to be used
> + * @slot_mask: [in] slot mask of the device
> + * @nonce: [in] nonce to use for the challenge, 32 bytes long
> + * @chall: [out] buffer for chall response, 204 bytes long, caller allocated
> + *
> + * Context: task context, might sleep.
> + *
> + * Possible errors:
> + * - EINVAL : NULL input pointer or invalid slot value
> + * - ECOMM : failed to send or receive message from the device
> + *
> + * Return: If successful, zero. Otherwise, a negative error number.
> + */
> +static int usb_authent_challenge_dev(struct usb_device *dev, uint8_t *buffer,
> + const uint8_t slot, const uint8_t slot_mask, const uint8_t *const nonce,
> + uint8_t *const chall)
> +{
> + int ret = -1;
> +
> + if (unlikely(buffer == NULL || slot >= 8 || nonce == NULL)) {
> + pr_err("invalid arguments\n");
> + return -EINVAL;
> + }
> +
> + // AUTH OUT challenge request transfer
> + memcpy(buffer, nonce, 32);
> + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), AUTH_OUT,
> + USB_DIR_OUT,
> + (USB_SECURITY_PROTOCOL_VERSION << 8) +
> + USB_AUTHENT_CHALLENGE_REQ_TYPE,
> + (slot << 8), buffer, 32, USB_CTRL_GET_TIMEOUT);
> + if (ret < 0) {
> + pr_err("Failed to send challenge request: %d\n", ret);
> + ret = -ECOMM;
> + goto cleanup;
> + }
> +
> + // Complete the challenge with the request
> + chall[1] = USB_SECURITY_PROTOCOL_VERSION;
> + chall[0] = USB_AUTHENT_CHALLENGE_REQ_TYPE;
> + chall[2] = slot;
> + chall[3] = 0x00;
> + memcpy(chall+4, nonce, 32);
This may be worth a definition.
> +
> + // AUTH IN challenge response transfer
> + ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), AUTH_IN, USB_DIR_IN,
> + (USB_SECURITY_PROTOCOL_VERSION << 8) +
> + USB_AUTHENT_CHALLENGE_RESP_TYPE,
> + (slot << 8) + slot_mask, buffer, 168,
> + USB_CTRL_GET_TIMEOUT);
> + if (ret < 0) {
> + pr_err("Failed to get challenge response: %d\n", ret);
> + ret = -ECOMM;
> + goto cleanup;
> + }
> +
> + pr_notice("received challenge response\n");
> +
> + // Complete last part of the challenge with what is returned by the device
> + memcpy(chall+USB_AUTH_CHAIN_HEADER_SIZE, buffer, 168);
The 168 comes whence?
> +
> + ret = 0;
> +
> +cleanup:
> +
> + return ret;
> +}
> +/**
> + * @brief Check that the authentication can resume after a sleep
> + *
> + * @param [in] dev : the usb device
> + * @param [in] hub : the parent hub
> + *
> + * Possible error codes:
> + * - ENODEV : hub has been disconnected
> + *
> + * @return 0 if possible to resume, else an error code
> + */
> +static int usb_auth_try_resume(struct usb_device *dev, struct usb_device *hub)
> +{
> + // Test if the hub or the device has been disconnected
> + if (unlikely(hub == NULL || dev == NULL ||
> + dev->port_is_suspended == 1 ||
> + dev->reset_in_progress == 1)) {
> + return -ENODEV;
> + }
> +
> + // TODO: test if the device has not been disconnected
> + // TODO: test if the device has not been disconnected then replaced with another one
> +
> + return 0;
> +}
> +
> +/**
> + * usb_authenticate_device - Challenge a device
> + * @dev: [inout] pointer to device
> + *
> + * Context: task context, might sleep.
> + *
> + * Authentication is done in the following steps:
> + * 1. Get device certificates digest to determine if it is already known
> + * if yes, go to 3.
> + * 2. Get device certificates
> + * 3. Challenge device
> + * 4. Based on previous result, determine if device is allowed under local
> + * security policy.
> + *
> + * Possible error code:
> + * - ENOMEM : failed to allocate memory for exchange
> + * - TODO: complete all possible error case
> + *
> + * Return: If successful, zero. Otherwise, a negative error number.
> + */
> +int usb_authenticate_device(struct usb_device *dev)
> +{
> + int ret = 0;
> +
> + uint8_t is_valid = 0;
> + uint8_t is_known = 0;
> + uint8_t is_blocked = 0;
> + uint8_t chain_nb = 0;
> + uint8_t slot_mask = 0;
> + uint8_t slot = 0;
> + uint8_t digests[256] = { 0 };
> + uint8_t nonce[32] = {0};
> + uint8_t chall[204] = {0};
> + uint32_t dev_id = 0;
> + size_t ctx_size = 0;
> + int i = 0;
> +
> + uint8_t *cert_der = NULL;
> + size_t cert_len = 0;
> +
> + if (unlikely(dev == NULL || dev->parent == NULL))
> + return -ENODEV;
> +
> + struct usb_device *hub = dev->parent;
> +
> + // By default set authorization status at false
> + dev->authorized = 0;
> + dev->authenticated = 0;
> +
> + uint8_t *buffer = NULL;
> + // Buffer to hold responses
> + buffer = kzalloc(512, GFP_KERNEL);
Should this not be cached for comparison after resume?
Regards
Oliver
Powered by blists - more mailing lists