[4/9] nvme: add nvme_auth_derive_tls_psk()

Add a function to derive the TLS PSK as specified TP8018.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/common/auth.c | 90 ++++++++++++++++++++++++++++++++++++++
 include/linux/nvme-auth.h  |  2 +
 2 files changed, 92 insertions(+)

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

On 7/23/24 03:47, Eric Biggers wrote:
> 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.
> 
Aw. Guess you are right. Will be fixing it up.

Cheers,

Hannes