public inbox for linux-nvme@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Remove secret-size restrictions for hashes
@ 2023-10-17 10:52 Mark O'Donovan
  2023-10-17 10:52 ` [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer Mark O'Donovan
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Mark O'Donovan @ 2023-10-17 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-nvme, sagi, hch, axboe, kbusch, hare, 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.

v1:
- Initial submission

v2:
- Added transformed_len as member of struct nvme_dhchap_key

v3:
- Return a struct nvme_dhchap_key from nvme_auth_transform_key()

v4:
- added helper to caclulate key struct size using struct_size()
- Break up lines which were too long
- Replace ternary operator with if
- Add missing ERR_CAST()

Mark O'Donovan (3):
  nvme-auth: alloc nvme_dhchap_key as single buffer
  nvme-auth: use transformed key size to create resp
  nvme-auth: allow mixing of secret and hash lengths

 drivers/nvme/common/auth.c | 66 ++++++++++++++++++++++----------------
 drivers/nvme/host/auth.c   | 30 ++++++++---------
 drivers/nvme/target/auth.c | 31 ++++++++++--------
 include/linux/nvme-auth.h  |  7 ++--
 4 files changed, 75 insertions(+), 59 deletions(-)

-- 
2.39.2



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer
  2023-10-17 10:52 [PATCH v4 0/3] Remove secret-size restrictions for hashes Mark O'Donovan
@ 2023-10-17 10:52 ` Mark O'Donovan
  2023-10-17 15:55   ` kernel test robot
  2023-10-17 10:52 ` [PATCH v4 2/3] nvme-auth: use transformed key size to create resp Mark O'Donovan
  2023-10-17 10:52 ` [PATCH v4 3/3] nvme-auth: allow mixing of secret and hash lengths Mark O'Donovan
  2 siblings, 1 reply; 5+ messages in thread
From: Mark O'Donovan @ 2023-10-17 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-nvme, sagi, hch, axboe, kbusch, hare, Mark O'Donovan,
	Akash Appaiah

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>
---
V2 -> V3: initial version

V3 -> V4: added function to get size of key struct

 drivers/nvme/common/auth.c | 35 ++++++++++++++++++++++++-----------
 include/linux/nvme-auth.h  |  4 +++-
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index d90e4f0c08b7..984f4320aca3 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -150,6 +150,14 @@ size_t nvme_auth_hmac_hash_len(u8 hmac_id)
 }
 EXPORT_SYMBOL_GPL(nvme_auth_hmac_hash_len);
 
+u32 nvme_auth_key_struct_size(u32 key_len)
+{
+	struct nvme_dhchap_key key;
+
+	return struct_size(&key, key, key_len);
+}
+EXPORT_SYMBOL_GPL(nvme_auth_key_struct_size);
+
 struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
 					      u8 key_hash)
 {
@@ -163,14 +171,9 @@ struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
 	p = strrchr(secret, ':');
 	if (p)
 		allocated_len = p - secret;
-	key = kzalloc(sizeof(*key), GFP_KERNEL);
+	key = nvme_auth_alloc_key(allocated_len, 0);
 	if (!key)
 		return ERR_PTR(-ENOMEM);
-	key->key = kzalloc(allocated_len, GFP_KERNEL);
-	if (!key->key) {
-		ret = -ENOMEM;
-		goto out_free_key;
-	}
 
 	key_len = base64_decode(secret, allocated_len, key->key);
 	if (key_len < 0) {
@@ -213,19 +216,29 @@ struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
 	key->hash = key_hash;
 	return key;
 out_free_secret:
-	kfree_sensitive(key->key);
-out_free_key:
-	kfree(key);
+	nvme_auth_free_key(key);
 	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(nvme_auth_extract_key);
 
+struct nvme_dhchap_key *nvme_auth_alloc_key(u32 len, u8 hash)
+{
+	u32 num_bytes = nvme_auth_key_struct_size(len);
+	struct nvme_dhchap_key *key = kzalloc(num_bytes, GFP_KERNEL);
+
+	if (key) {
+		key->len = len;
+		key->hash = hash;
+	}
+	return key;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_alloc_key);
+
 void nvme_auth_free_key(struct nvme_dhchap_key *key)
 {
 	if (!key)
 		return;
-	kfree_sensitive(key->key);
-	kfree(key);
+	kfree_sensitive(key);
 }
 EXPORT_SYMBOL_GPL(nvme_auth_free_key);
 
diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
index dcb8030062dd..a5ae9abe1ef6 100644
--- a/include/linux/nvme-auth.h
+++ b/include/linux/nvme-auth.h
@@ -9,9 +9,9 @@
 #include <crypto/kpp.h>
 
 struct nvme_dhchap_key {
-	u8 *key;
 	size_t len;
 	u8 hash;
+	u8 key[];
 };
 
 u32 nvme_auth_get_seqnum(void);
@@ -24,9 +24,11 @@ const char *nvme_auth_digest_name(u8 hmac_id);
 size_t nvme_auth_hmac_hash_len(u8 hmac_id);
 u8 nvme_auth_hmac_id(const char *hmac_name);
 
+u32 nvme_auth_key_struct_size(u32 key_len);
 struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
 					      u8 key_hash);
 void nvme_auth_free_key(struct nvme_dhchap_key *key);
+struct nvme_dhchap_key *nvme_auth_alloc_key(u32 len, u8 hash);
 u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn);
 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,
-- 
2.39.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 2/3] nvme-auth: use transformed key size to create resp
  2023-10-17 10:52 [PATCH v4 0/3] Remove secret-size restrictions for hashes Mark O'Donovan
  2023-10-17 10:52 ` [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer Mark O'Donovan
@ 2023-10-17 10:52 ` Mark O'Donovan
  2023-10-17 10:52 ` [PATCH v4 3/3] nvme-auth: allow mixing of secret and hash lengths Mark O'Donovan
  2 siblings, 0 replies; 5+ messages in thread
From: Mark O'Donovan @ 2023-10-17 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-nvme, sagi, hch, axboe, kbusch, hare, Mark O'Donovan,
	Akash Appaiah

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>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
V1 -> V2: support target implementation and controller secrets also
V2 -> V3: return key from nvme_auth_transform_key
V3 -> V4: use helper to get key struct len, added ERR_CAST

 drivers/nvme/common/auth.c | 23 ++++++++++++++---------
 drivers/nvme/host/auth.c   | 30 +++++++++++++++---------------
 drivers/nvme/target/auth.c | 31 +++++++++++++++++--------------
 include/linux/nvme-auth.h  |  3 ++-
 4 files changed, 48 insertions(+), 39 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index 984f4320aca3..f9e7ed8e729d 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -242,21 +242,25 @@ 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)
+struct nvme_dhchap_key *nvme_auth_transform_key(
+		struct nvme_dhchap_key *key, char *nqn)
 {
 	const char *hmac_name;
 	struct crypto_shash *key_tfm;
 	struct shash_desc *shash;
-	u8 *transformed_key;
-	int ret;
+	struct nvme_dhchap_key *transformed_key;
+	int ret, key_len;
 
 	if (!key || !key->key) {
 		pr_warn("No key specified\n");
 		return ERR_PTR(-ENOKEY);
 	}
 	if (key->hash == 0) {
-		transformed_key = kmemdup(key->key, key->len, GFP_KERNEL);
-		return transformed_key ? transformed_key : ERR_PTR(-ENOMEM);
+		key_len = nvme_auth_key_struct_size(key->len);
+		transformed_key = kmemdup(key, key_len, GFP_KERNEL);
+		if (!transformed_key)
+			return ERR_PTR(-ENOMEM);
+		return transformed_key;
 	}
 	hmac_name = nvme_auth_hmac_name(key->hash);
 	if (!hmac_name) {
@@ -266,7 +270,7 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
 
 	key_tfm = crypto_alloc_shash(hmac_name, 0, 0);
 	if (IS_ERR(key_tfm))
-		return (u8 *)key_tfm;
+		return ERR_CAST(key_tfm);
 
 	shash = kmalloc(sizeof(struct shash_desc) +
 			crypto_shash_descsize(key_tfm),
@@ -276,7 +280,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_len = crypto_shash_digestsize(key_tfm);
+	transformed_key = nvme_auth_alloc_key(key_len, key->hash);
 	if (!transformed_key) {
 		ret = -ENOMEM;
 		goto out_free_shash;
@@ -295,7 +300,7 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
 	ret = crypto_shash_update(shash, "NVMe-over-Fabrics", 17);
 	if (ret < 0)
 		goto out_free_transformed_key;
-	ret = crypto_shash_final(shash, transformed_key);
+	ret = crypto_shash_final(shash, transformed_key->key);
 	if (ret < 0)
 		goto out_free_transformed_key;
 
@@ -305,7 +310,7 @@ u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
 	return transformed_key;
 
 out_free_transformed_key:
-	kfree_sensitive(transformed_key);
+	nvme_auth_free_key(transformed_key);
 out_free_shash:
 	kfree(shash);
 out_free_key:
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index daf5d144a8ea..de1390d705dc 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -23,6 +23,7 @@ struct nvme_dhchap_queue_context {
 	struct nvme_ctrl *ctrl;
 	struct crypto_shash *shash_tfm;
 	struct crypto_kpp *dh_tfm;
+	struct nvme_dhchap_key *transformed_key;
 	void *buf;
 	int qid;
 	int error;
@@ -36,7 +37,6 @@ struct nvme_dhchap_queue_context {
 	u8 c1[64];
 	u8 c2[64];
 	u8 response[64];
-	u8 *host_response;
 	u8 *ctrl_key;
 	u8 *host_key;
 	u8 *sess_key;
@@ -428,12 +428,12 @@ static int nvme_auth_dhchap_setup_host_response(struct nvme_ctrl *ctrl,
 	dev_dbg(ctrl->device, "%s: qid %d host response seq %u transaction %d\n",
 		__func__, chap->qid, chap->s1, chap->transaction);
 
-	if (!chap->host_response) {
-		chap->host_response = nvme_auth_transform_key(ctrl->host_key,
+	if (!chap->transformed_key) {
+		chap->transformed_key = nvme_auth_transform_key(ctrl->host_key,
 						ctrl->opts->host->nqn);
-		if (IS_ERR(chap->host_response)) {
-			ret = PTR_ERR(chap->host_response);
-			chap->host_response = NULL;
+		if (IS_ERR(chap->transformed_key)) {
+			ret = PTR_ERR(chap->transformed_key);
+			chap->transformed_key = NULL;
 			return ret;
 		}
 	} else {
@@ -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->transformed_key->key, chap->transformed_key->len);
 	if (ret) {
 		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
 			 chap->qid, ret);
@@ -508,19 +508,19 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
 		struct nvme_dhchap_queue_context *chap)
 {
 	SHASH_DESC_ON_STACK(shash, chap->shash_tfm);
-	u8 *ctrl_response;
+	struct nvme_dhchap_key *transformed_key;
 	u8 buf[4], *challenge = chap->c2;
 	int ret;
 
-	ctrl_response = nvme_auth_transform_key(ctrl->ctrl_key,
+	transformed_key = nvme_auth_transform_key(ctrl->ctrl_key,
 				ctrl->opts->subsysnqn);
-	if (IS_ERR(ctrl_response)) {
-		ret = PTR_ERR(ctrl_response);
+	if (IS_ERR(transformed_key)) {
+		ret = PTR_ERR(transformed_key);
 		return ret;
 	}
 
 	ret = crypto_shash_setkey(chap->shash_tfm,
-			ctrl_response, ctrl->ctrl_key->len);
+			transformed_key->key, transformed_key->len);
 	if (ret) {
 		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
 			 chap->qid, ret);
@@ -586,7 +586,7 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
 out:
 	if (challenge != chap->c2)
 		kfree(challenge);
-	kfree(ctrl_response);
+	nvme_auth_free_key(transformed_key);
 	return ret;
 }
 
@@ -648,8 +648,8 @@ static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl,
 
 static void nvme_auth_reset_dhchap(struct nvme_dhchap_queue_context *chap)
 {
-	kfree_sensitive(chap->host_response);
-	chap->host_response = NULL;
+	nvme_auth_free_key(chap->transformed_key);
+	chap->transformed_key = NULL;
 	kfree_sensitive(chap->host_key);
 	chap->host_key = NULL;
 	chap->host_key_len = 0;
diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 4dcddcf95279..3ddbc3880cac 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -267,7 +267,8 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
 	struct shash_desc *shash;
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	const char *hash_name;
-	u8 *challenge = req->sq->dhchap_c1, *host_response;
+	u8 *challenge = req->sq->dhchap_c1;
+	struct nvme_dhchap_key *transformed_key;
 	u8 buf[4];
 	int ret;
 
@@ -291,14 +292,15 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
 		goto out_free_tfm;
 	}
 
-	host_response = nvme_auth_transform_key(ctrl->host_key, ctrl->hostnqn);
-	if (IS_ERR(host_response)) {
-		ret = PTR_ERR(host_response);
+	transformed_key = nvme_auth_transform_key(ctrl->host_key,
+						  ctrl->hostnqn);
+	if (IS_ERR(transformed_key)) {
+		ret = PTR_ERR(transformed_key);
 		goto out_free_tfm;
 	}
 
-	ret = crypto_shash_setkey(shash_tfm, host_response,
-				  ctrl->host_key->len);
+	ret = crypto_shash_setkey(shash_tfm, transformed_key->key,
+				  transformed_key->len);
 	if (ret)
 		goto out_free_response;
 
@@ -365,7 +367,7 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
 		kfree(challenge);
 	kfree(shash);
 out_free_response:
-	kfree_sensitive(host_response);
+	nvme_auth_free_key(transformed_key);
 out_free_tfm:
 	crypto_free_shash(shash_tfm);
 	return 0;
@@ -378,7 +380,8 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 	struct shash_desc *shash;
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	const char *hash_name;
-	u8 *challenge = req->sq->dhchap_c2, *ctrl_response;
+	u8 *challenge = req->sq->dhchap_c2;
+	struct nvme_dhchap_key *transformed_key;
 	u8 buf[4];
 	int ret;
 
@@ -402,15 +405,15 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 		goto out_free_tfm;
 	}
 
-	ctrl_response = nvme_auth_transform_key(ctrl->ctrl_key,
+	transformed_key = nvme_auth_transform_key(ctrl->ctrl_key,
 						ctrl->subsysnqn);
-	if (IS_ERR(ctrl_response)) {
-		ret = PTR_ERR(ctrl_response);
+	if (IS_ERR(transformed_key)) {
+		ret = PTR_ERR(transformed_key);
 		goto out_free_tfm;
 	}
 
-	ret = crypto_shash_setkey(shash_tfm, ctrl_response,
-				  ctrl->ctrl_key->len);
+	ret = crypto_shash_setkey(shash_tfm, transformed_key->key,
+				  transformed_key->len);
 	if (ret)
 		goto out_free_response;
 
@@ -474,7 +477,7 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 		kfree(challenge);
 	kfree(shash);
 out_free_response:
-	kfree_sensitive(ctrl_response);
+	nvme_auth_free_key(transformed_key);
 out_free_tfm:
 	crypto_free_shash(shash_tfm);
 	return 0;
diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
index a5ae9abe1ef6..c1d0bc5d9624 100644
--- a/include/linux/nvme-auth.h
+++ b/include/linux/nvme-auth.h
@@ -29,7 +29,8 @@ struct nvme_dhchap_key *nvme_auth_extract_key(unsigned char *secret,
 					      u8 key_hash);
 void nvme_auth_free_key(struct nvme_dhchap_key *key);
 struct nvme_dhchap_key *nvme_auth_alloc_key(u32 len, u8 hash);
-u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn);
+struct nvme_dhchap_key *nvme_auth_transform_key(
+				struct nvme_dhchap_key *key, char *nqn);
 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.39.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 3/3] nvme-auth: allow mixing of secret and hash lengths
  2023-10-17 10:52 [PATCH v4 0/3] Remove secret-size restrictions for hashes Mark O'Donovan
  2023-10-17 10:52 ` [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer Mark O'Donovan
  2023-10-17 10:52 ` [PATCH v4 2/3] nvme-auth: use transformed key size to create resp Mark O'Donovan
@ 2023-10-17 10:52 ` Mark O'Donovan
  2 siblings, 0 replies; 5+ messages in thread
From: Mark O'Donovan @ 2023-10-17 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-nvme, sagi, hch, axboe, kbusch, hare, Mark O'Donovan,
	Akash Appaiah

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 f9e7ed8e729d..ddfcc3889fec 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -190,14 +190,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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer
  2023-10-17 10:52 ` [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer Mark O'Donovan
@ 2023-10-17 15:55   ` kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-10-17 15:55 UTC (permalink / raw)
  To: Mark O'Donovan, linux-kernel
  Cc: oe-kbuild-all, linux-nvme, sagi, hch, axboe, kbusch, hare,
	Mark O'Donovan, Akash Appaiah

Hi Mark,

kernel test robot noticed the following build warnings:

[auto build test WARNING on hch-configfs/for-next]
[also build test WARNING on linus/master v6.6-rc6 next-20231017]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mark-O-Donovan/nvme-auth-alloc-nvme_dhchap_key-as-single-buffer/20231017-185421
base:   git://git.infradead.org/users/hch/configfs.git for-next
patch link:    https://lore.kernel.org/r/20231017105251.3274652-2-shiftee%40posteo.net
patch subject: [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231017/202310172318.IgK0V5EX-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231017/202310172318.IgK0V5EX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310172318.IgK0V5EX-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/nvme/common/auth.c: In function 'nvme_auth_transform_key':
>> drivers/nvme/common/auth.c:253:21: warning: the comparison will always evaluate as 'true' for the address of 'key' will never be NULL [-Waddress]
     253 |         if (!key || !key->key) {
         |                     ^
   In file included from drivers/nvme/common/auth.c:15:
   include/linux/nvme-auth.h:14:12: note: 'key' declared here
      14 |         u8 key[];
         |            ^~~


vim +253 drivers/nvme/common/auth.c

f50fff73d620cd Hannes Reinecke 2022-06-27  244  
f50fff73d620cd Hannes Reinecke 2022-06-27  245  u8 *nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn)
f50fff73d620cd Hannes Reinecke 2022-06-27  246  {
f50fff73d620cd Hannes Reinecke 2022-06-27  247  	const char *hmac_name;
f50fff73d620cd Hannes Reinecke 2022-06-27  248  	struct crypto_shash *key_tfm;
f50fff73d620cd Hannes Reinecke 2022-06-27  249  	struct shash_desc *shash;
f50fff73d620cd Hannes Reinecke 2022-06-27  250  	u8 *transformed_key;
f50fff73d620cd Hannes Reinecke 2022-06-27  251  	int ret;
f50fff73d620cd Hannes Reinecke 2022-06-27  252  
f50fff73d620cd Hannes Reinecke 2022-06-27 @253  	if (!key || !key->key) {
f50fff73d620cd Hannes Reinecke 2022-06-27  254  		pr_warn("No key specified\n");
f50fff73d620cd Hannes Reinecke 2022-06-27  255  		return ERR_PTR(-ENOKEY);
f50fff73d620cd Hannes Reinecke 2022-06-27  256  	}
f50fff73d620cd Hannes Reinecke 2022-06-27  257  	if (key->hash == 0) {
f50fff73d620cd Hannes Reinecke 2022-06-27  258  		transformed_key = kmemdup(key->key, key->len, GFP_KERNEL);
f50fff73d620cd Hannes Reinecke 2022-06-27  259  		return transformed_key ? transformed_key : ERR_PTR(-ENOMEM);
f50fff73d620cd Hannes Reinecke 2022-06-27  260  	}
f50fff73d620cd Hannes Reinecke 2022-06-27  261  	hmac_name = nvme_auth_hmac_name(key->hash);
f50fff73d620cd Hannes Reinecke 2022-06-27  262  	if (!hmac_name) {
f50fff73d620cd Hannes Reinecke 2022-06-27  263  		pr_warn("Invalid key hash id %d\n", key->hash);
f50fff73d620cd Hannes Reinecke 2022-06-27  264  		return ERR_PTR(-EINVAL);
f50fff73d620cd Hannes Reinecke 2022-06-27  265  	}
f50fff73d620cd Hannes Reinecke 2022-06-27  266  
f50fff73d620cd Hannes Reinecke 2022-06-27  267  	key_tfm = crypto_alloc_shash(hmac_name, 0, 0);
f50fff73d620cd Hannes Reinecke 2022-06-27  268  	if (IS_ERR(key_tfm))
f50fff73d620cd Hannes Reinecke 2022-06-27  269  		return (u8 *)key_tfm;
f50fff73d620cd Hannes Reinecke 2022-06-27  270  
f50fff73d620cd Hannes Reinecke 2022-06-27  271  	shash = kmalloc(sizeof(struct shash_desc) +
f50fff73d620cd Hannes Reinecke 2022-06-27  272  			crypto_shash_descsize(key_tfm),
f50fff73d620cd Hannes Reinecke 2022-06-27  273  			GFP_KERNEL);
f50fff73d620cd Hannes Reinecke 2022-06-27  274  	if (!shash) {
f50fff73d620cd Hannes Reinecke 2022-06-27  275  		ret = -ENOMEM;
f50fff73d620cd Hannes Reinecke 2022-06-27  276  		goto out_free_key;
f50fff73d620cd Hannes Reinecke 2022-06-27  277  	}
f50fff73d620cd Hannes Reinecke 2022-06-27  278  
f50fff73d620cd Hannes Reinecke 2022-06-27  279  	transformed_key = kzalloc(crypto_shash_digestsize(key_tfm), GFP_KERNEL);
f50fff73d620cd Hannes Reinecke 2022-06-27  280  	if (!transformed_key) {
f50fff73d620cd Hannes Reinecke 2022-06-27  281  		ret = -ENOMEM;
f50fff73d620cd Hannes Reinecke 2022-06-27  282  		goto out_free_shash;
f50fff73d620cd Hannes Reinecke 2022-06-27  283  	}
f50fff73d620cd Hannes Reinecke 2022-06-27  284  
f50fff73d620cd Hannes Reinecke 2022-06-27  285  	shash->tfm = key_tfm;
f50fff73d620cd Hannes Reinecke 2022-06-27  286  	ret = crypto_shash_setkey(key_tfm, key->key, key->len);
f50fff73d620cd Hannes Reinecke 2022-06-27  287  	if (ret < 0)
80e2768496a494 Dan Carpenter   2022-07-18  288  		goto out_free_transformed_key;
f50fff73d620cd Hannes Reinecke 2022-06-27  289  	ret = crypto_shash_init(shash);
f50fff73d620cd Hannes Reinecke 2022-06-27  290  	if (ret < 0)
80e2768496a494 Dan Carpenter   2022-07-18  291  		goto out_free_transformed_key;
f50fff73d620cd Hannes Reinecke 2022-06-27  292  	ret = crypto_shash_update(shash, nqn, strlen(nqn));
f50fff73d620cd Hannes Reinecke 2022-06-27  293  	if (ret < 0)
80e2768496a494 Dan Carpenter   2022-07-18  294  		goto out_free_transformed_key;
f50fff73d620cd Hannes Reinecke 2022-06-27  295  	ret = crypto_shash_update(shash, "NVMe-over-Fabrics", 17);
f50fff73d620cd Hannes Reinecke 2022-06-27  296  	if (ret < 0)
80e2768496a494 Dan Carpenter   2022-07-18  297  		goto out_free_transformed_key;
f50fff73d620cd Hannes Reinecke 2022-06-27  298  	ret = crypto_shash_final(shash, transformed_key);
80e2768496a494 Dan Carpenter   2022-07-18  299  	if (ret < 0)
80e2768496a494 Dan Carpenter   2022-07-18  300  		goto out_free_transformed_key;
80e2768496a494 Dan Carpenter   2022-07-18  301  
80e2768496a494 Dan Carpenter   2022-07-18  302  	kfree(shash);
80e2768496a494 Dan Carpenter   2022-07-18  303  	crypto_free_shash(key_tfm);
80e2768496a494 Dan Carpenter   2022-07-18  304  
80e2768496a494 Dan Carpenter   2022-07-18  305  	return transformed_key;
80e2768496a494 Dan Carpenter   2022-07-18  306  
80e2768496a494 Dan Carpenter   2022-07-18  307  out_free_transformed_key:
80e2768496a494 Dan Carpenter   2022-07-18  308  	kfree_sensitive(transformed_key);
f50fff73d620cd Hannes Reinecke 2022-06-27  309  out_free_shash:
f50fff73d620cd Hannes Reinecke 2022-06-27  310  	kfree(shash);
f50fff73d620cd Hannes Reinecke 2022-06-27  311  out_free_key:
f50fff73d620cd Hannes Reinecke 2022-06-27  312  	crypto_free_shash(key_tfm);
80e2768496a494 Dan Carpenter   2022-07-18  313  
f50fff73d620cd Hannes Reinecke 2022-06-27  314  	return ERR_PTR(ret);
f50fff73d620cd Hannes Reinecke 2022-06-27  315  }
f50fff73d620cd Hannes Reinecke 2022-06-27  316  EXPORT_SYMBOL_GPL(nvme_auth_transform_key);
f50fff73d620cd Hannes Reinecke 2022-06-27  317  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-10-17 15:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-17 10:52 [PATCH v4 0/3] Remove secret-size restrictions for hashes Mark O'Donovan
2023-10-17 10:52 ` [PATCH v4 1/3] nvme-auth: alloc nvme_dhchap_key as single buffer Mark O'Donovan
2023-10-17 15:55   ` kernel test robot
2023-10-17 10:52 ` [PATCH v4 2/3] nvme-auth: use transformed key size to create resp Mark O'Donovan
2023-10-17 10:52 ` [PATCH v4 3/3] nvme-auth: allow mixing of secret and hash lengths Mark O'Donovan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox