Re: [PATCH 2/8] nvme-keyring: add 'dhchap' key type

[Chris Leech <cleech@redhat.com>]

On Tue, Mar 17, 2026 at 02:00:57PM +0100, Hannes Reinecke wrote:
> Add a 'dhchap' keytype to store DH-HMAC-CHAP secret keys.
> Keys are stored with a 'user-type' compatible payload, such
> that one can use 'user_read()' to access the raw contents
> and the 'read()' callback to get the base64-encoded key
> data in the DH-HMAC-CHAP secret representation.
> 
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
>  drivers/nvme/common/keyring.c | 216 ++++++++++++++++++++++++++++++++++
>  1 file changed, 216 insertions(+)
> 
> diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
> index 32d16c53133b..f7e18df438e6 100644
> --- a/drivers/nvme/common/keyring.c
> +++ b/drivers/nvme/common/keyring.c
> @@ -4,6 +4,9 @@
>   */
...  
> +/**
> + * nvme_dhchap_psk_preparse - prepare DH-HMAC-CHAP key data
> + * @prep: preparsed payload of the key data
> + *
> + * Decode the DH-HMAC-CHAP key data passed in in @prep and
> + * store the resulting binary data. The binary data includes
> + * space for the CRC, the version, and the hmac identifier,
> + * but the data length is just the key data without the CRC.
> + * This allows the user to read the key data via the
> + * 'user_read()' function. The additional 'version' ahd 'hmac'
> + * data is used in the ->read() callback to generate the
> + * base64 encoded key.
> + */
> +static int nvme_dhchap_psk_preparse(struct key_preparsed_payload *prep)
> +{
> +	struct user_key_payload *upayload;
> +	size_t datalen = prep->datalen, keylen;
> +	int ret;
> +	u32 crc;
> +	u8 version, hmac;
> +
> +	if (!prep->data) {
> +		pr_debug("%s: Empty data", __func__);
> +		prep->payload.data[0] = NULL;
> +		prep->quotalen = 0;
> +		return -EINVAL;
> +	}
> +
> +	if (sscanf(prep->data, "DHHC-%01hhu:%02hhu:%*s",
> +		   &version, &hmac) != 2) {
> +		pr_debug("%s: invalid key data '%s'\n", __func__,
> +			 (char *)prep->data);
> +		prep->payload.data[0] = NULL;
> +		prep->quotalen = 0;
> +		return -EINVAL;
> +	}

version should be verified to be the expected value of 1 here
(or hardcoded to only accept 1 in the sscanf call like the repalced
code removed in the next patch)
 
> +	/* skip header and final ':' character */
> +	datalen -= 11;
> +
> +	/*
> +	 * payload is < key | version | hmac >
> +	 * base64 decode will always return less data
> +	 * than the encoded data, so allocating the size
> +	 * of the encoded data will be large enough.
> +	 */
> +	upayload = kzalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
> +	if (!upayload) {
> +		prep->payload.data[0] = NULL;
> +		prep->quotalen = 0;
> +		return -ENOMEM;
> +	}
> +
> +	/* decode the data */
> +	prep->quotalen = keylen;

Uninitialized keylen is being used here to set quotelen.

> +	prep->payload.data[0] = upayload;
> +	ret = base64_decode(prep->data + 10, datalen, upayload->data,
> +			    true, BASE64_STD);
> +	if (ret < 0) {
> +		pr_debug("%s: Failed to decode key %s\n",
> +			 __func__, (char *)prep->data + 10);
> +		return ret;
> +	}
> +	ret -= 4;
> +	crc = ~crc32(~0, upayload->data, ret);
> +	if (get_unaligned_le32(upayload->data + ret) != crc) {
> +		pr_debug("%s: CRC mismatch for key\n", __func__);
> +		/* CRC mismatch */
> +		return -EKEYREJECTED;
> +	}
> +	/* append version and hmac to the payload */
> +	upayload->data[ret + 4] = version;
> +	upayload->data[ret + 5] = hmac;
> +	upayload->datalen = ret;
> +	return 0;
> +}
> +
> +/**
> + * nvme_dhchap_decoded_key_size - Size of the base64-decoded key
> + * @size: size of the encoded key
> + *
> + * Returns the expected size of the key after base64 decoding.
> + */

Isn't this the expected size after encoding, and not decoding?
It's used before a call to base64_encode.

> +static inline int nvme_dhchap_decoded_key_size(int size)
> +{
> +	int keylen = -EINVAL;
> +
> +	switch (size) {
> +	case 32:
> +		keylen = 48;
> +		break;
> +	case 48:
> +		keylen = 72;
> +		break;
> +	case 64:
> +		keylen = 92;
> +		break;
> +	default:
> +		break;
> +	}
> +	return keylen;
> +}

Why don't these keylen numbers match BASE64_CHARS()?

> +/**
> + * nvme_dhchap_psk_read - read callback for dhchap key types
> + * @key: key to read from
> + * @buffer: buffer for the key contents
> + * @buflen: length of @buffer
> + *
> + * Formets the DH-HMAC-CHAP key in base64-encoded form as

spelling typo /Formets/Formats/

- Chris