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 <[email protected]>
Signed-off-by: Akash Appaiah <[email protected]>
Signed-off-by: Mark O'Donovan <[email protected]>
---
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);
--
2.39.2
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 <[email protected]>
> Signed-off-by: Akash Appaiah <[email protected]>
> Signed-off-by: Mark O'Donovan <[email protected]>
> ---
> 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
[email protected] +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
On 14/10/2023 12:41, Hannes Reinecke wrote:
> 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 <[email protected]>
>> Signed-off-by: Akash Appaiah <[email protected]>
>> Signed-off-by: Mark O'Donovan <[email protected]>
>> ---
>> 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
Hi Hannes,
I gave this a try and it ended up being easier to put it in struct nvme_dhchap_key.
V2 also does the nvme target code, and this means the length is stored in the same place.
Let me know if this works for you.
Thanks,
Mark