[PATCH v2 0/2] Remove secret-size restrictions for hashes

[PATCH v2 0/2] Remove secret-size restrictions for hashes

[Mark O'Donovan]

This relates to the hash functions used to transform the secret.
The kernel currently restricts us to using secrets equal in size
to the transformation hash function they use.
e.g. 32 byte secrets with the SHA-256(32 byte) hash function.

This restriction is not required by the spec and means
incompatibility with more permissive implementations.

With these patches the example secret from the spec should now
be permitted with any of the following:
DHHC-1:00:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:
DHHC-1:01:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:
DHHC-1:02:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:
DHHC-1:03:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:

Note: Secrets are still restricted to 32,48 or 64 bits.


Mark O'Donovan (2):
  nvme-auth: use transformed key size to create resp
  nvme-auth: allow mixing of secret and hash lengths

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

-- 
2.39.2

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

[Mark O'Donovan]

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

[PATCH v2 2/2] nvme-auth: allow mixing of secret and hash lengths

[Mark O'Donovan]

We can now use any of the secret transformation hashes with a
secret, regardless of the secret size.
e.g. a 32 byte key with the SHA-512(64 byte) hash.

The example secret from the spec should now be permitted with
any of the following:
DHHC-1:00:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:
DHHC-1:01:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:
DHHC-1:02:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:
DHHC-1:03:ia6zGodOr4SEG0Zzaw398rpY0wqipUWj4jWjUh4HWUz6aQ2n:

Note: Secrets are still restricted to 32,48 or 64 bits.

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>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/common/auth.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index 26a7fbdf4d55..712178e1fb02 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -187,14 +187,6 @@ struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
 		goto out_free_secret;
 	}
 
-	if (key_hash > 0 &&
-	    (key_len - 4) != nvme_auth_hmac_hash_len(key_hash)) {
-		pr_err("Mismatched key len %d for %s\n", key_len,
-		       nvme_auth_hmac_name(key_hash));
-		ret = -EINVAL;
-		goto out_free_secret;
-	}
-
 	/* The last four bytes is the CRC in little-endian format */
 	key_len -= 4;
 	/*
-- 
2.39.2

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

[Hannes Reinecke]

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)