diff mbox series

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

Message ID 20231013202827.2262708-2-shiftee@posteo.net
State New
Headers
Series Remove secret-size restrictions for hashes |

Commit Message

Mark O'Donovan Oct. 13, 2023, 8:28 p.m. UTC 
  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(-)

Comments

Hannes Reinecke Oct. 14, 2023, 11:41 a.m. UTC | #1 
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
Mark O'Donovan Oct. 16, 2023, 8:54 a.m. UTC | #2 
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 <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

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
diff mbox series

Patch

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);