nvme: fixes for secure concatenation

PATCH Superseded Hannes Reinecke 3 participants 17 messages Jul 18, 2024 – Jul 19, 2024

Subsystems (3)

NVMe NVMe target NVM express fabrics authentication

lore

Share & tools

lore

Hannes Reinecke

author

2024-07-18 14:49 UTC #0 lore raw

v6 cover nvme: fixes for secure concatenation

Hi all,

here's a list of fixes split off from the secure concatenation patchset as they really are unrelated and just are assorted fixes to get things rolling.
The most important here is the first patch, which implements TP8018 to support the 'version 1' format for TLS PSK identifiers. And it also updates the sysfs interface to allow us to re-construct the nvme-cli commandline from sysfs attributes.

As usual, comments and reviews are welcome.

Hannes Reinecke (8):
nvme-keyring: restrict match length for version '1' identifiers
nvme-tcp: sanitize TLS key handling
nvme-tcp: check for invalidated or revoked key
nvme: add a newline to the 'tls_key' sysfs attribute
nvme-sysfs: add 'tls_configured_key' sysfs attribute
nvme-sysfs: add 'tls_keyring' attribute
nvmet-auth: allow to clear DH-HMAC-CHAP keys
nvme-target: do not check authentication status for admin commands
twice

 drivers/nvme/common/keyring.c   | 53 ++++++++++++++++++++++++++++-----
 drivers/nvme/host/core.c        |  1 -
 drivers/nvme/host/fabrics.c     |  2 +-
 drivers/nvme/host/nvme.h        |  2 +-
 drivers/nvme/host/sysfs.c       | 34 +++++++++++++++++++--
 drivers/nvme/host/tcp.c         | 49 +++++++++++++++++++++---------
 drivers/nvme/target/admin-cmd.c |  2 --
 drivers/nvme/target/auth.c      | 12 ++++++++
 include/linux/nvme-keyring.h    |  3 +-
 9 files changed, 129 insertions(+), 29 deletions(-)

Signature · 1 line

2.35.3

Hannes Reinecke

author

2024-07-18 14:49 UTC #1 lore raw

PATCH v6 1/8 nvme-keyring: restrict match length for version '1' identifiers

TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup identifiers we should just consider the length of the match value, not the length of the identifiers to compare against.
And we should modify the PSK lookup algorithm to prefer v1 identifiers as they can be uniquely identified.

Signed-off-by Hannes Reinecke <[email protected]>

Reviewed-by Sagi Grimberg <[email protected]>

---
 drivers/nvme/common/keyring.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

drivers/nvme/common/keyring.c +25 -7

index 6f7e7a8fa5ae..c60ebbdc52b8 100644
@@ -36,14 +36,12 @@ static bool nvme_tls_psk_match(const struct key *key,
 		pr_debug("%s: no key description\n", __func__);
 		return false;
 	}
-	match_len = strlen(key->description);
-	pr_debug("%s: id %s len %zd\n", __func__, key->description, match_len);
-
 	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);

@@ -71,7 +69,7 @@ static struct key_type nvme_tls_psk_key_type = {
 
 static struct key *nvme_tls_psk_lookup(struct key *keyring,
 		const char *hostnqn, const char *subnqn,
-		int hmac, bool generated)
+		u8 hmac, u8 psk_ver, bool generated)
 {
 	char *identity;
 	size_t identity_len = (NVMF_NQN_SIZE) * 2 + 11;

@@ -82,8 +80,8 @@ static struct key *nvme_tls_psk_lookup(struct key *keyring,
 	if (!identity)
 		return ERR_PTR(-ENOMEM);
 
-	snprintf(identity, identity_len, "NVMe0%c%02d %s %s",
-		 generated ? 'G' : 'R', hmac, hostnqn, subnqn);
+	snprintf(identity, identity_len, "NVMe%u%c%02u %s %s",
+		 psk_ver, generated ? 'G' : 'R', hmac, hostnqn, subnqn);
 
 	if (!keyring)
 		keyring = nvme_keyring;

@@ -109,19 +107,38 @@ static struct key *nvme_tls_psk_lookup(struct key *keyring,
  *
  * 'Retained' PSKs (ie 'generated == false')
  * should be preferred to 'generated' PSKs,
+ * PSKs with hash (psk_ver 1) should be
+ * preferred to PSKs without (psk_ver 0),
  * and SHA-384 should be preferred to SHA-256.
  */
 static struct nvme_tls_psk_priority_list {
 	bool generated;
+	u8 psk_ver;
 	enum nvme_tcp_tls_cipher cipher;
 } nvme_tls_psk_prio[] = {
 	{ .generated = false,
+	  .psk_ver = 1,
+	  .cipher = NVME_TCP_TLS_CIPHER_SHA384, },
+	{ .generated = false,
+	  .psk_ver = 1,
+	  .cipher = NVME_TCP_TLS_CIPHER_SHA256, },
+	{ .generated = false,
+	  .psk_ver = 0,
 	  .cipher = NVME_TCP_TLS_CIPHER_SHA384, },
 	{ .generated = false,
+	  .psk_ver = 0,
+	  .cipher = NVME_TCP_TLS_CIPHER_SHA256, },
+	{ .generated = true,
+	  .psk_ver = 1,
+	  .cipher = NVME_TCP_TLS_CIPHER_SHA384, },
+	{ .generated = true,
+	  .psk_ver = 1,
 	  .cipher = NVME_TCP_TLS_CIPHER_SHA256, },
 	{ .generated = true,
+	  .psk_ver = 0,
 	  .cipher = NVME_TCP_TLS_CIPHER_SHA384, },
 	{ .generated = true,
+	  .psk_ver = 0,
 	  .cipher = NVME_TCP_TLS_CIPHER_SHA256, },
 };

@@ -137,10 +154,11 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
 
 	for (prio = 0; prio < ARRAY_SIZE(nvme_tls_psk_prio); prio++) {
 		bool generated = nvme_tls_psk_prio[prio].generated;
+		u8 ver = nvme_tls_psk_prio[prio].psk_ver;
 		enum nvme_tcp_tls_cipher cipher = nvme_tls_psk_prio[prio].cipher;
 
 		tls_key = nvme_tls_psk_lookup(keyring, hostnqn, subnqn,
-					      cipher, generated);
+					      cipher, ver, generated);
 		if (!IS_ERR(tls_key)) {
 			tls_key_id = tls_key->serial;
 			key_put(tls_key);

Signature · 1 line

2.35.3

Christoph Hellwig

maintainer

2024-07-19 05:34 UTC #2 lore raw

Re: [PATCH 1/8] nvme-keyring: restrict match length for version '1' identifiers

Quoted 6 lines · Hannes Reinecke

TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup identifiers we should just consider the length of the match value, not the length of the identifiers to compare against.
And we should modify the PSK lookup algorithm to prefer v1 identifiers as they can be uniquely identified.

Can you reword this a bit to remove the weird "we should" and state it in terms of requirements / recommendations from the standard. Bonus points for adding actual references to the specifications.

drivers/nvme/common/keyring.c static struct key *nvme_tls_psk_lookup(struct key *keyring, · line 112

Quoted 7 lines

drivers/nvme/common/keyring.c +2 -0

@@ -109,19 +107,38 @@ static struct key *nvme_tls_psk_lookup(struct key *keyring,
  *
  * 'Retained' PSKs (ie 'generated == false')
  * should be preferred to 'generated' PSKs,
+ * PSKs with hash (psk_ver 1) should be
+ * preferred to PSKs without (psk_ver 0),
  * and SHA-384 should be preferred to SHA-256.

Please reflow this to use up 80 characters and make the paragraph easily readable.

Hannes Reinecke

maintainer

2024-07-19 06:16 UTC #3 lore raw

Re: [PATCH 1/8] nvme-keyring: restrict match length for version '1' identifiers

Quoted 8 lines · Christoph Hellwig

Quoted 6 lines · Hannes Reinecke
TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup identifiers we should just consider the length of the match value, not the length of the identifiers to compare against.
And we should modify the PSK lookup algorithm to prefer v1 identifiers as they can be uniquely identified.

Quoted 4 lines

Can you reword this a bit to remove the weird "we should" and state it in terms of requirements / recommendations from the standard. Bonus points for adding actual references to the specifications.

Okay.

Quoted 11 lines

drivers/nvme/common/keyring.c static struct key *nvme_tls_psk_lookup(struct key *keyring, · line 112
Quoted 7 lines
drivers/nvme/common/keyring.c +2 -0
@@ -109,19 +107,38 @@ static struct key *nvme_tls_psk_lookup(struct key *keyring,
   *
   * 'Retained' PSKs (ie 'generated == false')
   * should be preferred to 'generated' PSKs,
+ * PSKs with hash (psk_ver 1) should be
+ * preferred to PSKs without (psk_ver 0),
   * and SHA-384 should be preferred to SHA-256.
Please reflow this to use up 80 characters and make the paragraph easily readable.

Sure.

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +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

Hannes Reinecke

author

2024-07-18 14:49 UTC #4 lore raw

PATCH v6 2/8 nvme-tcp: sanitize TLS key handling

In reply to Hannes Reinecke

There is a difference between TLS configured (ie the user has provisioned/requested a key) and TLS enabled (ie the connection is encrypted with TLS). This becomes important for secure concatenation, where the initial authentication is run on an unencrypted connection (ie with TLS configured, but not enabled), and then the queue is reset to run over TLS (ie TLS configured _and_ enabled).
So to differentiate between those two states store the generated key in opts->tls_key (as we're using the same TLS key for all queues), the key serial of the resulting TLS handshake in ctrl->tls_pskid (to signal that TLS on the admin queue is enabled), and a simple flag for the queues to indicated that TLS has been enabled.

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/nvme/host/core.c  |  1 -
 drivers/nvme/host/nvme.h  |  2 +-
 drivers/nvme/host/sysfs.c |  4 ++--
 drivers/nvme/host/tcp.c   | 47 ++++++++++++++++++++++++++++-----------
 4 files changed, 37 insertions(+), 17 deletions(-)

drivers/nvme/host/core.c +0 -1

index e78ef31eeef0..663fa14162ab 100644
@@ -4675,7 +4675,6 @@ static void nvme_free_ctrl(struct device *dev)
 
 	if (!subsys || ctrl->instance != subsys->instance)
 		ida_free(&nvme_instance_ida, ctrl->instance);
-	key_put(ctrl->tls_key);
 	nvme_free_cels(ctrl);
 	nvme_mpath_uninit(ctrl);
 	cleanup_srcu_struct(&ctrl->srcu);

drivers/nvme/host/nvme.h +1 -1

index 9ac587e64166..185fb986a5cc 100644
@@ -373,7 +373,7 @@ struct nvme_ctrl {
 	struct nvme_dhchap_key *ctrl_key;
 	u16 transaction;
 #endif
-	struct key *tls_key;
+	key_serial_t tls_pskid;
 
 	/* Power saving configuration */
 	u64 ps_max_latency_us;

drivers/nvme/host/sysfs.c +2 -2

index ba05faaac562..72675b59a7a7 100644
@@ -670,9 +670,9 @@ static ssize_t tls_key_show(struct device *dev,
 {
 	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
 
-	if (!ctrl->tls_key)
+	if (!ctrl->tls_pskid)
 		return 0;
-	return sysfs_emit(buf, "%08x", key_serial(ctrl->tls_key));
+	return sysfs_emit(buf, "%08x", ctrl->tls_pskid);
 }
 static DEVICE_ATTR_RO(tls_key);
 #endif

drivers/nvme/host/tcp.c +34 ~~-13~~

123 lines not on this page · raw message

Signature · 1 line

2.35.3

Christoph Hellwig

maintainer Reviewed-by on v6 2/8

2024-07-19 05:35 UTC #5 lore raw

Re: [PATCH 2/8] nvme-tcp: sanitize TLS key handling

Looks good:

Reviewed-by Christoph Hellwig <[email protected]>

Hannes Reinecke

author

2024-07-18 14:49 UTC #6 lore raw

PATCH v6 3/8 nvme-tcp: check for invalidated or revoked key

In reply to Hannes Reinecke

key_lookup() will always return a key, even if that key is revoked or invalidated. So check for invalid keys before continuing.

Signed-off-by Hannes Reinecke <[email protected]>

Reviewed-by Sagi Grimberg <[email protected]>

---
 drivers/nvme/common/keyring.c | 21 +++++++++++++++++++++
 drivers/nvme/host/fabrics.c   |  2 +-
 drivers/nvme/host/tcp.c       |  2 +-
 include/linux/nvme-keyring.h  |  3 ++-
 4 files changed, 25 insertions(+), 3 deletions(-)

drivers/nvme/common/keyring.c +21 -0

index c60ebbdc52b8..5b886fd6c3e7 100644
@@ -20,6 +20,27 @@ key_serial_t nvme_keyring_id(void)
 }
 EXPORT_SYMBOL_GPL(nvme_keyring_id);
 
+static bool nvme_tls_psk_revoked(struct key *psk)
+{
+	return (test_bit(KEY_FLAG_REVOKED, &psk->flags) ||
+		test_bit(KEY_FLAG_INVALIDATED, &psk->flags));
+}
+
+struct key *nvme_tls_key_lookup(key_serial_t key_id)
+{
+	struct key *key = key_lookup(key_id);
+	if (IS_ERR(key)) {
+		pr_err("key id %08x not found\n", key_id);
+		return key;
+	}
+	if (nvme_tls_psk_revoked(key)) {
+		pr_err("key id %08x revoked\n", key_id);
+		return ERR_PTR(-EKEYREVOKED);
+	}
+	return key;
+}
+EXPORT_SYMBOL_GPL(nvme_tls_key_lookup);
+
 static void nvme_tls_psk_describe(const struct key *key, struct seq_file *m)
 {
 	seq_puts(m, key->description);

drivers/nvme/host/fabrics.c +1 -1

index f5f545fa0103..432efcbf9e2f 100644
@@ -665,7 +665,7 @@ static struct key *nvmf_parse_key(int key_id)
 		return ERR_PTR(-EINVAL);
 	}
 
-	key = key_lookup(key_id);
+	key = nvme_tls_key_lookup(key_id);
 	if (IS_ERR(key))
 		pr_err("key id %08x not found\n", key_id);
 	else

drivers/nvme/host/tcp.c +1 -1

index 92ad5b8cc1b4..5885aa452aa1 100644
@@ -1590,7 +1590,7 @@ static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid)
 		goto out_complete;
 	}
 
-	tls_key = key_lookup(pskid);
+	tls_key = nvme_tls_key_lookup(pskid);
 	if (IS_ERR(tls_key)) {
 		dev_warn(ctrl->ctrl.device, "queue %d: Invalid key %x\n",
 			 qid, pskid);

include/linux/nvme-keyring.h +2 -1

index e10333d78dbb..e092e3e6ffb2 100644
@@ -12,7 +12,7 @@ key_serial_t nvme_tls_psk_default(struct key *keyring,
 		const char *hostnqn, const char *subnqn);
 
 key_serial_t nvme_keyring_id(void);
-
+struct key *nvme_tls_key_lookup(key_serial_t key_id);
 #else
 
 static inline key_serial_t nvme_tls_psk_default(struct key *keyring,

@@ -24,5 +24,6 @@ static inline key_serial_t nvme_keyring_id(void)
 {
 	return 0;
 }
+static inline struct key *nvme_tls_key_lookup(key_serial_t key_id) { return ERR_PTR(-ENOTSUPP); }
 #endif /* !CONFIG_NVME_KEYRING */
 #endif /* _NVME_KEYRING_H */

Signature · 1 line

2.35.3

Christoph Hellwig

maintainer

2024-07-19 05:37 UTC #7 lore raw

Re: [PATCH 3/8] nvme-tcp: check for invalidated or revoked key

drivers/nvme/common/keyring.c key_serial_t nvme_keyring_id(void) · line 26

Quoted 2 lines

drivers/nvme/common/keyring.c +2 -0

+	return (test_bit(KEY_FLAG_REVOKED, &psk->flags) ||
+		test_bit(KEY_FLAG_INVALIDATED, &psk->flags));

No need for the outer braces.

drivers/nvme/common/keyring.c key_serial_t nvme_keyring_id(void) · line 31

Quoted 3 lines

drivers/nvme/common/keyring.c +3 -0

+struct key *nvme_tls_key_lookup(key_serial_t key_id)
+{
+	struct key *key = key_lookup(key_id);

Empty line after variable declarations, please.

Quoted 1 line

+static inline struct key *nvme_tls_key_lookup(key_serial_t key_id) { return ERR_PTR(-ENOTSUPP); }

Please avoid the overly lone line.

Hannes Reinecke

author

2024-07-18 14:49 UTC #8 lore raw

PATCH v6 4/8 nvme: add a newline to the 'tls_key' sysfs attribute

In reply to Hannes Reinecke

Print a newline for easier userspace handling.

Signed-off-by Hannes Reinecke <[email protected]>

Reviewed-by Sagi Grimberg <[email protected]>

---
 drivers/nvme/host/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/nvme/host/sysfs.c +1 -1

index 72675b59a7a7..c391ad6c5a88 100644
@@ -672,7 +672,7 @@ static ssize_t tls_key_show(struct device *dev,
 
 	if (!ctrl->tls_pskid)
 		return 0;
-	return sysfs_emit(buf, "%08x", ctrl->tls_pskid);
+	return sysfs_emit(buf, "%08x\n", ctrl->tls_pskid);
 }
 static DEVICE_ATTR_RO(tls_key);
 #endif

Signature · 1 line

2.35.3

Christoph Hellwig

maintainer Reviewed-by on v6 4/8

2024-07-19 05:37 UTC #9 lore raw

Re: [PATCH 4/8] nvme: add a newline to the 'tls_key' sysfs attribute

Looks good:

Reviewed-by Christoph Hellwig <[email protected]>

Hannes Reinecke

author

2024-07-18 14:49 UTC #10 lore raw

PATCH v6 5/8 nvme-sysfs: add 'tls_configured_key' sysfs attribute

In reply to Hannes Reinecke

There is a difference between the negotiated TLS key (which is always present for a TLS encrypted connection) and the configured TLS key (which is specified with the --tls_key command line option). To differentate between these two add a new sysfs attribute 'tls_configured_key' to hold the specified on the command line.

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/nvme/host/sysfs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

drivers/nvme/host/sysfs.c +15 -0

index c391ad6c5a88..62f03aa530c8 100644
@@ -675,6 +675,16 @@ static ssize_t tls_key_show(struct device *dev,
 	return sysfs_emit(buf, "%08x\n", ctrl->tls_pskid);
 }
 static DEVICE_ATTR_RO(tls_key);
+
+static ssize_t tls_configured_key_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+	struct key *key = ctrl->opts->tls_key;
+
+	return sysfs_emit(buf, "%08x\n", key_serial(key));
+}
+static DEVICE_ATTR_RO(tls_configured_key);
 #endif
 
 static struct attribute *nvme_dev_attrs[] = {

@@ -706,6 +716,7 @@ static struct attribute *nvme_dev_attrs[] = {
 #endif
 #ifdef CONFIG_NVME_TCP_TLS
 	&dev_attr_tls_key.attr,
+	&dev_attr_tls_configured_key.attr,
 #endif
 	&dev_attr_adm_passthru_err_log_enabled.attr,
 	NULL

@@ -741,6 +752,10 @@ static umode_t nvme_dev_attrs_are_visible(struct kobject *kobj,
 	if (a == &dev_attr_tls_key.attr &&
 	    (!ctrl->opts || strcmp(ctrl->opts->transport, "tcp")))
 		return 0;
+	if (a == &dev_attr_tls_configured_key.attr &&
+	    (!ctrl->opts || !ctrl->opts->tls_key ||
+	     strcmp(ctrl->opts->transport, "tcp")))
+		return 0;
 #endif
 
 	return a->mode;

Signature · 1 line

2.35.3

Christoph Hellwig

maintainer

2024-07-19 05:44 UTC #11 lore raw

Re: [PATCH 5/8] nvme-sysfs: add 'tls_configured_key' sysfs attribute

Quoted 7 lines

#ifdef CONFIG_NVME_TCP_TLS

drivers/nvme/host/sysfs.c +1 -0

 	&dev_attr_tls_key.attr,
+	&dev_attr_tls_configured_key.attr,
 #endif
 	&dev_attr_adm_passthru_err_log_enabled.attr,
 	NULL

drivers/nvme/host/sysfs.c static umode_t nvme_dev_attrs_are_visible(struct kobject *kobj, · line 759

Quoted 8 lines

drivers/nvme/host/sysfs.c +4 -0

@@ -741,6 +752,10 @@ static umode_t nvme_dev_attrs_are_visible(struct kobject *kobj,
 	if (a == &dev_attr_tls_key.attr &&
 	    (!ctrl->opts || strcmp(ctrl->opts->transport, "tcp")))
 		return 0;
+	if (a == &dev_attr_tls_configured_key.attr &&
+	    (!ctrl->opts || !ctrl->opts->tls_key ||
+	     strcmp(ctrl->opts->transport, "tcp")))
+		return 0;
 #endif

The check for a specific transport hack was ok for a single attribute, but it probably is time to have a separate attribute_group provided by the transport now. Or maybe wait until we get another one..

Quoted 4 lines

drivers/nvme/host/sysfs.c +0 -0
 	return a->mode;
Signature · 1 line
2.35.3

---end quoted text---

Hannes Reinecke

maintainer

2024-07-19 06:29 UTC #12 lore raw

Re: [PATCH 5/8] nvme-sysfs: add 'tls_configured_key' sysfs attribute

Quoted 11 lines · Christoph Hellwig

Quoted 11 lines

  #ifdef CONFIG_NVME_TCP_TLS
  	&dev_attr_tls_key.attr,

drivers/nvme/host/sysfs.c +2 -0

+	&dev_attr_tls_configured_key.attr,
  #endif
  	&dev_attr_adm_passthru_err_log_enabled.attr,
  	NULL

@@ -741,6 +752,10 @@ static umode_t nvme_dev_attrs_are_visible(struct kobject *kobj,
  	if (a == &dev_attr_tls_key.attr &&
  	    (!ctrl->opts || strcmp(ctrl->opts->transport, "tcp")))
  		return 0;
+	if (a == &dev_attr_tls_configured_key.attr &&

Quoted 9 lines

drivers/nvme/host/sysfs.c static umode_t nvme_dev_attrs_are_visible(struct kobject *kobj, · line 759
Quoted 4 lines
drivers/nvme/host/sysfs.c +3 -0
+	    (!ctrl->opts || !ctrl->opts->tls_key ||
+	     strcmp(ctrl->opts->transport, "tcp")))
+		return 0;
  #endif
The check for a specific transport hack was ok for a single attribute, but it probably is time to have a separate attribute_group provided by the transport now. Or maybe wait until we get another one..

Not a bad idea. Will be doing that for the next round.

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke                  Kernel Storage Architect
[email protected]                                +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

Hannes Reinecke

author

2024-07-18 14:49 UTC #13 lore raw

PATCH v6 6/8 nvme-sysfs: add 'tls_keyring' attribute

In reply to Hannes Reinecke

Add a 'tls_keyring' attribute to display the contents of the --keyring option from the connect string. Adding this attribute allows us to recreate the original connect string from sysfs settings.

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/nvme/host/sysfs.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

drivers/nvme/host/sysfs.c +15 -0

index 62f03aa530c8..7a7a4ade59db 100644
@@ -685,6 +685,16 @@ static ssize_t tls_configured_key_show(struct device *dev,
 	return sysfs_emit(buf, "%08x\n", key_serial(key));
 }
 static DEVICE_ATTR_RO(tls_configured_key);
+
+static ssize_t tls_keyring_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+	struct key *keyring = ctrl->opts->keyring;
+
+	return sysfs_emit(buf, "%s\n", keyring->description);
+}
+static DEVICE_ATTR_RO(tls_keyring);
 #endif
 
 static struct attribute *nvme_dev_attrs[] = {

@@ -717,6 +727,7 @@ static struct attribute *nvme_dev_attrs[] = {
 #ifdef CONFIG_NVME_TCP_TLS
 	&dev_attr_tls_key.attr,
 	&dev_attr_tls_configured_key.attr,
+	&dev_attr_tls_keyring.attr,
 #endif
 	&dev_attr_adm_passthru_err_log_enabled.attr,
 	NULL

@@ -756,6 +767,10 @@ static umode_t nvme_dev_attrs_are_visible(struct kobject *kobj,
 	    (!ctrl->opts || !ctrl->opts->tls_key ||
 	     strcmp(ctrl->opts->transport, "tcp")))
 		return 0;
+	if (a == &dev_attr_tls_keyring.attr &&
+	    (!ctrl->opts || !ctrl->opts->keyring ||
+	     strcmp(ctrl->opts->transport, "tcp")))
+		return 0;
 #endif
 
 	return a->mode;

Signature · 1 line

2.35.3

Hannes Reinecke

author

2024-07-18 14:49 UTC #14 lore raw

PATCH v6 7/8 nvmet-auth: allow to clear DH-HMAC-CHAP keys

In reply to Hannes Reinecke

As we can set DH-HMAC-CHAP keys, we should also be able to unset them.

Signed-off-by Hannes Reinecke <[email protected]>

Reviewed-by Sagi Grimberg <[email protected]>

---
 drivers/nvme/target/auth.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/nvme/target/auth.c +12 -0

index 8bc3f431c77f..7897d02c681d 100644
@@ -25,6 +25,18 @@ int nvmet_auth_set_key(struct nvmet_host *host, const char *secret,
 	unsigned char key_hash;
 	char *dhchap_secret;
 
+	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;
+		}
+		return 0;
+	}
 	if (sscanf(secret, "DHHC-1:%hhd:%*s", &key_hash) != 1)
 		return -EINVAL;
 	if (key_hash > 3) {

Signature · 1 line

2.35.3

Hannes Reinecke

author

2024-07-18 14:49 UTC #15 lore raw

PATCH v6 8/8 nvme-target: do not check authentication status for admin commands twice

In reply to Hannes Reinecke

nvmet_check_ctrl_status() checks the authentication status, so we don't need to do that prior to calling it.

Signed-off-by Hannes Reinecke <[email protected]>

Reviewed-by Sagi Grimberg <[email protected]>

---
 drivers/nvme/target/admin-cmd.c | 2 --
 1 file changed, 2 deletions(-)

drivers/nvme/target/admin-cmd.c +0 -2

index f7e1156ac7ec..d64480f01f4a 100644
@@ -1005,8 +1005,6 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 
 	if (nvme_is_fabrics(cmd))
 		return nvmet_parse_fabrics_admin_cmd(req);
-	if (unlikely(!nvmet_check_auth_status(req)))
-		return NVME_SC_AUTH_REQUIRED | NVME_STATUS_DNR;
 	if (nvmet_is_disc_subsys(nvmet_req_subsys(req)))
 		return nvmet_parse_discovery_cmd(req);

Signature · 1 line

2.35.3

Christoph Hellwig

maintainer Reviewed-by on v6 8/8

2024-07-19 05:45 UTC #16 lore raw

Re: [PATCH 8/8] nvme-target: do not check authentication status for admin commands twice

Looks good:

Reviewed-by Christoph Hellwig <[email protected]>