Re: [PATCH 12/18] nvme-fabrics: parse options 'keyring' and 'tls_key'

[Sagi Grimberg <sagi@grimberg.me>]

On 4/3/23 17:11, Hannes Reinecke wrote:
> On 4/3/23 15:07, Sagi Grimberg wrote:
>>
>>>>>>> Parse the fabrics options 'keyring' and 'tls_key' and store the
>>>>>>> referenced keys in the options structure.
>>>>>>
>>>>>> Can you explain the reasoning to why a user need to pass a keyring
>>>>>> given that we already set up one?
>>>>>>
>>>>> Choice.
>>>>> With a single keyring we can only have a single identity.
>>>>> But there might be configurations where we want to have different PSKs
>>>>> for the same identity (eg for key rotation).
>>>>
>>>> How do you expect that rotation would work with this?
>>>>
>>> The user creates a new keyring, adds the necessary keys to it, and then
>>> either
>>> - updates the keyring for nvmet (via configfs)
>>> or
>>> - reconnects using the new keyring
>>
>> And the user cannot update the nvme keyring?
>>
> Sure, he can. All I'm saying is that he might choose to create a 
> different one (Different tenants, setups, what do I know).
> 
>>>> How does nvmet handle a non-nvme keyring?
>>>>
>>> There is a configfs attribute 'param_keyring', allowing you to use a 
>>> different keyring (the nvme one is just the default).
>>
>> OK.
>>
>>>>> With this option we can prepare a new keyring, and use that instead 
>>>>> of the old one.
>>>>
>>>> On an existing controller?
>>>>
>>> Sure; when we are updating the keyring and the key id the controller 
>>> will use that after the next reset.
>>> Much like we do for DH-CHAP nowadays.
>>>
>>>>> (And it really doesn't add much complexity...)
>>>>
>>>> I know, it just adds one more argument, and I want to understand if it
>>>> is really needed.
>>>
>>> I do agree that the keyring argument would not necessarily required 
>>> if we pass in the key id, but I'll be needing the keyring for secure 
>>> concatenation. And plan is to move DH-HMAC-CHAP over to keyrings, too.
>>
>> I'm not sure I understand why authentication move to keyrings requires
>> that the user is able to bring its own keyring.
>>
>>> So I'd prefer to keep it.
>>
>> I'm not against it, but I failing to understand in what situation a user
>> _must_ send a different keyring to the driver (and not use the nvme
>> keyring).
> 
> Oh, there is no 'must'. He _can_. And under certain circumstances
> (in a multi-tenant environment, say) there are reasons to keep the keys 
> separate into several keyrings.
> Bit like the '--key' parameter; the code will try to select the 
> 'default' key, but if the user wants to use a different one by all 
> means, do.
> Same for the keyring; we do have the default keyring, but if the user 
> for whatever reason doesn't want to use it we're giving him the 
> possibility for that.

I'm not overly keen to overload the argument space without proper
real-life justification. But maybe it does make sense to allow users
to store their secrets in separate keyrings...

So I'm not really opposed to this one.