[PATCH 08/17] nvme-tcp: sanitize TLS key handling

[Hannes Reinecke]

On 4/8/24 23:07, Sagi Grimberg wrote:
> 
> 
> On 08/04/2024 9:48, Hannes Reinecke wrote:
>> On 4/7/24 23:15, Sagi Grimberg wrote:
>>>
>>>
>>> On 18/03/2024 17:03, Hannes Reinecke wrote:
>>>> From: Hannes Reinecke <hare at suse.de>
>>>>
>>>> There is a difference between TLS configured (ie the user has
>>>> provisioned/requested a key) and TLS enabled (ie the connection
>>>> is encrypted with TLS). This becomes important for secure 
>>>> concatenation,
>>>> where the initial authentication is run unencrypted (ie with
>>>> TLS configured, but not enabled), and then the queue is reset to
>>>> run over TLS (ie TLS configured _and_ enabled).
>>>
>>> 1. configured/enabled confuse (me at least) in this context.
>>> 2. What does "queue is reset" mean?
> 
> Can you answer this?
> 
Sorry, I meant 'controller reset'.

>>>
>>>> So to differentiate between those two states store the provisioned
>>>> key in opts->tls_key (as we're using the same TLS key for all queues)
>>>> and only the key serial of the key negotiated by the TLS handshake
>>>> in queue->tls_pskid.
>>>
>>> I have some recollection of asking about it, but can you explain why is
>>> a tls_pskid now stored per-queue? What do we lose if tls_pskid moves 
>>> from
>>> nvme_tcp_queue to nvme_tcp_ctrl ?
>>>
>> We need to have a distinction between 'the queue is TLS enabled' (ie TLS
>> handshake should be started on that queue) and 'the queue is tls 
>> encrypted' (ie TLS handshake has completed on that queue).
>> The former indeed can be derived from controller settings, and for the
>> latter we use the 'tls_pskid' setting.
>> Additionally it's the value for the tls_key sysfs attribute.
>> I could have used a per-queue flag, but using the tls_pskid was just
>> too easy here :-)
> 
> I think a flag would be clearer, because a per-queue pskid suggest that 
> it is unique per queue while you concur that it isn't.

Ok. Let's see how this works out.

Cheerc,

Hannes