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

[Sagi Grimberg]

On 08/04/2024 9:25, Hannes Reinecke wrote:
> On 4/7/24 23:49, Sagi Grimberg wrote:
>>
>>
>> On 18/03/2024 17:03, Hannes Reinecke wrote:
>>> From: Hannes Reinecke <hare at 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 at 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.

So what is the point of setting the derived psk with a timeout?

Seems rather silly doing that _every_ reset/reconnect... But ok...