[PATCH 6/9] nvme-tcp: request secure channel concatenation
Hannes Reinecke
hare at suse.de
Mon Oct 21 00:22:39 PDT 2024
On 10/20/24 23:04, Sagi Grimberg wrote:
>
>
>
> On 18/10/2024 9:33, Hannes Reinecke wrote:
>> Add a fabrics option 'concat' to request secure channel concatenation.
>> When secure channel concatenation is enabled a 'generated PSK' is
>> inserted
>> into the keyring such that it's available after reset.
>>
>> Signed-off-by: Hannes Reinecke <hare at kernel.org>
>> ---
>> drivers/nvme/host/auth.c | 108 +++++++++++++++++++++++++++++++++++-
>> drivers/nvme/host/fabrics.c | 34 +++++++++++-
>> drivers/nvme/host/fabrics.h | 3 +
>> drivers/nvme/host/nvme.h | 2 +
>> drivers/nvme/host/sysfs.c | 4 +-
>> drivers/nvme/host/tcp.c | 47 ++++++++++++++--
>> include/linux/nvme.h | 7 +++
>> 7 files changed, 191 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
>> index 371e14f0a203..902c8ba59562 100644
>> --- a/drivers/nvme/host/auth.c
>> +++ b/drivers/nvme/host/auth.c
[ .. ]
>> @@ -833,6 +921,14 @@ static void nvme_queue_auth_work(struct
>> work_struct *work)
>> }
>> if (!ret) {
>> chap->error = 0;
>> + /* Secure concatenation can only be enabled on the admin
>> queue */
>
> I'd add a warning if that is not the case here.
>
Okay.
>> + if (!chap->qid && ctrl->opts->concat &&
>> + (ret = nvme_auth_secure_concat(ctrl, chap))) {
>> + dev_warn(ctrl->device,
>> + "%s: qid %d failed to enable secure concatenation\n",
>> + __func__, chap->qid);
>> + chap->error = ret;
>> + }
>> return;
>> }
>> @@ -912,6 +1008,12 @@ static void nvme_ctrl_auth_work(struct
>> work_struct *work)
>> "qid 0: authentication failed\n");
>> return;
>> }
>> + /*
>> + * Only run authentication on the admin queue for
>> + * secure concatenation
>> + */
>> + if (ctrl->opts->concat)
>> + return;
>> for (q = 1; q < ctrl->queue_count; q++) {
>> ret = nvme_auth_negotiate(ctrl, q);
>> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
>> index 432efcbf9e2f..93e9041b9657 100644
>> --- a/drivers/nvme/host/fabrics.c
>> +++ b/drivers/nvme/host/fabrics.c
>> @@ -472,8 +472,9 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
>> result = le32_to_cpu(res.u32);
>> ctrl->cntlid = result & 0xFFFF;
>> if (result & (NVME_CONNECT_AUTHREQ_ATR |
>> NVME_CONNECT_AUTHREQ_ASCR)) {
>> - /* Secure concatenation is not implemented */
>> - if (result & NVME_CONNECT_AUTHREQ_ASCR) {
>> + /* Check for secure concatenation */
>> + if ((result & NVME_CONNECT_AUTHREQ_ASCR) &&
>> + !ctrl->opts->concat) {
>> dev_warn(ctrl->device,
>> "qid 0: secure concatenation is not supported\n");
>> ret = -EOPNOTSUPP;
>> @@ -550,7 +551,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl,
>> u16 qid)
>> /* Secure concatenation is not implemented */
>> if (result & NVME_CONNECT_AUTHREQ_ASCR) {
>> dev_warn(ctrl->device,
>> - "qid 0: secure concatenation is not supported\n");
>> + "qid %d: secure concatenation is not supported\n",
>> qid);
>> ret = -EOPNOTSUPP;
>> goto out_free_data;
>> }
>> @@ -706,6 +707,7 @@ static const match_table_t opt_tokens = {
>> #endif
>> #ifdef CONFIG_NVME_TCP_TLS
>> { NVMF_OPT_TLS, "tls" },
>> + { NVMF_OPT_CONCAT, "concat" },
>> #endif
>> { NVMF_OPT_ERR, NULL }
>> };
>> @@ -735,6 +737,7 @@ static int nvmf_parse_options(struct
>> nvmf_ctrl_options *opts,
>> opts->tls = false;
>> opts->tls_key = NULL;
>> opts->keyring = NULL;
>> + opts->concat = false;
>> options = o = kstrdup(buf, GFP_KERNEL);
>> if (!options)
>> @@ -1053,6 +1056,14 @@ static int nvmf_parse_options(struct
>> nvmf_ctrl_options *opts,
>> }
>> opts->tls = true;
>> break;
>> + case NVMF_OPT_CONCAT:
>> + if (!IS_ENABLED(CONFIG_NVME_TCP_TLS)) {
>> + pr_err("TLS is not supported\n");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + opts->concat = true;
>> + break;
>> default:
>> pr_warn("unknown parameter or missing value '%s' in ctrl
>> creation request\n",
>> p);
>> @@ -1079,6 +1090,23 @@ static int nvmf_parse_options(struct
>> nvmf_ctrl_options *opts,
>> pr_warn("failfast tmo (%d) larger than controller loss
>> tmo (%d)\n",
>> opts->fast_io_fail_tmo, ctrl_loss_tmo);
>> }
>> + if (opts->concat) {
>> + if (opts->tls) {
>> + pr_err("Secure concatenation over TLS is not supported\n");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + if (opts->tls_key) {
>> + pr_err("Cannot specify a TLS key for secure
>> concatenation\n");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + if (!opts->dhchap_secret) {
>> + pr_err("Need to enable DH-CHAP for secure concatenation\n");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + }
>> opts->host = nvmf_host_add(hostnqn, &hostid);
>> if (IS_ERR(opts->host)) {
>> diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
>> index 21d75dc4a3a0..9cf5b020adba 100644
>> --- a/drivers/nvme/host/fabrics.h
>> +++ b/drivers/nvme/host/fabrics.h
>> @@ -66,6 +66,7 @@ enum {
>> NVMF_OPT_TLS = 1 << 25,
>> NVMF_OPT_KEYRING = 1 << 26,
>> NVMF_OPT_TLS_KEY = 1 << 27,
>> + NVMF_OPT_CONCAT = 1 << 28,
>> };
>> /**
>> @@ -101,6 +102,7 @@ enum {
>> * @keyring: Keyring to use for key lookups
>> * @tls_key: TLS key for encrypted connections (TCP)
>> * @tls: Start TLS encrypted connections (TCP)
>> + * @concat: Enabled Secure channel concatenation (TCP)
>> * @disable_sqflow: disable controller sq flow control
>> * @hdr_digest: generate/verify header digest (TCP)
>> * @data_digest: generate/verify data digest (TCP)
>> @@ -130,6 +132,7 @@ struct nvmf_ctrl_options {
>> struct key *keyring;
>> struct key *tls_key;
>> bool tls;
>> + bool concat;
>> bool disable_sqflow;
>> bool hdr_digest;
>> bool data_digest;
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 313a4f978a2c..4c735b88b434 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -1132,6 +1132,7 @@ void nvme_auth_stop(struct nvme_ctrl *ctrl);
>> int nvme_auth_negotiate(struct nvme_ctrl *ctrl, int qid);
>> int nvme_auth_wait(struct nvme_ctrl *ctrl, int qid);
>> void nvme_auth_free(struct nvme_ctrl *ctrl);
>> +void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl);
>> #else
>> static inline int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
>> {
>> @@ -1154,6 +1155,7 @@ static inline int nvme_auth_wait(struct
>> nvme_ctrl *ctrl, int qid)
>> return -EPROTONOSUPPORT;
>> }
>> static inline void nvme_auth_free(struct nvme_ctrl *ctrl) {};
>> +static void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl) {};
>> #endif
>> u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
>> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
>> index b68a9e5f1ea3..efb35eef1915 100644
>> --- a/drivers/nvme/host/sysfs.c
>> +++ b/drivers/nvme/host/sysfs.c
>> @@ -780,10 +780,10 @@ static umode_t nvme_tls_attrs_are_visible(struct
>> kobject *kobj,
>> return 0;
>> if (a == &dev_attr_tls_key.attr &&
>> - !ctrl->opts->tls)
>> + !ctrl->opts->tls && !ctrl->opts->concat)
>> return 0;
>> if (a == &dev_attr_tls_configured_key.attr &&
>> - !ctrl->opts->tls_key)
>> + (!ctrl->opts->tls_key || ctrl->opts->concat))
>> return 0;
>> if (a == &dev_attr_tls_keyring.attr &&
>> !ctrl->opts->keyring)
>> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
>> index 3e416af2659f..b8a3461b617c 100644
>> --- a/drivers/nvme/host/tcp.c
>> +++ b/drivers/nvme/host/tcp.c
>> @@ -233,7 +233,7 @@ static inline bool nvme_tcp_tls_configured(struct
>> nvme_ctrl *ctrl)
>> if (!IS_ENABLED(CONFIG_NVME_TCP_TLS))
>> return 0;
>> - return ctrl->opts->tls;
>> + return ctrl->opts->tls || ctrl->opts->concat;
>> }
>> static inline struct blk_mq_tags *nvme_tcp_tagset(struct
>> nvme_tcp_queue *queue)
>> @@ -1948,7 +1948,7 @@ static int nvme_tcp_alloc_admin_queue(struct
>> nvme_ctrl *ctrl)
>> if (nvme_tcp_tls_configured(ctrl)) {
>> if (ctrl->opts->tls_key)
>> pskid = key_serial(ctrl->opts->tls_key);
>> - else {
>> + else if (ctrl->opts->tls) {
>> pskid = nvme_tls_psk_default(ctrl->opts->keyring,
>> ctrl->opts->host->nqn,
>> ctrl->opts->subsysnqn);
>> @@ -1978,9 +1978,25 @@ static int __nvme_tcp_alloc_io_queues(struct
>> nvme_ctrl *ctrl)
>> {
>> int i, ret;
>> - if (nvme_tcp_tls_configured(ctrl) && !ctrl->tls_pskid) {
>> - dev_err(ctrl->device, "no PSK negotiated\n");
>> - return -ENOKEY;
>> + if (nvme_tcp_tls_configured(ctrl)) {
>> + if (ctrl->opts->concat) {
>> + /*
>> + * The generated PSK is stored in the
>> + * fabric options
>> + */
>> + if (!ctrl->opts->tls_key) {
>> + dev_err(ctrl->device, "no PSK generated\n");
>> + return -ENOKEY;
>> + }
>> + if (ctrl->tls_pskid &&
>> + ctrl->tls_pskid != key_serial(ctrl->opts->tls_key)) {
>> + dev_err(ctrl->device, "Stale PSK id %08x\n", ctrl-
>> >tls_pskid);
>> + ctrl->tls_pskid = 0;
>> + }
>> + } else if (!ctrl->tls_pskid) {
>> + dev_err(ctrl->device, "no PSK negotiated\n");
>> + return -ENOKEY;
>> + }
>> }
>> for (i = 1; i < ctrl->queue_count; i++) {
>> @@ -2211,6 +2227,21 @@ static void nvme_tcp_reconnect_or_remove(struct
>> nvme_ctrl *ctrl,
>> }
>> }
>> +/*
>> + * The TLS key needs to be revoked when:
>> + * - concatenation is enabled and
>> + * -> This is a generated key and only valid for this session
>> + * - the generated key is present in ctrl->tls_key and
>> + * -> authentication has completed and the key has been generated
>> + * - tls has been enabled
>> + * -> otherwise we are about to reset the admin queue after
>> authentication
>> + * to enable TLS with the generated key
>> + */
>> +static bool nvme_tcp_key_revoke_needed(struct nvme_ctrl *ctrl)
>> +{
>> + return ctrl->opts->concat && ctrl->opts->tls_key && ctrl->tls_pskid;
>> +}
>> +
>> static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
>> {
>> struct nvmf_ctrl_options *opts = ctrl->opts;
>> @@ -2314,6 +2345,8 @@ static void nvme_tcp_error_recovery_work(struct
>> work_struct *work)
>> struct nvme_tcp_ctrl, err_work);
>> struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl;
>> + if (nvme_tcp_key_revoke_needed(ctrl))
>> + nvme_auth_revoke_tls_key(ctrl);
>
> Having this sprayed in various places in the code is really confusing.
>
> Can you please add a small comment on each call-site? just for our
> future selves
> reading this code?
>
Sure.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare at 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
More information about the Linux-nvme
mailing list