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

[Sagi Grimberg]

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?

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