Re: [PATCH 13/17] nvme-tcp: reset after recovery for secure concatenation

[Hannes Reinecke <hare@suse.de>]

On 4/7/24 23:49, Sagi Grimberg wrote:
> 
> 
> On 18/03/2024 17:03, Hannes Reinecke wrote:
>> From: Hannes Reinecke <hare@suse.de>
>>
>> With TP8018 a new key will be generated from the DH-HMAC-CHAP
>> protocol after reset or recovery, but we need to start over
>> to establish a new TLS connection with the new keys.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/nvme/host/tcp.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
>> index 94152ded123a..3811ee9cd040 100644
>> --- a/drivers/nvme/host/tcp.c
>> +++ b/drivers/nvme/host/tcp.c
>> @@ -2219,6 +2219,22 @@ static void nvme_tcp_reconnect_or_remove(struct 
>> nvme_ctrl *ctrl)
>>       }
>>   }
>> +static bool nvme_tcp_reset_for_secure_concat(struct nvme_ctrl *ctrl)
>> +{
>> +    if (!ctrl->opts->concat)
>> +        return false;
>> +    /*
>> +     * If a key has been generated and TLS has not been enabled
>> +     * reset the queue to start TLS handshake.
>> +     */
>> +    if (ctrl->opts->tls_key && !ctrl->tls_key) {
>> +        dev_info(ctrl->device, "Reset to enable TLS with generated 
>> PSK\n");
>> +        nvme_reset_ctrl(ctrl);
>> +        return true;
>> +    }
>> +    return false;
>> +}
>> +
>>   static void nvme_tcp_revoke_generated_tls_key(struct nvme_ctrl *ctrl)
>>   {
>>       if (!ctrl->opts->concat)
>> @@ -2321,6 +2337,9 @@ static void nvme_tcp_reconnect_ctrl_work(struct 
>> work_struct *work)
>>       if (nvme_tcp_setup_ctrl(ctrl, false))
>>           goto requeue;
>> +    if (nvme_tcp_reset_for_secure_concat(ctrl))
>> +        return;
>> +
>>       dev_info(ctrl->device, "Successfully reconnected (%d attempt)\n",
>>               ctrl->nr_reconnects);
>> @@ -2396,6 +2415,7 @@ static void nvme_reset_ctrl_work(struct 
>> work_struct *work)
>>       if (nvme_tcp_setup_ctrl(ctrl, false))
>>           goto out_fail;
>> +    nvme_tcp_reset_for_secure_concat(ctrl);
> 
> This is really strange. I'd imagine that this would be needed only if 
> the derived psk expired no?

You are absolutely correct. But once you reset the connection the 
derived PSK of the previous connection _is_ expired as DH-HMAC-CHAP
generated a new one.

Remember: for secure concatenation we _always_ have a two-step 
connection setup. The initial connection runs for DH-HMAC-CHAP
to generate the PSK, and then a connection reset to start over
with TLS and the generated PSK.
So upon reset we have to invalidate the generated PSK, reset the
connection to run DH-HMAC-CHAP, and reset _again_ to start over
with the new PSK.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.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