[PATCH 4/9] nvme: parse dhchap keys during option parsing
Sagi Grimberg
sagi at grimberg.me
Wed Nov 26 01:04:29 PST 2025
On 28/05/2025 17:05, Hannes Reinecke wrote:
> We really should parse the dhchap keys during option parsing to avoid
> having to pass around the plain dhchap secret. During options parsing
> we will create a 'dhchap' key with a random UUID as description, and
> store the key serial in the 'opts' structure.
> This simplifies key handling as on every access the key needs to be
> looked up and checked for validity before accessing the key data.
>
> Signed-off-by: Hannes Reinecke <hare at kernel.org>
> ---
> drivers/nvme/host/auth.c | 113 ++++++++++++++++------
> drivers/nvme/host/fabrics.c | 82 +++++++++++-----
> drivers/nvme/host/fabrics.h | 8 +-
> drivers/nvme/host/sysfs.c | 185 ++++++++++++++++++++++++++----------
> 4 files changed, 278 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
> index c5be0c13e85b..0e8a5b544f63 100644
> --- a/drivers/nvme/host/auth.c
> +++ b/drivers/nvme/host/auth.c
> @@ -532,7 +532,7 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
> ret = crypto_shash_setkey(chap->shash_tfm,
> transformed_secret, transformed_len);
> if (ret) {
> - dev_warn(ctrl->device, "qid %d: failed to set key, error %d\n",
> + dev_warn(ctrl->device, "qid %d: failed to set ctrl key, error %d\n",
> chap->qid, ret);
> goto out;
> }
> @@ -970,11 +970,6 @@ int nvme_auth_negotiate(struct nvme_ctrl *ctrl, int qid)
> return -ENOKEY;
> }
>
> - if (ctrl->opts->dhchap_ctrl_secret && !ctrl->ctrl_key) {
> - dev_warn(ctrl->device, "qid %d: invalid ctrl key\n", qid);
> - return -ENOKEY;
> - }
> -
> chap = &ctrl->dhchap_ctxs[qid];
> cancel_work_sync(&chap->auth_work);
> queue_work(nvme_auth_wq, &chap->auth_work);
> @@ -1059,6 +1054,26 @@ static void nvme_ctrl_auth_work(struct work_struct *work)
> }
> }
>
> +static void nvme_auth_clear_key(struct nvme_ctrl *ctrl, bool is_ctrl)
> +{
> + struct key *key;
> +
> + if (is_ctrl) {
> + key = ctrl->ctrl_key;
> + ctrl->ctrl_key = NULL;
> + } else {
> + key = ctrl->host_key;
> + ctrl->host_key = NULL;
> + }
> + if (key) {
> + dev_dbg(ctrl->device, "%s: revoke%s key %08x\n",
> + __func__, is_ctrl ? " ctrl" : "",
> + key_serial(key));
> + key_revoke(key);
> + key_put(key);
> + }
> +}
> +
> int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
> {
> struct nvme_dhchap_queue_context *chap;
> @@ -1068,31 +1083,70 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
> INIT_WORK(&ctrl->dhchap_auth_work, nvme_ctrl_auth_work);
> if (!ctrl->opts)
> return 0;
> - ctrl->host_key = nvme_auth_extract_key(ctrl->opts->keyring,
> - ctrl->opts->dhchap_secret,
> - strlen(ctrl->opts->dhchap_secret));
> - if (IS_ERR(ctrl->host_key)) {
> - ret = PTR_ERR(ctrl->host_key);
> - ctrl->host_key = NULL;
> - return ret;
> + if (!ctrl->opts->dhchap_key) {
> + nvme_auth_clear_key(ctrl, false);
> + nvme_auth_clear_key(ctrl, true);
consider adding nvme_auth_clear_host_key and nvme_auth_clear_ctrl_key
helpers.
> + return 0;
> }
> - ctrl->ctrl_key = nvme_auth_extract_key(ctrl->opts->keyring,
> - ctrl->opts->dhchap_ctrl_secret,
> - strlen(ctrl->opts->dhchap_ctrl_secret));
> - if (IS_ERR(ctrl->ctrl_key)) {
> - ret = PTR_ERR(ctrl->ctrl_key);
> - ctrl->ctrl_key = NULL;
> - goto err_free_dhchap_secret;
> +
> + if (ctrl->host_key)
> + nvme_auth_clear_key(ctrl, false);
> +
> + ctrl->host_key = key_get(ctrl->opts->dhchap_key);
> + if (!ctrl->host_key) {
> + dev_warn(ctrl->device,
> + "dhchap key %08x not present\n",
> + key_serial(ctrl->opts->dhchap_key));
> + return -ENOKEY;
> + }
> + down_read(&ctrl->host_key->sem);
> + ret = key_validate(ctrl->host_key);
> + up_read(&ctrl->host_key->sem);
> + if (ret) {
> + dev_warn(ctrl->device,
> + "dhchap key %08x invalidated\n",
> + key_serial(ctrl->host_key));
> + key_put(ctrl->host_key);
> + ctrl->host_key = NULL;
> + return -ENOKEY;
> }
> + dev_dbg(ctrl->device,
> + "%s: using dhchap key %08x\n",
> + __func__, key_serial(ctrl->host_key));
>
> - if (!ctrl->opts->dhchap_secret && !ctrl->opts->dhchap_ctrl_secret)
> - return 0;
> + if (ctrl->ctrl_key)
> + nvme_auth_clear_key(ctrl, true);
> +
> + if (ctrl->opts->dhchap_ctrl_key) {
> + ctrl->ctrl_key = key_get(ctrl->opts->dhchap_ctrl_key);
> + if (!ctrl->ctrl_key) {
> + dev_warn(ctrl->device,
> + "dhchap ctrl key %08x not present\n",
> + key_serial(ctrl->opts->dhchap_ctrl_key));
> + return -ENOKEY;
> + }
> + down_read(&ctrl->ctrl_key->sem);
> + ret = key_validate(ctrl->ctrl_key);
> + up_read(&ctrl->ctrl_key->sem);
> + if (ret) {
> + dev_warn(ctrl->device,
> + "dhchap ctrl key %08x invalidated\n",
> + key_serial(ctrl->ctrl_key));
> + key_put(ctrl->ctrl_key);
> + ctrl->ctrl_key = NULL;
> + return -EKEYREVOKED;
> + }
> + dev_dbg(ctrl->device,
> + "%s: using dhchap ctrl key %08x\n",
> + __func__, key_serial(ctrl->ctrl_key));
> + }
>
> ctrl->dhchap_ctxs = kvcalloc(ctrl_max_dhchaps(ctrl),
> sizeof(*chap), GFP_KERNEL);
> if (!ctrl->dhchap_ctxs) {
> - ret = -ENOMEM;
> - goto err_free_dhchap_ctrl_secret;
> + nvme_auth_clear_key(ctrl, true);
> + nvme_auth_clear_key(ctrl, false);
> + return -ENOMEM;
> }
>
> for (i = 0; i < ctrl_max_dhchaps(ctrl); i++) {
> @@ -1104,13 +1158,6 @@ int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
> }
>
> return 0;
> -err_free_dhchap_ctrl_secret:
> - key_put(ctrl->ctrl_key);
> - ctrl->ctrl_key = NULL;
> -err_free_dhchap_secret:
> - key_put(ctrl->host_key);
> - ctrl->host_key = NULL;
> - return ret;
> }
> EXPORT_SYMBOL_GPL(nvme_auth_init_ctrl);
>
> @@ -1130,10 +1177,14 @@ void nvme_auth_free(struct nvme_ctrl *ctrl)
> kfree(ctrl->dhchap_ctxs);
> }
> if (ctrl->host_key) {
> + dev_dbg(ctrl->device, "%s: drop host key %08x\n",
> + __func__, key_serial(ctrl->host_key));
> key_put(ctrl->host_key);
> ctrl->host_key = NULL;
> }
> if (ctrl->ctrl_key) {
> + dev_dbg(ctrl->device, "%s: drop ctrl key %08x\n",
> + __func__, key_serial(ctrl->ctrl_key));
> key_put(ctrl->ctrl_key);
> ctrl->ctrl_key = NULL;
> }
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 93e9041b9657..009a6cf8a86b 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -12,6 +12,7 @@
> #include <linux/seq_file.h>
> #include "nvme.h"
> #include "fabrics.h"
> +#include <linux/nvme-auth.h>
> #include <linux/nvme-keyring.h>
>
> static LIST_HEAD(nvmf_transports);
> @@ -717,6 +718,7 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
> {
> substring_t args[MAX_OPT_ARGS];
> char *options, *o, *p;
> + char *host_secret = NULL, *ctrl_secret = NULL;
> int token, ret = 0;
> size_t nqnlen = 0;
> int ctrl_loss_tmo = NVMF_DEF_CTRL_LOSS_TMO, key_id;
> @@ -738,6 +740,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
> opts->tls_key = NULL;
> opts->keyring = NULL;
> opts->concat = false;
> + opts->dhchap_key = NULL;
> + opts->dhchap_ctrl_key = NULL;
>
> options = o = kstrdup(buf, GFP_KERNEL);
> if (!options)
> @@ -1026,13 +1030,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
> ret = -ENOMEM;
> goto out;
> }
> - if (strlen(p) < 11 || strncmp(p, "DHHC-1:", 7)) {
> - pr_err("Invalid DH-CHAP secret %s\n", p);
> - ret = -EINVAL;
> - goto out;
> - }
> - kfree(opts->dhchap_secret);
> - opts->dhchap_secret = p;
> + kfree(host_secret);
> + host_secret = p;
> break;
> case NVMF_OPT_DHCHAP_CTRL_SECRET:
> p = match_strdup(args);
> @@ -1040,13 +1039,8 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
> ret = -ENOMEM;
> goto out;
> }
> - if (strlen(p) < 11 || strncmp(p, "DHHC-1:", 7)) {
> - pr_err("Invalid DH-CHAP secret %s\n", p);
> - ret = -EINVAL;
> - goto out;
> - }
> - kfree(opts->dhchap_ctrl_secret);
> - opts->dhchap_ctrl_secret = p;
> + kfree(ctrl_secret);
> + ctrl_secret = p;
> break;
> case NVMF_OPT_TLS:
> if (!IS_ENABLED(CONFIG_NVME_TCP_TLS)) {
> @@ -1090,6 +1084,41 @@ 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);
> }
> +
> + opts->host = nvmf_host_add(hostnqn, &hostid);
> + if (IS_ERR(opts->host)) {
> + ret = PTR_ERR(opts->host);
> + opts->host = NULL;
> + goto out;
> + }
> +
> + if (host_secret) {
> + pr_debug("lookup host identity '%s'\n", host_secret);
> + key = nvme_auth_extract_key(opts->keyring, host_secret,
> + strlen(host_secret));
> + if (IS_ERR(key)) {
> + ret = PTR_ERR(key);
> + goto out;
> + }
> + pr_debug("using dhchap key %08x\n", key_serial(key));
> + opts->dhchap_key = key;
> + }
> + if (ctrl_secret) {
> + if (!opts->dhchap_key) {
> + ret = -EINVAL;
> + goto out;
> + }
> + pr_debug("lookup ctrl identity '%s'\n", ctrl_secret);
> + key = nvme_auth_extract_key(opts->keyring, ctrl_secret,
> + strlen(ctrl_secret));
> + if (IS_ERR(key)) {
> + ret = PTR_ERR(key);
> + goto out;
> + }
> + pr_debug("using dhchap ctrl key %08x\n", key_serial(key));
> + opts->dhchap_ctrl_key = key;
> + }
> +
> if (opts->concat) {
> if (opts->tls) {
> pr_err("Secure concatenation over TLS is not supported\n");
> @@ -1101,21 +1130,16 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
> ret = -EINVAL;
> goto out;
> }
> - if (!opts->dhchap_secret) {
> + if (!opts->dhchap_key) {
> 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)) {
> - ret = PTR_ERR(opts->host);
> - opts->host = NULL;
> - goto out;
> - }
> -
> out:
> + kfree(ctrl_secret);
> + kfree(host_secret);
> kfree(options);
> return ret;
> }
> @@ -1290,8 +1314,18 @@ void nvmf_free_options(struct nvmf_ctrl_options *opts)
> kfree(opts->subsysnqn);
> kfree(opts->host_traddr);
> kfree(opts->host_iface);
> - kfree(opts->dhchap_secret);
> - kfree(opts->dhchap_ctrl_secret);
> + if (opts->dhchap_key) {
> + pr_debug("revoke dhchap key %08x\n",
> + key_serial(opts->dhchap_key));
> + key_revoke(opts->dhchap_key);
> + key_put(opts->dhchap_key);
> + }
> + if (opts->dhchap_ctrl_key) {
> + pr_debug("revoke dhchap ctrl key %08x\n",
> + key_serial(opts->dhchap_ctrl_key));
> + key_revoke(opts->dhchap_key);
> + key_put(opts->dhchap_ctrl_key);
> + }
revoking directly from opts? opts was always used as a store for what
the user passed in, any transformation
is kept under the ctrl.
More information about the Linux-nvme
mailing list