[PATCH] nvme: add 'static' suffix for user-specified TLS keys

[Hannes Reinecke]

On 3/8/24 11:42, Sagi Grimberg wrote:
> 
> 
> On 07/03/2024 12:47, Hannes Reinecke wrote:
>> On 3/7/24 10:45, Sagi Grimberg wrote:
>>>
>>>
>>> On 26/02/2024 14:24, Hannes Reinecke wrote:
>>>> From: Hannes Reinecke <hare at suse.de>
>>>>
>>>> If the user specifies a specific TLS key via the '--tls_key'
>>>> option to nvme-cli we should be adding a ';static' suffix
>>>> to the 'tls_key' sysfs attribute to signal that this is a
>>>> user-specific key, and not one auto-selected during TLS
>>>> protocol negotiation.
>>>
>>> Static is not the best wording. Doesn't the user already
>>> know what it provided? How is this indication useful?
>>
>> What I'm trying to achieve here is to differentiate between
>>
>> nvme connect --tls
>>
>> and
>>
>> nvme connect --tls_key=<key_id>
>>
>> both calls will result in key serial number in the
>> 'tls_key' attribute, so if we try to generate the json
>> configuration file from an existing connection we
>> have no idea which of the arguments the admin has
>> used. Problem is that first invocation will auto-select
>> the TLS key, and hence we can do a key rotation.
>> The second invocation will always refer to a specific
>> key, and hence we cannot do a key rotation.
>>
>> So we'll need an additional setting allowing us
>> to differentate here.
>>
>> I do agree that adding a field to an existing
>> attribute is probably suboptimal, so I guess
>> I'll retract this patch and send another introducing
>> a new sysfs attribute ('tls_provisioning' ?)
> 
> What are the outputs for this new attributes?
> 
I've just sent a patch for this.

>>
>> The alternative would be to add a new attribute for
>> the keyring (ie introduce a 'tls_keyring' attribute).
>> That would be empty for a pre-configured key (as we
>> are looking up the key by serial, and don't care on
>> which keyring the key is in), and contain the keyring
>> name otherwise.
>>
>> Hmm. I guess I like this better, as then we would
>> also be able to display the 'keyring' setting which
>> currently is also not present in sysfs.
> 
> Agree that displaying the keyring in sysfs is useful to
> have.

Indeed.

Cheers,

Hannes