Re: [RFC PATCH 2/4] usb: core: Introduce usb authentication feature

[Oliver Neukum <oneukum@suse.com>]

On 20.06.25 16:27, nicolas.bouchinet@oss.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