* [PATCH v3 0/4] Support PSK reauthentication (REPLACETLSPSK)
@ 2025-11-14 4:58 alistair23
2025-11-14 4:58 ` [PATCH v3 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset alistair23
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: alistair23 @ 2025-11-14 4:58 UTC (permalink / raw)
To: kbusch, axboe, hch, sagi, hare, kch, linux-nvme
Cc: linux-kernel, alistair23, Alistair Francis
From: Alistair Francis <alistair.francis@wdc.com>
Allow userspace on the host to trigger a reauth (REPLACETLSPSK) from
sysfs. This will replace the PSK for the admin queue when using
a secure concat connection.
This can be done by writing 0 to the `tls_configured_key` sysfs file,
for example something like this
```shell
echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key
```
`tls_configured_key` will only appear for concat connections as that is
all that is supported.
Reading `tls_configured_key` will return the current configured key, which
changes after each REPLACETLSPSK operation.
This series also include some fixes for the NVMe target code to ensure
this works against a Linux NVMe target.
v3:
- Only trigger if a 0 is written to `tls_configured_key`
- Add documentation
Alistair Francis (4):
nvmet-tcp: Don't error if TLS is enabed on a reset
nvmet-tcp: Don't free SQ on authentication success
nvme: Expose the tls_configured sysfs for secure concat connections
nvme: Allow reauth from sysfs
Documentation/ABI/testing/sysfs-nvme | 13 ++++++++++
drivers/nvme/host/sysfs.c | 36 ++++++++++++++++++++++++--
drivers/nvme/target/auth.c | 4 +--
drivers/nvme/target/core.c | 2 +-
drivers/nvme/target/fabrics-cmd-auth.c | 12 ++++-----
drivers/nvme/target/nvmet.h | 4 +--
6 files changed, 58 insertions(+), 13 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-nvme
--
2.51.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset
2025-11-14 4:58 [PATCH v3 0/4] Support PSK reauthentication (REPLACETLSPSK) alistair23
@ 2025-11-14 4:58 ` alistair23
2025-11-14 4:58 ` [PATCH v3 2/4] nvmet-tcp: Don't free SQ on authentication success alistair23
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: alistair23 @ 2025-11-14 4:58 UTC (permalink / raw)
To: kbusch, axboe, hch, sagi, hare, kch, linux-nvme
Cc: linux-kernel, alistair23, Alistair Francis, Wilfred Mallawa
From: Alistair Francis <alistair.francis@wdc.com>
If the host sends a AUTH_Negotiate Message on the admin queue with
REPLACETLSPSK set then we expect and require a TLS connection and
shouldn't report an error if TLS is enabled.
This change only enforces the nvmet_queue_tls_keyid() check if we aren't
resetting the negotiation.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
v3:
- No change
v2:
- Fixup long line
drivers/nvme/target/auth.c | 4 ++--
drivers/nvme/target/core.c | 2 +-
drivers/nvme/target/fabrics-cmd-auth.c | 3 ++-
drivers/nvme/target/nvmet.h | 4 ++--
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 300d5e032f6d..58d80fc72fda 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -140,7 +140,7 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id)
return ret;
}
-u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
+u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq, bool reset)
{
int ret = 0;
struct nvmet_host_link *p;
@@ -166,7 +166,7 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
goto out_unlock;
}
- if (nvmet_queue_tls_keyid(sq)) {
+ if (!reset && nvmet_queue_tls_keyid(sq)) {
pr_debug("host %s tls enabled\n", ctrl->hostnqn);
goto out_unlock;
}
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 5d7d483bfbe3..bd9746715ffc 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1689,7 +1689,7 @@ struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_alloc_ctrl_args *args)
if (args->hostid)
uuid_copy(&ctrl->hostid, args->hostid);
- dhchap_status = nvmet_setup_auth(ctrl, args->sq);
+ dhchap_status = nvmet_setup_auth(ctrl, args->sq, false);
if (dhchap_status) {
pr_err("Failed to setup authentication, dhchap status %u\n",
dhchap_status);
diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index 5946681cb0e3..2e828f7717ad 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -293,7 +293,8 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
pr_debug("%s: ctrl %d qid %d reset negotiation\n",
__func__, ctrl->cntlid, req->sq->qid);
if (!req->sq->qid) {
- dhchap_status = nvmet_setup_auth(ctrl, req->sq);
+ dhchap_status = nvmet_setup_auth(ctrl, req->sq,
+ true);
if (dhchap_status) {
pr_err("ctrl %d qid 0 failed to setup re-authentication\n",
ctrl->cntlid);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index f3b09f4099f0..20be2fe43307 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -896,7 +896,7 @@ void nvmet_execute_auth_receive(struct nvmet_req *req);
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);
-u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq);
+u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq, bool reset);
void nvmet_auth_sq_init(struct nvmet_sq *sq);
void nvmet_destroy_auth(struct nvmet_ctrl *ctrl);
void nvmet_auth_sq_free(struct nvmet_sq *sq);
@@ -917,7 +917,7 @@ int nvmet_auth_ctrl_sesskey(struct nvmet_req *req,
void nvmet_auth_insert_psk(struct nvmet_sq *sq);
#else
static inline u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl,
- struct nvmet_sq *sq)
+ struct nvmet_sq *sq, bool reset)
{
return 0;
}
--
2.51.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/4] nvmet-tcp: Don't free SQ on authentication success
2025-11-14 4:58 [PATCH v3 0/4] Support PSK reauthentication (REPLACETLSPSK) alistair23
2025-11-14 4:58 ` [PATCH v3 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset alistair23
@ 2025-11-14 4:58 ` alistair23
2025-11-18 0:41 ` Wilfred Mallawa
2025-11-14 4:58 ` [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections alistair23
2025-11-14 4:58 ` [PATCH v3 4/4] nvme: Allow reauth from sysfs alistair23
3 siblings, 1 reply; 14+ messages in thread
From: alistair23 @ 2025-11-14 4:58 UTC (permalink / raw)
To: kbusch, axboe, hch, sagi, hare, kch, linux-nvme
Cc: linux-kernel, alistair23, Alistair Francis
From: Alistair Francis <alistair.francis@wdc.com>
Curently after the host sends a REPLACETLSPSK we free the TLS keys as
part of calling nvmet_auth_sq_free() on success. This means when the
host sends a follow up REPLACETLSPSK we return CONCAT_MISMATCH as the
check for !nvmet_queue_tls_keyid(req->sq) fails.
This patch ensures we don't free the TLS key on success as we might need
it again in the future.
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
v3:
- No change
v2:
- Don't call nvmet_auth_sq_free() in nvmet_execute_auth_send() either
drivers/nvme/target/fabrics-cmd-auth.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index 2e828f7717ad..0cd722ebfa75 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -397,9 +397,10 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
goto complete;
}
/* Final states, clear up variables */
- nvmet_auth_sq_free(req->sq);
- if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE2)
+ if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE2) {
+ nvmet_auth_sq_free(req->sq);
nvmet_ctrl_fatal_error(ctrl);
+ }
complete:
nvmet_req_complete(req, status);
@@ -575,9 +576,7 @@ void nvmet_execute_auth_receive(struct nvmet_req *req)
status = nvmet_copy_to_sgl(req, 0, d, al);
kfree(d);
done:
- if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2)
- nvmet_auth_sq_free(req->sq);
- else if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE1) {
+ if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE1) {
nvmet_auth_sq_free(req->sq);
nvmet_ctrl_fatal_error(ctrl);
}
--
2.51.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections
2025-11-14 4:58 [PATCH v3 0/4] Support PSK reauthentication (REPLACETLSPSK) alistair23
2025-11-14 4:58 ` [PATCH v3 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset alistair23
2025-11-14 4:58 ` [PATCH v3 2/4] nvmet-tcp: Don't free SQ on authentication success alistair23
@ 2025-11-14 4:58 ` alistair23
2025-11-14 7:00 ` Hannes Reinecke
2025-11-18 0:41 ` Wilfred Mallawa
2025-11-14 4:58 ` [PATCH v3 4/4] nvme: Allow reauth from sysfs alistair23
3 siblings, 2 replies; 14+ messages in thread
From: alistair23 @ 2025-11-14 4:58 UTC (permalink / raw)
To: kbusch, axboe, hch, sagi, hare, kch, linux-nvme
Cc: linux-kernel, alistair23, Alistair Francis
From: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
v3:
- No change
v2:
- New patch
drivers/nvme/host/sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 29430949ce2f..6d10e12136d0 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -838,7 +838,7 @@ static umode_t nvme_tls_attrs_are_visible(struct kobject *kobj,
!ctrl->opts->tls && !ctrl->opts->concat)
return 0;
if (a == &dev_attr_tls_configured_key.attr &&
- (!ctrl->opts->tls_key || ctrl->opts->concat))
+ !ctrl->opts->concat)
return 0;
if (a == &dev_attr_tls_keyring.attr &&
!ctrl->opts->keyring)
--
2.51.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/4] nvme: Allow reauth from sysfs
2025-11-14 4:58 [PATCH v3 0/4] Support PSK reauthentication (REPLACETLSPSK) alistair23
` (2 preceding siblings ...)
2025-11-14 4:58 ` [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections alistair23
@ 2025-11-14 4:58 ` alistair23
2025-11-14 7:15 ` Hannes Reinecke
3 siblings, 1 reply; 14+ messages in thread
From: alistair23 @ 2025-11-14 4:58 UTC (permalink / raw)
To: kbusch, axboe, hch, sagi, hare, kch, linux-nvme
Cc: linux-kernel, alistair23, Alistair Francis
From: Alistair Francis <alistair.francis@wdc.com>
Allow userspace to trigger a reauth (REPLACETLSPSK) from sysfs.
This can be done by writing a zero to the sysfs file.
echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
v3:
- Only trigger if a 0 is written to `tls_configured_key`
- Add documentation
v2:
- Trigger on any value written to `tls_configured_key`
Documentation/ABI/testing/sysfs-nvme | 13 +++++++++++
drivers/nvme/host/sysfs.c | 34 +++++++++++++++++++++++++++-
2 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/sysfs-nvme
diff --git a/Documentation/ABI/testing/sysfs-nvme b/Documentation/ABI/testing/sysfs-nvme
new file mode 100644
index 000000000000..16aaf0dca9e2
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-nvme
@@ -0,0 +1,13 @@
+What: /sys/devices/virtual/nvme-fabrics/ctl/.../tls_configured_key
+Date: November 2025
+KernelVersion: 6.19
+Contact: Linux NVMe mailing list <linux-nvme@lists.infradead.org>
+Description:
+ The file is avaliable when using a secure concatanation
+ connection to a NVMe taget. Reading the file will return
+ the serial of the currently negotiated key.
+
+ Writing 0 to the file will trigger a PSK reauthentication
+ (REPLACETLSPSK) with the target. After a reauthentication
+ the value returned by tls_configured_key will be the new
+ serial.
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 6d10e12136d0..7ff9a5053c3f 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -806,7 +806,39 @@ 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_configured_key_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+ int error, qid;
+
+ error = kstrtoint(buf, 10, &qid);
+ if (error)
+ return error;
+
+ /*
+ * We currently only allow userspace to write a `0` indicating
+ * generate a new key.
+ */
+ if (!qid)
+ return -EINVAL;
+
+ if (!ctrl->opts || !ctrl->opts->concat)
+ return -EOPNOTSUPP;
+
+ error = nvme_auth_negotiate(ctrl, 0);
+ if (error < 0)
+ return error;
+
+ error = nvme_auth_wait(ctrl, 0);
+ if (error < 0)
+ return error;
+
+ return count;
+}
+static DEVICE_ATTR_RW(tls_configured_key);
static ssize_t tls_keyring_show(struct device *dev,
struct device_attribute *attr, char *buf)
--
2.51.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections
2025-11-14 4:58 ` [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections alistair23
@ 2025-11-14 7:00 ` Hannes Reinecke
2025-11-18 0:41 ` Wilfred Mallawa
1 sibling, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2025-11-14 7:00 UTC (permalink / raw)
To: alistair23, kbusch, axboe, hch, sagi, kch, linux-nvme
Cc: linux-kernel, Alistair Francis
On 11/14/25 05:58, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> v3:
> - No change
> v2:
> - New patch
>
> drivers/nvme/host/sysfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 29430949ce2f..6d10e12136d0 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -838,7 +838,7 @@ static umode_t nvme_tls_attrs_are_visible(struct kobject *kobj,
> !ctrl->opts->tls && !ctrl->opts->concat)
> return 0;
> if (a == &dev_attr_tls_configured_key.attr &&
> - (!ctrl->opts->tls_key || ctrl->opts->concat))
> + !ctrl->opts->concat)
> return 0;
> if (a == &dev_attr_tls_keyring.attr &&
> !ctrl->opts->keyring)
Reviewed-by: Hannes Reinecke <hare@suse.de>
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: [PATCH v3 4/4] nvme: Allow reauth from sysfs
2025-11-14 4:58 ` [PATCH v3 4/4] nvme: Allow reauth from sysfs alistair23
@ 2025-11-14 7:15 ` Hannes Reinecke
2025-11-18 0:52 ` Alistair Francis
0 siblings, 1 reply; 14+ messages in thread
From: Hannes Reinecke @ 2025-11-14 7:15 UTC (permalink / raw)
To: alistair23, kbusch, axboe, hch, sagi, kch, linux-nvme
Cc: linux-kernel, Alistair Francis
On 11/14/25 05:58, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Allow userspace to trigger a reauth (REPLACETLSPSK) from sysfs.
> This can be done by writing a zero to the sysfs file.
>
> echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> v3:
> - Only trigger if a 0 is written to `tls_configured_key`
> - Add documentation
> v2:
> - Trigger on any value written to `tls_configured_key`
>
> Documentation/ABI/testing/sysfs-nvme | 13 +++++++++++
> drivers/nvme/host/sysfs.c | 34 +++++++++++++++++++++++++++-
> 2 files changed, 46 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/ABI/testing/sysfs-nvme
>
> diff --git a/Documentation/ABI/testing/sysfs-nvme b/Documentation/ABI/testing/sysfs-nvme
> new file mode 100644
> index 000000000000..16aaf0dca9e2
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-nvme
> @@ -0,0 +1,13 @@
> +What: /sys/devices/virtual/nvme-fabrics/ctl/.../tls_configured_key
> +Date: November 2025
> +KernelVersion: 6.19
> +Contact: Linux NVMe mailing list <linux-nvme@lists.infradead.org>
> +Description:
> + The file is avaliable when using a secure concatanation
> + connection to a NVMe taget. Reading the file will return
> + the serial of the currently negotiated key.
> +
> + Writing 0 to the file will trigger a PSK reauthentication
> + (REPLACETLSPSK) with the target. After a reauthentication
> + the value returned by tls_configured_key will be the new
> + serial.
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 6d10e12136d0..7ff9a5053c3f 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -806,7 +806,39 @@ 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_configured_key_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> + int error, qid;
> +
> + error = kstrtoint(buf, 10, &qid);
> + if (error)
> + return error;
> +
> + /*
> + * We currently only allow userspace to write a `0` indicating
> + * generate a new key.
> + */
> + if (!qid)
> + return -EINVAL;
> +
> + if (!ctrl->opts || !ctrl->opts->concat)
> + return -EOPNOTSUPP;
> +
> + error = nvme_auth_negotiate(ctrl, 0);
> + if (error < 0)
> + return error;
> +
> + error = nvme_auth_wait(ctrl, 0);
> + if (error < 0)
> + return error;
> +
> + return count;
> +}
> +static DEVICE_ATTR_RW(tls_configured_key);
>
> static ssize_t tls_keyring_show(struct device *dev,
> struct device_attribute *attr, char *buf)
Hmm.
Now we are just running (re-) authentication, but that does
not affect the TLS connection (which continues to use the
original key). So you would need to reset the connection
here to re-establish a new TLS connection.
Also: if the authentication fails you need to tear down
the connection (cf NVMe Base Spec 2.1, section 8.3.4.5.1
Protocol Operations).
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: [PATCH v3 2/4] nvmet-tcp: Don't free SQ on authentication success
2025-11-14 4:58 ` [PATCH v3 2/4] nvmet-tcp: Don't free SQ on authentication success alistair23
@ 2025-11-18 0:41 ` Wilfred Mallawa
0 siblings, 0 replies; 14+ messages in thread
From: Wilfred Mallawa @ 2025-11-18 0:41 UTC (permalink / raw)
To: alistair23, kbusch, axboe, hch, sagi, hare, kch, linux-nvme
Cc: linux-kernel, Alistair Francis
On Fri, 2025-11-14 at 14:58 +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Curently after the host sends a REPLACETLSPSK we free the TLS keys as
> part of calling nvmet_auth_sq_free() on success. This means when the
> host sends a follow up REPLACETLSPSK we return CONCAT_MISMATCH as the
> check for !nvmet_queue_tls_keyid(req->sq) fails.
>
> This patch ensures we don't free the TLS key on success as we might
> need
> it again in the future.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> ---
> v3:
> - No change
> v2:
> - Don't call nvmet_auth_sq_free() in nvmet_execute_auth_send()
> either
>
> drivers/nvme/target/fabrics-cmd-auth.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c
> b/drivers/nvme/target/fabrics-cmd-auth.c
> index 2e828f7717ad..0cd722ebfa75 100644
> --- a/drivers/nvme/target/fabrics-cmd-auth.c
> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
> @@ -397,9 +397,10 @@ void nvmet_execute_auth_send(struct nvmet_req
> *req)
> goto complete;
> }
> /* Final states, clear up variables */
> - nvmet_auth_sq_free(req->sq);
> - if (req->sq->dhchap_step ==
> NVME_AUTH_DHCHAP_MESSAGE_FAILURE2)
> + if (req->sq->dhchap_step ==
> NVME_AUTH_DHCHAP_MESSAGE_FAILURE2) {
> + nvmet_auth_sq_free(req->sq);
> nvmet_ctrl_fatal_error(ctrl);
> + }
>
> complete:
> nvmet_req_complete(req, status);
> @@ -575,9 +576,7 @@ void nvmet_execute_auth_receive(struct nvmet_req
> *req)
> status = nvmet_copy_to_sgl(req, 0, d, al);
> kfree(d);
> done:
> - if (req->sq->dhchap_step ==
> NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2)
> - nvmet_auth_sq_free(req->sq);
> - else if (req->sq->dhchap_step ==
> NVME_AUTH_DHCHAP_MESSAGE_FAILURE1) {
> + if (req->sq->dhchap_step ==
> NVME_AUTH_DHCHAP_MESSAGE_FAILURE1) {
> nvmet_auth_sq_free(req->sq);
> nvmet_ctrl_fatal_error(ctrl);
> }
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Wilfred
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections
2025-11-14 4:58 ` [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections alistair23
2025-11-14 7:00 ` Hannes Reinecke
@ 2025-11-18 0:41 ` Wilfred Mallawa
1 sibling, 0 replies; 14+ messages in thread
From: Wilfred Mallawa @ 2025-11-18 0:41 UTC (permalink / raw)
To: alistair23, kbusch, axboe, hch, sagi, hare, kch, linux-nvme
Cc: linux-kernel, Alistair Francis
On Fri, 2025-11-14 at 14:58 +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> v3:
> - No change
> v2:
> - New patch
>
> drivers/nvme/host/sysfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 29430949ce2f..6d10e12136d0 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -838,7 +838,7 @@ static umode_t nvme_tls_attrs_are_visible(struct
> kobject *kobj,
> !ctrl->opts->tls && !ctrl->opts->concat)
> return 0;
> if (a == &dev_attr_tls_configured_key.attr &&
> - (!ctrl->opts->tls_key || ctrl->opts->concat))
> + !ctrl->opts->concat)
> return 0;
> if (a == &dev_attr_tls_keyring.attr &&
> !ctrl->opts->keyring)
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Wilfred
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/4] nvme: Allow reauth from sysfs
2025-11-14 7:15 ` Hannes Reinecke
@ 2025-11-18 0:52 ` Alistair Francis
2025-11-18 11:50 ` Hannes Reinecke
0 siblings, 1 reply; 14+ messages in thread
From: Alistair Francis @ 2025-11-18 0:52 UTC (permalink / raw)
To: Hannes Reinecke
Cc: kbusch, axboe, hch, sagi, kch, linux-nvme, linux-kernel,
Alistair Francis
On Fri, Nov 14, 2025 at 5:15 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 11/14/25 05:58, alistair23@gmail.com wrote:
> > From: Alistair Francis <alistair.francis@wdc.com>
> >
> > Allow userspace to trigger a reauth (REPLACETLSPSK) from sysfs.
> > This can be done by writing a zero to the sysfs file.
> >
> > echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> > v3:
> > - Only trigger if a 0 is written to `tls_configured_key`
> > - Add documentation
> > v2:
> > - Trigger on any value written to `tls_configured_key`
> >
> > Documentation/ABI/testing/sysfs-nvme | 13 +++++++++++
> > drivers/nvme/host/sysfs.c | 34 +++++++++++++++++++++++++++-
> > 2 files changed, 46 insertions(+), 1 deletion(-)
> > create mode 100644 Documentation/ABI/testing/sysfs-nvme
> >
> > diff --git a/Documentation/ABI/testing/sysfs-nvme b/Documentation/ABI/testing/sysfs-nvme
> > new file mode 100644
> > index 000000000000..16aaf0dca9e2
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-nvme
> > @@ -0,0 +1,13 @@
> > +What: /sys/devices/virtual/nvme-fabrics/ctl/.../tls_configured_key
> > +Date: November 2025
> > +KernelVersion: 6.19
> > +Contact: Linux NVMe mailing list <linux-nvme@lists.infradead.org>
> > +Description:
> > + The file is avaliable when using a secure concatanation
> > + connection to a NVMe taget. Reading the file will return
> > + the serial of the currently negotiated key.
> > +
> > + Writing 0 to the file will trigger a PSK reauthentication
> > + (REPLACETLSPSK) with the target. After a reauthentication
> > + the value returned by tls_configured_key will be the new
> > + serial.
> > diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> > index 6d10e12136d0..7ff9a5053c3f 100644
> > --- a/drivers/nvme/host/sysfs.c
> > +++ b/drivers/nvme/host/sysfs.c
> > @@ -806,7 +806,39 @@ 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_configured_key_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> > + int error, qid;
> > +
> > + error = kstrtoint(buf, 10, &qid);
> > + if (error)
> > + return error;
> > +
> > + /*
> > + * We currently only allow userspace to write a `0` indicating
> > + * generate a new key.
> > + */
> > + if (!qid)
> > + return -EINVAL;
> > +
> > + if (!ctrl->opts || !ctrl->opts->concat)
> > + return -EOPNOTSUPP;
> > +
> > + error = nvme_auth_negotiate(ctrl, 0);
> > + if (error < 0)
> > + return error;
> > +
> > + error = nvme_auth_wait(ctrl, 0);
> > + if (error < 0)
> > + return error;
> > +
> > + return count;
> > +}
> > +static DEVICE_ATTR_RW(tls_configured_key);
> >
> > static ssize_t tls_keyring_show(struct device *dev,
> > struct device_attribute *attr, char *buf)
>
> Hmm.
> Now we are just running (re-) authentication, but that does
> not affect the TLS connection (which continues to use the
> original key). So you would need to reset the connection
> here to re-establish a new TLS connection.
Is the connection supposed to be reset? I don't see any mention of
that in the spec
>
> Also: if the authentication fails you need to tear down
> the connection (cf NVMe Base Spec 2.1, section 8.3.4.5.1
> Protocol Operations).
Ah good point
Alistair
>
> 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: [PATCH v3 4/4] nvme: Allow reauth from sysfs
2025-11-18 0:52 ` Alistair Francis
@ 2025-11-18 11:50 ` Hannes Reinecke
2025-11-19 0:24 ` Alistair Francis
0 siblings, 1 reply; 14+ messages in thread
From: Hannes Reinecke @ 2025-11-18 11:50 UTC (permalink / raw)
To: Alistair Francis
Cc: kbusch, axboe, hch, sagi, kch, linux-nvme, linux-kernel,
Alistair Francis
On 11/18/25 01:52, Alistair Francis wrote:
> On Fri, Nov 14, 2025 at 5:15 PM Hannes Reinecke <hare@suse.de> wrote:
>>
>> On 11/14/25 05:58, alistair23@gmail.com wrote:
>>> From: Alistair Francis <alistair.francis@wdc.com>
>>>
>>> Allow userspace to trigger a reauth (REPLACETLSPSK) from sysfs.
>>> This can be done by writing a zero to the sysfs file.
>>>
>>> echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>> v3:
>>> - Only trigger if a 0 is written to `tls_configured_key`
>>> - Add documentation
>>> v2:
>>> - Trigger on any value written to `tls_configured_key`
>>>
>>> Documentation/ABI/testing/sysfs-nvme | 13 +++++++++++
>>> drivers/nvme/host/sysfs.c | 34 +++++++++++++++++++++++++++-
>>> 2 files changed, 46 insertions(+), 1 deletion(-)
>>> create mode 100644 Documentation/ABI/testing/sysfs-nvme
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-nvme b/Documentation/ABI/testing/sysfs-nvme
>>> new file mode 100644
>>> index 000000000000..16aaf0dca9e2
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-nvme
>>> @@ -0,0 +1,13 @@
>>> +What: /sys/devices/virtual/nvme-fabrics/ctl/.../tls_configured_key
>>> +Date: November 2025
>>> +KernelVersion: 6.19
>>> +Contact: Linux NVMe mailing list <linux-nvme@lists.infradead.org>
>>> +Description:
>>> + The file is avaliable when using a secure concatanation
>>> + connection to a NVMe taget. Reading the file will return
>>> + the serial of the currently negotiated key.
>>> +
>>> + Writing 0 to the file will trigger a PSK reauthentication
>>> + (REPLACETLSPSK) with the target. After a reauthentication
>>> + the value returned by tls_configured_key will be the new
>>> + serial.
>>> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
>>> index 6d10e12136d0..7ff9a5053c3f 100644
>>> --- a/drivers/nvme/host/sysfs.c
>>> +++ b/drivers/nvme/host/sysfs.c
>>> @@ -806,7 +806,39 @@ 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_configured_key_store(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>>> + int error, qid;
>>> +
>>> + error = kstrtoint(buf, 10, &qid);
>>> + if (error)
>>> + return error;
>>> +
>>> + /*
>>> + * We currently only allow userspace to write a `0` indicating
>>> + * generate a new key.
>>> + */
>>> + if (!qid)
>>> + return -EINVAL;
>>> +
>>> + if (!ctrl->opts || !ctrl->opts->concat)
>>> + return -EOPNOTSUPP;
>>> +
>>> + error = nvme_auth_negotiate(ctrl, 0);
>>> + if (error < 0)
>>> + return error;
>>> +
>>> + error = nvme_auth_wait(ctrl, 0);
>>> + if (error < 0)
>>> + return error;
>>> +
>>> + return count;
>>> +}
>>> +static DEVICE_ATTR_RW(tls_configured_key);
>>>
>>> static ssize_t tls_keyring_show(struct device *dev,
>>> struct device_attribute *attr, char *buf)
>>
>> Hmm.
>> Now we are just running (re-) authentication, but that does
>> not affect the TLS connection (which continues to use the
>> original key). So you would need to reset the connection
>> here to re-establish a new TLS connection.
>
> Is the connection supposed to be reset? I don't see any mention of
> that in the spec
>
Yeah, that's a bit hard to read (as usual).
The base spec just claims (Fig. 733, Secure Channel Protocol Identifiers):
03h: This {PSK, PSK Identity} pair replaces the {PSK, PSK Identity}
pair that was used to set up the TLS secure channel over which the
authentication transaction is performed.
So from that your implementation is correct, as it just replaces the
PSK (without actually using them). However, the TCP spec clarifies
(section 3.6.1.4: PSK Use):
Once the TLS secure channel for the Admin Queue of an association
has been set up with a generated {PSK, PSK Identity} pair, that
generated {PSK, PSK Identity} pair should be replaced periodically
(e.g., every hour) or on demand by performing a reauthentication
with the SC_C field in the AUTH_Negotiate message set to REPLACETLSPSK
(refer to the AUTH_Negotiate Message section of the NVM Express
Base Specification) over the Admin Queue of that association. The most
recently generated PSK, if any, is the generated PSK associated with
that Admin Queue.
And the only way to associate a PSK with the admin queue is to use
it for the TLS encryption, ie re-run the TLS handshake.
Or indeed use the KeyUpdate mechanism.
But I'll ask the FMDS group for clarification.
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: [PATCH v3 4/4] nvme: Allow reauth from sysfs
2025-11-18 11:50 ` Hannes Reinecke
@ 2025-11-19 0:24 ` Alistair Francis
2025-11-19 7:45 ` Hannes Reinecke
0 siblings, 1 reply; 14+ messages in thread
From: Alistair Francis @ 2025-11-19 0:24 UTC (permalink / raw)
To: Hannes Reinecke
Cc: kbusch, axboe, hch, sagi, kch, linux-nvme, linux-kernel,
Alistair Francis
On Tue, Nov 18, 2025 at 9:50 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 11/18/25 01:52, Alistair Francis wrote:
> > On Fri, Nov 14, 2025 at 5:15 PM Hannes Reinecke <hare@suse.de> wrote:
> >>
> >> On 11/14/25 05:58, alistair23@gmail.com wrote:
> >>> From: Alistair Francis <alistair.francis@wdc.com>
> >>>
> >>> Allow userspace to trigger a reauth (REPLACETLSPSK) from sysfs.
> >>> This can be done by writing a zero to the sysfs file.
> >>>
> >>> echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key
> >>>
> >>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >>> ---
> >>> v3:
> >>> - Only trigger if a 0 is written to `tls_configured_key`
> >>> - Add documentation
> >>> v2:
> >>> - Trigger on any value written to `tls_configured_key`
> >>>
> >>> Documentation/ABI/testing/sysfs-nvme | 13 +++++++++++
> >>> drivers/nvme/host/sysfs.c | 34 +++++++++++++++++++++++++++-
> >>> 2 files changed, 46 insertions(+), 1 deletion(-)
> >>> create mode 100644 Documentation/ABI/testing/sysfs-nvme
> >>>
> >>> diff --git a/Documentation/ABI/testing/sysfs-nvme b/Documentation/ABI/testing/sysfs-nvme
> >>> new file mode 100644
> >>> index 000000000000..16aaf0dca9e2
> >>> --- /dev/null
> >>> +++ b/Documentation/ABI/testing/sysfs-nvme
> >>> @@ -0,0 +1,13 @@
> >>> +What: /sys/devices/virtual/nvme-fabrics/ctl/.../tls_configured_key
> >>> +Date: November 2025
> >>> +KernelVersion: 6.19
> >>> +Contact: Linux NVMe mailing list <linux-nvme@lists.infradead.org>
> >>> +Description:
> >>> + The file is avaliable when using a secure concatanation
> >>> + connection to a NVMe taget. Reading the file will return
> >>> + the serial of the currently negotiated key.
> >>> +
> >>> + Writing 0 to the file will trigger a PSK reauthentication
> >>> + (REPLACETLSPSK) with the target. After a reauthentication
> >>> + the value returned by tls_configured_key will be the new
> >>> + serial.
> >>> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> >>> index 6d10e12136d0..7ff9a5053c3f 100644
> >>> --- a/drivers/nvme/host/sysfs.c
> >>> +++ b/drivers/nvme/host/sysfs.c
> >>> @@ -806,7 +806,39 @@ 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_configured_key_store(struct device *dev,
> >>> + struct device_attribute *attr,
> >>> + const char *buf, size_t count)
> >>> +{
> >>> + struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> >>> + int error, qid;
> >>> +
> >>> + error = kstrtoint(buf, 10, &qid);
> >>> + if (error)
> >>> + return error;
> >>> +
> >>> + /*
> >>> + * We currently only allow userspace to write a `0` indicating
> >>> + * generate a new key.
> >>> + */
> >>> + if (!qid)
> >>> + return -EINVAL;
> >>> +
> >>> + if (!ctrl->opts || !ctrl->opts->concat)
> >>> + return -EOPNOTSUPP;
> >>> +
> >>> + error = nvme_auth_negotiate(ctrl, 0);
> >>> + if (error < 0)
> >>> + return error;
> >>> +
> >>> + error = nvme_auth_wait(ctrl, 0);
> >>> + if (error < 0)
> >>> + return error;
> >>> +
> >>> + return count;
> >>> +}
> >>> +static DEVICE_ATTR_RW(tls_configured_key);
> >>>
> >>> static ssize_t tls_keyring_show(struct device *dev,
> >>> struct device_attribute *attr, char *buf)
> >>
> >> Hmm.
> >> Now we are just running (re-) authentication, but that does
> >> not affect the TLS connection (which continues to use the
> >> original key). So you would need to reset the connection
> >> here to re-establish a new TLS connection.
> >
> > Is the connection supposed to be reset? I don't see any mention of
> > that in the spec
> >
> Yeah, that's a bit hard to read (as usual).
> The base spec just claims (Fig. 733, Secure Channel Protocol Identifiers):
>
> 03h: This {PSK, PSK Identity} pair replaces the {PSK, PSK Identity}
> pair that was used to set up the TLS secure channel over which the
> authentication transaction is performed.
>
> So from that your implementation is correct, as it just replaces the
> PSK (without actually using them). However, the TCP spec clarifies
> (section 3.6.1.4: PSK Use):
>
> Once the TLS secure channel for the Admin Queue of an association
> has been set up with a generated {PSK, PSK Identity} pair, that
> generated {PSK, PSK Identity} pair should be replaced periodically
> (e.g., every hour) or on demand by performing a reauthentication
> with the SC_C field in the AUTH_Negotiate message set to REPLACETLSPSK
> (refer to the AUTH_Negotiate Message section of the NVM Express
> Base Specification) over the Admin Queue of that association. The most
> recently generated PSK, if any, is the generated PSK associated with
> that Admin Queue.
Yeah, to me "associated with" doesn't necessarily mean that we reset
the connection to use the new PSK.
>
> And the only way to associate a PSK with the admin queue is to use
> it for the TLS encryption, ie re-run the TLS handshake.
>
> Or indeed use the KeyUpdate mechanism.
This is the part that I think is weird.
If we do need to use the new PSK, once a host issues a REPLACETLSPSK
we have to tear down and restart the TLS connection. Which seems
really clunky and the spec doesn't seem to mention that at all.
If the host wants to replace the TLS keys it can just issue a
KeyUpdate, which doesn't involve tearing down the entire connection.
So do we actually need to reset the TLS connection after a
REPLACETLSPSK?
>
> But I'll ask the FMDS group for clarification.
Thanks!
Alistair
>
> 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: [PATCH v3 4/4] nvme: Allow reauth from sysfs
2025-11-19 0:24 ` Alistair Francis
@ 2025-11-19 7:45 ` Hannes Reinecke
2025-11-19 10:21 ` Alistair Francis
0 siblings, 1 reply; 14+ messages in thread
From: Hannes Reinecke @ 2025-11-19 7:45 UTC (permalink / raw)
To: Alistair Francis
Cc: kbusch, axboe, hch, sagi, kch, linux-nvme, linux-kernel,
Alistair Francis
On 11/19/25 01:24, Alistair Francis wrote:
> On Tue, Nov 18, 2025 at 9:50 PM Hannes Reinecke <hare@suse.de> wrote:
>>
>> On 11/18/25 01:52, Alistair Francis wrote:
>>> On Fri, Nov 14, 2025 at 5:15 PM Hannes Reinecke <hare@suse.de> wrote:
[ .. ]>>>>
>>>> Hmm.
>>>> Now we are just running (re-) authentication, but that does
>>>> not affect the TLS connection (which continues to use the
>>>> original key). So you would need to reset the connection
>>>> here to re-establish a new TLS connection.
>>>
>>> Is the connection supposed to be reset? I don't see any mention of
>>> that in the spec
>>>
>> Yeah, that's a bit hard to read (as usual).
>> The base spec just claims (Fig. 733, Secure Channel Protocol Identifiers):
>>
>> 03h: This {PSK, PSK Identity} pair replaces the {PSK, PSK Identity}
>> pair that was used to set up the TLS secure channel over which the
>> authentication transaction is performed.
>>
>> So from that your implementation is correct, as it just replaces the
>> PSK (without actually using them). However, the TCP spec clarifies
>> (section 3.6.1.4: PSK Use):
>>
>> Once the TLS secure channel for the Admin Queue of an association
>> has been set up with a generated {PSK, PSK Identity} pair, that
>> generated {PSK, PSK Identity} pair should be replaced periodically
>> (e.g., every hour) or on demand by performing a reauthentication
>> with the SC_C field in the AUTH_Negotiate message set to REPLACETLSPSK
>> (refer to the AUTH_Negotiate Message section of the NVM Express
>> Base Specification) over the Admin Queue of that association. The most
>> recently generated PSK, if any, is the generated PSK associated with
>> that Admin Queue.
>
> Yeah, to me "associated with" doesn't necessarily mean that we reset
> the connection to use the new PSK.
>
But then why would we want to replace the PSK if it's not used?Which
would be completely pointless for secure concatenation, as
for _any_ connection establishment a new PSK will be generated.
And I've checked with FMDS, the intention really was that REPLACETLSPSK
should result in the new PSK to be _used_ for existing connections.
There's now a bug for this:
https://bugzilla.nvmexpress.org/show_bug.cgi?id=638
and it should be addressed with an ECN.
>>
>> And the only way to associate a PSK with the admin queue is to use
>> it for the TLS encryption, ie re-run the TLS handshake.
>>
>> Or indeed use the KeyUpdate mechanism.
>
> This is the part that I think is weird.
>
> If we do need to use the new PSK, once a host issues a REPLACETLSPSK
> we have to tear down and restart the TLS connection. Which seems
> really clunky and the spec doesn't seem to mention that at all.
>
> If the host wants to replace the TLS keys it can just issue a
> KeyUpdate, which doesn't involve tearing down the entire connection.
> So do we actually need to reset the TLS connection after a
> REPLACETLSPSK?
>
Oh, I fully agree. KeyUpdate would be the way to go to get a seamless
PSK replacement. But not all implementations do it (currently not even
the linux kernel :-), so in the absence of that we have to reset the
queue to start a new TLS handshake.
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: [PATCH v3 4/4] nvme: Allow reauth from sysfs
2025-11-19 7:45 ` Hannes Reinecke
@ 2025-11-19 10:21 ` Alistair Francis
0 siblings, 0 replies; 14+ messages in thread
From: Alistair Francis @ 2025-11-19 10:21 UTC (permalink / raw)
To: Hannes Reinecke
Cc: kbusch, axboe, hch, sagi, kch, linux-nvme, linux-kernel,
Alistair Francis
On Wed, Nov 19, 2025 at 5:45 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 11/19/25 01:24, Alistair Francis wrote:
> > On Tue, Nov 18, 2025 at 9:50 PM Hannes Reinecke <hare@suse.de> wrote:
> >>
> >> On 11/18/25 01:52, Alistair Francis wrote:
> >>> On Fri, Nov 14, 2025 at 5:15 PM Hannes Reinecke <hare@suse.de> wrote:
> [ .. ]>>>>
> >>>> Hmm.
> >>>> Now we are just running (re-) authentication, but that does
> >>>> not affect the TLS connection (which continues to use the
> >>>> original key). So you would need to reset the connection
> >>>> here to re-establish a new TLS connection.
> >>>
> >>> Is the connection supposed to be reset? I don't see any mention of
> >>> that in the spec
> >>>
> >> Yeah, that's a bit hard to read (as usual).
> >> The base spec just claims (Fig. 733, Secure Channel Protocol Identifiers):
> >>
> >> 03h: This {PSK, PSK Identity} pair replaces the {PSK, PSK Identity}
> >> pair that was used to set up the TLS secure channel over which the
> >> authentication transaction is performed.
> >>
> >> So from that your implementation is correct, as it just replaces the
> >> PSK (without actually using them). However, the TCP spec clarifies
> >> (section 3.6.1.4: PSK Use):
> >>
> >> Once the TLS secure channel for the Admin Queue of an association
> >> has been set up with a generated {PSK, PSK Identity} pair, that
> >> generated {PSK, PSK Identity} pair should be replaced periodically
> >> (e.g., every hour) or on demand by performing a reauthentication
> >> with the SC_C field in the AUTH_Negotiate message set to REPLACETLSPSK
> >> (refer to the AUTH_Negotiate Message section of the NVM Express
> >> Base Specification) over the Admin Queue of that association. The most
> >> recently generated PSK, if any, is the generated PSK associated with
> >> that Admin Queue.
> >
> > Yeah, to me "associated with" doesn't necessarily mean that we reset
> > the connection to use the new PSK.
> >
> But then why would we want to replace the PSK if it's not used?Which
> would be completely pointless for secure concatenation, as
> for _any_ connection establishment a new PSK will be generated.
>
> And I've checked with FMDS, the intention really was that REPLACETLSPSK
> should result in the new PSK to be _used_ for existing connections.
Ok, so tear down the connection and reconnect. I'll work on adding that
>
> There's now a bug for this:
> https://bugzilla.nvmexpress.org/show_bug.cgi?id=638
> and it should be addressed with an ECN.
>
> >>
> >> And the only way to associate a PSK with the admin queue is to use
> >> it for the TLS encryption, ie re-run the TLS handshake.
> >>
> >> Or indeed use the KeyUpdate mechanism.
> >
> > This is the part that I think is weird.
> >
> > If we do need to use the new PSK, once a host issues a REPLACETLSPSK
> > we have to tear down and restart the TLS connection. Which seems
> > really clunky and the spec doesn't seem to mention that at all.
> >
> > If the host wants to replace the TLS keys it can just issue a
> > KeyUpdate, which doesn't involve tearing down the entire connection.
> > So do we actually need to reset the TLS connection after a
> > REPLACETLSPSK?
> >
> Oh, I fully agree. KeyUpdate would be the way to go to get a seamless
> PSK replacement. But not all implementations do it (currently not even
> the linux kernel :-), so in the absence of that we have to reset the
> queue to start a new TLS handshake.
Urgh, let's get everyone supporting KeyUpdate then :)
Alistair
>
> 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-11-19 10:21 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 4:58 [PATCH v3 0/4] Support PSK reauthentication (REPLACETLSPSK) alistair23
2025-11-14 4:58 ` [PATCH v3 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset alistair23
2025-11-14 4:58 ` [PATCH v3 2/4] nvmet-tcp: Don't free SQ on authentication success alistair23
2025-11-18 0:41 ` Wilfred Mallawa
2025-11-14 4:58 ` [PATCH v3 3/4] nvme: Expose the tls_configured sysfs for secure concat connections alistair23
2025-11-14 7:00 ` Hannes Reinecke
2025-11-18 0:41 ` Wilfred Mallawa
2025-11-14 4:58 ` [PATCH v3 4/4] nvme: Allow reauth from sysfs alistair23
2025-11-14 7:15 ` Hannes Reinecke
2025-11-18 0:52 ` Alistair Francis
2025-11-18 11:50 ` Hannes Reinecke
2025-11-19 0:24 ` Alistair Francis
2025-11-19 7:45 ` Hannes Reinecke
2025-11-19 10:21 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox