[PATCH 09/18] nvme-tcp: add connect option 'tls'

[Hannes Reinecke]

On 3/22/23 10:24, Sagi Grimberg wrote:
>> Add a connect option 'tls' to request TLS1.3 in-band encryption, and
>> abort the connection attempt if TLS could not be established.
>>
>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>> ---
>>   drivers/nvme/host/fabrics.c | 5 +++++
>>   drivers/nvme/host/fabrics.h | 2 ++
>>   drivers/nvme/host/tcp.c     | 7 ++++++-
>>   3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
>> index bbaa04a0c502..fdff7cdff029 100644
>> --- a/drivers/nvme/host/fabrics.c
>> +++ b/drivers/nvme/host/fabrics.c
>> @@ -609,6 +609,7 @@ static const match_table_t opt_tokens = {
>>       { NVMF_OPT_DISCOVERY,        "discovery"        },
>>       { NVMF_OPT_DHCHAP_SECRET,    "dhchap_secret=%s"    },
>>       { NVMF_OPT_DHCHAP_CTRL_SECRET,    "dhchap_ctrl_secret=%s"    },
>> +    { NVMF_OPT_TLS,            "tls"            },
>>       { NVMF_OPT_ERR,            NULL            }
>>   };
>> @@ -632,6 +633,7 @@ static int nvmf_parse_options(struct 
>> nvmf_ctrl_options *opts,
>>       opts->hdr_digest = false;
>>       opts->data_digest = false;
>>       opts->tos = -1; /* < 0 == use transport default */
>> +    opts->tls = false;
>>       options = o = kstrdup(buf, GFP_KERNEL);
>>       if (!options)
>> @@ -918,6 +920,9 @@ static int nvmf_parse_options(struct 
>> nvmf_ctrl_options *opts,
>>               kfree(opts->dhchap_ctrl_secret);
>>               opts->dhchap_ctrl_secret = p;
>>               break;
>> +        case NVMF_OPT_TLS:
>> +            opts->tls = true;
>> +            break;
>>           default:
>>               pr_warn("unknown parameter or missing value '%s' in ctrl 
>> creation request\n",
>>                   p);
>> diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
>> index dcac3df8a5f7..c4538a9d437c 100644
>> --- a/drivers/nvme/host/fabrics.h
>> +++ b/drivers/nvme/host/fabrics.h
>> @@ -70,6 +70,7 @@ enum {
>>       NVMF_OPT_DISCOVERY    = 1 << 22,
>>       NVMF_OPT_DHCHAP_SECRET    = 1 << 23,
>>       NVMF_OPT_DHCHAP_CTRL_SECRET = 1 << 24,
>> +    NVMF_OPT_TLS        = 1 << 25,
>>   };
>>   /**
>> @@ -128,6 +129,7 @@ struct nvmf_ctrl_options {
>>       int            max_reconnects;
>>       char            *dhchap_secret;
>>       char            *dhchap_ctrl_secret;
>> +    bool            tls;
>>       bool            disable_sqflow;
>>       bool            hdr_digest;
>>       bool            data_digest;
>> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
>> index bcf24e9a08e1..bbff1f52a167 100644
>> --- a/drivers/nvme/host/tcp.c
>> +++ b/drivers/nvme/host/tcp.c
>> @@ -1902,6 +1902,9 @@ static int nvme_tcp_alloc_admin_queue(struct 
>> nvme_ctrl *ctrl)
>>               break;
>>       }
>>       if (ret) {
>> +        /* Abort if TLS is requested */
>> +        if (num_keys && ctrl->opts->tls)
>> +            goto out_free_queue;
>>           /* Try without TLS */
>>           ret = nvme_tcp_alloc_queue(ctrl, 0, 0);
>>           if (ret)
>> @@ -1934,6 +1937,8 @@ static int __nvme_tcp_alloc_io_queues(struct 
>> nvme_ctrl *ctrl)
>>                   break;
>>           }
>>           if (ret) {
>> +            if (num_keys && ctrl->opts->tls)
>> +                goto out_free_queues;
> 
> I don't see why we even attempt tls if we're not explicitly told to.

Because we don't know.

It's all easy if we do a discovery before connect, as the discovery log 
page tells us what to do.
But if we do _not_ do a discovery, how would we know what to do?

We could try to start off with no TLS, but if the server requires TLS 
then all we get is a 'connection refused' error, leaving us none the wiser.
So really we have to start off with TLS (if we have a matching 
identity). The server then can always refuse the connection (with the 
same error), in which case we should re-try without TLS.
That's where the 'tls' option comes in: to avoid having a fallback 
without TLS.

So in the end we have three modes for the client:

1) TLS not supported
2) TLS allowed (with fallback to non-TLS)
3) TLS required (no fallback to non-TLS)

For modes 2) and 3) a PSK has to be present, and
the 'tls' option is used to switch from mode 2) to mode 3)

Cheers,

Hannes