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>
---
V1 -> V2: support target implementation and controller secrets also

 drivers/nvme/common/auth.c | 6 +++++-
 drivers/nvme/host/auth.c   | 4 ++--
 drivers/nvme/target/auth.c | 4 ++--
 include/linux/nvme-auth.h  | 3 ++-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index d90e4f0c08b7..26a7fbdf4d55 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -243,6 +243,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)
+			key->transformed_len = key->len;
 		return transformed_key ? transformed_key : ERR_PTR(-ENOMEM);
 	}
 	hmac_name = nvme_auth_hmac_name(key->hash);
@@ -263,7 +265,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);
+	key->transformed_len = crypto_shash_digestsize(key_tfm);
+	transformed_key = kzalloc(key->transformed_len, GFP_KERNEL);
 	if (!transformed_key) {
 		ret = -ENOMEM;
 		goto out_free_shash;
@@ -297,6 +300,7 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
 	kfree(shash);
 out_free_key:
 	crypto_free_shash(key_tfm);
+	key->transformed_len = 0;
 
 	return ERR_PTR(ret);
 }
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index daf5d144a8ea..89293da3210e 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -442,7 +442,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, ctrl->host_key->transformed_len);
 	if (ret) {
 		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
 			 chap->qid, ret);
@@ -520,7 +520,7 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
 	}
 
 	ret = crypto_shash_setkey(chap->shash_tfm,
-			ctrl_response, ctrl->ctrl_key->len);
+			ctrl_response, ctrl->ctrl_key->transformed_len);
 	if (ret) {
 		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
 			 chap->qid, ret);
diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 4dcddcf95279..c46473b383b1 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -298,7 +298,7 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
 	}
 
 	ret = crypto_shash_setkey(shash_tfm, host_response,
-				  ctrl->host_key->len);
+				  ctrl->host_key->transformed_len);
 	if (ret)
 		goto out_free_response;
 
@@ -410,7 +410,7 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 	}
 
 	ret = crypto_shash_setkey(shash_tfm, ctrl_response,
-				  ctrl->ctrl_key->len);
+				  ctrl->ctrl_key->transformed_len);
 	if (ret)
 		goto out_free_response;
 
diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
index dcb8030062dd..cf24d885dfd9 100644
--- a/include/linux/nvme-auth.h
+++ b/include/linux/nvme-auth.h
@@ -10,8 +10,9 @@
 
 struct nvme_dhchap_key {
 	u8 *key;
-	size_t len;
 	u8 hash;
+	size_t len;
+	size_t transformed_len;
 };
 
 u32 nvme_auth_get_seqnum(void);
-- 
2.39.2

On 10/16/23 10:57, 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>
> ---
> V1 -> V2: support target implementation and controller secrets also
> 
>   drivers/nvme/common/auth.c | 6 +++++-
>   drivers/nvme/host/auth.c   | 4 ++--
>   drivers/nvme/target/auth.c | 4 ++--
>   include/linux/nvme-auth.h  | 3 ++-
>   4 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
> index d90e4f0c08b7..26a7fbdf4d55 100644
> --- a/drivers/nvme/common/auth.c
> +++ b/drivers/nvme/common/auth.c
> @@ -243,6 +243,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)
> +			key->transformed_len = key->len;
>   		return transformed_key ? transformed_key : ERR_PTR(-ENOMEM);

Hmm. But now we're inconsistent.
The input structure 'key' doesn't know (nor does care) if the key has 
been transformed; the transformation itself is returned in a different 
structure.

If we were to go that way the it'll be better to return a 'struct 
nvme_dhchap_key' from nvme_auth_transform_key(), which will then 
encapsulate the data and the correct length.

Which probably is not a bad idea, seeing that a key transform really 
should generate a new key.

Hmm?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg
Managing Directors: I. Totev, A. Myers, A. McDonald, M. B. Moerman
(HRB 36809, AG Nürnberg)