Message-ID: <fd9c30d3-2a9f-4ddc-9d71-436034637acb@suse.de>
Date: Tue, 18 Nov 2025 12:50:46 +0100
From: Hannes Reinecke <hare@...e.de>
To: Alistair Francis <alistair23@...il.com>
Cc: kbusch@...nel.org, axboe@...nel.dk, hch@....de, sagi@...mberg.me,
 kch@...dia.com, linux-nvme@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Alistair Francis <alistair.francis@....com>
Subject: Re: [PATCH v3 4/4] nvme: Allow reauth from sysfs

On 11/18/25 01:52, Alistair Francis wrote:
> On Fri, Nov 14, 2025 at 5:15 PM Hannes Reinecke <hare@...e.de> wrote:
>>
>> On 11/14/25 05:58, alistair23@...il.com wrote:
>>> From: Alistair Francis <alistair.francis@....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@....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@...ts.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@...e.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

Powered by blists - more mailing lists