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

Sagi Grimberg
2023-03-22T09:24:40Z

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