Re: [PATCH 1/2] nvme-auth: use transformed key size to create resp

[Hannes Reinecke <hare@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1983 bytes --]

On 10/13/23 22:28, Mark O'Donovan wrote:
> This does not change current behaviour as the driver currently
> verifies that the secret size is the same size as the length of
> the transformation hash.
> 
> Co-developed-by: Akash Appaiah <Akash.Appaiah@dell.com>
> Signed-off-by: Akash Appaiah <Akash.Appaiah@dell.com>
> Signed-off-by: Mark O'Donovan <shiftee@posteo.net>
> ---
>   drivers/nvme/host/auth.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
> index daf5d144a8ea..e7d478d17b06 100644
> --- a/drivers/nvme/host/auth.c
> +++ b/drivers/nvme/host/auth.c
> @@ -418,6 +418,14 @@ static int nvme_auth_set_dhchap_failure2_data(struct nvme_ctrl *ctrl,
>   	return size;
>   }
>   
> +static int nvme_auth_dhchap_transformed_key_len(struct nvme_dhchap_key *key)
> +{
> +	if (key->hash)
> +		return nvme_auth_hmac_hash_len(key->hash);
> +
> +	return key->len;
> +}
> +
>   static int nvme_auth_dhchap_setup_host_response(struct nvme_ctrl *ctrl,
>   		struct nvme_dhchap_queue_context *chap)
>   {
> @@ -442,7 +450,8 @@ static int nvme_auth_dhchap_setup_host_response(struct nvme_ctrl *ctrl,
>   	}
>   
>   	ret = crypto_shash_setkey(chap->shash_tfm,
> -			chap->host_response, ctrl->host_key->len);
> +			chap->host_response,
> +			nvme_auth_dhchap_transformed_key_len(ctrl->host_key));
>   	if (ret) {
>   		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
>   			 chap->qid, ret);

Hmm. Yeah, hash size vs secret size always gets me.
However, wouldn't it be better to return the key size from
nvme_auth_transform_key and us that directly?
(cf the attached patch)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

[-- Attachment #2: 0001-nvme-auth-use-length-of-the-transformed-key.patch --]
[-- Type: text/x-patch, Size: 3998 bytes --]

From 14ac06a586c44ca186b5270095f551f3bdb77f18 Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <hare@suse.de>
Date: Sat, 14 Oct 2023 13:37:31 +0200
Subject: [PATCH] nvme-auth: use length of the transformed key

The key length for generating the host response is the length
of the transformed host response, which may be different from
the length of host key.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/common/auth.c | 9 +++++++--
 drivers/nvme/host/auth.c   | 7 +++++--
 include/linux/nvme-auth.h  | 3 ++-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index d90e4f0c08b7..dd5f1a221f52 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -229,7 +229,8 @@ void nvme_auth_free_key(struct nvme_dhchap_key *key)
 }
 EXPORT_SYMBOL_GPL(nvme_auth_free_key);
 
-u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
+u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
+			    size_t *transformed_len)
 {
 	const char *hmac_name;
 	struct crypto_shash *key_tfm;
@@ -243,6 +244,8 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
 	}
 	if (key->hash == 0) {
 		transformed_key = kmemdup(key->key, key->len, GFP_KERNEL);
+		if (transformed_key)
+			*transformed_len = key->len;
 		return transformed_key ? transformed_key : ERR_PTR(-ENOMEM);
 	}
 	hmac_name = nvme_auth_hmac_name(key->hash);
@@ -263,7 +266,8 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
 		goto out_free_key;
 	}
 
-	transformed_key = kzalloc(crypto_shash_digestsize(key_tfm), GFP_KERNEL);
+	*transformed_len = crypto_shash_digestsize(key_tfm);
+	transformed_key = kzalloc(*transformed_len, GFP_KERNEL);
 	if (!transformed_key) {
 		ret = -ENOMEM;
 		goto out_free_shash;
@@ -294,6 +298,7 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
 out_free_transformed_key:
 	kfree_sensitive(transformed_key);
 out_free_shash:
+	*transformed_len = 0;
 	kfree(shash);
 out_free_key:
 	crypto_free_shash(key_tfm);
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index daf5d144a8ea..929b37502f12 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -40,6 +40,7 @@ struct nvme_dhchap_queue_context {
 	u8 *ctrl_key;
 	u8 *host_key;
 	u8 *sess_key;
+	int host_response_len;
 	int ctrl_key_len;
 	int host_key_len;
 	int sess_key_len;
@@ -430,10 +431,12 @@ static int nvme_auth_dhchap_setup_host_response(struct nvme_ctrl *ctrl,
 
 	if (!chap->host_response) {
 		chap->host_response = nvme_auth_transform_key(ctrl->host_key,
-						ctrl->opts->host->nqn);
+						ctrl->opts->host->nqn,
+						&chap->host_response_len);
 		if (IS_ERR(chap->host_response)) {
 			ret = PTR_ERR(chap->host_response);
 			chap->host_response = NULL;
+			chap->host_response_len = 0;
 			return ret;
 		}
 	} else {
@@ -442,7 +445,7 @@ static int nvme_auth_dhchap_setup_host_response(struct nvme_ctrl *ctrl,
 	}
 
 	ret = crypto_shash_setkey(chap->shash_tfm,
-			chap->host_response, ctrl->host_key->len);
+			chap->host_response, chap->host_response_len);
 	if (ret) {
 		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
 			 chap->qid, ret);
diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
index dcb8030062dd..72f59632bf38 100644
--- a/include/linux/nvme-auth.h
+++ b/include/linux/nvme-auth.h
@@ -27,7 +27,8 @@ u8 nvme_auth_hmac_id(const char *hmac_name);
 struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
 					      u8 key_hash);
 void nvme_auth_free_key(struct nvme_dhchap_key *key);
-u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn);
+u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
+			    size_t *transformed_len);
 int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key);
 int nvme_auth_augmented_challenge(u8 hmac_id, u8 *skey, size_t skey_len,
 				  u8 *challenge, u8 *aug, size_t hlen);
-- 
2.35.3