linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring
@ 2025-05-28 14:05 Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 1/9] nvme-auth: modify nvme_auth_transform_key() to return status Hannes Reinecke
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Hey all,

the current NVMe authentication code is using a hand-crafted key
structure; idea was to have the initial implementation with a minimal
set of dependencies.
(And me not having a good grasp on how to use the kernel keyring :-)
That had the drawback that keys always had to be specified on the
nvme-cli commandline, which is far from ideal from a security standpoint.

So this patchset switches the authentication code over to use the
kernel keyring. User-facing interface (namely argument to 'nvme
connect') remain the same, but the key data is converted into keys
which are stored as a new key type 'dhchap' with a random UUID as
description in the kernel keyring.

With this I have updated the dhchap arguments to 'nvme connect' and
the configfs interface to either be the keydata (ie the original
interface) _or_ a key description referring to a pre-populated dhchap
key in the kernel keyring. This allows for easier provisioning of keys
and avoids the security risk from having to specify the key data on
the kernel commandline.

The entire patchset can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/hare/nvme.git
branch dhchap-keyring.v2

There is a pull request to blktests (PR#175) which adds a test
to exercise the new interface.

As usual, comments and reviews are welcome.

Changes to the original submission:
- Dropped patches merged with upstream
- Modified the interface to refer to keys via the description
  and not the serial number

Hannes Reinecke (9):
  nvme-auth: modify nvme_auth_transform_key() to return status
  nvme-keyring: add 'dhchap' key type
  nvme-auth: switch to use 'struct key'
  nvme: parse dhchap keys during option parsing
  nvmet-auth: parse dhchap key from configfs attribute
  nvme: allow to pass in key description as dhchap secret
  nvme-auth: wait for authentication to finish when changing keys
  nvme-fabrics: allow to pass in keyring by name
  nvmet: add configfs attribute 'dhchap_keyring'

 drivers/nvme/common/Kconfig    |   1 +
 drivers/nvme/common/auth.c     | 227 ++++++++++++----------------
 drivers/nvme/common/keyring.c  | 266 +++++++++++++++++++++++++++++++++
 drivers/nvme/host/Kconfig      |   1 -
 drivers/nvme/host/auth.c       | 166 ++++++++++++++------
 drivers/nvme/host/fabrics.c    | 119 +++++++++++----
 drivers/nvme/host/fabrics.h    |  12 +-
 drivers/nvme/host/nvme.h       |   6 +-
 drivers/nvme/host/sysfs.c      | 204 ++++++++++++++++++-------
 drivers/nvme/target/Kconfig    |   1 -
 drivers/nvme/target/auth.c     | 226 ++++++++++++++++++----------
 drivers/nvme/target/configfs.c | 146 ++++++++++++++++--
 drivers/nvme/target/nvmet.h    |  14 +-
 include/linux/nvme-auth.h      |  18 +--
 include/linux/nvme-keyring.h   |  22 ++-
 15 files changed, 1039 insertions(+), 390 deletions(-)

-- 
2.35.3



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

* [PATCH 1/9] nvme-auth: modify nvme_auth_transform_key() to return status
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 2/9] nvme-keyring: add 'dhchap' key type Hannes Reinecke
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Modify nvme_auth_transform_key() to return a status and provide
the transformed data as argument on the command line as raw data.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/common/auth.c | 72 +++++++++++++++++++++++---------------
 drivers/nvme/host/auth.c   | 40 +++++++++++----------
 drivers/nvme/target/auth.c | 36 +++++++++----------
 include/linux/nvme-auth.h  |  4 +--
 4 files changed, 85 insertions(+), 67 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index 3b6d759bcdf2..918c92cbd8c5 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -237,70 +237,86 @@ void nvme_auth_free_key(struct nvme_dhchap_key *key)
 }
 EXPORT_SYMBOL_GPL(nvme_auth_free_key);
 
-struct nvme_dhchap_key *nvme_auth_transform_key(
-		struct nvme_dhchap_key *key, char *nqn)
+int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
+			    u8 **transformed_secret)
 {
 	const char *hmac_name;
 	struct crypto_shash *key_tfm;
 	SHASH_DESC_ON_STACK(shash, key_tfm);
-	struct nvme_dhchap_key *transformed_key;
-	int ret, key_len;
+	u8 *transformed_data;
+	u8 *key_data;
+	size_t transformed_len;
+	int ret;
 
 	if (!key) {
 		pr_warn("No key specified\n");
-		return ERR_PTR(-ENOKEY);
+		return -ENOKEY;
 	}
+	key_data = kzalloc(key->len, GFP_KERNEL);
+	if (!key_data)
+		return -ENOMEM;
+	memcpy(key_data, key->key, key->len);
 	if (key->hash == 0) {
-		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;
+		*transformed_secret = key_data;
+		return key->len;
 	}
 	hmac_name = nvme_auth_hmac_name(key->hash);
 	if (!hmac_name) {
 		pr_warn("Invalid key hash id %d\n", key->hash);
-		return ERR_PTR(-EINVAL);
+		ret = -EINVAL;
+		goto out_free_data;
 	}
 
 	key_tfm = crypto_alloc_shash(hmac_name, 0, 0);
-	if (IS_ERR(key_tfm))
-		return ERR_CAST(key_tfm);
+	if (IS_ERR(key_tfm)) {
+		ret = PTR_ERR(key_tfm);
+		goto out_free_data;
+	}
 
-	key_len = crypto_shash_digestsize(key_tfm);
-	transformed_key = nvme_auth_alloc_key(key_len, key->hash);
-	if (!transformed_key) {
+	transformed_len = crypto_shash_digestsize(key_tfm);
+	if (transformed_len != key->len) {
+		pr_warn("incompatible digest size %ld for key (hash %s, len %ld)\n",
+			transformed_len, hmac_name, key->len);
+		ret = -EINVAL;
+		goto out_free_tfm;
+	}
+
+	transformed_data = kzalloc(transformed_len, GFP_KERNEL);
+	if (!transformed_data) {
 		ret = -ENOMEM;
-		goto out_free_key;
+		goto out_free_tfm;
 	}
 
 	shash->tfm = key_tfm;
 	ret = crypto_shash_setkey(key_tfm, key->key, key->len);
 	if (ret < 0)
-		goto out_free_transformed_key;
+		goto out_free_transformed_data;
 	ret = crypto_shash_init(shash);
 	if (ret < 0)
-		goto out_free_transformed_key;
+		goto out_free_transformed_data;
 	ret = crypto_shash_update(shash, nqn, strlen(nqn));
 	if (ret < 0)
-		goto out_free_transformed_key;
+		goto out_free_transformed_data;
 	ret = crypto_shash_update(shash, "NVMe-over-Fabrics", 17);
 	if (ret < 0)
-		goto out_free_transformed_key;
-	ret = crypto_shash_final(shash, transformed_key->key);
+		goto out_free_transformed_data;
+	ret = crypto_shash_final(shash, transformed_data);
 	if (ret < 0)
-		goto out_free_transformed_key;
+		goto out_free_transformed_data;
 
 	crypto_free_shash(key_tfm);
+	*transformed_secret = transformed_data;
 
-	return transformed_key;
+	return transformed_len;
 
-out_free_transformed_key:
-	nvme_auth_free_key(transformed_key);
-out_free_key:
+out_free_transformed_data:
+	kfree(transformed_data);
+out_free_tfm:
 	crypto_free_shash(key_tfm);
+out_free_data:
+	kfree(key_data);
 
-	return ERR_PTR(ret);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(nvme_auth_transform_key);
 
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index f6ddbe553289..9e7c2e889ee0 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -24,7 +24,8 @@ 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;
+	u8 *transformed_secret;
+	size_t transformed_len;
 	void *buf;
 	int qid;
 	int error;
@@ -437,21 +438,20 @@ 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->transformed_key) {
-		chap->transformed_key = nvme_auth_transform_key(ctrl->host_key,
-						ctrl->opts->host->nqn);
-		if (IS_ERR(chap->transformed_key)) {
-			ret = PTR_ERR(chap->transformed_key);
-			chap->transformed_key = NULL;
+	if (!chap->transformed_secret) {
+		ret = nvme_auth_transform_key(ctrl->host_key,
+					      ctrl->opts->host->nqn,
+					      &chap->transformed_secret);
+		if (ret < 0)
 			return ret;
-		}
+		chap->transformed_len = ret;
 	} else {
 		dev_dbg(ctrl->device, "%s: qid %d re-using host response\n",
 			__func__, chap->qid);
 	}
 
 	ret = crypto_shash_setkey(chap->shash_tfm,
-			chap->transformed_key->key, chap->transformed_key->len);
+			chap->transformed_secret, chap->transformed_len);
 	if (ret) {
 		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
 			 chap->qid, ret);
@@ -517,19 +517,20 @@ 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);
-	struct nvme_dhchap_key *transformed_key;
+	u8 *transformed_secret;
+	size_t transformed_len;
 	u8 buf[4], *challenge = chap->c2;
 	int ret;
 
-	transformed_key = nvme_auth_transform_key(ctrl->ctrl_key,
-				ctrl->opts->subsysnqn);
-	if (IS_ERR(transformed_key)) {
-		ret = PTR_ERR(transformed_key);
+	ret = nvme_auth_transform_key(ctrl->ctrl_key,
+			ctrl->opts->subsysnqn,
+			&transformed_secret);
+	if (ret < 0)
 		return ret;
-	}
+	transformed_len = ret;
 
 	ret = crypto_shash_setkey(chap->shash_tfm,
-			transformed_key->key, transformed_key->len);
+			transformed_secret, transformed_len);
 	if (ret) {
 		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
 			 chap->qid, ret);
@@ -595,7 +596,7 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
 out:
 	if (challenge != chap->c2)
 		kfree(challenge);
-	nvme_auth_free_key(transformed_key);
+	kfree(transformed_secret);
 	return ret;
 }
 
@@ -657,8 +658,9 @@ static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl,
 
 static void nvme_auth_reset_dhchap(struct nvme_dhchap_queue_context *chap)
 {
-	nvme_auth_free_key(chap->transformed_key);
-	chap->transformed_key = NULL;
+	kfree(chap->transformed_secret);
+	chap->transformed_secret = NULL;
+	chap->transformed_len = 0;
 	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 b340380f3892..c70d9d259a7f 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -297,7 +297,8 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	const char *hash_name;
 	u8 *challenge = req->sq->dhchap_c1;
-	struct nvme_dhchap_key *transformed_key;
+	u8 *transformed_secret;
+	size_t transformed_len;
 	u8 buf[4];
 	int ret;
 
@@ -321,15 +322,14 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
 		goto out_free_tfm;
 	}
 
-	transformed_key = nvme_auth_transform_key(ctrl->host_key,
-						  ctrl->hostnqn);
-	if (IS_ERR(transformed_key)) {
-		ret = PTR_ERR(transformed_key);
+	ret = nvme_auth_transform_key(ctrl->host_key, ctrl->hostnqn,
+				      &transformed_secret);
+	if (ret < 0)
 		goto out_free_tfm;
-	}
 
-	ret = crypto_shash_setkey(shash_tfm, transformed_key->key,
-				  transformed_key->len);
+	transformed_len = ret;
+	ret = crypto_shash_setkey(shash_tfm, transformed_secret,
+				  transformed_len);
 	if (ret)
 		goto out_free_response;
 
@@ -389,7 +389,7 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response,
 	if (challenge != req->sq->dhchap_c1)
 		kfree(challenge);
 out_free_response:
-	nvme_auth_free_key(transformed_key);
+	kfree(transformed_secret);
 out_free_tfm:
 	crypto_free_shash(shash_tfm);
 	return ret;
@@ -403,7 +403,8 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	const char *hash_name;
 	u8 *challenge = req->sq->dhchap_c2;
-	struct nvme_dhchap_key *transformed_key;
+	u8 *transformed_secret;
+	size_t transformed_len;
 	u8 buf[4];
 	int ret;
 
@@ -427,15 +428,14 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 		goto out_free_tfm;
 	}
 
-	transformed_key = nvme_auth_transform_key(ctrl->ctrl_key,
-						ctrl->subsysnqn);
-	if (IS_ERR(transformed_key)) {
-		ret = PTR_ERR(transformed_key);
+	ret = nvme_auth_transform_key(ctrl->ctrl_key, ctrl->subsysnqn,
+				      &transformed_secret);
+	if (ret < 0)
 		goto out_free_tfm;
-	}
+	transformed_len = ret;
 
-	ret = crypto_shash_setkey(shash_tfm, transformed_key->key,
-				  transformed_key->len);
+	ret = crypto_shash_setkey(shash_tfm, transformed_secret,
+				  transformed_len);
 	if (ret)
 		goto out_free_response;
 
@@ -500,7 +500,7 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 	if (challenge != req->sq->dhchap_c2)
 		kfree(challenge);
 out_free_response:
-	nvme_auth_free_key(transformed_key);
+	kfree(transformed_secret);
 out_free_tfm:
 	crypto_free_shash(shash_tfm);
 	return ret;
diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
index 60e069a6757f..fd43fa042c88 100644
--- a/include/linux/nvme-auth.h
+++ b/include/linux/nvme-auth.h
@@ -29,8 +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);
-struct nvme_dhchap_key *nvme_auth_transform_key(
-				struct nvme_dhchap_key *key, char *nqn);
+int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
+			    u8 **transformed_secret) ;
 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.35.3



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

* [PATCH 2/9] nvme-keyring: add 'dhchap' key type
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 1/9] nvme-auth: modify nvme_auth_transform_key() to return status Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-06-03  0:32   ` Shinichiro Kawasaki
  2025-05-28 14:05 ` [PATCH 3/9] nvme-auth: switch to use 'struct key' Hannes Reinecke
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Add a 'dhchap' keytype to store DH-HMAC-CHAP secret keys.
Keys are stored with a 'user-type' compatible payload, such
that one can use 'user_read()' to access the raw contents
and the 'read()' callback to get the base64-encoded key
data in the DH-HMAC-CHAP secret representation.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/common/keyring.c | 266 ++++++++++++++++++++++++++++++++++
 include/linux/nvme-keyring.h  |  22 ++-
 2 files changed, 287 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
index 32d16c53133b..a58c93c6d495 100644
--- a/drivers/nvme/common/keyring.c
+++ b/drivers/nvme/common/keyring.c
@@ -4,6 +4,9 @@
  */
 
 #include <linux/module.h>
+#include <linux/crc32.h>
+#include <linux/base64.h>
+#include <linux/unaligned.h>
 #include <linux/seq_file.h>
 #include <linux/key-type.h>
 #include <keys/user-type.h>
@@ -252,6 +255,262 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
 }
 EXPORT_SYMBOL_GPL(nvme_tls_psk_default);
 
+static void nvme_dhchap_psk_describe(const struct key *key, struct seq_file *m)
+{
+	seq_puts(m, key->description);
+	seq_printf(m, ": %u", key->datalen);
+}
+
+static bool nvme_dhchap_psk_match(const struct key *key,
+				  const struct key_match_data *match_data)
+{
+	const char *match_id;
+	size_t match_len;
+
+	if (!key->description) {
+		pr_debug("%s: no key description\n", __func__);
+		return false;
+	}
+	if (!match_data->raw_data) {
+		pr_debug("%s: no match data\n", __func__);
+		return false;
+	}
+	match_id = match_data->raw_data;
+	match_len = strlen(match_id);
+	pr_debug("%s: match '%s' '%s' len %zd\n",
+		 __func__, match_id, key->description, match_len);
+
+	return !memcmp(key->description, match_id, match_len);
+}
+
+static int nvme_dhchap_psk_match_preparse(struct key_match_data *match_data)
+{
+	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	match_data->cmp = nvme_dhchap_psk_match;
+	return 0;
+}
+
+/**
+ * nvme_dhchap_psk_preparse - prepare DH-HMAC-CHAP key data
+ * @prep: preparsed payload of the key data
+ *
+ * Decode the DH-HMAC-CHAP key data passed in in @prep and
+ * store the resulting binary data. The binary data includes
+ * space for the CRC, the version, and the hmac identifier,
+ * but the data length is just the key data without the CRC.
+ * This allows the user to read the key data via the
+ * 'user_read()' function, but also to export the full
+ * key data including CRC via the 'read' callback from the key type.
+ */
+static int nvme_dhchap_psk_preparse(struct key_preparsed_payload *prep)
+{
+	struct user_key_payload *upayload;
+	size_t datalen = prep->datalen, keylen;
+	int ret;
+	u32 crc;
+	u8 version, hmac;
+
+	if (!prep->data) {
+		pr_debug("%s: Empty data", __func__);
+		prep->payload.data[0] = NULL;
+		prep->quotalen = 0;
+		return -EINVAL;
+	}
+
+	if (sscanf(prep->data, "DHHC-%01hhu:%02hhu:%*s", &version, &hmac) != 2) {
+		pr_debug("%s: invalid key data '%s'\n", __func__,
+			 (char *)prep->data);
+		prep->payload.data[0] = NULL;
+		prep->quotalen = 0;
+		return -EINVAL;
+	}
+
+	/* skip header and final ':' character */
+	datalen -= 11;
+
+	/* keylen is the key length, 4 bytes CRC, 1 byte version, and 1 byte HMAC identifier */
+	switch (datalen) {
+	case 48:
+		keylen = 38;
+		break;
+	case 72:
+		keylen = 54;
+		break;
+	case 92:
+		keylen = 70;
+		break;
+	default:
+		pr_debug("%s: Invalid data length %lu\n", __func__, datalen);
+		prep->payload.data[0] = NULL;
+		prep->quotalen = 0;
+		return -EINVAL;
+	}
+
+	upayload = kzalloc(sizeof(*upayload) + keylen, GFP_KERNEL);
+	if (!upayload) {
+		prep->payload.data[0] = NULL;
+		prep->quotalen = 0;
+		return -ENOMEM;
+	}
+
+	/* decode the data */
+	prep->quotalen = keylen;
+	prep->payload.data[0] = upayload;
+	ret = base64_decode(prep->data + 10, datalen, upayload->data);
+	if (ret < 0) {
+		pr_debug("%s: Failed to decode key %s\n",
+			 __func__, (char *)prep->data + 10);
+		return ret;
+	}
+	ret -= 4;
+	crc = ~crc32(~0, upayload->data, ret);
+	if (get_unaligned_le32(upayload->data + ret) != crc) {
+		pr_debug("%s: CRC mismatch for key\n", __func__);
+		/* CRC mismatch */
+		return -EKEYREJECTED;
+	}
+	upayload->data[ret + 4] = version;
+	upayload->data[ret + 5] = hmac;
+	upayload->datalen = ret;
+	return 0;
+}
+
+static long nvme_dhchap_psk_read(const struct key *key, char *buffer, size_t buflen)
+{
+	const struct user_key_payload *upayload;
+	size_t datalen, keylen;
+	u8 version, hmac;
+	long ret;
+
+	upayload = user_key_payload_locked(key);
+	switch (upayload->datalen) {
+	    case 32:
+		keylen = 59;
+		break;
+	    case 48:
+		keylen = 83;
+		break;
+	    case 64:
+		keylen = 103;
+		break;
+	    default:
+		return -EINVAL;
+	}
+
+	if (!buffer || buflen == 0)
+		return keylen;
+
+	if (buflen < keylen)
+		return -EINVAL;
+
+	memset(buffer, 0, buflen);
+	version = upayload->data[upayload->datalen + 4];
+	hmac = upayload->data[upayload->datalen + 5];
+	ret = sprintf(buffer, "DHHC-%01hhu:%02hhu:", version, hmac);
+	if (ret < 0)
+		return -ENOKEY;
+	/* Include the trailing CRC */
+	datalen = upayload->datalen + 4;
+	ret += base64_encode(upayload->data, datalen, buffer + ret);
+	buffer[ret] = ':';
+	ret++;
+	return ret;
+}
+
+static struct key_type nvme_dhchap_psk_key_type = {
+	.name           = "dhchap",
+	.flags          = KEY_TYPE_NET_DOMAIN,
+	.preparse       = nvme_dhchap_psk_preparse,
+	.free_preparse  = user_free_preparse,
+	.match_preparse = nvme_dhchap_psk_match_preparse,
+	.instantiate    = generic_key_instantiate,
+	.revoke         = user_revoke,
+	.destroy        = user_destroy,
+	.describe       = nvme_dhchap_psk_describe,
+	.read           = nvme_dhchap_psk_read,
+};
+
+struct key *nvme_dhchap_psk_refresh(struct key *keyring,
+		const u8 *data, size_t data_len)
+{
+	key_perm_t keyperm =
+		KEY_POS_SEARCH | KEY_POS_VIEW | KEY_POS_READ |
+		KEY_POS_WRITE | KEY_POS_LINK | KEY_POS_SETATTR |
+		KEY_USR_SEARCH | KEY_USR_VIEW | KEY_USR_READ;
+	char *identity;
+	key_ref_t keyref;
+	key_serial_t keyring_id;
+	struct key *key;
+	uuid_t key_uuid;
+
+	if (data == NULL || !data_len)
+		return ERR_PTR(-EINVAL);
+
+	generate_random_uuid(key_uuid.b);
+	identity = kasprintf(GFP_KERNEL, "%pU", &key_uuid);
+	if (!identity)
+		return ERR_PTR(-ENOMEM);
+	if (!keyring)
+		keyring = nvme_keyring;
+	keyring_id = key_serial(keyring);
+
+	pr_debug("keyring %x generate dhchap psk '%s'\n",
+		 keyring_id, identity);
+	keyref = key_create(make_key_ref(keyring, true),
+			    "dhchap", identity, data, data_len,
+			    keyperm, KEY_ALLOC_NOT_IN_QUOTA |
+			    KEY_ALLOC_BUILT_IN |
+			    KEY_ALLOC_BYPASS_RESTRICTION);
+	if (IS_ERR(keyref)) {
+		pr_warn("refresh dhchap psk '%s' failed, error %ld\n",
+			identity, PTR_ERR(keyref));
+		kfree(identity);
+		return ERR_PTR(-ENOKEY);
+	}
+	key = key_ref_to_ptr(keyref);
+	pr_debug("generated dhchap psk %08x\n", key_serial(key));
+	kfree(identity);
+	return key;
+}
+EXPORT_SYMBOL_GPL(nvme_dhchap_psk_refresh);
+
+struct key *nvme_dhchap_psk_lookup(struct key *keyring, const char *identity)
+{
+	key_ref_t keyref;
+	key_serial_t keyring_id;
+
+	if (!keyring)
+		keyring = nvme_keyring;
+	keyring_id = key_serial(keyring);
+	pr_debug("keyring %x lookup dhchap psk '%s'\n",
+		 keyring_id, identity);
+
+	keyref = keyring_search(make_key_ref(keyring, true),
+				&nvme_dhchap_psk_key_type,
+				identity, false);
+	if (IS_ERR(keyref)) {
+		pr_debug("lookup dhchap psk '%s' failed, error %ld\n",
+			 identity, PTR_ERR(keyref));
+		return ERR_PTR(-ENOKEY);
+	}
+
+	return key_ref_to_ptr(keyref);
+}
+EXPORT_SYMBOL_GPL(nvme_dhchap_psk_lookup);
+
+u8 nvme_dhchap_psk_hash(struct key *key)
+{
+	const struct user_key_payload *upayload;
+	u8 hmac;
+
+	if (!key)
+		return 0;
+	upayload = user_key_payload_locked(key);
+	hmac = upayload->data[upayload->datalen + 5];
+	return hmac;
+}
+EXPORT_SYMBOL_GPL(nvme_dhchap_psk_hash);
+
 static int __init nvme_keyring_init(void)
 {
 	int err;
@@ -270,11 +529,18 @@ static int __init nvme_keyring_init(void)
 		key_put(nvme_keyring);
 		return err;
 	}
+	err = register_key_type(&nvme_dhchap_psk_key_type);
+	if (err) {
+		unregister_key_type(&nvme_tls_psk_key_type);
+		key_put(nvme_keyring);
+		return err;
+	}
 	return 0;
 }
 
 static void __exit nvme_keyring_exit(void)
 {
+	unregister_key_type(&nvme_dhchap_psk_key_type);
 	unregister_key_type(&nvme_tls_psk_key_type);
 	key_revoke(nvme_keyring);
 	key_put(nvme_keyring);
diff --git a/include/linux/nvme-keyring.h b/include/linux/nvme-keyring.h
index ab8971afa973..d8baa71f061d 100644
--- a/include/linux/nvme-keyring.h
+++ b/include/linux/nvme-keyring.h
@@ -18,9 +18,14 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
 
 key_serial_t nvme_keyring_id(void);
 struct key *nvme_tls_key_lookup(key_serial_t key_id);
+
+struct key *nvme_dhchap_psk_refresh(struct key *keyring,
+		const u8 *data, size_t data_len);
+struct key *nvme_dhchap_psk_lookup(struct key *keyring, const char *identity);
+u8 nvme_dhchap_psk_hash(struct key *key);
+
 #else
 static inline struct key *nvme_tls_psk_refresh(struct key *keyring,
-		const char *hostnqn, char *subnqn, u8 hmac_id,
 		u8 *data, size_t data_len, const char *digest)
 {
 	return ERR_PTR(-ENOTSUPP);
@@ -38,5 +43,20 @@ static inline struct key *nvme_tls_key_lookup(key_serial_t key_id)
 {
 	return ERR_PTR(-ENOTSUPP);
 }
+static inline struct key *nvme_dhchap_psk_refresh(struct key *keyring,
+		const char *hostnqn, const char *subnqn,
+		u8 *data, size_t data_len)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+static inline struct key *nvme_dhchap_psk_lookup(struct key *keyring,
+		const char *hostnqn, const char *subnqn, u8 hmac);
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+u8 nvme_dhchap_psk_hash(struct key *key)
+{
+	return 0;
+}
 #endif /* !CONFIG_NVME_KEYRING */
 #endif /* _NVME_KEYRING_H */
-- 
2.35.3



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

* [PATCH 3/9] nvme-auth: switch to use 'struct key'
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 1/9] nvme-auth: modify nvme_auth_transform_key() to return status Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 2/9] nvme-keyring: add 'dhchap' key type Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 4/9] nvme: parse dhchap keys during option parsing Hannes Reinecke
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Use the new key type 'dhchap' to store the DH-HMAC-CHAP keys and modify
handling function to use 'struct key'. With that we can drop the now
unused 'struct nvme_dhchap_key' definitions.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/common/Kconfig |   1 +
 drivers/nvme/common/auth.c  | 170 ++++++++++++------------------------
 drivers/nvme/host/Kconfig   |   1 -
 drivers/nvme/host/auth.c    |  28 +++---
 drivers/nvme/host/nvme.h    |   4 +-
 drivers/nvme/host/sysfs.c   |  25 +++---
 drivers/nvme/target/Kconfig |   1 -
 drivers/nvme/target/auth.c  |  40 +++++----
 drivers/nvme/target/nvmet.h |   4 +-
 include/linux/nvme-auth.h   |  17 +---
 10 files changed, 113 insertions(+), 178 deletions(-)

diff --git a/drivers/nvme/common/Kconfig b/drivers/nvme/common/Kconfig
index da963e4f3f1f..8a5521c038c5 100644
--- a/drivers/nvme/common/Kconfig
+++ b/drivers/nvme/common/Kconfig
@@ -13,3 +13,4 @@ config NVME_AUTH
 	select CRYPTO_DH
 	select CRYPTO_DH_RFC7919_GROUPS
 	select CRYPTO_HKDF
+	select NVME_KEYRING
diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index 918c92cbd8c5..8c2ccbfb9986 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -14,6 +14,8 @@
 #include <crypto/hkdf.h>
 #include <linux/nvme.h>
 #include <linux/nvme-auth.h>
+#include <linux/nvme-keyring.h>
+#include <keys/user-type.h>
 
 #define HKDF_MAX_HASHLEN 64
 
@@ -153,98 +155,28 @@ 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 key *nvme_auth_extract_key(struct key *keyring, const u8 *secret,
+				  size_t secret_len)
 {
-	struct nvme_dhchap_key key;
+	struct 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)
-{
-	struct nvme_dhchap_key *key;
-	unsigned char *p;
-	u32 crc;
-	int ret, key_len;
-	size_t allocated_len = strlen(secret);
-
-	/* Secret might be affixed with a ':' */
-	p = strrchr(secret, ':');
-	if (p)
-		allocated_len = p - secret;
-	key = nvme_auth_alloc_key(allocated_len, 0);
-	if (!key)
-		return ERR_PTR(-ENOMEM);
-
-	key_len = base64_decode(secret, allocated_len, key->key);
-	if (key_len < 0) {
-		pr_debug("base64 key decoding error %d\n",
-			 key_len);
-		ret = key_len;
-		goto out_free_secret;
-	}
-
-	if (key_len != 36 && key_len != 52 &&
-	    key_len != 68) {
-		pr_err("Invalid key len %d\n", key_len);
-		ret = -EINVAL;
-		goto out_free_secret;
-	}
-
-	/* The last four bytes is the CRC in little-endian format */
-	key_len -= 4;
-	/*
-	 * The linux implementation doesn't do pre- and post-increments,
-	 * so we have to do it manually.
-	 */
-	crc = ~crc32(~0, key->key, key_len);
-
-	if (get_unaligned_le32(key->key + key_len) != crc) {
-		pr_err("key crc mismatch (key %08x, crc %08x)\n",
-		       get_unaligned_le32(key->key + key_len), crc);
-		ret = -EKEYREJECTED;
-		goto out_free_secret;
-	}
-	key->len = key_len;
-	key->hash = key_hash;
+	key = nvme_dhchap_psk_refresh(keyring, secret, secret_len);
+	if (!IS_ERR(key))
+		pr_debug("generated dhchap key %08x\n",
+			 key_serial(key));
 	return key;
-out_free_secret:
-	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);
-}
-EXPORT_SYMBOL_GPL(nvme_auth_free_key);
-
-int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
+int nvme_auth_transform_key(struct key *key, char *nqn,
 			    u8 **transformed_secret)
 {
 	const char *hmac_name;
 	struct crypto_shash *key_tfm;
 	SHASH_DESC_ON_STACK(shash, key_tfm);
+	long key_len = 0;
 	u8 *transformed_data;
-	u8 *key_data;
+	u8 *key_data, key_hash;
 	size_t transformed_len;
 	int ret;
 
@@ -252,17 +184,47 @@ int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
 		pr_warn("No key specified\n");
 		return -ENOKEY;
 	}
-	key_data = kzalloc(key->len, GFP_KERNEL);
-	if (!key_data)
+	down_read(&key->sem);
+	ret = key_validate(key);
+	if (ret) {
+		pr_warn("%s: key %08x invalidated\n",
+			__func__, key_serial(key));
+		up_read(&key->sem);
+		return ret;
+	}
+	key_len = user_read(key, NULL, 0);
+	if (key_len <= 0) {
+		pr_warn("failed to get length from key %08x: error %ld\n",
+			key_serial(key), key_len);
+		up_read(&key->sem);
+		return key_len;
+	}
+	key_data = kzalloc(key_len, GFP_KERNEL);
+	if (!key_data) {
+		up_read(&key->sem);
 		return -ENOMEM;
-	memcpy(key_data, key->key, key->len);
-	if (key->hash == 0) {
+	}
+
+	ret = user_read(key, key_data, key_len);
+	key_hash = nvme_dhchap_psk_hash(key);
+	up_read(&key->sem);
+	if (ret != key_len) {
+		if (ret < 0) {
+			pr_warn("failed to read data from key %08x: error %d\n",
+				key_serial(key), ret);
+		} else {
+			pr_warn("only read %d of %ld bytes from key %08x\n",
+				ret, key_len, key_serial(key));
+		}
+		goto out_free_data;
+	}
+	if (key_hash == 0) {
 		*transformed_secret = key_data;
-		return key->len;
+		return key_len;
 	}
-	hmac_name = nvme_auth_hmac_name(key->hash);
+	hmac_name = nvme_auth_hmac_name(key_hash);
 	if (!hmac_name) {
-		pr_warn("Invalid key hash id %d\n", key->hash);
+		pr_warn("Invalid key hash id %d\n", key_hash);
 		ret = -EINVAL;
 		goto out_free_data;
 	}
@@ -274,9 +236,9 @@ int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
 	}
 
 	transformed_len = crypto_shash_digestsize(key_tfm);
-	if (transformed_len != key->len) {
+	if (transformed_len != key_len) {
 		pr_warn("incompatible digest size %ld for key (hash %s, len %ld)\n",
-			transformed_len, hmac_name, key->len);
+			transformed_len, hmac_name, key_len);
 		ret = -EINVAL;
 		goto out_free_tfm;
 	}
@@ -288,7 +250,7 @@ int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
 	}
 
 	shash->tfm = key_tfm;
-	ret = crypto_shash_setkey(key_tfm, key->key, key->len);
+	ret = crypto_shash_setkey(key_tfm, key_data, key_len);
 	if (ret < 0)
 		goto out_free_transformed_data;
 	ret = crypto_shash_init(shash);
@@ -304,8 +266,9 @@ int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
 	if (ret < 0)
 		goto out_free_transformed_data;
 
-	crypto_free_shash(key_tfm);
 	*transformed_secret = transformed_data;
+	crypto_free_shash(key_tfm);
+	kfree(key_data);
 
 	return transformed_len;
 
@@ -454,31 +417,6 @@ int nvme_auth_gen_shared_secret(struct crypto_kpp *dh_tfm,
 }
 EXPORT_SYMBOL_GPL(nvme_auth_gen_shared_secret);
 
-int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key)
-{
-	struct nvme_dhchap_key *key;
-	u8 key_hash;
-
-	if (!secret) {
-		*ret_key = NULL;
-		return 0;
-	}
-
-	if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
-		return -EINVAL;
-
-	/* Pass in the secret without the 'DHHC-1:XX:' prefix */
-	key = nvme_auth_extract_key(secret + 10, key_hash);
-	if (IS_ERR(key)) {
-		*ret_key = NULL;
-		return PTR_ERR(key);
-	}
-
-	*ret_key = key;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(nvme_auth_generate_key);
-
 /**
  * nvme_auth_generate_psk - Generate a PSK for TLS
  * @hmac_id: Hash function identifier
diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index 4d64b6935bb9..65a5a5fd82f9 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -115,7 +115,6 @@ config NVME_HOST_AUTH
 	bool "NVMe over Fabrics In-Band Authentication in host side"
 	depends on NVME_CORE
 	select NVME_AUTH
-	select NVME_KEYRING
 	help
 	  This provides support for NVMe over Fabrics In-Band Authentication in
 	  host side.
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index 9e7c2e889ee0..c5be0c13e85b 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -1068,14 +1068,22 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
 	INIT_WORK(&ctrl->dhchap_auth_work, nvme_ctrl_auth_work);
 	if (!ctrl->opts)
 		return 0;
-	ret = nvme_auth_generate_key(ctrl->opts->dhchap_secret,
-			&ctrl->host_key);
-	if (ret)
+	ctrl->host_key = nvme_auth_extract_key(ctrl->opts->keyring,
+					       ctrl->opts->dhchap_secret,
+					       strlen(ctrl->opts->dhchap_secret));
+	if (IS_ERR(ctrl->host_key)) {
+		ret = PTR_ERR(ctrl->host_key);
+		ctrl->host_key = NULL;
 		return ret;
-	ret = nvme_auth_generate_key(ctrl->opts->dhchap_ctrl_secret,
-			&ctrl->ctrl_key);
-	if (ret)
+	}
+	ctrl->ctrl_key = nvme_auth_extract_key(ctrl->opts->keyring,
+					       ctrl->opts->dhchap_ctrl_secret,
+					       strlen(ctrl->opts->dhchap_ctrl_secret));
+	if (IS_ERR(ctrl->ctrl_key)) {
+		ret = PTR_ERR(ctrl->ctrl_key);
+		ctrl->ctrl_key = NULL;
 		goto err_free_dhchap_secret;
+	}
 
 	if (!ctrl->opts->dhchap_secret && !ctrl->opts->dhchap_ctrl_secret)
 		return 0;
@@ -1097,10 +1105,10 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
 
 	return 0;
 err_free_dhchap_ctrl_secret:
-	nvme_auth_free_key(ctrl->ctrl_key);
+	key_put(ctrl->ctrl_key);
 	ctrl->ctrl_key = NULL;
 err_free_dhchap_secret:
-	nvme_auth_free_key(ctrl->host_key);
+	key_put(ctrl->host_key);
 	ctrl->host_key = NULL;
 	return ret;
 }
@@ -1122,11 +1130,11 @@ void nvme_auth_free(struct nvme_ctrl *ctrl)
 		kfree(ctrl->dhchap_ctxs);
 	}
 	if (ctrl->host_key) {
-		nvme_auth_free_key(ctrl->host_key);
+		key_put(ctrl->host_key);
 		ctrl->host_key = NULL;
 	}
 	if (ctrl->ctrl_key) {
-		nvme_auth_free_key(ctrl->ctrl_key);
+		key_put(ctrl->ctrl_key);
 		ctrl->ctrl_key = NULL;
 	}
 }
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 1de1b843afa5..3dd83a48532a 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -378,8 +378,8 @@ struct nvme_ctrl {
 	struct work_struct dhchap_auth_work;
 	struct mutex dhchap_auth_mutex;
 	struct nvme_dhchap_queue_context *dhchap_ctxs;
-	struct nvme_dhchap_key *host_key;
-	struct nvme_dhchap_key *ctrl_key;
+	struct key *host_key;
+	struct key *ctrl_key;
 	u16 transaction;
 #endif
 	key_serial_t tls_pskid;
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index a48d30c31d51..b3b63453df1c 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -596,8 +596,6 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 		return -EINVAL;
 	if (count < 7)
 		return -EINVAL;
-	if (memcmp(buf, "DHHC-1:", 7))
-		return -EINVAL;
 
 	dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
 	if (!dhchap_secret)
@@ -605,13 +603,12 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 	memcpy(dhchap_secret, buf, count);
 	nvme_auth_stop(ctrl);
 	if (strcmp(dhchap_secret, opts->dhchap_secret)) {
-		struct nvme_dhchap_key *key, *host_key;
-		int ret;
+		struct key *key, *host_key;
 
-		ret = nvme_auth_generate_key(dhchap_secret, &key);
-		if (ret) {
+		key = nvme_auth_extract_key(opts->keyring, dhchap_secret, count);
+		if (IS_ERR(key)) {
 			kfree(dhchap_secret);
-			return ret;
+			return PTR_ERR(key);
 		}
 		kfree(opts->dhchap_secret);
 		opts->dhchap_secret = dhchap_secret;
@@ -619,7 +616,7 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 		mutex_lock(&ctrl->dhchap_auth_mutex);
 		ctrl->host_key = key;
 		mutex_unlock(&ctrl->dhchap_auth_mutex);
-		nvme_auth_free_key(host_key);
+		key_put(host_key);
 	} else
 		kfree(dhchap_secret);
 	/* Start re-authentication */
@@ -663,13 +660,13 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 	memcpy(dhchap_secret, buf, count);
 	nvme_auth_stop(ctrl);
 	if (strcmp(dhchap_secret, opts->dhchap_ctrl_secret)) {
-		struct nvme_dhchap_key *key, *ctrl_key;
-		int ret;
+		struct key *key, *ctrl_key;
 
-		ret = nvme_auth_generate_key(dhchap_secret, &key);
-		if (ret) {
+		key = nvme_auth_extract_key(opts->keyring,
+					    dhchap_secret, count);
+		if (IS_ERR(key)) {
 			kfree(dhchap_secret);
-			return ret;
+			return PTR_ERR(key);
 		}
 		kfree(opts->dhchap_ctrl_secret);
 		opts->dhchap_ctrl_secret = dhchap_secret;
@@ -677,7 +674,7 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 		mutex_lock(&ctrl->dhchap_auth_mutex);
 		ctrl->ctrl_key = key;
 		mutex_unlock(&ctrl->dhchap_auth_mutex);
-		nvme_auth_free_key(ctrl_key);
+		key_put(ctrl_key);
 	} else
 		kfree(dhchap_secret);
 	/* Start re-authentication */
diff --git a/drivers/nvme/target/Kconfig b/drivers/nvme/target/Kconfig
index 4c253b433bf7..5761bb52c46c 100644
--- a/drivers/nvme/target/Kconfig
+++ b/drivers/nvme/target/Kconfig
@@ -5,7 +5,6 @@ config NVME_TARGET
 	depends on BLOCK
 	depends on CONFIGFS_FS
 	select NVME_KEYRING if NVME_TARGET_TCP_TLS
-	select KEYS if NVME_TARGET_TCP_TLS
 	select SGL_ALLOC
 	help
 	  This enabled target side support for the NVMe protocol, that is
diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index c70d9d259a7f..7f87dc39a2de 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -145,6 +145,7 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
 	int ret = 0;
 	struct nvmet_host_link *p;
 	struct nvmet_host *host = NULL;
+	u8 host_hash, ctrl_hash;
 
 	down_read(&nvmet_config_sem);
 	if (nvmet_is_disc_subsys(ctrl->subsys))
@@ -190,42 +191,43 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
 		ctrl->shash_id = host->dhchap_hash_id;
 	}
 
-	/* Skip the 'DHHC-1:XX:' prefix */
-	nvme_auth_free_key(ctrl->host_key);
-	ctrl->host_key = nvme_auth_extract_key(host->dhchap_secret + 10,
-					       host->dhchap_key_hash);
+	key_put(ctrl->host_key);
+	ctrl->host_key = nvme_auth_extract_key(NULL, host->dhchap_secret,
+					       strlen(host->dhchap_secret));
 	if (IS_ERR(ctrl->host_key)) {
 		ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
 		ctrl->host_key = NULL;
 		goto out_free_hash;
 	}
-	pr_debug("%s: using hash %s key %*ph\n", __func__,
-		 ctrl->host_key->hash > 0 ?
-		 nvme_auth_hmac_name(ctrl->host_key->hash) : "none",
-		 (int)ctrl->host_key->len, ctrl->host_key->key);
+	host_hash = nvme_dhchap_psk_hash(ctrl->host_key);
+	pr_debug("%s: using hash %s key %u\n", __func__,
+		 ctrl_hash > 0 ?
+		 nvme_auth_hmac_name(ctrl_hash) : "none",
+		 key_serial(ctrl->host_key));
 
-	nvme_auth_free_key(ctrl->ctrl_key);
+	key_put(ctrl->ctrl_key);
 	if (!host->dhchap_ctrl_secret) {
 		ctrl->ctrl_key = NULL;
 		goto out_unlock;
 	}
 
-	ctrl->ctrl_key = nvme_auth_extract_key(host->dhchap_ctrl_secret + 10,
-					       host->dhchap_ctrl_key_hash);
+	ctrl->ctrl_key = nvme_auth_extract_key(NULL, host->dhchap_ctrl_secret,
+					       strlen(host->dhchap_ctrl_secret));
 	if (IS_ERR(ctrl->ctrl_key)) {
 		ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
 		ctrl->ctrl_key = NULL;
 		goto out_free_hash;
 	}
-	pr_debug("%s: using ctrl hash %s key %*ph\n", __func__,
-		 ctrl->ctrl_key->hash > 0 ?
-		 nvme_auth_hmac_name(ctrl->ctrl_key->hash) : "none",
-		 (int)ctrl->ctrl_key->len, ctrl->ctrl_key->key);
+	ctrl_hash = nvme_dhchap_psk_hash(ctrl->ctrl_key);
+	pr_debug("%s: using ctrl hash %s key %u\n", __func__,
+		 ctrl_hash > 0 ?
+		 nvme_auth_hmac_name(ctrl_hash) : "none",
+		 key_serial(ctrl->ctrl_key));
 
 out_free_hash:
 	if (ret) {
 		if (ctrl->host_key) {
-			nvme_auth_free_key(ctrl->host_key);
+			key_put(ctrl->host_key);
 			ctrl->host_key = NULL;
 		}
 		ctrl->shash_id = 0;
@@ -263,11 +265,13 @@ void nvmet_destroy_auth(struct nvmet_ctrl *ctrl)
 	ctrl->dh_key = NULL;
 
 	if (ctrl->host_key) {
-		nvme_auth_free_key(ctrl->host_key);
+		key_revoke(ctrl->host_key);
+		key_put(ctrl->host_key);
 		ctrl->host_key = NULL;
 	}
 	if (ctrl->ctrl_key) {
-		nvme_auth_free_key(ctrl->ctrl_key);
+		key_revoke(ctrl->ctrl_key);
+		key_put(ctrl->ctrl_key);
 		ctrl->ctrl_key = NULL;
 	}
 #ifdef CONFIG_NVME_TARGET_TCP_TLS
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index df69a9dee71c..ca6053fbaac8 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -298,8 +298,8 @@ struct nvmet_ctrl {
 	bool			pi_support;
 	bool			concat;
 #ifdef CONFIG_NVME_TARGET_AUTH
-	struct nvme_dhchap_key	*host_key;
-	struct nvme_dhchap_key	*ctrl_key;
+	struct key		*host_key;
+	struct key		*ctrl_key;
 	u8			shash_id;
 	struct crypto_kpp	*dh_tfm;
 	u8			dh_gid;
diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
index fd43fa042c88..4e53ef96eea7 100644
--- a/include/linux/nvme-auth.h
+++ b/include/linux/nvme-auth.h
@@ -8,12 +8,6 @@
 
 #include <crypto/kpp.h>
 
-struct nvme_dhchap_key {
-	size_t len;
-	u8 hash;
-	u8 key[];
-};
-
 u32 nvme_auth_get_seqnum(void);
 const char *nvme_auth_dhgroup_name(u8 dhgroup_id);
 const char *nvme_auth_dhgroup_kpp(u8 dhgroup_id);
@@ -24,14 +18,9 @@ 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);
-int nvme_auth_transform_key(struct nvme_dhchap_key *key, char *nqn,
-			    u8 **transformed_secret) ;
-int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key);
+struct key *nvme_auth_extract_key(struct key *keyring, const u8 *secret, size_t secret_len);
+int nvme_auth_transform_key(struct key *key, char *nqn,
+			    u8 **transformed_secret);
 int nvme_auth_augmented_challenge(u8 hmac_id, u8 *skey, size_t skey_len,
 				  u8 *challenge, u8 *aug, size_t hlen);
 int nvme_auth_gen_privkey(struct crypto_kpp *dh_tfm, u8 dh_gid);
-- 
2.35.3



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

* [PATCH 4/9] nvme: parse dhchap keys during option parsing
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
                   ` (2 preceding siblings ...)
  2025-05-28 14:05 ` [PATCH 3/9] nvme-auth: switch to use 'struct key' Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 5/9] nvmet-auth: parse dhchap key from configfs attribute Hannes Reinecke
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

We really should parse the dhchap keys during option parsing to avoid
having to pass around the plain dhchap secret. During options parsing
we will create a 'dhchap' key with a random UUID as description, and
store the key serial in the 'opts' structure.
This simplifies key handling as on every access the key needs to be
looked up and checked for validity before accessing the key data.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/host/auth.c    | 113 ++++++++++++++++------
 drivers/nvme/host/fabrics.c |  82 +++++++++++-----
 drivers/nvme/host/fabrics.h |   8 +-
 drivers/nvme/host/sysfs.c   | 185 ++++++++++++++++++++++++++----------
 4 files changed, 278 insertions(+), 110 deletions(-)

diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index c5be0c13e85b..0e8a5b544f63 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -532,7 +532,7 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
 	ret = crypto_shash_setkey(chap->shash_tfm,
 			transformed_secret, transformed_len);
 	if (ret) {
-		dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
+		dev_warn(ctrl->device, "qid %d: failed to set ctrl key, error %d\n",
 			 chap->qid, ret);
 		goto out;
 	}
@@ -970,11 +970,6 @@ int nvme_auth_negotiate(struct nvme_ctrl *ctrl, int qid)
 		return -ENOKEY;
 	}
 
-	if (ctrl->opts->dhchap_ctrl_secret && !ctrl->ctrl_key) {
-		dev_warn(ctrl->device, "qid %d: invalid ctrl key\n", qid);
-		return -ENOKEY;
-	}
-
 	chap = &ctrl->dhchap_ctxs[qid];
 	cancel_work_sync(&chap->auth_work);
 	queue_work(nvme_auth_wq, &chap->auth_work);
@@ -1059,6 +1054,26 @@ static void nvme_ctrl_auth_work(struct work_struct *work)
 	}
 }
 
+static void nvme_auth_clear_key(struct nvme_ctrl *ctrl, bool is_ctrl)
+{
+	struct key *key;
+
+	if (is_ctrl) {
+		key = ctrl->ctrl_key;
+		ctrl->ctrl_key = NULL;
+	} else {
+		key = ctrl->host_key;
+		ctrl->host_key = NULL;
+	}
+	if (key) {
+		dev_dbg(ctrl->device, "%s: revoke%s key %08x\n",
+			__func__, is_ctrl ? " ctrl" : "",
+			key_serial(key));
+		key_revoke(key);
+		key_put(key);
+	}
+}
+
 int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
 {
 	struct nvme_dhchap_queue_context *chap;
@@ -1068,31 +1083,70 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
 	INIT_WORK(&ctrl->dhchap_auth_work, nvme_ctrl_auth_work);
 	if (!ctrl->opts)
 		return 0;
-	ctrl->host_key = nvme_auth_extract_key(ctrl->opts->keyring,
-					       ctrl->opts->dhchap_secret,
-					       strlen(ctrl->opts->dhchap_secret));
-	if (IS_ERR(ctrl->host_key)) {
-		ret = PTR_ERR(ctrl->host_key);
-		ctrl->host_key = NULL;
-		return ret;
+	if (!ctrl->opts->dhchap_key) {
+		nvme_auth_clear_key(ctrl, false);
+		nvme_auth_clear_key(ctrl, true);
+		return 0;
 	}
-	ctrl->ctrl_key = nvme_auth_extract_key(ctrl->opts->keyring,
-					       ctrl->opts->dhchap_ctrl_secret,
-					       strlen(ctrl->opts->dhchap_ctrl_secret));
-	if (IS_ERR(ctrl->ctrl_key)) {
-		ret = PTR_ERR(ctrl->ctrl_key);
-		ctrl->ctrl_key = NULL;
-		goto err_free_dhchap_secret;
+
+	if (ctrl->host_key)
+		nvme_auth_clear_key(ctrl, false);
+
+	ctrl->host_key = key_get(ctrl->opts->dhchap_key);
+	if (!ctrl->host_key) {
+		dev_warn(ctrl->device,
+			 "dhchap key %08x not present\n",
+			 key_serial(ctrl->opts->dhchap_key));
+		return -ENOKEY;
+	}
+	down_read(&ctrl->host_key->sem);
+	ret = key_validate(ctrl->host_key);
+	up_read(&ctrl->host_key->sem);
+	if (ret) {
+		dev_warn(ctrl->device,
+			 "dhchap key %08x invalidated\n",
+			 key_serial(ctrl->host_key));
+		key_put(ctrl->host_key);
+		ctrl->host_key = NULL;
+		return -ENOKEY;
 	}
+	dev_dbg(ctrl->device,
+		"%s: using dhchap key %08x\n",
+		__func__, key_serial(ctrl->host_key));
 
-	if (!ctrl->opts->dhchap_secret && !ctrl->opts->dhchap_ctrl_secret)
-		return 0;
+	if (ctrl->ctrl_key)
+		nvme_auth_clear_key(ctrl, true);
+
+	if (ctrl->opts->dhchap_ctrl_key) {
+		ctrl->ctrl_key = key_get(ctrl->opts->dhchap_ctrl_key);
+		if (!ctrl->ctrl_key) {
+			dev_warn(ctrl->device,
+				 "dhchap ctrl key %08x not present\n",
+				 key_serial(ctrl->opts->dhchap_ctrl_key));
+			return -ENOKEY;
+		}
+		down_read(&ctrl->ctrl_key->sem);
+		ret = key_validate(ctrl->ctrl_key);
+		up_read(&ctrl->ctrl_key->sem);
+		if (ret) {
+			dev_warn(ctrl->device,
+				 "dhchap ctrl key %08x invalidated\n",
+				 key_serial(ctrl->ctrl_key));
+			key_put(ctrl->ctrl_key);
+			ctrl->ctrl_key = NULL;
+			return -EKEYREVOKED;
+		}
+		dev_dbg(ctrl->device,
+			"%s: using dhchap ctrl key %08x\n",
+			__func__, key_serial(ctrl->ctrl_key));
+	}
 
 	ctrl->dhchap_ctxs = kvcalloc(ctrl_max_dhchaps(ctrl),
 				sizeof(*chap), GFP_KERNEL);
 	if (!ctrl->dhchap_ctxs) {
-		ret = -ENOMEM;
-		goto err_free_dhchap_ctrl_secret;
+		nvme_auth_clear_key(ctrl, true);
+		nvme_auth_clear_key(ctrl, false);
+		return -ENOMEM;
 	}
 
 	for (i = 0; i < ctrl_max_dhchaps(ctrl); i++) {
@@ -1104,13 +1158,6 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
 	}
 
 	return 0;
-err_free_dhchap_ctrl_secret:
-	key_put(ctrl->ctrl_key);
-	ctrl->ctrl_key = NULL;
-err_free_dhchap_secret:
-	key_put(ctrl->host_key);
-	ctrl->host_key = NULL;
-	return ret;
 }
 EXPORT_SYMBOL_GPL(nvme_auth_init_ctrl);
 
@@ -1130,10 +1177,14 @@ void nvme_auth_free(struct nvme_ctrl *ctrl)
 		kfree(ctrl->dhchap_ctxs);
 	}
 	if (ctrl->host_key) {
+		dev_dbg(ctrl->device, "%s: drop host key %08x\n",
+			__func__, key_serial(ctrl->host_key));
 		key_put(ctrl->host_key);
 		ctrl->host_key = NULL;
 	}
 	if (ctrl->ctrl_key) {
+		dev_dbg(ctrl->device, "%s: drop ctrl key %08x\n",
+			__func__, key_serial(ctrl->ctrl_key));
 		key_put(ctrl->ctrl_key);
 		ctrl->ctrl_key = NULL;
 	}
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 93e9041b9657..009a6cf8a86b 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -12,6 +12,7 @@
 #include <linux/seq_file.h>
 #include "nvme.h"
 #include "fabrics.h"
+#include <linux/nvme-auth.h>
 #include <linux/nvme-keyring.h>
 
 static LIST_HEAD(nvmf_transports);
@@ -717,6 +718,7 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *options, *o, *p;
+	char *host_secret = NULL, *ctrl_secret = NULL;
 	int token, ret = 0;
 	size_t nqnlen  = 0;
 	int ctrl_loss_tmo = NVMF_DEF_CTRL_LOSS_TMO, key_id;
@@ -738,6 +740,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 	opts->tls_key = NULL;
 	opts->keyring = NULL;
 	opts->concat = false;
+	opts->dhchap_key = NULL;
+	opts->dhchap_ctrl_key = NULL;
 
 	options = o = kstrdup(buf, GFP_KERNEL);
 	if (!options)
@@ -1026,13 +1030,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 				ret = -ENOMEM;
 				goto out;
 			}
-			if (strlen(p) < 11 || strncmp(p, "DHHC-1:", 7)) {
-				pr_err("Invalid DH-CHAP secret %s\n", p);
-				ret = -EINVAL;
-				goto out;
-			}
-			kfree(opts->dhchap_secret);
-			opts->dhchap_secret = p;
+			kfree(host_secret);
+			host_secret = p;
 			break;
 		case NVMF_OPT_DHCHAP_CTRL_SECRET:
 			p = match_strdup(args);
@@ -1040,13 +1039,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 				ret = -ENOMEM;
 				goto out;
 			}
-			if (strlen(p) < 11 || strncmp(p, "DHHC-1:", 7)) {
-				pr_err("Invalid DH-CHAP secret %s\n", p);
-				ret = -EINVAL;
-				goto out;
-			}
-			kfree(opts->dhchap_ctrl_secret);
-			opts->dhchap_ctrl_secret = p;
+			kfree(ctrl_secret);
+			ctrl_secret = p;
 			break;
 		case NVMF_OPT_TLS:
 			if (!IS_ENABLED(CONFIG_NVME_TCP_TLS)) {
@@ -1090,6 +1084,41 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 			pr_warn("failfast tmo (%d) larger than controller loss tmo (%d)\n",
 				opts->fast_io_fail_tmo, ctrl_loss_tmo);
 	}
+
+	opts->host = nvmf_host_add(hostnqn, &hostid);
+	if (IS_ERR(opts->host)) {
+		ret = PTR_ERR(opts->host);
+		opts->host = NULL;
+		goto out;
+	}
+
+	if (host_secret) {
+		pr_debug("lookup host identity '%s'\n", host_secret);
+		key = nvme_auth_extract_key(opts->keyring, host_secret,
+					    strlen(host_secret));
+		if (IS_ERR(key)) {
+			ret = PTR_ERR(key);
+			goto out;
+		}
+		pr_debug("using dhchap key %08x\n", key_serial(key));
+		opts->dhchap_key = key;
+	}
+	if (ctrl_secret) {
+		if (!opts->dhchap_key) {
+			ret = -EINVAL;
+			goto out;
+		}
+		pr_debug("lookup ctrl identity '%s'\n", ctrl_secret);
+		key = nvme_auth_extract_key(opts->keyring, ctrl_secret,
+					    strlen(ctrl_secret));
+		if (IS_ERR(key)) {
+			ret = PTR_ERR(key);
+			goto out;
+		}
+		pr_debug("using dhchap ctrl key %08x\n", key_serial(key));
+		opts->dhchap_ctrl_key = key;
+	}
+
 	if (opts->concat) {
 		if (opts->tls) {
 			pr_err("Secure concatenation over TLS is not supported\n");
@@ -1101,21 +1130,16 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 			ret = -EINVAL;
 			goto out;
 		}
-		if (!opts->dhchap_secret) {
+		if (!opts->dhchap_key) {
 			pr_err("Need to enable DH-CHAP for secure concatenation\n");
 			ret = -EINVAL;
 			goto out;
 		}
 	}
 
-	opts->host = nvmf_host_add(hostnqn, &hostid);
-	if (IS_ERR(opts->host)) {
-		ret = PTR_ERR(opts->host);
-		opts->host = NULL;
-		goto out;
-	}
-
 out:
+	kfree(ctrl_secret);
+	kfree(host_secret);
 	kfree(options);
 	return ret;
 }
@@ -1290,8 +1314,18 @@ void nvmf_free_options(struct nvmf_ctrl_options *opts)
 	kfree(opts->subsysnqn);
 	kfree(opts->host_traddr);
 	kfree(opts->host_iface);
-	kfree(opts->dhchap_secret);
-	kfree(opts->dhchap_ctrl_secret);
+	if (opts->dhchap_key) {
+		pr_debug("revoke dhchap key %08x\n",
+			 key_serial(opts->dhchap_key));
+		key_revoke(opts->dhchap_key);
+		key_put(opts->dhchap_key);
+	}
+	if (opts->dhchap_ctrl_key) {
+		pr_debug("revoke dhchap ctrl key %08x\n",
+			 key_serial(opts->dhchap_ctrl_key));
+		key_revoke(opts->dhchap_key);
+		key_put(opts->dhchap_ctrl_key);
+	}
 	kfree(opts);
 }
 EXPORT_SYMBOL_GPL(nvmf_free_options);
diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index 9cf5b020adba..95a3d8ce1201 100644
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -96,8 +96,8 @@ enum {
  * @discovery_nqn: indicates if the subsysnqn is the well-known discovery NQN.
  * @kato:	Keep-alive timeout.
  * @host:	Virtual NVMe host, contains the NQN and Host ID.
- * @dhchap_secret: DH-HMAC-CHAP secret
- * @dhchap_ctrl_secret: DH-HMAC-CHAP controller secret for bi-directional
+ * @dhchap_key: DH-HMAC-CHAP pre-shared key
+ * @dhchap_ctrl_key: DH-HMAC-CHAP controller pre-shared key for bi-directional
  *              authentication
  * @keyring:    Keyring to use for key lookups
  * @tls_key:    TLS key for encrypted connections (TCP)
@@ -127,8 +127,8 @@ struct nvmf_ctrl_options {
 	bool			duplicate_connect;
 	unsigned int		kato;
 	struct nvmf_host	*host;
-	char			*dhchap_secret;
-	char			*dhchap_ctrl_secret;
+	struct key		*dhchap_key;
+	struct key		*dhchap_ctrl_key;
 	struct key		*keyring;
 	struct key		*tls_key;
 	bool			tls;
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index b3b63453df1c..8a96228ff244 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -6,6 +6,8 @@
  */
 
 #include <linux/nvme-auth.h>
+#include <linux/nvme-keyring.h>
+#include <linux/key-type.h>
 
 #include "nvme.h"
 #include "fabrics.h"
@@ -578,11 +580,23 @@ static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct key *key = ctrl->host_key;
+	ssize_t count;
 
-	if (!opts->dhchap_secret)
+	if (!key)
 		return sysfs_emit(buf, "none\n");
-	return sysfs_emit(buf, "%s\n", opts->dhchap_secret);
+	down_read(&key->sem);
+	if (key_validate(key))
+		count = sysfs_emit(buf, "<invalidated>\n");
+	else {
+		count = key->type->read(key, buf, PAGE_SIZE);
+		if (count > 0) {
+			buf[count] = '\n';
+			count++;
+		}
+	}
+	up_read(&key->sem);
+	return count;
 }
 
 static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
@@ -590,35 +604,47 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 {
 	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
 	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct key *key, *old_key;
 	char *dhchap_secret;
+	size_t len;
+	int ret;
 
-	if (!ctrl->opts->dhchap_secret)
-		return -EINVAL;
-	if (count < 7)
+	if (!ctrl->host_key || !strlen(buf))
 		return -EINVAL;
 
-	dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
+	len = strcspn(buf, "\n");
+	dhchap_secret = kzalloc(len + 1, GFP_KERNEL);
 	if (!dhchap_secret)
 		return -ENOMEM;
-	memcpy(dhchap_secret, buf, count);
+	memcpy(dhchap_secret, buf, len);
 	nvme_auth_stop(ctrl);
-	if (strcmp(dhchap_secret, opts->dhchap_secret)) {
-		struct key *key, *host_key;
-
-		key = nvme_auth_extract_key(opts->keyring, dhchap_secret, count);
-		if (IS_ERR(key)) {
-			kfree(dhchap_secret);
-			return PTR_ERR(key);
-		}
-		kfree(opts->dhchap_secret);
-		opts->dhchap_secret = dhchap_secret;
-		host_key = ctrl->host_key;
-		mutex_lock(&ctrl->dhchap_auth_mutex);
-		ctrl->host_key = key;
-		mutex_unlock(&ctrl->dhchap_auth_mutex);
-		key_put(host_key);
-	} else
+	key = nvme_auth_extract_key(opts->keyring,
+				    dhchap_secret, count);
+	if (IS_ERR(key)) {
 		kfree(dhchap_secret);
+		return PTR_ERR(key);
+	}
+	down_read(&key->sem);
+	ret = key_validate(key);
+	up_read(&key->sem);
+	if (ret) {
+		dev_warn(ctrl->dev, "key %08x invalidated\n", key_serial(key));
+		dev_dbg(ctrl->dev, "revoke key %08x\n", key_serial(key));
+		key_revoke(key);
+		key_put(key);
+		kfree(dhchap_secret);
+		return ret;
+	}
+	mutex_lock(&ctrl->dhchap_auth_mutex);
+	old_key = ctrl->host_key;
+	dev_dbg(ctrl->dev, "revoke key %08x\n",
+		key_serial(old_key));
+	key_revoke(old_key);
+
+	ctrl->host_key = key;
+	mutex_unlock(&ctrl->dhchap_auth_mutex);
+	key_put(old_key);
+	kfree(dhchap_secret);
 	/* Start re-authentication */
 	dev_info(ctrl->device, "re-authenticating controller\n");
 	queue_work(nvme_wq, &ctrl->dhchap_auth_work);
@@ -633,11 +659,23 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct key *key = ctrl->ctrl_key;
+	size_t count;
 
-	if (!opts->dhchap_ctrl_secret)
+	if (!key)
 		return sysfs_emit(buf, "none\n");
-	return sysfs_emit(buf, "%s\n", opts->dhchap_ctrl_secret);
+	down_read(&key->sem);
+	if (key_validate(key))
+		count = sysfs_emit(buf, "<invalidated>");
+	else {
+		count = key->type->read(key, buf, PAGE_SIZE);
+		if (count > 0) {
+			buf[count] = '\n';
+			count++;
+		}
+	}
+	up_read(&key->sem);
+	return count;
 }
 
 static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
@@ -645,38 +683,46 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 {
 	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
 	struct nvmf_ctrl_options *opts = ctrl->opts;
+	struct key *key, *old_key;
 	char *dhchap_secret;
+	size_t len;
+	int ret;
 
-	if (!ctrl->opts->dhchap_ctrl_secret)
-		return -EINVAL;
-	if (count < 7)
-		return -EINVAL;
-	if (memcmp(buf, "DHHC-1:", 7))
+	if (!ctrl->ctrl_key || !strlen(buf))
 		return -EINVAL;
 
-	dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
+	len = strcspn(buf, "\n");
+	dhchap_secret = kzalloc(len + 1, GFP_KERNEL);
 	if (!dhchap_secret)
 		return -ENOMEM;
-	memcpy(dhchap_secret, buf, count);
+	memcpy(dhchap_secret, buf, len);
 	nvme_auth_stop(ctrl);
-	if (strcmp(dhchap_secret, opts->dhchap_ctrl_secret)) {
-		struct key *key, *ctrl_key;
-
-		key = nvme_auth_extract_key(opts->keyring,
-					    dhchap_secret, count);
-		if (IS_ERR(key)) {
-			kfree(dhchap_secret);
-			return PTR_ERR(key);
-		}
-		kfree(opts->dhchap_ctrl_secret);
-		opts->dhchap_ctrl_secret = dhchap_secret;
-		ctrl_key = ctrl->ctrl_key;
-		mutex_lock(&ctrl->dhchap_auth_mutex);
-		ctrl->ctrl_key = key;
-		mutex_unlock(&ctrl->dhchap_auth_mutex);
-		key_put(ctrl_key);
-	} else
+	key = nvme_auth_extract_key(opts->keyring,
+				    dhchap_secret, count);
+	if (IS_ERR(key)) {
 		kfree(dhchap_secret);
+		return PTR_ERR(key);
+	}
+	down_read(&key->sem);
+	ret = key_validate(key);
+	up_read(&key->sem);
+	if (ret) {
+		dev_warn(ctrl->dev, "key %08x invalidated\n", key_serial(key));
+		dev_dbg(ctrl->dev, "revoke key %08x\n", key_serial(key));
+		key_revoke(key);
+		key_put(key);
+		kfree(dhchap_secret);
+		return ret;
+	}
+	mutex_lock(&ctrl->dhchap_auth_mutex);
+	old_key = ctrl->ctrl_key;
+	dev_dbg(ctrl->dev, "revoke key %08x\n",
+		key_serial(old_key));
+	key_revoke(old_key);
+	ctrl->ctrl_key = key;
+	mutex_unlock(&ctrl->dhchap_auth_mutex);
+	key_put(old_key);
+	kfree(dhchap_secret);
 	/* Start re-authentication */
 	dev_info(ctrl->device, "re-authenticating controller\n");
 	queue_work(nvme_wq, &ctrl->dhchap_auth_work);
@@ -686,6 +732,41 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 
 static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
 	nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);
+
+static ssize_t dhchap_key_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+	struct key *key = ctrl->host_key;
+	size_t count;
+
+	if (!key)
+		return 0;
+	down_read(&key->sem);
+	if (key_validate(key))
+		count = sysfs_emit(buf, "<invalidated>\n");
+	else {
+		count = key->type->read(key, buf, PAGE_SIZE);
+		if (count > 0) {
+			buf[count] = '\n';
+			count++;
+		}
+	}
+	up_read(&key->sem);
+	return count;
+}
+static DEVICE_ATTR_RO(dhchap_key);
+
+static ssize_t dhchap_ctrl_key_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+
+	if (!ctrl->ctrl_key)
+		return 0;
+	return sysfs_emit(buf, "%08x\n", key_serial(ctrl->ctrl_key));
+}
+static DEVICE_ATTR_RO(dhchap_ctrl_key);
 #endif
 
 static struct attribute *nvme_dev_attrs[] = {
@@ -714,6 +795,8 @@ static struct attribute *nvme_dev_attrs[] = {
 #ifdef CONFIG_NVME_HOST_AUTH
 	&dev_attr_dhchap_secret.attr,
 	&dev_attr_dhchap_ctrl_secret.attr,
+	&dev_attr_dhchap_key.attr,
+	&dev_attr_dhchap_ctrl_key.attr,
 #endif
 	&dev_attr_adm_passthru_err_log_enabled.attr,
 	NULL
-- 
2.35.3



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

* [PATCH 5/9] nvmet-auth: parse dhchap key from configfs attribute
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
                   ` (3 preceding siblings ...)
  2025-05-28 14:05 ` [PATCH 4/9] nvme: parse dhchap keys during option parsing Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 6/9] nvme: allow to pass in key description as dhchap secret Hannes Reinecke
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

When writing a new dhchap key to the 'dhchap_secret' attribute
we should be parsing it directly and only use the pointer to the
inserted key. This avoids having to pass around a pointer with the
actual key material.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/target/auth.c     | 176 +++++++++++++++++++++------------
 drivers/nvme/target/configfs.c |  84 +++++++++++++---
 drivers/nvme/target/nvmet.h    |   7 +-
 3 files changed, 182 insertions(+), 85 deletions(-)

diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 7f87dc39a2de..2058e35908f8 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -20,54 +20,76 @@
 
 #include "nvmet.h"
 
+void nvmet_auth_revoke_key(struct nvmet_host *host, bool set_ctrl)
+{
+	struct key *key = ERR_PTR(-ENOKEY);
+
+	if (set_ctrl) {
+		if (host->dhchap_ctrl_key) {
+			key = host->dhchap_ctrl_key;
+			host->dhchap_ctrl_key = NULL;
+		}
+	} else {
+		if (host->dhchap_key) {
+			key = host->dhchap_key;
+			host->dhchap_key = NULL;
+		}
+	}
+	if (!IS_ERR(key)) {
+		pr_debug("%s: revoke %s key %08x\n",
+			 __func__, set_ctrl ? "ctrl" : "host",
+			 key_serial(key));
+		key_revoke(key);
+		key_put(key);
+	}
+}
+
 int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 		       bool set_ctrl)
 {
 	unsigned char key_hash;
-	char *dhchap_secret;
+	struct key *key;
+	size_t len;
 
 	if (!strlen(secret)) {
-		if (set_ctrl) {
-			kfree(host->dhchap_ctrl_secret);
-			host->dhchap_ctrl_secret = NULL;
-			host->dhchap_ctrl_key_hash = 0;
-		} else {
-			kfree(host->dhchap_secret);
-			host->dhchap_secret = NULL;
-			host->dhchap_key_hash = 0;
-		}
+		nvmet_auth_revoke_key(host, set_ctrl);
 		return 0;
 	}
-	if (sscanf(secret, "DHHC-1:%hhd:%*s", &key_hash) != 1)
-		return -EINVAL;
-	if (key_hash > 3) {
-		pr_warn("Invalid DH-HMAC-CHAP hash id %d\n",
-			 key_hash);
-		return -EINVAL;
+
+	len = strcspn(secret, "\n");
+	key = nvme_auth_extract_key(NULL, secret, len);
+	if (IS_ERR(key)) {
+		pr_debug("%s: invalid key specification\n", __func__);
+		return PTR_ERR(key);
+	}
+	down_read(&key->sem);
+	if (key_validate(key)) {
+		pr_warn("%s: key %08x invalidated\n",
+			__func__, key_serial(key));
+		up_read(&key->sem);
+		key_put(key);
+		return -EKEYREVOKED;
 	}
+
+	key_hash = nvme_dhchap_psk_hash(key);
 	if (key_hash > 0) {
 		/* Validate selected hash algorithm */
 		const char *hmac = nvme_auth_hmac_name(key_hash);
 
 		if (!crypto_has_shash(hmac, 0, 0)) {
 			pr_err("DH-HMAC-CHAP hash %s unsupported\n", hmac);
+			up_read(&key->sem);
+			key_put(key);
 			return -ENOTSUPP;
 		}
 	}
-	dhchap_secret = kstrdup(secret, GFP_KERNEL);
-	if (!dhchap_secret)
-		return -ENOMEM;
-	down_write(&nvmet_config_sem);
+	up_read(&key->sem);
+	nvmet_auth_revoke_key(host, set_ctrl);
 	if (set_ctrl) {
-		kfree(host->dhchap_ctrl_secret);
-		host->dhchap_ctrl_secret = strim(dhchap_secret);
-		host->dhchap_ctrl_key_hash = key_hash;
+		host->dhchap_ctrl_key = key;
 	} else {
-		kfree(host->dhchap_secret);
-		host->dhchap_secret = strim(dhchap_secret);
-		host->dhchap_key_hash = key_hash;
+		host->dhchap_key = key;
 	}
-	up_write(&nvmet_config_sem);
 	return 0;
 }
 
@@ -145,7 +167,8 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
 	int ret = 0;
 	struct nvmet_host_link *p;
 	struct nvmet_host *host = NULL;
-	u8 host_hash, ctrl_hash;
+	struct key *key;
+	key_serial_t key_id;
 
 	down_read(&nvmet_config_sem);
 	if (nvmet_is_disc_subsys(ctrl->subsys))
@@ -179,7 +202,7 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
 		goto out_unlock;
 	}
 
-	if (!host->dhchap_secret) {
+	if (!host->dhchap_key) {
 		pr_debug("No authentication provided\n");
 		goto out_unlock;
 	}
@@ -191,47 +214,68 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
 		ctrl->shash_id = host->dhchap_hash_id;
 	}
 
-	key_put(ctrl->host_key);
-	ctrl->host_key = nvme_auth_extract_key(NULL, host->dhchap_secret,
-					       strlen(host->dhchap_secret));
-	if (IS_ERR(ctrl->host_key)) {
-		ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
-		ctrl->host_key = NULL;
-		goto out_free_hash;
+	key = key_get(host->dhchap_key);
+	if (!key) {
+		pr_warn("%s: host key %08x not found\n",
+			 __func__, key_serial(host->dhchap_key));
+		goto out_unlock;
 	}
-	host_hash = nvme_dhchap_psk_hash(ctrl->host_key);
-	pr_debug("%s: using hash %s key %u\n", __func__,
-		 ctrl_hash > 0 ?
-		 nvme_auth_hmac_name(ctrl_hash) : "none",
-		 key_serial(ctrl->host_key));
-
-	key_put(ctrl->ctrl_key);
-	if (!host->dhchap_ctrl_secret) {
-		ctrl->ctrl_key = NULL;
+	down_read(&key->sem);
+	ret = key_validate(key);
+	if (!ret) {
+		if (ctrl->host_key) {
+			pr_debug("%s: drop host key %08x\n",
+				 __func__, key_serial(ctrl->host_key));
+			key_put(ctrl->host_key);
+		}
+		ctrl->host_key = key;
+	}
+	key_id = key_serial(key);
+	up_read(&key->sem);
+	if (ret) {
+		pr_debug("key id %08x invalidated\n", key_id);
+		key_put(key);
+		key = ERR_PTR(-EKEYREVOKED);
+	}
+	pr_debug("%s: using dhchap hash %s key %08x\n", __func__,
+		 nvme_auth_hmac_name(ctrl->shash_id), key_id);
+
+	if (!host->dhchap_ctrl_key) {
+		if (ctrl->ctrl_key) {
+			pr_debug("%s: drop ctrl key %08x\n",
+				 __func__, key_serial(ctrl->ctrl_key));
+			key_put(ctrl->ctrl_key);
+			ctrl->ctrl_key = NULL;
+		}
 		goto out_unlock;
 	}
 
-	ctrl->ctrl_key = nvme_auth_extract_key(NULL, host->dhchap_ctrl_secret,
-					       strlen(host->dhchap_ctrl_secret));
-	if (IS_ERR(ctrl->ctrl_key)) {
-		ret = NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE;
-		ctrl->ctrl_key = NULL;
-		goto out_free_hash;
+	key = key_get(host->dhchap_ctrl_key);
+	if (!key) {
+		pr_warn("%s: ctrl key %08x not found\n",
+			__func__, key_serial(host->dhchap_ctrl_key));
+		goto out_unlock;
 	}
-	ctrl_hash = nvme_dhchap_psk_hash(ctrl->ctrl_key);
-	pr_debug("%s: using ctrl hash %s key %u\n", __func__,
-		 ctrl_hash > 0 ?
-		 nvme_auth_hmac_name(ctrl_hash) : "none",
-		 key_serial(ctrl->ctrl_key));
-
-out_free_hash:
-	if (ret) {
-		if (ctrl->host_key) {
-			key_put(ctrl->host_key);
-			ctrl->host_key = NULL;
+	down_read(&key->sem);
+	ret = key_validate(key);
+	if (!ret) {
+		if (ctrl->ctrl_key) {
+			pr_debug("%s: drop ctrl key %08x\n",
+				 __func__, key_serial(ctrl->ctrl_key));
+			key_put(ctrl->ctrl_key);
 		}
-		ctrl->shash_id = 0;
+		ctrl->ctrl_key = key;
 	}
+	key_id = key_serial(key);
+	up_read(&key->sem);
+	if (ret) {
+		pr_debug("ctrl key id %08x invalidated\n", key_id);
+		key_put(key);
+		goto out_unlock;
+	}
+	pr_debug("%s: using dhchap ctrl hash %s key %08x\n", __func__,
+		 nvme_auth_hmac_name(ctrl->shash_id), key_id);
+
 out_unlock:
 	up_read(&nvmet_config_sem);
 
@@ -265,12 +309,14 @@ void nvmet_destroy_auth(struct nvmet_ctrl *ctrl)
 	ctrl->dh_key = NULL;
 
 	if (ctrl->host_key) {
-		key_revoke(ctrl->host_key);
+		pr_debug("%s: drop host key %08x\n",
+			 __func__, key_serial(ctrl->host_key));
 		key_put(ctrl->host_key);
 		ctrl->host_key = NULL;
 	}
 	if (ctrl->ctrl_key) {
-		key_revoke(ctrl->ctrl_key);
+		pr_debug("%s: drop ctrl key %08x\n",
+			 __func__, key_serial(ctrl->ctrl_key));
 		key_put(ctrl->ctrl_key);
 		ctrl->ctrl_key = NULL;
 	}
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e44ef69dffc2..3c5ccc012168 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -14,6 +14,7 @@
 #include <linux/pci-p2pdma.h>
 #ifdef CONFIG_NVME_TARGET_AUTH
 #include <linux/nvme-auth.h>
+#include <linux/key-type.h>
 #endif
 #include <linux/nvme-keyring.h>
 #include <crypto/hash.h>
@@ -2100,15 +2101,29 @@ static struct config_group nvmet_ports_group;
 static ssize_t nvmet_host_dhchap_key_show(struct config_item *item,
 		char *page)
 {
-	u8 *dhchap_secret;
+	struct nvmet_host *host = to_host(item);
+	struct key *key;
 	ssize_t ret;
 
 	down_read(&nvmet_config_sem);
-	dhchap_secret = to_host(item)->dhchap_secret;
-	if (!dhchap_secret)
-		ret = sprintf(page, "\n");
-	else
-		ret = sprintf(page, "%s\n", dhchap_secret);
+	key = key_get(host->dhchap_key);
+	if (!key) {
+		page[0] = '\0';
+		ret = 0;
+	} else {
+		down_read(&key->sem);
+		if (key_validate(key))
+			ret = sprintf(page, "<invalidated>\n");
+		else {
+			ret = key->type->read(key, page, PAGE_SIZE);
+			if (ret > 0) {
+				page[ret] = '\n';
+				ret++;
+			}
+		}
+		up_read(&key->sem);
+		key_put(key);
+	}
 	up_read(&nvmet_config_sem);
 	return ret;
 }
@@ -2117,9 +2132,21 @@ static ssize_t nvmet_host_dhchap_key_store(struct config_item *item,
 		const char *page, size_t count)
 {
 	struct nvmet_host *host = to_host(item);
+	u8 *keydata;
+	size_t len;
 	int ret;
 
-	ret = nvmet_auth_set_key(host, page, false);
+	len = strcspn(page, "\n");
+	if (!len)
+		return -EINVAL;
+	keydata = kstrndup(page, len, GFP_KERNEL);
+	if (!keydata)
+		return -ENOMEM;
+
+	down_write(&nvmet_config_sem);
+	ret = nvmet_auth_set_key(host, keydata, false);
+	up_write(&nvmet_config_sem);
+	kfree(keydata);
 	/*
 	 * Re-authentication is a soft state, so keep the
 	 * current authentication valid until the host
@@ -2133,15 +2160,29 @@ CONFIGFS_ATTR(nvmet_host_, dhchap_key);
 static ssize_t nvmet_host_dhchap_ctrl_key_show(struct config_item *item,
 		char *page)
 {
-	u8 *dhchap_secret = to_host(item)->dhchap_ctrl_secret;
+	struct nvmet_host *host = to_host(item);
+	struct key *key;
 	ssize_t ret;
 
 	down_read(&nvmet_config_sem);
-	dhchap_secret = to_host(item)->dhchap_ctrl_secret;
-	if (!dhchap_secret)
-		ret = sprintf(page, "\n");
-	else
-		ret = sprintf(page, "%s\n", dhchap_secret);
+	key = key_get(host->dhchap_ctrl_key);
+	if (!key) {
+		page[0] = '\0';
+		ret = 0;
+	} else {
+		down_read(&key->sem);
+		if (key_validate(key))
+			ret = sprintf(page, "<invalidated>\n");
+		else {
+			ret = key->type->read(key, page, PAGE_SIZE);
+			if (ret > 0) {
+				page[ret] = '\n';
+				ret++;
+			}
+		}
+		up_read(&key->sem);
+		key_put(key);
+	}
 	up_read(&nvmet_config_sem);
 	return ret;
 }
@@ -2150,9 +2191,20 @@ static ssize_t nvmet_host_dhchap_ctrl_key_store(struct config_item *item,
 		const char *page, size_t count)
 {
 	struct nvmet_host *host = to_host(item);
+	u8 *keydata;
+	size_t len;
 	int ret;
 
-	ret = nvmet_auth_set_key(host, page, true);
+	len = strcspn(page, "\n");
+	if (!len)
+		return -EINVAL;
+	keydata = kstrndup(page, len, GFP_KERNEL);
+	if (!keydata)
+		return -ENOMEM;
+
+	down_write(&nvmet_config_sem);
+	ret = nvmet_auth_set_key(host, keydata, true);
+	up_write(&nvmet_config_sem);
 	/*
 	 * Re-authentication is a soft state, so keep the
 	 * current authentication valid until the host
@@ -2233,8 +2285,8 @@ static void nvmet_host_release(struct config_item *item)
 	struct nvmet_host *host = to_host(item);
 
 #ifdef CONFIG_NVME_TARGET_AUTH
-	kfree(host->dhchap_secret);
-	kfree(host->dhchap_ctrl_secret);
+	nvmet_auth_revoke_key(host, false);
+	nvmet_auth_revoke_key(host, true);
 #endif
 	kfree(host);
 }
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index ca6053fbaac8..409a1580afe9 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -378,10 +378,8 @@ static inline struct nvmet_subsys *namespaces_to_subsys(
 
 struct nvmet_host {
 	struct config_group	group;
-	u8			*dhchap_secret;
-	u8			*dhchap_ctrl_secret;
-	u8			dhchap_key_hash;
-	u8			dhchap_ctrl_key_hash;
+	struct key		*dhchap_key;
+	struct key		*dhchap_ctrl_key;
 	u8			dhchap_hash_id;
 	u8			dhchap_dhgroup_id;
 };
@@ -890,6 +888,7 @@ u32 nvmet_auth_send_data_len(struct nvmet_req *req);
 void nvmet_execute_auth_send(struct nvmet_req *req);
 u32 nvmet_auth_receive_data_len(struct nvmet_req *req);
 void nvmet_execute_auth_receive(struct nvmet_req *req);
+void nvmet_auth_revoke_key(struct nvmet_host *host, bool set_ctrl);
 int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 		       bool set_ctrl);
 int nvmet_auth_set_host_hash(struct nvmet_host *host, const char *hash);
-- 
2.35.3



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

* [PATCH 6/9] nvme: allow to pass in key description as dhchap secret
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
                   ` (4 preceding siblings ...)
  2025-05-28 14:05 ` [PATCH 5/9] nvmet-auth: parse dhchap key from configfs attribute Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 7/9] nvme-auth: wait for authentication to finish when changing keys Hannes Reinecke
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

In order to use pre-populated keys update the parsing for
DH-HMAC-CHAP secret to allow for a key description instead
of an encoded binary secret.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/common/auth.c     | 15 ++++++---
 drivers/nvme/host/auth.c       | 17 ++++++++--
 drivers/nvme/host/fabrics.c    | 24 ++++++++++----
 drivers/nvme/host/fabrics.h    |  4 +++
 drivers/nvme/host/nvme.h       |  2 ++
 drivers/nvme/host/sysfs.c      | 60 ++++++++++++++++++++++------------
 drivers/nvme/target/auth.c     | 20 ++++++++++--
 drivers/nvme/target/configfs.c | 10 +++---
 drivers/nvme/target/nvmet.h    |  2 ++
 include/linux/nvme-auth.h      |  3 +-
 10 files changed, 116 insertions(+), 41 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index 8c2ccbfb9986..cbf35a7c3105 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -156,14 +156,21 @@ size_t nvme_auth_hmac_hash_len(u8 hmac_id)
 EXPORT_SYMBOL_GPL(nvme_auth_hmac_hash_len);
 
 struct key *nvme_auth_extract_key(struct key *keyring, const u8 *secret,
-				  size_t secret_len)
+				  size_t secret_len, bool *generated)
 {
 	struct key *key;
 
+	key = nvme_dhchap_psk_lookup(keyring, secret);
+	if (!IS_ERR(key)) {
+		*generated = false;
+		return key;
+	}
 	key = nvme_dhchap_psk_refresh(keyring, secret, secret_len);
-	if (!IS_ERR(key))
-		pr_debug("generated dhchap key %08x\n",
-			 key_serial(key));
+	if (!IS_ERR(key)) {
+		*generated = true;
+		pr_debug("generated dhchap key %s\n",
+			 key->description);
+	}
 	return key;
 }
 EXPORT_SYMBOL_GPL(nvme_auth_extract_key);
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index 0e8a5b544f63..0464e23b2a21 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -1057,19 +1057,29 @@ static void nvme_ctrl_auth_work(struct work_struct *work)
 static void nvme_auth_clear_key(struct nvme_ctrl *ctrl, bool is_ctrl)
 {
 	struct key *key;
+	bool generated;
 
 	if (is_ctrl) {
 		key = ctrl->ctrl_key;
 		ctrl->ctrl_key = NULL;
+		generated = ctrl->ctrl_key_generated;
+		ctrl->ctrl_key_generated = false;
 	} else {
 		key = ctrl->host_key;
 		ctrl->host_key = NULL;
+		generated = ctrl->host_key_generated;
+		ctrl->host_key_generated = false;
 	}
 	if (key) {
-		dev_dbg(ctrl->device, "%s: revoke%s key %08x\n",
+		if (generated) {
+			dev_dbg(ctrl->device, "%s: revoke%s key %08x\n",
+				__func__, is_ctrl ? " ctrl" : "",
+				key_serial(key));
+			key_revoke(key);
+		}
+		dev_dbg(ctrl->device, "%s: drop%s key %08x\n",
 			__func__, is_ctrl ? " ctrl" : "",
 			key_serial(key));
-		key_revoke(key);
 		key_put(key);
 	}
 }
@@ -1099,6 +1109,7 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
 			 key_serial(ctrl->opts->dhchap_key));
 		return -ENOKEY;
 	}
+	ctrl->host_key_generated = ctrl->opts->dhchap_key_generated;
 	down_read(&ctrl->host_key->sem);
 	ret = key_validate(ctrl->host_key);
 	up_read(&ctrl->host_key->sem);
@@ -1125,6 +1136,8 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
 				 key_serial(ctrl->opts->dhchap_ctrl_key));
 			return -ENOKEY;
 		}
+		ctrl->ctrl_key_generated =
+			ctrl->opts->dhchap_ctrl_key_generated;
 		down_read(&ctrl->ctrl_key->sem);
 		ret = key_validate(ctrl->ctrl_key);
 		up_read(&ctrl->ctrl_key->sem);
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 009a6cf8a86b..6ec94c4f6075 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -741,7 +741,9 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 	opts->keyring = NULL;
 	opts->concat = false;
 	opts->dhchap_key = NULL;
+	opts->dhchap_key_generated = false;
 	opts->dhchap_ctrl_key = NULL;
+	opts->dhchap_ctrl_key_generated = false;
 
 	options = o = kstrdup(buf, GFP_KERNEL);
 	if (!options)
@@ -1095,7 +1097,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 	if (host_secret) {
 		pr_debug("lookup host identity '%s'\n", host_secret);
 		key = nvme_auth_extract_key(opts->keyring, host_secret,
-					    strlen(host_secret));
+					    strlen(host_secret),
+					    &opts->dhchap_key_generated);
 		if (IS_ERR(key)) {
 			ret = PTR_ERR(key);
 			goto out;
@@ -1110,7 +1113,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 		}
 		pr_debug("lookup ctrl identity '%s'\n", ctrl_secret);
 		key = nvme_auth_extract_key(opts->keyring, ctrl_secret,
-					    strlen(ctrl_secret));
+					    strlen(ctrl_secret),
+					    &opts->dhchap_ctrl_key_generated);
 		if (IS_ERR(key)) {
 			ret = PTR_ERR(key);
 			goto out;
@@ -1315,15 +1319,23 @@ void nvmf_free_options(struct nvmf_ctrl_options *opts)
 	kfree(opts->host_traddr);
 	kfree(opts->host_iface);
 	if (opts->dhchap_key) {
-		pr_debug("revoke dhchap key %08x\n",
+		if (opts->dhchap_key_generated) {
+			pr_debug("revoke dhchap key %08x\n",
+				 key_serial(opts->dhchap_key));
+			key_revoke(opts->dhchap_key);
+		}
+		pr_debug("drop dhchap key %08x\n",
 			 key_serial(opts->dhchap_key));
-		key_revoke(opts->dhchap_key);
 		key_put(opts->dhchap_key);
 	}
 	if (opts->dhchap_ctrl_key) {
-		pr_debug("revoke dhchap ctrl key %08x\n",
+		if (opts->dhchap_ctrl_key_generated) {
+			pr_debug("revoke dhchap ctrl key %08x\n",
+				 key_serial(opts->dhchap_ctrl_key));
+			key_revoke(opts->dhchap_key);
+		}
+		pr_debug("drop dhchap ctrl key %08x\n",
 			 key_serial(opts->dhchap_ctrl_key));
-		key_revoke(opts->dhchap_key);
 		key_put(opts->dhchap_ctrl_key);
 	}
 	kfree(opts);
diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index 95a3d8ce1201..4b0ef3182f4c 100644
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -99,6 +99,8 @@ enum {
  * @dhchap_key: DH-HMAC-CHAP pre-shared key
  * @dhchap_ctrl_key: DH-HMAC-CHAP controller pre-shared key for bi-directional
  *              authentication
+ * @dhchap_key_generated: True if the @dhchap_key has been auto-generated
+ * @dhchap_ctrl_key_generated: True if @dhchap_ctrl_key has been auto-generated
  * @keyring:    Keyring to use for key lookups
  * @tls_key:    TLS key for encrypted connections (TCP)
  * @tls:        Start TLS encrypted connections (TCP)
@@ -129,6 +131,8 @@ struct nvmf_ctrl_options {
 	struct nvmf_host	*host;
 	struct key		*dhchap_key;
 	struct key		*dhchap_ctrl_key;
+	bool			dhchap_key_generated;
+	bool			dhchap_ctrl_key_generated;
 	struct key		*keyring;
 	struct key		*tls_key;
 	bool			tls;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 3dd83a48532a..45beb701011b 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -381,6 +381,8 @@ struct nvme_ctrl {
 	struct key *host_key;
 	struct key *ctrl_key;
 	u16 transaction;
+	bool host_key_generated;
+	bool ctrl_key_generated;
 #endif
 	key_serial_t tls_pskid;
 
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 8a96228ff244..f1ab165c1f86 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -588,13 +588,14 @@ static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
 	down_read(&key->sem);
 	if (key_validate(key))
 		count = sysfs_emit(buf, "<invalidated>\n");
-	else {
+	else if (ctrl->host_key_generated) {
 		count = key->type->read(key, buf, PAGE_SIZE);
 		if (count > 0) {
 			buf[count] = '\n';
 			count++;
 		}
-	}
+	} else
+		count = sysfs_emit(buf, "%s\n", key->description);
 	up_read(&key->sem);
 	return count;
 }
@@ -606,6 +607,7 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 	struct nvmf_ctrl_options *opts = ctrl->opts;
 	struct key *key, *old_key;
 	char *dhchap_secret;
+	bool generated = false;
 	size_t len;
 	int ret;
 
@@ -618,8 +620,8 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 		return -ENOMEM;
 	memcpy(dhchap_secret, buf, len);
 	nvme_auth_stop(ctrl);
-	key = nvme_auth_extract_key(opts->keyring,
-				    dhchap_secret, count);
+	key = nvme_auth_extract_key(opts->keyring, dhchap_secret, len,
+				    &generated);
 	if (IS_ERR(key)) {
 		kfree(dhchap_secret);
 		return PTR_ERR(key);
@@ -629,19 +631,25 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 	up_read(&key->sem);
 	if (ret) {
 		dev_warn(ctrl->dev, "key %08x invalidated\n", key_serial(key));
-		dev_dbg(ctrl->dev, "revoke key %08x\n", key_serial(key));
-		key_revoke(key);
+		if (generated) {
+			dev_dbg(ctrl->dev, "revoke key %08x\n", key_serial(key));
+			key_revoke(key);
+			synchronize_rcu();
+		}
 		key_put(key);
 		kfree(dhchap_secret);
 		return ret;
 	}
 	mutex_lock(&ctrl->dhchap_auth_mutex);
 	old_key = ctrl->host_key;
-	dev_dbg(ctrl->dev, "revoke key %08x\n",
-		key_serial(old_key));
-	key_revoke(old_key);
-
+	if (ctrl->host_key_generated) {
+		dev_dbg(ctrl->dev, "revoke key %08x\n",
+			key_serial(old_key));
+		key_revoke(old_key);
+		synchronize_rcu();
+	}
 	ctrl->host_key = key;
+	ctrl->host_key_generated = generated;
 	mutex_unlock(&ctrl->dhchap_auth_mutex);
 	key_put(old_key);
 	kfree(dhchap_secret);
@@ -667,13 +675,14 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
 	down_read(&key->sem);
 	if (key_validate(key))
 		count = sysfs_emit(buf, "<invalidated>");
-	else {
+	else if (ctrl->ctrl_key_generated) {
 		count = key->type->read(key, buf, PAGE_SIZE);
 		if (count > 0) {
 			buf[count] = '\n';
 			count++;
 		}
-	}
+	} else
+		count = sysfs_emit(buf, "%s\n", key->description);
 	up_read(&key->sem);
 	return count;
 }
@@ -685,6 +694,7 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 	struct nvmf_ctrl_options *opts = ctrl->opts;
 	struct key *key, *old_key;
 	char *dhchap_secret;
+	bool generated = false;
 	size_t len;
 	int ret;
 
@@ -697,8 +707,8 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 		return -ENOMEM;
 	memcpy(dhchap_secret, buf, len);
 	nvme_auth_stop(ctrl);
-	key = nvme_auth_extract_key(opts->keyring,
-				    dhchap_secret, count);
+	key = nvme_auth_extract_key(opts->keyring, dhchap_secret, len,
+				    &generated);
 	if (IS_ERR(key)) {
 		kfree(dhchap_secret);
 		return PTR_ERR(key);
@@ -708,18 +718,25 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 	up_read(&key->sem);
 	if (ret) {
 		dev_warn(ctrl->dev, "key %08x invalidated\n", key_serial(key));
-		dev_dbg(ctrl->dev, "revoke key %08x\n", key_serial(key));
-		key_revoke(key);
+		if (generated) {
+			dev_dbg(ctrl->dev, "revoke key %08x\n", key_serial(key));
+			key_revoke(key);
+			synchronize_rcu();
+		}
 		key_put(key);
 		kfree(dhchap_secret);
 		return ret;
 	}
 	mutex_lock(&ctrl->dhchap_auth_mutex);
 	old_key = ctrl->ctrl_key;
-	dev_dbg(ctrl->dev, "revoke key %08x\n",
-		key_serial(old_key));
-	key_revoke(old_key);
+	if (ctrl->ctrl_key_generated) {
+		dev_dbg(ctrl->dev, "revoke key %08x\n",
+			key_serial(old_key));
+		key_revoke(old_key);
+		synchronize_rcu();
+	}
 	ctrl->ctrl_key = key;
+	ctrl->ctrl_key_generated = generated;
 	mutex_unlock(&ctrl->dhchap_auth_mutex);
 	key_put(old_key);
 	kfree(dhchap_secret);
@@ -745,13 +762,14 @@ static ssize_t dhchap_key_show(struct device *dev,
 	down_read(&key->sem);
 	if (key_validate(key))
 		count = sysfs_emit(buf, "<invalidated>\n");
-	else {
+	else if (ctrl->host_key_generated) {
 		count = key->type->read(key, buf, PAGE_SIZE);
 		if (count > 0) {
 			buf[count] = '\n';
 			count++;
 		}
-	}
+	} else
+		count = sysfs_emit(buf, "%08x\n", key_serial(ctrl->host_key));
 	up_read(&key->sem);
 	return count;
 }
diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 2058e35908f8..036652de3489 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -23,23 +23,34 @@
 void nvmet_auth_revoke_key(struct nvmet_host *host, bool set_ctrl)
 {
 	struct key *key = ERR_PTR(-ENOKEY);
+	bool generated = false;
 
 	if (set_ctrl) {
 		if (host->dhchap_ctrl_key) {
 			key = host->dhchap_ctrl_key;
+			generated = host->dhchap_ctrl_key_generated;
 			host->dhchap_ctrl_key = NULL;
+			host->dhchap_ctrl_key_generated = false;
 		}
 	} else {
 		if (host->dhchap_key) {
 			key = host->dhchap_key;
+			generated = host->dhchap_key_generated;
 			host->dhchap_key = NULL;
+			host->dhchap_key_generated = false;
 		}
 	}
 	if (!IS_ERR(key)) {
-		pr_debug("%s: revoke %s key %08x\n",
+		if (generated) {
+			pr_debug("%s: revoke %s key %08x\n",
+				 __func__, set_ctrl ? "ctrl" : "host",
+				 key_serial(key));
+			key_revoke(key);
+			synchronize_rcu();
+		}
+		pr_debug("%s: drop %s key %08x\n",
 			 __func__, set_ctrl ? "ctrl" : "host",
 			 key_serial(key));
-		key_revoke(key);
 		key_put(key);
 	}
 }
@@ -48,6 +59,7 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 		       bool set_ctrl)
 {
 	unsigned char key_hash;
+	bool generated = false;
 	struct key *key;
 	size_t len;
 
@@ -57,7 +69,7 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 	}
 
 	len = strcspn(secret, "\n");
-	key = nvme_auth_extract_key(NULL, secret, len);
+	key = nvme_auth_extract_key(NULL, secret, len, &generated);
 	if (IS_ERR(key)) {
 		pr_debug("%s: invalid key specification\n", __func__);
 		return PTR_ERR(key);
@@ -87,8 +99,10 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 	nvmet_auth_revoke_key(host, set_ctrl);
 	if (set_ctrl) {
 		host->dhchap_ctrl_key = key;
+		host->dhchap_ctrl_key_generated = generated;
 	} else {
 		host->dhchap_key = key;
+		host->dhchap_key_generated = generated;
 	}
 	return 0;
 }
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 3c5ccc012168..e165905fab31 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -2114,13 +2114,14 @@ static ssize_t nvmet_host_dhchap_key_show(struct config_item *item,
 		down_read(&key->sem);
 		if (key_validate(key))
 			ret = sprintf(page, "<invalidated>\n");
-		else {
+		else if (host->dhchap_key_generated) {
 			ret = key->type->read(key, page, PAGE_SIZE);
 			if (ret > 0) {
 				page[ret] = '\n';
 				ret++;
 			}
-		}
+		} else
+			ret = sprintf(page, "%s\n", key->description);
 		up_read(&key->sem);
 		key_put(key);
 	}
@@ -2173,13 +2174,14 @@ static ssize_t nvmet_host_dhchap_ctrl_key_show(struct config_item *item,
 		down_read(&key->sem);
 		if (key_validate(key))
 			ret = sprintf(page, "<invalidated>\n");
-		else {
+		else if (host->dhchap_ctrl_key_generated) {
 			ret = key->type->read(key, page, PAGE_SIZE);
 			if (ret > 0) {
 				page[ret] = '\n';
 				ret++;
 			}
-		}
+		} else
+			ret = sprintf(page, "%s\n", key->description);
 		up_read(&key->sem);
 		key_put(key);
 	}
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 409a1580afe9..772a3fc69162 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -379,7 +379,9 @@ static inline struct nvmet_subsys *namespaces_to_subsys(
 struct nvmet_host {
 	struct config_group	group;
 	struct key		*dhchap_key;
+	bool			dhchap_key_generated;
 	struct key		*dhchap_ctrl_key;
+	bool			dhchap_ctrl_key_generated;
 	u8			dhchap_hash_id;
 	u8			dhchap_dhgroup_id;
 };
diff --git a/include/linux/nvme-auth.h b/include/linux/nvme-auth.h
index 4e53ef96eea7..a9ae1d60a5f9 100644
--- a/include/linux/nvme-auth.h
+++ b/include/linux/nvme-auth.h
@@ -18,7 +18,8 @@ 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);
 
-struct key *nvme_auth_extract_key(struct key *keyring, const u8 *secret, size_t secret_len);
+struct key *nvme_auth_extract_key(struct key *keyring, const u8 *secret, size_t secret_len,
+				  bool *generated);
 int nvme_auth_transform_key(struct key *key, char *nqn,
 			    u8 **transformed_secret);
 int nvme_auth_augmented_challenge(u8 hmac_id, u8 *skey, size_t skey_len,
-- 
2.35.3



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

* [PATCH 7/9] nvme-auth: wait for authentication to finish when changing keys
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
                   ` (5 preceding siblings ...)
  2025-05-28 14:05 ` [PATCH 6/9] nvme: allow to pass in key description as dhchap secret Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 8/9] nvme-fabrics: allow to pass in keyring by name Hannes Reinecke
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

When changing DH-HMAC-CHAP keys in sysfs we need to wait for authentication
to complete on all queues. Otherwise the user might change the keys while
authentication is in progress, causing the authentication to fail and
I/O to be interrupted.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/host/sysfs.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index f1ab165c1f86..3ca10db0653c 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -656,7 +656,7 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
 	/* Start re-authentication */
 	dev_info(ctrl->device, "re-authenticating controller\n");
 	queue_work(nvme_wq, &ctrl->dhchap_auth_work);
-
+	flush_work(&ctrl->dhchap_auth_work);
 	return count;
 }
 
@@ -721,7 +721,6 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 		if (generated) {
 			dev_dbg(ctrl->dev, "revoke key %08x\n", key_serial(key));
 			key_revoke(key);
-			synchronize_rcu();
 		}
 		key_put(key);
 		kfree(dhchap_secret);
@@ -733,7 +732,6 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 		dev_dbg(ctrl->dev, "revoke key %08x\n",
 			key_serial(old_key));
 		key_revoke(old_key);
-		synchronize_rcu();
 	}
 	ctrl->ctrl_key = key;
 	ctrl->ctrl_key_generated = generated;
@@ -743,7 +741,7 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
 	/* Start re-authentication */
 	dev_info(ctrl->device, "re-authenticating controller\n");
 	queue_work(nvme_wq, &ctrl->dhchap_auth_work);
-
+	flush_work(&ctrl->dhchap_auth_work);
 	return count;
 }
 
-- 
2.35.3



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

* [PATCH 8/9] nvme-fabrics: allow to pass in keyring by name
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
                   ` (6 preceding siblings ...)
  2025-05-28 14:05 ` [PATCH 7/9] nvme-auth: wait for authentication to finish when changing keys Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-05-28 14:05 ` [PATCH 9/9] nvmet: add configfs attribute 'dhchap_keyring' Hannes Reinecke
  2025-07-03  9:39 ` [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Christoph Hellwig
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

The JSON configuration should be system independent, so we cannot
list any key serial numbers. So this patch allows to specify the TLS keyring
by name and not only by key serial number.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/host/fabrics.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 6ec94c4f6075..d0f8c40cebb8 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -14,6 +14,7 @@
 #include "fabrics.h"
 #include <linux/nvme-auth.h>
 #include <linux/nvme-keyring.h>
+#include <linux/key-type.h>
 
 static LIST_HEAD(nvmf_transports);
 static DECLARE_RWSEM(nvmf_transports_rwsem);
@@ -999,13 +1000,23 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 			break;
 		case NVMF_OPT_KEYRING:
 			if (match_int(args, &key_id) || key_id <= 0) {
-				ret = -EINVAL;
-				goto out;
-			}
-			key = nvmf_parse_key(key_id);
-			if (IS_ERR(key)) {
-				ret = PTR_ERR(key);
-				goto out;
+				p = match_strdup(args);
+				if (!p) {
+					ret = -ENOMEM;
+					goto out;
+				}
+				key = request_key(&key_type_keyring, p, NULL);
+				kfree(p);
+				if (IS_ERR(key)) {
+					ret = PTR_ERR(key);
+					goto out;
+				}
+			} else {
+				key = nvmf_parse_key(key_id);
+				if (IS_ERR(key)) {
+					ret = PTR_ERR(key);
+					goto out;
+				}
 			}
 			key_put(opts->keyring);
 			opts->keyring = key;
-- 
2.35.3



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

* [PATCH 9/9] nvmet: add configfs attribute 'dhchap_keyring'
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
                   ` (7 preceding siblings ...)
  2025-05-28 14:05 ` [PATCH 8/9] nvme-fabrics: allow to pass in keyring by name Hannes Reinecke
@ 2025-05-28 14:05 ` Hannes Reinecke
  2025-07-03  9:39 ` [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Christoph Hellwig
  9 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-05-28 14:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

The authentication code now fetches the key from the kernel keystore, so
we should be able to specify which keyring to use for looking up keys.
So add a configfs attribute 'dhchap_keyring' for the 'host' directory
to specify the keyring to use.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/target/auth.c     |  2 +-
 drivers/nvme/target/configfs.c | 60 ++++++++++++++++++++++++++++++++++
 drivers/nvme/target/nvmet.h    |  1 +
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 036652de3489..090cc1dc0655 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -69,7 +69,7 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 	}
 
 	len = strcspn(secret, "\n");
-	key = nvme_auth_extract_key(NULL, secret, len, &generated);
+	key = nvme_auth_extract_key(host->dhchap_keyring, secret, len, &generated);
 	if (IS_ERR(key)) {
 		pr_debug("%s: invalid key specification\n", __func__);
 		return PTR_ERR(key);
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e165905fab31..2642e3148f3f 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -2217,6 +2217,60 @@ static ssize_t nvmet_host_dhchap_ctrl_key_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_host_, dhchap_ctrl_key);
 
+static ssize_t nvmet_host_dhchap_keyring_show(struct config_item *item,
+		char *page)
+{
+	struct nvmet_host *host = to_host(item);
+	struct key *keyring;
+	ssize_t ret;
+
+	down_read(&nvmet_config_sem);
+	keyring = key_get(host->dhchap_keyring);
+	if (!keyring) {
+		page[0] = '\0';
+		ret = 0;
+	} else {
+		down_read(&keyring->sem);
+		if (key_validate(keyring))
+			ret = sprintf(page, "<invalidated>\n");
+		else
+			ret = sprintf(page, "%s\n", keyring->description);
+		up_read(&keyring->sem);
+		key_put(keyring);
+	}
+	up_read(&nvmet_config_sem);
+	return ret;
+}
+
+static ssize_t nvmet_host_dhchap_keyring_store(struct config_item *item,
+		const char *page, size_t count)
+{
+	struct nvmet_host *host = to_host(item);
+	struct key *keyring;
+	char *desc;
+	size_t len;
+	int ret = 0;
+
+	len = strcspn(page, "\n");
+	if (!len)
+		return -EINVAL;
+	desc = kstrndup(page, len, GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+	keyring = request_key(&key_type_keyring, desc, NULL);
+	if (IS_ERR(keyring)) {
+		ret = PTR_ERR(keyring);
+	} else {
+		key_put(host->dhchap_keyring);
+		host->dhchap_keyring = keyring;
+	}
+	kfree(desc);
+
+	return ret ? -ret : count;
+}
+
+CONFIGFS_ATTR(nvmet_host_, dhchap_keyring);
+
 static ssize_t nvmet_host_dhchap_hash_show(struct config_item *item,
 		char *page)
 {
@@ -2276,6 +2330,7 @@ CONFIGFS_ATTR(nvmet_host_, dhchap_dhgroup);
 static struct configfs_attribute *nvmet_host_attrs[] = {
 	&nvmet_host_attr_dhchap_key,
 	&nvmet_host_attr_dhchap_ctrl_key,
+	&nvmet_host_attr_dhchap_keyring,
 	&nvmet_host_attr_dhchap_hash,
 	&nvmet_host_attr_dhchap_dhgroup,
 	NULL,
@@ -2317,6 +2372,11 @@ static struct config_group *nvmet_hosts_make_group(struct config_group *group,
 #ifdef CONFIG_NVME_TARGET_AUTH
 	/* Default to SHA256 */
 	host->dhchap_hash_id = NVME_AUTH_HASH_SHA256;
+	host->dhchap_keyring = key_lookup(nvme_keyring_id());
+	if (IS_ERR(host->dhchap_keyring)) {
+		kfree(host);
+		return ERR_PTR(-ENOKEY);
+	}
 #endif
 
 	config_group_init_type_name(&host->group, name, &nvmet_host_type);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 772a3fc69162..30ec38142ef3 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -382,6 +382,7 @@ struct nvmet_host {
 	bool			dhchap_key_generated;
 	struct key		*dhchap_ctrl_key;
 	bool			dhchap_ctrl_key_generated;
+	struct key		*dhchap_keyring;
 	u8			dhchap_hash_id;
 	u8			dhchap_dhgroup_id;
 };
-- 
2.35.3



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

* Re: [PATCH 2/9] nvme-keyring: add 'dhchap' key type
  2025-05-28 14:05 ` [PATCH 2/9] nvme-keyring: add 'dhchap' key type Hannes Reinecke
@ 2025-06-03  0:32   ` Shinichiro Kawasaki
  2025-06-03  6:11     ` Hannes Reinecke
  0 siblings, 1 reply; 14+ messages in thread
From: Shinichiro Kawasaki @ 2025-06-03  0:32 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: hch, Sagi Grimberg, Keith Busch, linux-nvme@lists.infradead.org

On May 28, 2025 / 16:05, Hannes Reinecke wrote:
> Add a 'dhchap' keytype to store DH-HMAC-CHAP secret keys.
> Keys are stored with a 'user-type' compatible payload, such
> that one can use 'user_read()' to access the raw contents
> and the 'read()' callback to get the base64-encoded key
> data in the DH-HMAC-CHAP secret representation.
> 
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
>  drivers/nvme/common/keyring.c | 266 ++++++++++++++++++++++++++++++++++
>  include/linux/nvme-keyring.h  |  22 ++-
>  2 files changed, 287 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
> index 32d16c53133b..a58c93c6d495 100644
> --- a/drivers/nvme/common/keyring.c
> +++ b/drivers/nvme/common/keyring.c
[...]
> +static int nvme_dhchap_psk_preparse(struct key_preparsed_payload *prep)
> +{
> +	struct user_key_payload *upayload;
> +	size_t datalen = prep->datalen, keylen;
> +	int ret;
> +	u32 crc;
> +	u8 version, hmac;
> +
> +	if (!prep->data) {
> +		pr_debug("%s: Empty data", __func__);
> +		prep->payload.data[0] = NULL;
> +		prep->quotalen = 0;
> +		return -EINVAL;
> +	}
> +
> +	if (sscanf(prep->data, "DHHC-%01hhu:%02hhu:%*s", &version, &hmac) != 2) {

I built the kernel at nvme-6.16 branch at git hash bf4d87cba2d7 applying
this series. Then I ran the blktests test case corresponding to this series,
and observed the BUG KASAN slab-out-of-bounds below [1].

The sscanf() above has three "%"s in the conversion format string, while it has
two following pointer arguments. I think this gap between the numbers of "%"s
and the pointer arguments causes the BUG. I removed "%*s" from the format
string, then the BUG looks disappearing.

[1]

Jun 03 08:48:30 testnode2 unknown: run blktests nvme/064 at 2025-06-03 08:48:30
Jun 03 08:48:30 testnode2 kernel: ==================================================================
Jun 03 08:48:30 testnode2 kernel: BUG: KASAN: slab-out-of-bounds in vsscanf+0xd55/0x2d20
Jun 03 08:48:30 testnode2 kernel: Read of size 1 at addr ffff888100a5303b by task keyctl/1116
Jun 03 08:48:30 testnode2 kernel: 
Jun 03 08:48:30 testnode2 kernel: CPU: 1 UID: 0 PID: 1116 Comm: keyctl Not tainted 6.15.0-rc3+ #43 PREEMPT(voluntary) 
Jun 03 08:48:30 testnode2 kernel: Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-3.fc41 04/01/2014
Jun 03 08:48:30 testnode2 kernel: Call Trace:
Jun 03 08:48:30 testnode2 kernel:  <TASK>
Jun 03 08:48:30 testnode2 kernel:  dump_stack_lvl+0x6a/0x90
Jun 03 08:48:30 testnode2 kernel:  print_report+0x174/0x554
Jun 03 08:48:30 testnode2 kernel:  ? __virt_addr_valid+0x208/0x430
Jun 03 08:48:30 testnode2 kernel:  ? vsscanf+0xd55/0x2d20
Jun 03 08:48:30 testnode2 kernel:  kasan_report+0xae/0x170
Jun 03 08:48:30 testnode2 kernel:  ? vsscanf+0xd55/0x2d20
Jun 03 08:48:30 testnode2 kernel:  vsscanf+0xd55/0x2d20
Jun 03 08:48:30 testnode2 kernel:  ? __pfx_vsscanf+0x10/0x10
Jun 03 08:48:30 testnode2 kernel:  sscanf+0xac/0xe0
Jun 03 08:48:30 testnode2 kernel:  ? __pfx_sscanf+0x10/0x10
Jun 03 08:48:30 testnode2 kernel:  ? lock_acquire+0x180/0x310
Jun 03 08:48:30 testnode2 kernel:  ? __pfx___might_resched+0x10/0x10
Jun 03 08:48:30 testnode2 kernel:  nvme_dhchap_psk_preparse+0xde/0x5a0 [nvme_keyring]
Jun 03 08:48:30 testnode2 kernel:  ? __pfx_nvme_dhchap_psk_preparse+0x10/0x10 [nvme_keyring]
Jun 03 08:48:30 testnode2 kernel:  ? __pfx_down_read+0x1/0x10
Jun 03 08:48:30 testnode2 kernel:  ? avc_has_perm+0xa6/0x160
Jun 03 08:48:30 testnode2 kernel:  ? lock_is_held_type+0x41/0x130
Jun 03 08:48:30 testnode2 kernel:  ? __pfx_avc_has_perm+0x10/0x10
Jun 03 08:48:30 testnode2 kernel:  __key_create_or_update+0x3f1/0xc60
Jun 03 08:48:30 testnode2 kernel:  ? __pfx___key_create_or_update+0x10/0x10
Jun 03 08:48:30 testnode2 kernel:  ? __pfx_lookup_user_key+0x10/0x10
Jun 03 08:48:30 testnode2 kernel:  ? lock_release+0x17d/0x2c0
Jun 03 08:48:30 testnode2 kernel:  key_create_or_update+0x10/0x20
Jun 03 08:48:30 testnode2 kernel:  __do_sys_add_key+0x1e5/0x300
Jun 03 08:48:30 testnode2 kernel:  ? __pfx___do_sys_add_key+0x10/0x10
Jun 03 08:48:30 testnode2 kernel:  ? fput_close_sync+0x100/0x170
Jun 03 08:48:30 testnode2 kernel:  do_syscall_64+0x93/0x190
Jun 03 08:48:30 testnode2 kernel:  ? do_syscall_64+0x9f/0x190
Jun 03 08:48:30 testnode2 kernel:  ? lockdep_hardirqs_on+0x78/0x100
Jun 03 08:48:30 testnode2 kernel:  ? do_syscall_64+0x9f/0x190
Jun 03 08:48:30 testnode2 kernel:  ? kasan_save_track+0x10/0x30
Jun 03 08:48:30 testnode2 kernel:  ? kasan_save_free_info+0x37/0x60
Jun 03 08:48:30 testnode2 kernel:  ? __kasan_slab_free+0x4b/0x70
Jun 03 08:48:30 testnode2 kernel:  ? kfree+0x13a/0x4b0
Jun 03 08:48:30 testnode2 kernel:  ? keyctl_describe_key+0x29c/0x420
Jun 03 08:48:30 testnode2 kernel:  ? do_syscall_64+0x93/0x190
Jun 03 08:48:30 testnode2 kernel:  ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
Jun 03 08:48:30 testnode2 kernel:  ? kasan_quarantine_put+0xd8/0x1e0
Jun 03 08:48:30 testnode2 kernel:  ? kasan_quarantine_put+0xd8/0x1e0
Jun 03 08:48:30 testnode2 kernel:  ? lockdep_hardirqs_on+0x78/0x100
Jun 03 08:48:30 testnode2 kernel:  ? kasan_quarantine_put+0xd8/0x1e0
Jun 03 08:48:30 testnode2 kernel:  ? kfree+0x13a/0x4b0
Jun 03 08:48:30 testnode2 kernel:  ? lock_release+0x17d/0x2c0
Jun 03 08:48:30 testnode2 kernel:  ? keyctl_describe_key+0x29c/0x420
Jun 03 08:48:30 testnode2 kernel:  ? key_put+0x25/0x280
Jun 03 08:48:30 testnode2 kernel:  ? syscall_exit_to_user_mode+0x8e/0x280
Jun 03 08:48:30 testnode2 kernel:  ? rcu_is_watching+0x11/0xb0
Jun 03 08:48:30 testnode2 kernel:  ? do_syscall_64+0x9f/0x190
Jun 03 08:48:30 testnode2 kernel:  ? lockdep_hardirqs_on+0x78/0x100
Jun 03 08:48:30 testnode2 kernel:  ? do_syscall_64+0x9f/0x190
Jun 03 08:48:30 testnode2 kernel:  ? lock_release+0x17d/0x2c0
Jun 03 08:48:30 testnode2 kernel:  ? do_user_addr_fault+0x4a2/0xa00
Jun 03 08:48:30 testnode2 kernel:  ? irqentry_exit_to_user_mode+0x84/0x270
Jun 03 08:48:30 testnode2 kernel:  ? rcu_is_watching+0x11/0xb0
Jun 03 08:48:30 testnode2 kernel:  entry_SYSCALL_64_after_hwframe+0x76/0x7e
Jun 03 08:48:30 testnode2 kernel: RIP: 0033:0x7f256cc0b95d
Jun 03 08:48:30 testnode2 kernel: Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 83 64 0f 00 f7 d8 64 89 01 48
Jun 03 08:48:30 testnode2 kernel: RSP: 002b:00007ffd307fd8e8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
Jun 03 08:48:30 testnode2 kernel: RAX: ffffffffffffffda RBX: 00007ffd307fdae0 RCX: 00007f256cc0b95d
Jun 03 08:48:30 testnode2 kernel: RDX: 00007ffd307fe63d RSI: 00007ffd307fe618 RDI: 00007ffd307fe611
Jun 03 08:48:30 testnode2 kernel: RBP: 00007ffd307fd8f0 R08: 000000000ae4be97 R09: 00007ffd307fd940
Jun 03 08:48:30 testnode2 kernel: R10: 000000000000003b R11: 0000000000000246 R12: 00007ffd307fe63d
Jun 03 08:48:30 testnode2 kernel: R13: 000000000000003b R14: 000055e2e6923cdb R15: 000055e2e69264b8
Jun 03 08:48:30 testnode2 kernel:  </TASK>
Jun 03 08:48:30 testnode2 kernel: 
Jun 03 08:48:30 testnode2 kernel: Allocated by task 1116:
Jun 03 08:48:30 testnode2 kernel:  kasan_save_stack+0x2c/0x50
Jun 03 08:48:30 testnode2 kernel:  kasan_save_track+0x10/0x30
Jun 03 08:48:30 testnode2 kernel:  __kasan_kmalloc+0xa6/0xb0
Jun 03 08:48:30 testnode2 kernel:  __kvmalloc_node_noprof+0x1c7/0x660
Jun 03 08:48:30 testnode2 kernel:  __do_sys_add_key+0x16d/0x300
Jun 03 08:48:30 testnode2 kernel:  do_syscall_64+0x93/0x190
Jun 03 08:48:30 testnode2 kernel:  entry_SYSCALL_64_after_hwframe+0x76/0x7e
Jun 03 08:48:30 testnode2 kernel: 
Jun 03 08:48:30 testnode2 kernel: The buggy address belongs to the object at ffff888100a53000
                                   which belongs to the cache kmalloc-64 of size 64
Jun 03 08:48:30 testnode2 kernel: The buggy address is located 0 bytes to the right of
                                   allocated 59-byte region [ffff888100a53000, ffff888100a5303b)
Jun 03 08:48:30 testnode2 kernel: 
Jun 03 08:48:30 testnode2 kernel: The buggy address belongs to the physical page:
Jun 03 08:48:30 testnode2 kernel: page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x100a53
Jun 03 08:48:30 testnode2 kernel: ksm flags: 0x17ffffc0000000(node=0|zone=2|lastcpupid=0x1fffff)
Jun 03 08:48:30 testnode2 kernel: page_type: f5(slab)
Jun 03 08:48:30 testnode2 kernel: raw: 0017ffffc0000000 ffff8881000428c0 ffffea000445e340 dead000000000003
Jun 03 08:48:30 testnode2 kernel: raw: 0000000000000000 0000000080200020 00000000f5000000 0000000000000000
Jun 03 08:48:30 testnode2 kernel: page dumped because: kasan: bad access detected
Jun 03 08:48:30 testnode2 kernel: 
Jun 03 08:48:30 testnode2 kernel: Memory state around the buggy address:
Jun 03 08:48:30 testnode2 kernel:  ffff888100a52f00: 00 fc fc fc 00 fc fc fc 00 fc fc fc 00 fc fc fc
Jun 03 08:48:30 testnode2 kernel:  ffff888100a52f80: 00 fc fc fc 00 fc fc fc 00 fc fc fc 00 fc fc fc
Jun 03 08:48:30 testnode2 kernel: >ffff888100a53000: 00 00 00 00 00 00 00 03 fc fc fc fc fc fc fc fc
Jun 03 08:48:30 testnode2 kernel:                                         ^
Jun 03 08:48:30 testnode2 kernel:  ffff888100a53080: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
Jun 03 08:48:30 testnode2 kernel:  ffff888100a53100: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
Jun 03 08:48:30 testnode2 kernel: ==================================================================
Jun 03 08:48:30 testnode2 kernel: Disabling lock debugging due to kernel taint

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

* Re: [PATCH 2/9] nvme-keyring: add 'dhchap' key type
  2025-06-03  0:32   ` Shinichiro Kawasaki
@ 2025-06-03  6:11     ` Hannes Reinecke
  0 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-06-03  6:11 UTC (permalink / raw)
  To: Shinichiro Kawasaki, Hannes Reinecke
  Cc: hch, Sagi Grimberg, Keith Busch, linux-nvme@lists.infradead.org

On 6/3/25 02:32, Shinichiro Kawasaki wrote:
> On May 28, 2025 / 16:05, Hannes Reinecke wrote:
>> Add a 'dhchap' keytype to store DH-HMAC-CHAP secret keys.
>> Keys are stored with a 'user-type' compatible payload, such
>> that one can use 'user_read()' to access the raw contents
>> and the 'read()' callback to get the base64-encoded key
>> data in the DH-HMAC-CHAP secret representation.
>>
>> Signed-off-by: Hannes Reinecke <hare@kernel.org>
>> ---
>>   drivers/nvme/common/keyring.c | 266 ++++++++++++++++++++++++++++++++++
>>   include/linux/nvme-keyring.h  |  22 ++-
>>   2 files changed, 287 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
>> index 32d16c53133b..a58c93c6d495 100644
>> --- a/drivers/nvme/common/keyring.c
>> +++ b/drivers/nvme/common/keyring.c
> [...]
>> +static int nvme_dhchap_psk_preparse(struct key_preparsed_payload *prep)
>> +{
>> +	struct user_key_payload *upayload;
>> +	size_t datalen = prep->datalen, keylen;
>> +	int ret;
>> +	u32 crc;
>> +	u8 version, hmac;
>> +
>> +	if (!prep->data) {
>> +		pr_debug("%s: Empty data", __func__);
>> +		prep->payload.data[0] = NULL;
>> +		prep->quotalen = 0;
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (sscanf(prep->data, "DHHC-%01hhu:%02hhu:%*s", &version, &hmac) != 2) {
> 
> I built the kernel at nvme-6.16 branch at git hash bf4d87cba2d7 applying
> this series. Then I ran the blktests test case corresponding to this series,
> and observed the BUG KASAN slab-out-of-bounds below [1].
> 
> The sscanf() above has three "%"s in the conversion format string, while it has
> two following pointer arguments. I think this gap between the numbers of "%"s
> and the pointer arguments causes the BUG. I removed "%*s" from the format
> string, then the BUG looks disappearing.
> 
Well ... '%*s' translates as 'ignore following string argument'.
So there _might_ be an issue if there is no following string argument.
And actually this was meant to be a 'cheap' way of stripping and pending
newlines after the key.
Guess I'll rewrite that to actually strip the newline.
Thanks for testing.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich


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

* Re: [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring
  2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
                   ` (8 preceding siblings ...)
  2025-05-28 14:05 ` [PATCH 9/9] nvmet: add configfs attribute 'dhchap_keyring' Hannes Reinecke
@ 2025-07-03  9:39 ` Christoph Hellwig
  2025-07-03  9:43   ` Hannes Reinecke
  9 siblings, 1 reply; 14+ messages in thread
From: Christoph Hellwig @ 2025-07-03  9:39 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

Can I get some reviews for this series?



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

* Re: [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring
  2025-07-03  9:39 ` [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Christoph Hellwig
@ 2025-07-03  9:43   ` Hannes Reinecke
  0 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-07-03  9:43 UTC (permalink / raw)
  To: Christoph Hellwig, Hannes Reinecke; +Cc: Sagi Grimberg, Keith Busch, linux-nvme

On 7/3/25 11:39, Christoph Hellwig wrote:
> Can I get some reviews for this series?
> 
Was about to ask the same question ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich


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

end of thread, other threads:[~2025-07-03  9:45 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-28 14:05 [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Hannes Reinecke
2025-05-28 14:05 ` [PATCH 1/9] nvme-auth: modify nvme_auth_transform_key() to return status Hannes Reinecke
2025-05-28 14:05 ` [PATCH 2/9] nvme-keyring: add 'dhchap' key type Hannes Reinecke
2025-06-03  0:32   ` Shinichiro Kawasaki
2025-06-03  6:11     ` Hannes Reinecke
2025-05-28 14:05 ` [PATCH 3/9] nvme-auth: switch to use 'struct key' Hannes Reinecke
2025-05-28 14:05 ` [PATCH 4/9] nvme: parse dhchap keys during option parsing Hannes Reinecke
2025-05-28 14:05 ` [PATCH 5/9] nvmet-auth: parse dhchap key from configfs attribute Hannes Reinecke
2025-05-28 14:05 ` [PATCH 6/9] nvme: allow to pass in key description as dhchap secret Hannes Reinecke
2025-05-28 14:05 ` [PATCH 7/9] nvme-auth: wait for authentication to finish when changing keys Hannes Reinecke
2025-05-28 14:05 ` [PATCH 8/9] nvme-fabrics: allow to pass in keyring by name Hannes Reinecke
2025-05-28 14:05 ` [PATCH 9/9] nvmet: add configfs attribute 'dhchap_keyring' Hannes Reinecke
2025-07-03  9:39 ` [PATCHv2 0/9] nvme-auth: switch to use the kernel keyring Christoph Hellwig
2025-07-03  9:43   ` Hannes Reinecke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).