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

Chris Leech
2026-04-01T18:13:24Z

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