[PATCH v2 0/4] Support PSK reauthentication (REPLACETLSPSK)

[PATCH v2 0/4] Support PSK reauthentication (REPLACETLSPSK)

[alistair23]

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 anything to the `tls_configured_key` sysfs file,
for example something like this

```shell
echo 1 > /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.

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

 drivers/nvme/host/sysfs.c              | 25 +++++++++++++++++++++++--
 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 ++--
 5 files changed, 35 insertions(+), 14 deletions(-)

-- 
2.51.1

[PATCH v2 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset

[alistair23]

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>
---
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

Re: [PATCH v2 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset

[Wilfred Mallawa]

On Wed, 2025-11-12 at 09:45 +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> 
[...]

Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>

Wilfred

Re: [PATCH v2 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 1/4] nvmet-tcp: Don't error if TLS is enabed on a reset

[Hannes Reinecke]

On 11/12/25 00:45, alistair23@gmail.com wrote:
> 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>
> ---
> 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(-)
> 
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

[PATCH v2 2/4] nvmet-tcp: Don't free SQ on authentication success

[alistair23]

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>
---
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

Re: [PATCH v2 2/4] nvmet-tcp: Don't free SQ on authentication success

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 2/4] nvmet-tcp: Don't free SQ on authentication success

[Hannes Reinecke]

On 11/12/25 00:45, 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>
> ---
> 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(-)
> 
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

[PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[alistair23]

From: Alistair Francis <alistair.francis@wdc.com>

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
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

Re: [PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[Hannes Reinecke]

On 11/12/25 00:45, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> 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)

??

How can you have a configured TLS Key for secure concatenation?
We generate the keys from the DH-CHAP key material, don't we?

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

Re: [PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[Alistair Francis]

On Wed, Nov 12, 2025 at 5:08 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 11/12/25 00:45, alistair23@gmail.com wrote:
> > From: Alistair Francis <alistair.francis@wdc.com>
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> > 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)
>
> ??
>
> How can you have a configured TLS Key for secure concatenation?

I'm not sure I follow

`ctrl->opts->tls_key` is directly set at the end of the
`nvme_auth_secure_concat()` function, so it will be set for secure
concatenation.

> We generate the keys from the DH-CHAP key material, don't we?

I haven't checked that closely, but the key seems to be generated from
the CHAP hash id

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

Re: [PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[Hannes Reinecke]

On 11/13/25 03:08, Alistair Francis wrote:
> On Wed, Nov 12, 2025 at 5:08 PM Hannes Reinecke <hare@suse.de> wrote:
>>
>> On 11/12/25 00:45, alistair23@gmail.com wrote:
>>> From: Alistair Francis <alistair.francis@wdc.com>
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>> 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)
>>
>> ??
>>
>> How can you have a configured TLS Key for secure concatenation?
> 
> I'm not sure I follow
> 
> `ctrl->opts->tls_key` is directly set at the end of the
> `nvme_auth_secure_concat()` function, so it will be set for secure
> concatenation.
> 
Right, sorry. Of course you are right.

But I'm still a bit unsure about the interface here; just writing
anything into it doesn't feel like the correct way of doing it.

I would rather modify the interface to allow a key serial number (or 0).
That would allow us to modify the configured key, which currently is
fixed for the lifetime of the connection.
And writing '0' would reset the configured key, reverting to automatic
key selection.
Having such an interface would actually be beneficial, as it would
remove some limitations from the current interface.

Hmm?

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

Re: [PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[Alistair Francis]

On Thu, Nov 13, 2025 at 5:21 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 11/13/25 03:08, Alistair Francis wrote:
> > On Wed, Nov 12, 2025 at 5:08 PM Hannes Reinecke <hare@suse.de> wrote:
> >>
> >> On 11/12/25 00:45, alistair23@gmail.com wrote:
> >>> From: Alistair Francis <alistair.francis@wdc.com>
> >>>
> >>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >>> ---
> >>> 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)
> >>
> >> ??
> >>
> >> How can you have a configured TLS Key for secure concatenation?
> >
> > I'm not sure I follow
> >
> > `ctrl->opts->tls_key` is directly set at the end of the
> > `nvme_auth_secure_concat()` function, so it will be set for secure
> > concatenation.
> >
> Right, sorry. Of course you are right.
>
> But I'm still a bit unsure about the interface here; just writing
> anything into it doesn't feel like the correct way of doing it.

I'm happy to change that.

I was thinking a bool, so basically write 1 to trigger

>
> I would rather modify the interface to allow a key serial number (or 0).
> That would allow us to modify the configured key, which currently is
> fixed for the lifetime of the connection.

Wouldn't a configured key (`tls_key`) be changed via the tlshd
interface, not writing a key serial to sysfs?

> And writing '0' would reset the configured key, reverting to automatic
> key selection.
> Having such an interface would actually be beneficial, as it would
> remove some limitations from the current interface.

Anyway, I'm not sure your vision conflicts with allowing "write 1 to
`tls_configured_key` to trigger a REPLACETLSPSK"

Alistair

>
> Hmm?
>
> 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

Re: [PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[Hannes Reinecke]

On 11/13/25 13:22, Alistair Francis wrote:
> On Thu, Nov 13, 2025 at 5:21 PM Hannes Reinecke <hare@suse.de> wrote:
>>
>> On 11/13/25 03:08, Alistair Francis wrote:
>>> On Wed, Nov 12, 2025 at 5:08 PM Hannes Reinecke <hare@suse.de> wrote:
>>>>
>>>> On 11/12/25 00:45, alistair23@gmail.com wrote:
>>>>> From: Alistair Francis <alistair.francis@wdc.com>
>>>>>
>>>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>>>> ---
>>>>> 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)
>>>>
>>>> ??
>>>>
>>>> How can you have a configured TLS Key for secure concatenation?
>>>
>>> I'm not sure I follow
>>>
>>> `ctrl->opts->tls_key` is directly set at the end of the
>>> `nvme_auth_secure_concat()` function, so it will be set for secure
>>> concatenation.
>>>
>> Right, sorry. Of course you are right.
>>
>> But I'm still a bit unsure about the interface here; just writing
>> anything into it doesn't feel like the correct way of doing it.
> 
> I'm happy to change that.
> 
> I was thinking a bool, so basically write 1 to trigger
> 

I would like to keep the interface consistent, so as it's displaying
a key serial number it should also accept a key serial number on writing.
(Or '0', which is 'invalid key serial number');

>>
>> I would rather modify the interface to allow a key serial number (or 0).
>> That would allow us to modify the configured key, which currently is
>> fixed for the lifetime of the connection.
> 
> Wouldn't a configured key (`tls_key`) be changed via the tlshd
> interface, not writing a key serial to sysfs?
> 

We would need to figure out the effects of REPLACETLSPSK with secure
concatenation.
Clearly for secure concatenation the resulting key is generated from
the secure key material negotiated during authentication. So the only
valid keys here should be generated keys, not persistent ones.
But in order to have _new_ generated keys we need to run authentication.
Consequently the key serial number can't be known when starting a
REPLACETLSPSK operation, and we need to write '0' into the
tls_configured_key attribute to a) reset the generated key and b)
start the authentication protocol.
That will then generate a new key, which will be visible via
'tls_configured_key'.

So all good, methinks.

>> And writing '0' would reset the configured key, reverting to automatic
>> key selection.
>> Having such an interface would actually be beneficial, as it would
>> remove some limitations from the current interface.
> 
> Anyway, I'm not sure your vision conflicts with allowing "write 1 to
> `tls_configured_key` to trigger a REPLACETLSPSK"
> 

Please don't use '1'. As mentioned earlier, I would like the sysfs
attribute to represent entries in the keystore (by displaying the key
serial number). '1' _is_ a valid number, and so the interface will try
to lookup a key with serial number '1'.
(And failing to do so :-)
'0' is _not_ valid key serial number, so writing '0' is equivalent to
'disable that key'.

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

Re: [PATCH v2 3/4] nvme: Expose the tls_configured sysfs for secure concat connections

[Alistair Francis]

On Thu, Nov 13, 2025 at 10:45 PM Hannes Reinecke <hare@suse.de> wrote:
>
> On 11/13/25 13:22, Alistair Francis wrote:
> > On Thu, Nov 13, 2025 at 5:21 PM Hannes Reinecke <hare@suse.de> wrote:
> >>
> >> On 11/13/25 03:08, Alistair Francis wrote:
> >>> On Wed, Nov 12, 2025 at 5:08 PM Hannes Reinecke <hare@suse.de> wrote:
> >>>>
> >>>> On 11/12/25 00:45, alistair23@gmail.com wrote:
> >>>>> From: Alistair Francis <alistair.francis@wdc.com>
> >>>>>
> >>>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> >>>>> ---
> >>>>> 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)
> >>>>
> >>>> ??
> >>>>
> >>>> How can you have a configured TLS Key for secure concatenation?
> >>>
> >>> I'm not sure I follow
> >>>
> >>> `ctrl->opts->tls_key` is directly set at the end of the
> >>> `nvme_auth_secure_concat()` function, so it will be set for secure
> >>> concatenation.
> >>>
> >> Right, sorry. Of course you are right.
> >>
> >> But I'm still a bit unsure about the interface here; just writing
> >> anything into it doesn't feel like the correct way of doing it.
> >
> > I'm happy to change that.
> >
> > I was thinking a bool, so basically write 1 to trigger
> >
>
> I would like to keep the interface consistent, so as it's displaying
> a key serial number it should also accept a key serial number on writing.
> (Or '0', which is 'invalid key serial number');

But you don't know what the new key serial will be

>
> >>
> >> I would rather modify the interface to allow a key serial number (or 0).
> >> That would allow us to modify the configured key, which currently is
> >> fixed for the lifetime of the connection.
> >
> > Wouldn't a configured key (`tls_key`) be changed via the tlshd
> > interface, not writing a key serial to sysfs?
> >
>
> We would need to figure out the effects of REPLACETLSPSK with secure
> concatenation.
> Clearly for secure concatenation the resulting key is generated from
> the secure key material negotiated during authentication. So the only
> valid keys here should be generated keys, not persistent ones.
> But in order to have _new_ generated keys we need to run authentication.
> Consequently the key serial number can't be known when starting a
> REPLACETLSPSK operation, and we need to write '0' into the
> tls_configured_key attribute to a) reset the generated key and b)
> start the authentication protocol.
> That will then generate a new key, which will be visible via
> 'tls_configured_key'.
>
> So all good, methinks.

Ok, so trigger a REPLACETLSPSK when the user writes `0` to
`tls_configured_key`. I can do that

Alistair

>
> >> And writing '0' would reset the configured key, reverting to automatic
> >> key selection.
> >> Having such an interface would actually be beneficial, as it would
> >> remove some limitations from the current interface.
> >
> > Anyway, I'm not sure your vision conflicts with allowing "write 1 to
> > `tls_configured_key` to trigger a REPLACETLSPSK"
> >
>
> Please don't use '1'. As mentioned earlier, I would like the sysfs
> attribute to represent entries in the keystore (by displaying the key
> serial number). '1' _is_ a valid number, and so the interface will try
> to lookup a key with serial number '1'.
> (And failing to do so :-)
> '0' is _not_ valid key serial number, so writing '0' is equivalent to
> 'disable that key'.
>
> 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

[PATCH v2 4/4] nvme: Allow reauth from sysfs

[alistair23]

From: Alistair Francis <alistair.francis@wdc.com>

Allow userspace to trigger a reauth (REPLACETLSPSK) from sysfs.
This can be done by writing 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>
---
v2:
 - Trigger on any value written to `tls_configured_key`

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

diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 6d10e12136d0..db1c53e00501 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -806,7 +806,28 @@ 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;
+
+	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

Re: [PATCH v2 4/4] nvme: Allow reauth from sysfs

[Chaitanya Kulkarni]

On 11/11/25 3:45 PM, 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 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>


LGTM,

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck

Re: [PATCH v2 4/4] nvme: Allow reauth from sysfs

[Christoph Hellwig]

On Wed, Nov 12, 2025 at 09:45:18AM +1000, 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 to the sysfs file.
> 
> echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key

This now takes absolutely any write, right?  I think we should at least
be strict to force some value, and document it.

Which reminds me that Documentation/ABI/testing/sysfs-* is missing a
lot of nvme attributes it seems :(

Re: [PATCH v2 4/4] nvme: Allow reauth from sysfs

[Alistair Francis]

On Wed, Nov 12, 2025 at 4:56 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Nov 12, 2025 at 09:45:18AM +1000, 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 to the sysfs file.
> >
> > echo 0 > /sys/devices/virtual/nvme-fabrics/ctl/nvme0/tls_configured_key
>
> This now takes absolutely any write, right?  I think we should at least
> be strict to force some value, and document it.

Yes, it now takes any value. I can change it to a bool-ish (so write 1
to trigger)

>
> Which reminds me that Documentation/ABI/testing/sysfs-* is missing a
> lot of nvme attributes it seems :(

Yeah, that's why I didn't add this one. I'll have a look at updating this

Alistair

>

Re: [PATCH v2 4/4] nvme: Allow reauth from sysfs

[Hannes Reinecke]

On 11/12/25 00:45, 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 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>
> ---
> v2:
>   - Trigger on any value written to `tls_configured_key`
> 
>   drivers/nvme/host/sysfs.c | 23 ++++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 6d10e12136d0..db1c53e00501 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -806,7 +806,28 @@ 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;
> +
> +	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)

Errm. Why 'configured_key'?

The configured_key sysfs attribute is a representation of the --tls-key
attribute when calling 'nvme connect', which in itself should be used
for debugging purposes only. And as such it really cannot be modified.

Please use the 'tls_key' attribute (which holds the negotiated key,
and is available always).

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