Re: [PATCH 4/9] nvme: add nvme_auth_derive_tls_psk()

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Jul 22, 2024 at 04:21:17PM +0200, Hannes Reinecke wrote:
> +/*
> + * Derive a TLS PSK as specified in TP8018 Section 3.6.1.3:
> + *   TLS PSK and PSK identity Derivation
> + *
> + * The TLS PSK shall be derived as follows from an input PSK
> + * (i.e., either a retained PSK or a generated PSK) and a PSK
> + * identity using the HKDF-Extract and HKDF-Expand-Label operations
> + * (refer to RFC 5869 and RFC 8446) where the hash function is the
> + * one specified by the hash specifier of the PSK identity:
> + * 1. PRK = HKDF-Extract(0, Input PSK); and
> + * 2. TLS PSK = HKDF-Expand-Label(PRK, "nvme-tls-psk", PskIdentityContext, L),
> + * where PskIdentityContext is the hash identifier indicated in
> + * the PSK identity concatenated to a space character and to the
> + * Base64 PSK digest (i.e., "<hash> <PSK digest>") and L is the
> + * output size in bytes of the hash function (i.e., 32 for SHA-256
> + * and 48 for SHA-384).
> + */
> +int nvme_auth_derive_tls_psk(int hmac_id, u8 *psk, size_t psk_len,
> +		u8 *psk_digest, u8 **ret_psk)
> +{
> +	struct crypto_shash *hmac_tfm;
> +	const char *hmac_name;
> +	const char *psk_prefix = "tls13 nvme-tls-psk";
> +	size_t info_len, prk_len;
> +	char *info;
> +	unsigned char *prk, *tls_key;
> +	int ret;
> +
> +	hmac_name = nvme_auth_hmac_name(hmac_id);
> +	if (!hmac_name) {
> +		pr_warn("%s: invalid hash algoritm %d\n",
> +			__func__, hmac_id);
> +		return -EINVAL;
> +	}
> +	if (hmac_id == NVME_AUTH_HASH_SHA512) {
> +		pr_warn("%s: unsupported hash algorithm %s\n",
> +			__func__, hmac_name);
> +		return -EINVAL;
> +	}
> +
> +	hmac_tfm = crypto_alloc_shash(hmac_name, 0, 0);
> +	if (IS_ERR(hmac_tfm))
> +		return PTR_ERR(hmac_tfm);
> +
> +	prk_len = crypto_shash_digestsize(hmac_tfm);
> +	prk = kzalloc(prk_len, GFP_KERNEL);
> +	if (!prk) {
> +		ret = -ENOMEM;
> +		goto out_free_shash;
> +	}
> +
> +	ret = hkdf_extract(hmac_tfm, psk, psk_len, prk);
> +	if (ret)
> +		goto out_free_prk;
> +
> +	ret = crypto_shash_setkey(hmac_tfm, prk, prk_len);
> +	if (ret)
> +		goto out_free_prk;
> +
> +	info_len = strlen(psk_digest) + strlen(psk_prefix) + 1;
> +	info = kzalloc(info_len, GFP_KERNEL);
> +	if (!info)
> +		goto out_free_prk;
> +
> +	memcpy(info, psk_prefix, strlen(psk_prefix));
> +	memcpy(info + strlen(psk_prefix), psk_digest, strlen(psk_digest));

The code doesn't match the description given in the function comment (which
looks like it was quoted from a specification).

The code does HKDF-Expand with info="tls13 nvme-tls-psk<PSK digest>".

The description does HKDF-Expand-Label with Label="nvme-tls-psk",
Context="<hash identifier> <PSK digest>", Length=digest_size.

Not only does the code not actually use <hash identifier>, but it doesn't follow
the definition of HKDF-Expand-Label from RFC8446
(https://datatracker.ietf.org/doc/html/rfc8446#section-7.1) in that it's missing
all the length fields.  So the info string used by the actual code ends up being
quite different from the one specified.

- Eric